selinux: libselinux: Enable multiple input files to selabel_open.
PATCH v3 1/1 selinux: libselinux: Enable multiple input files to selabel_open.
From: Dan Cashman <[email protected]>
The file_contexts labeling backend, specified in label_file.c, currently assumes that only one path will be specified as an option to selabel_open(). The split of platform and non-platform policy on device, however, will necessitate the loading of two disparate policy files. Rather than combining the files and then calling the existing API on a newly-formed file, just add the ability to specify multiple files to use. Order of opt specification to selabel_open matters.
This corresponds to AOSP commit 50400d38203e4db08314168e60c281cc61a717a8, which lead to a fork with upstream, which we'd like to correct.
--- libselinux/src/label.c | 21 +++++--- libselinux/src/label_db.c | 13 ++++- libselinux/src/label_file.c | 104 +++++++++++++++++++++++++++++----------- libselinux/src/label_internal.h | 5 +- libselinux/src/label_media.c | 10 +++- libselinux/src/label_x.c | 10 +++- 6 files changed, 124 insertions(+), 39 deletions(-)
libselinux/src/label.c +15 -6
@@ -143,7 +143,11 @@ static int selabel_fini(struct selabel_handle *rec,struct selabel_lookup_rec *lr, int translating) { - if (compat_validate(rec, lr, rec->spec_file, 0)) + char *path = NULL; + +if (rec->spec_files) + path = rec->spec_files[0 ]; +if (compat_validate (rec, lr, path,0 ))return -1 ; if (translating && !lr->ctx_trans &&
@@ -226,11 +230,9 @@ struct selabel_handle *selabel_open(unsigned int backend, rec->digest =selabel_is_digest_set (opts, nopts, rec->digest);if ((*initfuncs[backend])(rec, opts, nopts)) { -free (rec->spec_file); -free (rec); +selabel_close (rec); rec =NULL ; } - out:return rec; }
@@ -337,10 +339,17 @@ int selabel_digest(struct selabel_handle *rec,void selabel_close (struct selabel_handle *rec) { +size_t i; + +if (rec->spec_files) { +for (i =0 ; i < rec->spec_files_len; i++) +free (rec->spec_files[i]); +free (rec->spec_files); + }if (rec->digest)selabel_digest_fini (rec->digest); - rec->func_close (rec); -free (rec->spec_file); +if (rec->func_close) + rec->func_close (rec);free (rec); }
libselinux/src/label_db.c +12 -1
@@ -290,7 +290,18 @@ db_init(const struct selinux_opt *opts, unsigned nopts, errno =EINVAL ;return NULL ; } - rec->spec_file =strdup (path); + rec->spec_files_len =1 ; + rec->spec_files =calloc (rec->spec_files_len,sizeof (rec->spec_files[0 ])); +if (!rec->spec_files) { +free (catalog); +return NULL ; + } + rec->spec_files[0 ] =strdup (path); +if (!rec->spec_files[0 ]) { +free (catalog); +free (rec->spec_files); +return NULL ; + }/* * Parse for each lines
libselinux/src/label_file.c +76 -28
153 lines not on this page · raw message
libselinux/src/label_internal.h +3 -2
@@ -98,10 +98,11 @@ struct selabel_handle {void *data;/* -* The main spec file used. Note for file contexts the local and/or +* The main spec file(s) used. Note for file contexts the local and/or * homedirs could also have been used to resolve a context. */ -char *spec_file; +size_t spec_files_len; +char **spec_files;/* ptr to SHA1 hash information if SELABEL_OPT_DIGEST set */ struct selabel_digest *digest;
libselinux/src/label_media.c +9 -1
@@ -100,7 +100,15 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, errno =EINVAL ;return -1 ; } - rec->spec_file =strdup (path); + rec->spec_files_len =1 ; + rec->spec_files =calloc (rec->spec_files_len,sizeof (rec->spec_files[0 ])); +if (!rec->spec_files) +return -1 ; + rec->spec_files[0 ] =strdup (path); +if (!rec->spec_files[0 ]) { +free (rec->spec_files); +return -1 ; + }/* * Perform two passes over the specification file.
libselinux/src/label_x.c +9 -1
@@ -127,7 +127,15 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, errno =EINVAL ;return -1 ; } - rec->spec_file =strdup (path); + rec->spec_files_len =1 ; + rec->spec_files =calloc (rec->spec_files_len,sizeof (rec->spec_files[0 ])); +if (!rec->spec_files) +return -1 ; + rec->spec_files[0 ] =strdup (path); +if (!rec->spec_files[0 ]) { +free (rec->spec_files); +return -1 ; + }/* * Perform two passes over the specification file.
Signature · 1 line
2.15.0.rc0.271.g36b669edcc-goog
Quoted 158 lines · Daniel Cashman
From: Dan Cashman <[email protected]>
The
file_contextslabeling backend, specified inlabel_file.c, currently assumes
that only one path will be specified as an option to selabel_open(). The split
of platform and non-platform policy on device, however, will necessitate the
loading of two disparate policy files. Rather than combining the files and then
calling the existing API on a newly-formed file, just add the ability to specify
multiple files to use. Order of opt specification toselabel_openmatters.This corresponds to AOSP commit
50400d38203e4db08314168e60c281cc61a717a8, which lead to a fork with upstream, which we'd like to correct.Signed-off-by Dan Cashman <[email protected]>---
libselinux/src/label.c | 21 +++++--- libselinux/src/label_db.c | 13 ++++- libselinux/src/label_file.c | 104 +++++++++++++++++++++++++++++-
----------
libselinux/src/label_internal.h | 5 +- libselinux/src/label_media.c | 10 +++- libselinux/src/label_x.c | 10 +++-6 files changed, 124 insertions(+), 39 deletions(-)libselinux/src/label.c +5
-1@@ -143,7 +143,11 @@ static int selabel_fini(struct selabel_handle *rec, struct selabel_lookup_rec *lr, int translating) { -if (compat_validate (rec, lr, rec->spec_file,0 )) +char *path =NULL ; + +if (rec->spec_files) + path = rec->spec_files[0 ]; +if (compat_validate (rec, lr, path,0 )) return -1;
if (translating && !lr->ctx_trans &&libselinux/src/label.c +0
-0@@ -226,11 +230,9 @@ struct selabel_handle *selabel_open(unsigned int backend,
rec->digest= selabel_is_digest_set(opts, nopts, rec-Quoted 1 line
digest);
if ((*initfuncs[backend])(rec, opts, nopts)) {libselinux/src/label.c +1
-2-free (rec->spec_file); -free (rec); +selabel_close (rec);rec = NULL;
}
-
out:
return rec;
}libselinux/src/label.c +0
-0@@ -337,10 +339,17 @@ int selabel_digest(struct selabel_handle *rec,
voidselabel_close(struct selabel_handle *rec)
{libselinux/src/label.c +7
-0+size_t i; + +if (rec->spec_files) { +for (i =0 ; i < rec->spec_files_len; i++) +free (rec->spec_files[i]); +free (rec->spec_files); + }if (
rec->digest)
selabel_digest_fini(rec->digest);libselinux/src/label.c +2
-2- rec->func_close (rec); -free (rec->spec_file); +if (rec->func_close) + rec->func_close (rec);free(rec);
}libselinux/src/label_db.c +12
-1@@ -290,7 +290,18 @@ db_init(const struct selinux_opt *opts, unsigned nopts, errno = EINVAL; return NULL; } - rec->spec_file =strdup (path); + rec->spec_files_len =1 ; + rec->spec_files =calloc (rec->spec_files_len,sizeof (rec- >spec_files[0])); +if (!rec->spec_files) { +free (catalog); +return NULL ; + } + rec->spec_files[0 ] =strdup (path); +if (!rec->spec_files[0 ]) { +free (catalog); +free (rec->spec_files); +return NULL ; + }
/*
* Parse for each lineslibselinux/src/label_file.c +6
-2@@ -709,28 +709,61 @@ static int init(struct selabel_handle *rec,const struct selinux_opt *opts, unsigned n) { struct saved_data *data = (struct saved_data *)rec->data; -const char *path =NULL ; +size_t num_paths =0 ; +char **path =NULL ; const char *prefix = NULL; -int status =-1 , baseonly =0 ; +int status =-1 ; +size_t i; +bool baseonly = false; +bool path_provided;
/* Process arguments */libselinux/src/label_file.c +3
-2-while (n--) -switch (opts[n].type) { + i = n; +while (i--) +switch (opts[i].type) {case
SELABEL_OPT_PATH:libselinux/src/label_file.c +1
-1- path = opts[n].value; + num_paths++;break;
caseSELABEL_OPT_SUBSET:libselinux/src/label_file.c +1
-1- prefix = opts[n].value; + prefix = opts[i].value;break;
caseSELABEL_OPT_BASEONLY:libselinux/src/label_file.c +1
-1- baseonly = !!opts[n].value; + baseonly = !!opts[i].value;break;
}libselinux/src/label_file.c +9
-0+if (!num_paths) { + num_paths =1 ; + path_provided = false; + }else { + path_provided = true; + } + + path =calloc (num_paths,sizeof (*path)); +if (path ==NULL ) {
libselinux/src/label_file.c static int init(struct selabel_handle *rec, const struct selinux_opt *opts, · line 753
Quoted 10 lines
libselinux/src/label_file.c +10
-0+ goto finish; + } + rec->spec_files = path; + rec->spec_files_len = num_paths; + +if (path_provided) { +for (i =0 ; i < n; i++) { +switch (opts[i].type) { +case SELABEL_OPT_PATH : + *path =strdup (opts[i].value);
Perhaps surprisingly, opts[i].value can be NULL here and some callers rely on that. After applying your patch, coreutils, selabel_lookup, and other userspace programs all seg fault as a result. The use case is programs where the selinux_opt structure is declared with a SELABEL_OPT_PATH entry whose value is subsequently filled in, and left NULL if they want to use the default path for file_contexts. Internally, libselinux also does this from the matchpathcon_init_prefix() function.
In any event, you need to handle this case.
Quoted 188 lines
libselinux/src/label_file.c +9
-0+if (*path ==NULL ) + goto finish; + path++; +break ; +default : +break ; + } + } + }#if !defined(BUILD_HOST) && !defined(ANDROID)
char subs_file[PATH_MAX + 1];
/* Process local and distribution substitution files */libselinux/src/label_file.c +1
-1-if (!path) { +if (!path_provided) {status =
selabel_subs_init( selinux_file_context_subs_dist_path(), rec->digest, &data->dist_subs);libselinux/src/label_file.c +18
-11@@ -740,43 +773,52 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, rec->digest, &data->subs); if (status) goto finish; - path =selinux_file_context_path (); + rec->spec_files[0] = strdup(selinux_file_context_path()); + if (rec->spec_files[0] == NULL) + goto finish; } else { -snprintf (subs_file,sizeof (subs_file), "%s.subs_dist", path); - status =selabel_subs_init (subs_file, rec->digest, + for (i = 0; i < num_paths; i++) { + snprintf(subs_file, sizeof(subs_file), "%s.subs_dist", rec->spec_files[i]); + status = selabel_subs_init(subs_file, rec- >digest, &data->dist_subs); -if (status) - goto finish; -snprintf (subs_file,sizeof (subs_file),"%s.subs" , path); - status =selabel_subs_init (subs_file, rec->digest, + if (status) + goto finish; + snprintf(subs_file, sizeof(subs_file), "%s.subs", rec->spec_files[i]); + status = selabel_subs_init(subs_file, rec- >digest, &data->subs); -if (status) - goto finish; + if (status) + goto finish; + } + } +#else + if (!path_provided) { + selinux_log(SELINUX_ERROR, "No path given to file labeling backend\n"); + goto finish; } - #endif - rec->spec_file =strdup (path);
/*libselinux/src/label_file.c +0
-1- * The do detailed validation of the input and fill the specarray
libselinux/src/label_file.c +1
-0+ * Do detailed validation of the input and fill the specarray
*/libselinux/src/label_file.c +2
-6- status =process_file (path,NULL , rec, prefix, rec->digest); -if (status) - goto finish; - -if (rec->validating) { - status =nodups_specs (data, path); +for (i =0 ; i < num_paths; i++) { + status = process_file(rec->spec_files[i], NULL, rec,prefix,
rec->digest);
if (status)
goto finish;libselinux/src/label_file.c +3
-0+ +if (rec->validating) { + status = nodups_specs(data, rec-Quoted 1 line
spec_files[i]);
libselinux/src/label_file.c +3
-0+if (status) + goto finish; + }}
if (!baseonly) {libselinux/src/label_file.c +1
-1- status = process_file(path, "homedirs", rec, prefix, + status = process_file(rec->spec_files[0],"homedirs", rec, prefix,
rec-Quoted 1 line
digest);
if (status && errno != ENOENT)
goto finish;libselinux/src/label_file.c +1
-1- status = process_file(path, "local", rec, prefix, + status = process_file(rec->spec_files[0], "local",rec, prefix,
rec-Quoted 1 line
digest);
if (status && errno != ENOENT)
goto finish;libselinux/src/label_file.c +0
-0@@ -804,6 +846,12 @@ static void closef(struct selabel_handle *rec) struct stem *stem; unsigned int i;libselinux/src/label_file.c +6
-0+if (!data) +return ; + +/* make sure successive ->func_close() calls are harmless */ + rec->data =NULL ; +
selabel_subs_fini(data->subs);
selabel_subs_fini(data->dist_subs);libselinux/src/label_internal.h +0
-0@@ -98,10 +98,11 @@ struct selabel_handle { void *data;
/*libselinux/src/label_internal.h +0
-1- * The main spec file used. Note for file contexts the localand/or
libselinux/src/label_internal.h +1
-0+ * The main spec file(s) used. Note for file contexts thelocal and/or
* homedirs could also have been used to resolve a context. */libselinux/src/label_internal.h +2
-1-char *spec_file; +size_t spec_files_len; +char **spec_files;
/* ptr to SHA1 hash information ifSELABEL_OPT_DIGESTset */ structselabel_digest*digest;libselinux/src/label_media.c +9
-1@@ -100,7 +100,15 @@ static int init(struct selabel_handle *rec,const struct selinux_opt *opts, errno = EINVAL; return -1; } - rec->spec_file =strdup (path); + rec->spec_files_len =1 ; + rec->spec_files =calloc (rec->spec_files_len,sizeof (rec- >spec_files[0])); +if (!rec->spec_files) +return -1 ; + rec->spec_files[0 ] =strdup (path); +if (!rec->spec_files[0 ]) { +free (rec->spec_files); +return -1 ; + }
/*
* Perform two passes over the specification file.libselinux/src/label_x.c +9
-1@@ -127,7 +127,15 @@ static int init(struct selabel_handle *rec,const struct selinux_opt *opts, errno = EINVAL; return -1; } - rec->spec_file =strdup (path); + rec->spec_files_len =1 ; + rec->spec_files =calloc (rec->spec_files_len,sizeof (rec- >spec_files[0])); +if (!rec->spec_files) +return -1 ; + rec->spec_files[0 ] =strdup (path); +if (!rec->spec_files[0 ]) { +free (rec->spec_files); +return -1 ; + }
/*
* Perform two passes over the specification file.
Quoted 179 lines · Stephen Smalley
Quoted 158 lines · Daniel Cashman
From: Dan Cashman <[email protected]>
The
file_contextslabeling backend, specified inlabel_file.c, currently assumes
that only one path will be specified as an option toselabel_open(). The split
of platform and non-platform policy on device, however, will necessitate the
loading of two disparate policy files. Rather than combining the files and then
calling the existing API on a newly-formed file, just add the ability to specify
multiple files to use. Order of opt specification toselabel_openmatters.This corresponds to AOSP commit
50400d38203e4db08314168e60c281cc61a717a8, which lead to a fork with upstream, which we'd like to correct.Signed-off-by Dan Cashman <[email protected]>--- libselinux/src/label.c | 21 +++++--- libselinux/src/label_db.c | 13 ++++- libselinux/src/label_file.c | 104 +++++++++++++++++++++++++++++- ---------- libselinux/src/label_internal.h | 5 +- libselinux/src/label_media.c | 10 +++- libselinux/src/label_x.c | 10 +++- 6 files changed, 124 insertions(+), 39 deletions(-)libselinux/src/label.c +5
-1@@ -143,7 +143,11 @@ static int selabel_fini(struct selabel_handle *rec, struct selabel_lookup_rec *lr, int translating) { -if (compat_validate (rec, lr, rec->spec_file,0 )) +char *path =NULL ; + +if (rec->spec_files) + path = rec->spec_files[0 ]; +if (compat_validate (rec, lr, path,0 ))return -1 ;if (translating && !lr->ctx_trans &&libselinux/src/label.c +1
-3@@ -226,11 +230,9 @@ struct selabel_handle *selabel_open(unsigned int backend, rec->digest =selabel_is_digest_set (opts, nopts, rec- >digest);if ((*initfuncs[backend])(rec, opts, nopts)) { -free (rec->spec_file); -free (rec); +selabel_close (rec); rec =NULL ; } - out:return rec;}
libselinux/src/label.c +9
-2@@ -337,10 +339,17 @@ int selabel_digest(struct selabel_handle *rec,void selabel_close (struct selabel_handle *rec) { +size_t i; + +if (rec->spec_files) { +for (i =0 ; i < rec->spec_files_len; i++) +free (rec->spec_files[i]); +free (rec->spec_files); + }if (rec->digest)selabel_digest_fini (rec->digest); - rec->func_close (rec); -free (rec->spec_file); +if (rec->func_close) + rec->func_close (rec);free (rec); }libselinux/src/label_db.c +12
-1@@ -290,7 +290,18 @@ db_init(const struct selinux_opt *opts, unsigned nopts, errno =EINVAL ;return NULL ; } - rec->spec_file =strdup (path); + rec->spec_files_len =1 ; + rec->spec_files =calloc (rec->spec_files_len,sizeof (rec- >spec_files[0])); +if (!rec->spec_files) { +free (catalog); +return NULL ; + } + rec->spec_files[0 ] =strdup (path); +if (!rec->spec_files[0 ]) { +free (catalog); +free (rec->spec_files); +return NULL ; + }/*
* Parse for each lineslibselinux/src/label_file.c +21
-7@@ -709,28 +709,61 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, unsigned n) {struct saved_data *data = (struct saved_data *)rec->data; -const char *path =NULL ; +size_t num_paths =0 ; +char **path =NULL ;const char *prefix =NULL ; -int status =-1 , baseonly =0 ; +int status =-1 ; +size_t i; +bool baseonly = false; +bool path_provided;/* Process arguments */ -while (n--) -switch (opts[n].type) { + i = n; +while (i--) +switch (opts[i].type) {case SELABEL_OPT_PATH : - path = opts[n].value; + num_paths++;break ;case SELABEL_OPT_SUBSET : - prefix = opts[n].value; + prefix = opts[i].value;break ;case SELABEL_OPT_BASEONLY : - baseonly = !!opts[n].value; + baseonly = !!opts[i].value;break ; } +if (!num_paths) { + num_paths =1 ; + path_provided = false; + }else { + path_provided = true; + } + + path =calloc (num_paths,sizeof (*path)); +if (path ==NULL ) {libselinux/src/label_file.c static int init(struct selabel_handle *rec, · line 754
Quoted 10 lines
libselinux/src/label_file.c +10
-0+ goto finish; + } + rec->spec_files = path; + rec->spec_files_len = num_paths; + +if (path_provided) { +for (i =0 ; i < n; i++) { +switch (opts[i].type) { +case SELABEL_OPT_PATH : + *path =strdup (opts[i].value);Perhaps surprisingly, opts[i].value can be NULL here and some callers rely on that. After applying your patch, coreutils,
selabel_lookup, and other userspace programs all seg fault as a result. The use case is programs where theselinux_optstructure is declared with aSELABEL_OPT_PATHentry whose value is subsequently filled in, and left NULL if they want to use the default path forfile_contexts. Internally, libselinux also does this from thematchpathcon_init_prefix()function.
Quoted 2 lines
In any event, you need to handle this case.
Poor Stephen has turned into your test bot :-)
Does any of the test suite exercise this? Maybe make a PR on github first to get Travis to test these?
Stephen can you share with how your testing this so Dan can follow suit?
Quoted 188 lines
Quoted 188 lines
libselinux/src/label_file.c +10
-1+if (*path ==NULL ) + goto finish; + path++; +break ; +default : +break ; + } + } + }#if !defined(BUILD_HOST ) && !defined(ANDROID )char subs_file[PATH_MAX +1 ];/* Process local and distribution substitution files */ -if (!path) { +if (!path_provided) { status = selabel_subs_init( selinux_file_context_subs_dist_path(),rec->digest, &data->dist_subs);libselinux/src/label_file.c +18
-12@@ -740,43 +773,52 @@ static int init(struct selabel_handle *rec,const struct selinux_opt *opts, rec->digest, &data->subs);if (status) goto finish; - path =selinux_file_context_path (); + rec->spec_files[0 ] =strdup (selinux_file_context_path ()); +if (rec->spec_files[0 ] ==NULL ) + goto finish; } else { -snprintf (subs_file,sizeof (subs_file), "%s.subs_dist", path); - status =selabel_subs_init (subs_file, rec->digest, +for (i =0 ; i < num_paths; i++) { +snprintf (subs_file,sizeof (subs_file),"%s.subs_dist" , rec->spec_files[i]); + status =selabel_subs_init (subs_file, rec- >digest, &data->dist_subs); -if (status) - goto finish; -snprintf (subs_file,sizeof (subs_file),"%s.subs" , path); - status =selabel_subs_init (subs_file, rec->digest, +if (status) + goto finish; +snprintf (subs_file,sizeof (subs_file),"%s.subs" , rec->spec_files[i]); + status =selabel_subs_init (subs_file, rec- >digest, &data->subs); -if (status) - goto finish; +if (status) + goto finish; + } + } +#else +if (!path_provided) { + selinux_log(SELINUX_ERROR, "No path given to file labeling backend\n"); + goto finish; } -#endif - rec->spec_file =strdup (path);/* -* The do detailed validation of the input and fill the spec array
+ * Do detailed validation of the input and fill the spec
array*/libselinux/src/label_file.c +2
-6- status =process_file (path,NULL , rec, prefix, rec->digest); -if (status) - goto finish; - -if (rec->validating) { - status =nodups_specs (data, path); +for (i =0 ; i < num_paths; i++) { + status = process_file(rec->spec_files[i], NULL, rec,prefix, rec->digest); if (status) goto finish;libselinux/src/label_file.c +3
-0+ +if (rec->validating) { + status = nodups_specs(data, rec-Quoted 1 line
spec_files[i]);
libselinux/src/label_file.c +4
-1+if (status) + goto finish; + } }if (!baseonly) { - status = process_file(path, "homedirs", rec, prefix, + status = process_file(rec->spec_files[0],"homedirs", rec, prefix,
rec-Quoted 1 line
digest);
if (status && errno != ENOENT) goto finish;libselinux/src/label_file.c +1
-1- status = process_file(path, "local", rec, prefix, + status = process_file(rec->spec_files[0], "local",rec, prefix,
rec-Quoted 1 line
digest);
if (status && errno != ENOENT) goto finish;libselinux/src/label_file.c +6
-0@@ -804,6 +846,12 @@ static void closef(struct selabel_handle *rec)struct stem *stem;unsigned int i; +if (!data) +return ; + +/* make sure successive ->func_close() calls are harmless */ + rec->data =NULL ; +selabel_subs_fini (data->subs);selabel_subs_fini (data->dist_subs);libselinux/src/label_internal.h +0
-1@@ -98,10 +98,11 @@ struct selabel_handle {void *data;/* -* The main spec file used. Note for file contexts the local and/or
+ * The main spec file(s) used. Note for file contexts the
local and/or
* homedirs could also have been used to resolve a context. */libselinux/src/label_internal.h +2
-1-char *spec_file; +size_t spec_files_len; +char **spec_files;/* ptr to SHA1 hash information if SELABEL_OPT_DIGEST set */ struct selabel_digest *digest;libselinux/src/label_media.c +9
-1@@ -100,7 +100,15 @@ static int init(struct selabel_handle *rec,const struct selinux_opt *opts, errno =EINVAL ;return -1 ; } - rec->spec_file =strdup (path); + rec->spec_files_len =1 ; + rec->spec_files =calloc (rec->spec_files_len,sizeof (rec- >spec_files[0])); +if (!rec->spec_files) +return -1 ; + rec->spec_files[0 ] =strdup (path); +if (!rec->spec_files[0 ]) { +free (rec->spec_files); +return -1 ; + }/*
* Perform two passes over the specification file.libselinux/src/label_x.c +9
-1@@ -127,7 +127,15 @@ static int init(struct selabel_handle *rec,const struct selinux_opt *opts, errno =EINVAL ;return -1 ; } - rec->spec_file =strdup (path); + rec->spec_files_len =1 ; + rec->spec_files =calloc (rec->spec_files_len,sizeof (rec- >spec_files[0])); +if (!rec->spec_files) +return -1 ; + rec->spec_files[0 ] =strdup (path); +if (!rec->spec_files[0 ]) { +free (rec->spec_files); +return -1 ; + }/*
* Perform two passes over the specification file.
Quoted 180 lines · William Roberts
Quoted 179 lines · Stephen Smalley
Quoted 158 lines · Daniel Cashman
From: Dan Cashman <[email protected]>
The
file_contextslabeling backend, specified inlabel_file.c, currently assumes
that only one path will be specified as an option toselabel_open(). The split
of platform and non-platform policy on device, however, will necessitate the
loading of two disparate policy files. Rather than combining the files and then
calling the existing API on a newly-formed file, just add the ability to specify
multiple files to use. Order of opt specification toselabel_openmatters.This corresponds to AOSP commit
50400d38203e4db08314168e60c281cc61a717a8, which lead to a fork with upstream, which we'd like to correct.Signed-off-by Dan Cashman <[email protected]>--- libselinux/src/label.c | 21 +++++--- libselinux/src/label_db.c | 13 ++++- libselinux/src/label_file.c | 104 +++++++++++++++++++++++++++++- ---------- libselinux/src/label_internal.h | 5 +- libselinux/src/label_media.c | 10 +++- libselinux/src/label_x.c | 10 +++- 6 files changed, 124 insertions(+), 39 deletions(-)libselinux/src/label.c +5
-1@@ -143,7 +143,11 @@ static int selabel_fini(struct selabel_handle *rec, struct selabel_lookup_rec *lr, int translating) { -if (compat_validate (rec, lr, rec->spec_file,0 )) +char *path =NULL ; + +if (rec->spec_files) + path = rec->spec_files[0 ]; +if (compat_validate (rec, lr, path,0 ))return -1 ;if (translating && !lr->ctx_trans &&libselinux/src/label.c +1
-3@@ -226,11 +230,9 @@ struct selabel_handle *selabel_open(unsigned int backend, rec->digest =selabel_is_digest_set (opts, nopts, rec- >digest);if ((*initfuncs[backend])(rec, opts, nopts)) { -free (rec->spec_file); -free (rec); +selabel_close (rec); rec =NULL ; } - out:return rec;}
libselinux/src/label.c +9
-2@@ -337,10 +339,17 @@ int selabel_digest(struct selabel_handle *rec,void selabel_close (struct selabel_handle *rec) { +size_t i; + +if (rec->spec_files) { +for (i =0 ; i < rec->spec_files_len; i++) +free (rec->spec_files[i]); +free (rec->spec_files); + }if (rec->digest)selabel_digest_fini (rec->digest); - rec->func_close (rec); -free (rec->spec_file); +if (rec->func_close) + rec->func_close (rec);free (rec); }libselinux/src/label_db.c +12
-1@@ -290,7 +290,18 @@ db_init(const struct selinux_opt *opts, unsigned nopts, errno =EINVAL ;return NULL ; } - rec->spec_file =strdup (path); + rec->spec_files_len =1 ; + rec->spec_files =calloc (rec->spec_files_len,sizeof (rec- >spec_files[0])); +if (!rec->spec_files) { +free (catalog); +return NULL ; + } + rec->spec_files[0 ] =strdup (path); +if (!rec->spec_files[0 ]) { +free (catalog); +free (rec->spec_files); +return NULL ; + }/*
* Parse for each lineslibselinux/src/label_file.c +21
-7@@ -709,28 +709,61 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, unsigned n) {struct saved_data *data = (struct saved_data *)rec->data; -const char *path =NULL ; +size_t num_paths =0 ; +char **path =NULL ;const char *prefix =NULL ; -int status =-1 , baseonly =0 ; +int status =-1 ; +size_t i; +bool baseonly = false; +bool path_provided;/* Process arguments */ -while (n--) -switch (opts[n].type) { + i = n; +while (i--) +switch (opts[i].type) {case SELABEL_OPT_PATH : - path = opts[n].value; + num_paths++;break ;case SELABEL_OPT_SUBSET : - prefix = opts[n].value; + prefix = opts[i].value;break ;case SELABEL_OPT_BASEONLY : - baseonly = !!opts[n].value; + baseonly = !!opts[i].value;break ; } +if (!num_paths) { + num_paths =1 ; + path_provided = false; + }else { + path_provided = true; + } + + path =calloc (num_paths,sizeof (*path)); +if (path ==NULL ) {libselinux/src/label_file.c static int init(struct selabel_handle *rec, · line 754
Quoted 10 lines
libselinux/src/label_file.c +10
-0+ goto finish; + } + rec->spec_files = path; + rec->spec_files_len = num_paths; + +if (path_provided) { +for (i =0 ; i < n; i++) { +switch (opts[i].type) { +case SELABEL_OPT_PATH : + *path =strdup (opts[i].value);Perhaps surprisingly, opts[i].value can be NULL here and some callers rely on that. After applying your patch, coreutils,
selabel_lookup, and other userspace programs all seg fault as a result. The use case is programs where theselinux_optstructure is declared with aSELABEL_OPT_PATHentry whose value is subsequently filled in, and left NULL if they want to use the default path forfile_contexts. Internally, libselinux also does this from thematchpathcon_init_prefix()function.
Quoted 7 lines
Quoted 2 lines
In any event, you need to handle this case.
Poor Stephen has turned into your test bot :-)
Does any of the test suite exercise this? Maybe make a PR on github first to get Travis to test these?
Looks like Stephen tried that to see what would happen, and it incorrectly passed indicating we need more tests:
https://github.com/SELinuxProject/selinux/pull/67
Quoted 4 lines
Stephen can you share with how your testing this so Dan can follow suit?
Looks like we need to add whatever your doing.
Quoted 189 lines
Quoted 189 lines
Quoted 189 lines
libselinux/src/label_file.c +10
-1+if (*path ==NULL ) + goto finish; + path++; +break ; +default : +break ; + } + } + }#if !defined(BUILD_HOST ) && !defined(ANDROID )char subs_file[PATH_MAX +1 ];/* Process local and distribution substitution files */ -if (!path) { +if (!path_provided) { status = selabel_subs_init( selinux_file_context_subs_dist_path(),rec->digest, &data->dist_subs);libselinux/src/label_file.c +18
-12@@ -740,43 +773,52 @@ static int init(struct selabel_handle *rec,const struct selinux_opt *opts, rec->digest, &data->subs);if (status) goto finish; - path =selinux_file_context_path (); + rec->spec_files[0 ] =strdup (selinux_file_context_path ()); +if (rec->spec_files[0 ] ==NULL ) + goto finish; } else { -snprintf (subs_file,sizeof (subs_file), "%s.subs_dist", path); - status =selabel_subs_init (subs_file, rec->digest, +for (i =0 ; i < num_paths; i++) { +snprintf (subs_file,sizeof (subs_file),"%s.subs_dist" , rec->spec_files[i]); + status =selabel_subs_init (subs_file, rec- >digest, &data->dist_subs); -if (status) - goto finish; -snprintf (subs_file,sizeof (subs_file),"%s.subs" , path); - status =selabel_subs_init (subs_file, rec->digest, +if (status) + goto finish; +snprintf (subs_file,sizeof (subs_file),"%s.subs" , rec->spec_files[i]); + status =selabel_subs_init (subs_file, rec- >digest, &data->subs); -if (status) - goto finish; +if (status) + goto finish; + } + } +#else +if (!path_provided) { + selinux_log(SELINUX_ERROR, "No path given to file labeling backend\n"); + goto finish; } -#endif - rec->spec_file =strdup (path);/* -* The do detailed validation of the input and fill the spec array
+ * Do detailed validation of the input and fill the spec
array*/libselinux/src/label_file.c +2
-6- status =process_file (path,NULL , rec, prefix, rec->digest); -if (status) - goto finish; - -if (rec->validating) { - status =nodups_specs (data, path); +for (i =0 ; i < num_paths; i++) { + status = process_file(rec->spec_files[i], NULL, rec,prefix, rec->digest); if (status) goto finish;libselinux/src/label_file.c +3
-0+ +if (rec->validating) { + status = nodups_specs(data, rec-Quoted 1 line
spec_files[i]);
libselinux/src/label_file.c +4
-1+if (status) + goto finish; + } }if (!baseonly) { - status = process_file(path, "homedirs", rec, prefix, + status = process_file(rec->spec_files[0],"homedirs", rec, prefix,
rec-Quoted 1 line
digest);
if (status && errno != ENOENT) goto finish;libselinux/src/label_file.c +1
-1- status = process_file(path, "local", rec, prefix, + status = process_file(rec->spec_files[0], "local",rec, prefix,
rec-Quoted 1 line
digest);
if (status && errno != ENOENT) goto finish;libselinux/src/label_file.c +6
-0@@ -804,6 +846,12 @@ static void closef(struct selabel_handle *rec)struct stem *stem;unsigned int i; +if (!data) +return ; + +/* make sure successive ->func_close() calls are harmless */ + rec->data =NULL ; +selabel_subs_fini (data->subs);selabel_subs_fini (data->dist_subs);libselinux/src/label_internal.h +0
-1@@ -98,10 +98,11 @@ struct selabel_handle {void *data;/* -* The main spec file used. Note for file contexts the local and/or
+ * The main spec file(s) used. Note for file contexts the
local and/or
* homedirs could also have been used to resolve a context. */libselinux/src/label_internal.h +2
-1-char *spec_file; +size_t spec_files_len; +char **spec_files;/* ptr to SHA1 hash information if SELABEL_OPT_DIGEST set */ struct selabel_digest *digest;libselinux/src/label_media.c +9
-1@@ -100,7 +100,15 @@ static int init(struct selabel_handle *rec,const struct selinux_opt *opts, errno =EINVAL ;return -1 ; } - rec->spec_file =strdup (path); + rec->spec_files_len =1 ; + rec->spec_files =calloc (rec->spec_files_len,sizeof (rec- >spec_files[0])); +if (!rec->spec_files) +return -1 ; + rec->spec_files[0 ] =strdup (path); +if (!rec->spec_files[0 ]) { +free (rec->spec_files); +return -1 ; + }/*
* Perform two passes over the specification file.libselinux/src/label_x.c +9
-1@@ -127,7 +127,15 @@ static int init(struct selabel_handle *rec,const struct selinux_opt *opts, errno =EINVAL ;return -1 ; } - rec->spec_file =strdup (path); + rec->spec_files_len =1 ; + rec->spec_files =calloc (rec->spec_files_len,sizeof (rec- >spec_files[0])); +if (!rec->spec_files) +return -1 ; + rec->spec_files[0 ] =strdup (path); +if (!rec->spec_files[0 ]) { +free (rec->spec_files); +return -1 ; + }/*
* Perform two passes over the specification file.
Signature · 3 lines
Respectfully, William C Roberts
Quoted 194 lines · William Roberts
Quoted 192 lines · Stephen Smalley
Quoted 167 lines · Daniel Cashman
From: Dan Cashman <[email protected]>
The
file_contextslabeling backend, specified inlabel_file.c, currently assumes
that only one path will be specified as an option to selabel_open(). The split
of platform and non-platform policy on device, however, will necessitate the
loading of two disparate policy files. Rather than combining the files and then
calling the existing API on a newly-formed file, just add the ability
to specify
multiple files to use. Order of opt specification toselabel_open
matters.This corresponds to AOSP commit
50400d38203e4db08314168e60c281cc61a717a8, which lead to a fork with upstream, which we'd like to correct.Signed-off-by Dan Cashman <[email protected]>---
libselinux/src/label.c | 21 +++++---
libselinux/src/label_db.c | 13 ++++-
libselinux/src/label_file.c | 104
+++++++++++++++++++++++++++++-
----------
libselinux/src/label_internal.h | 5 +-
libselinux/src/label_media.c | 10 +++-
libselinux/src/label_x.c | 10 +++-6 files changed, 124 insertions(+), 39 deletions(-)libselinux/src/label.c +0
-0@@ -143,7 +143,11 @@ static int selabel_fini(struct
selabel_handle
*rec,
structselabel_lookup_rec*lr, int translating)
{
- if (compat_validate(rec, lr, rec->spec_file, 0)) + char *path = NULL;
+
+ if (rec->spec_files)
+ path = rec->spec_files[0];
+ if (compat_validate(rec, lr, path, 0)) return -1;if (translating && !lr->ctx_trans &&
libselinux/src/label.c +0
-0@@ -226,11 +230,9 @@ struct selabel_handle *selabel_open(unsignedint
backend,
rec->digest = selabel_is_digest_set(opts, nopts, rec-Quoted 1 line
digest);
if ((*initfuncs[backend])(rec, opts, nopts)) { - free(rec->spec_file);
- free(rec);
+ selabel_close(rec);
rec = NULL;
}
-
out:
return rec;
}libselinux/src/label.c +9
-2@@ -337,10 +339,17 @@ int selabel_digest(struct selabel_handle *rec, void selabel_close(struct selabel_handle *rec) { + size_t i; + + if (rec->spec_files) { + for (i = 0; i < rec->spec_files_len; i++) + free(rec->spec_files[i]); + free(rec->spec_files); + } if (rec->digest) selabel_digest_fini(rec->digest); - rec->func_close(rec); - free(rec->spec_file); + if (rec->func_close) + rec->func_close(rec); free(rec); }libselinux/src/label_db.c +0
-0@@ -290,7 +290,18 @@ db_init(const struct selinux_opt *opts,unsigned
nopts,
errno = EINVAL;
return NULL;
}
- rec->spec_file = strdup(path);
+ rec->spec_files_len = 1;
+ rec->spec_files = calloc(rec->spec_files_len, sizeof(rec-Quoted 1 line
spec_files[0]));
+ if (!rec->spec_files) {
+ free(catalog);
+ return NULL;
+ }
+ rec->spec_files[0] = strdup(path);
+ if (!rec->spec_files[0]) {
+ free(catalog);
+ free(rec->spec_files);
+ return NULL;
+ }/*
* Parse for each lineslibselinux/src/label_file.c +9
-4@@ -709,28 +709,61 @@ static int init(struct selabel_handle *rec,const struct selinux_opt *opts, unsigned n) { struct saved_data *data = (struct saved_data *)rec->data; - const char *path = NULL; + size_t num_paths = 0; + char **path = NULL; const char *prefix = NULL; - int status = -1, baseonly = 0; + int status = -1; + size_t i; + bool baseonly = false; + bool path_provided; /* Process arguments */ - while (n--) - switch(opts[n].type) { + i = n; + while (i--) + switch(opts[i].type) {case
SELABEL_OPT_PATH:
- path = opts[n].value;
+ num_paths++;
break;
caseSELABEL_OPT_SUBSET:
- prefix = opts[n].value;
+ prefix = opts[i].value;
break;
caseSELABEL_OPT_BASEONLY:
- baseonly = !!opts[n].value;
+ baseonly = !!opts[i].value;
break;
}+ if (!num_paths) {
+ num_paths = 1;
+ path_provided = false;
+ } else {
+ path_provided = true;
+ }libselinux/src/label_file.c +3
-0+ + path = calloc(num_paths, sizeof(*path)); + if (path == NULL) {libselinux/src/label_file.c static int init(struct selabel_handle *rec, const struct selinux_opt *opts, · line 753
Quoted 10 lines
libselinux/src/label_file.c +10
-0+ goto finish; + } + rec->spec_files = path; + rec->spec_files_len = num_paths; + + if (path_provided) { + for (i = 0; i < n; i++) { + switch(opts[i].type) { + case SELABEL_OPT_PATH: + *path = strdup(opts[i].value);Perhaps surprisingly, opts[i].value can be NULL here and some callers
rely on that. After applying your patch, coreutils,selabel_lookup,
and other userspace programs all seg fault as a result. The use case
is programs where theselinux_optstructure is declared with aSELABEL_OPT_PATHentry whose value is subsequently filled in, and left
NULL if they want to use the default path forfile_contexts. Internally, libselinux also does this from thematchpathcon_init_prefix()function.
Quoted 7 lines
Quoted 2 lines
In any event, you need to handle this case.
Poor Stephen has turned into your test bot :-)
Does any of the test suite exercise this? Maybe make a PR on github first to get Travis to test these?
Unfortunately the travis-ci tests don't catch this one currently.
Quoted 2 lines
Stephen can you share with how your testing this so Dan can follow suit?
In this case, you could build and install to a private directory as per the README, then set your LD_LIBRARY_PATH=~/obj/lib or whatever and run selabel_lookup.
Quoted 205 lines
Quoted 205 lines
Quoted 205 lines
+ if (*path == NULL)
+ goto finish; + path++;
+ break;
+ default:
+ break;
+ }
+ }
+ }
#if !defined(BUILD_HOST) && !defined(ANDROID)
char subs_file[PATH_MAX + 1];
/* Process local and distribution substitution files */ - if (!path) {
+ if (!path_provided) {
status =selabel_subs_init( selinux_file_context_subs_dist_path(), rec->digest, &data->dist_subs);libselinux/src/label_file.c +18
-12@@ -740,43 +773,52 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, rec->digest, &data->subs); if (status) goto finish; - path = selinux_file_context_path(); + rec->spec_files[0] = strdup(selinux_file_context_path()); + if (rec->spec_files[0] == NULL) + goto finish; } else { - snprintf(subs_file, sizeof(subs_file), "%s.subs_dist", path); - status = selabel_subs_init(subs_file, rec->digest, + for (i = 0; i < num_paths; i++) { + snprintf(subs_file, sizeof(subs_file), "%s.subs_dist", rec->spec_files[i]); + status = selabel_subs_init(subs_file, rec- > digest, &data->dist_subs); - if (status) - goto finish; - snprintf(subs_file, sizeof(subs_file), "%s.subs", path); - status = selabel_subs_init(subs_file, rec->digest, + if (status) + goto finish; + snprintf(subs_file, sizeof(subs_file), "%s.subs", rec->spec_files[i]); + status = selabel_subs_init(subs_file, rec- > digest, &data->subs); - if (status) - goto finish; + if (status) + goto finish; + } + } +#else + if (!path_provided) { + selinux_log(SELINUX_ERROR, "No path given to file labeling backend\n"); + goto finish; } - #endif - rec->spec_file = strdup(path); /* -* The do detailed validation of the input and fill the spec
array
+ * Do detailed validation of the input and fill the spec array
*/
- status = process_file(path, NULL, rec, prefix, rec-Quoted 1 line
digest);
- if (status)
- goto finish;
-
- if (rec->validating) {
- status =nodups_specs(data, path);
+ for (i = 0; i <num_paths; i++) {
+ status = process_file(rec->spec_files[i], NULL, rec,
prefix,rec->digest);
if (status)
goto finish;
+
+ if (rec->validating) {
+ status = nodups_specs(data, rec-Quoted 1 line
spec_files[i]);
+ if (status)
+ goto finish;
+ }
}if (!baseonly) {
- status = process_file(path, "homedirs", rec, prefix,
+ status = process_file(rec->spec_files[0], "homedirs", rec, prefix,
rec-Quoted 1 line
digest);
if (status && errno != ENOENT)
goto finish;- status = process_file(path, "local", rec, prefix, + status = process_file(rec->spec_files[0], "local", rec, prefix,
rec-Quoted 1 line
digest);
if (status && errno != ENOENT)
goto finish;libselinux/src/label_file.c +6
-0@@ -804,6 +846,12 @@ static void closef(struct selabel_handle *rec) struct stem *stem; unsigned int i; + if (!data) + return; + + /* make sure successive ->func_close() calls are harmless */ + rec->data = NULL; + selabel_subs_fini(data->subs);selabel_subs_fini(data->dist_subs);
libselinux/src/label_internal.h +0
-1@@ -98,10 +98,11 @@ struct selabel_handle { void *data; /* -* The main spec file used. Note for file contexts the local
and/or
+ * The main spec file(s) used. Note for file contexts the local and/or
* homedirs could also have been used to resolve a context. */
- char*spec_file;
+ size_tspec_files_len;
+ char **spec_files;/* ptr to SHA1 hash information if
SELABEL_OPT_DIGESTset */
structselabel_digest*digest;libselinux/src/label_media.c +9
-1@@ -100,7 +100,15 @@ static int init(struct selabel_handle *rec,const struct selinux_opt *opts, errno = EINVAL; return -1; } - rec->spec_file = strdup(path); + rec->spec_files_len = 1; + rec->spec_files = calloc(rec->spec_files_len, sizeof(rec- > spec_files[0])); + if (!rec->spec_files) + return -1; + rec->spec_files[0] = strdup(path); + if (!rec->spec_files[0]) { + free(rec->spec_files); + return -1; + }/*
* Perform two passes over the specification file.libselinux/src/label_x.c +9
-1@@ -127,7 +127,15 @@ static int init(struct selabel_handle *rec,const struct selinux_opt *opts, errno = EINVAL; return -1; } - rec->spec_file = strdup(path); + rec->spec_files_len = 1; + rec->spec_files = calloc(rec->spec_files_len, sizeof(rec- > spec_files[0])); + if (!rec->spec_files) + return -1; + rec->spec_files[0] = strdup(path); + if (!rec->spec_files[0]) { + free(rec->spec_files); + return -1; + }/*
* Perform two passes over the specification file.
Quoted 209 lines · Stephen Smalley
Quoted 198 lines · William Roberts
On Thu, Oct 19, 2017 at 7:26 AM, Stephen Smalley <[email protected]
Quoted 1 line
Quoted 195 lines · wrote:
Quoted 170 lines · Daniel Cashman
From: Dan Cashman <[email protected]>
The
file_contextslabeling backend, specified inlabel_file.c, currently assumes
that only one path will be specified as an option to selabel_open(). The split
of platform and non-platform policy on device, however, will necessitate the
loading of two disparate policy files. Rather than combining the
files and then
calling the existing API on a newly-formed file, just add the ability
to specify
multiple files to use. Order of opt specification toselabel_open
matters.This corresponds to AOSP commit
50400d38203e4db08314168e60c281cc61a717a8, which lead to a fork with upstream, which we'd like to correct.Signed-off-by Dan Cashman <[email protected]>---
libselinux/src/label.c | 21 +++++---
libselinux/src/label_db.c | 13 ++++-
libselinux/src/label_file.c | 104
+++++++++++++++++++++++++++++-
----------
libselinux/src/label_internal.h | 5 +-
libselinux/src/label_media.c | 10 +++-
libselinux/src/label_x.c | 10 +++-6 files changed, 124 insertions(+), 39 deletions(-)libselinux/src/label.c +0
-0@@ -143,7 +143,11 @@ static int selabel_fini(struct
selabel_handle
*rec,
structselabel_lookup_rec*lr, int translating)
{
- if (compat_validate(rec, lr, rec->spec_file, 0)) + char *path = NULL;
+
+ if (rec->spec_files)
+ path = rec->spec_files[0];
+ if (compat_validate(rec, lr, path, 0)) return -1;if (translating && !lr->ctx_trans &&
libselinux/src/label.c +0
-0@@ -226,11 +230,9 @@ struct selabel_handle *selabel_open(unsignedint
backend,
rec->digest = selabel_is_digest_set(opts, nopts, rec-Quoted 1 line
digest);
if ((*initfuncs[backend])(rec, opts, nopts)) { - free(rec->spec_file);
- free(rec);
+ selabel_close(rec);
rec = NULL;
}
-
out:
return rec;
}libselinux/src/label.c +9
-2@@ -337,10 +339,17 @@ int selabel_digest(struct selabel_handle *rec, void selabel_close(struct selabel_handle *rec) { + size_t i; + + if (rec->spec_files) { + for (i = 0; i < rec->spec_files_len; i++) + free(rec->spec_files[i]); + free(rec->spec_files); + } if (rec->digest) selabel_digest_fini(rec->digest); - rec->func_close(rec); - free(rec->spec_file); + if (rec->func_close) + rec->func_close(rec); free(rec); }libselinux/src/label_db.c +0
-0@@ -290,7 +290,18 @@ db_init(const struct selinux_opt *opts,unsigned
nopts,
errno = EINVAL;
return NULL;
}
- rec->spec_file = strdup(path);
+ rec->spec_files_len = 1;
+ rec->spec_files = calloc(rec->spec_files_len, sizeof(rec-Quoted 1 line
spec_files[0]));
+ if (!rec->spec_files) {
+ free(catalog);
+ return NULL;
+ }
+ rec->spec_files[0] = strdup(path);
+ if (!rec->spec_files[0]) {
+ free(catalog);
+ free(rec->spec_files);
+ return NULL;
+ }/*
* Parse for each lineslibselinux/src/label_file.c +9
-4@@ -709,28 +709,61 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, unsigned n) { struct saved_data *data = (struct saved_data *)rec->data; - const char *path = NULL; + size_t num_paths = 0; + char **path = NULL; const char *prefix = NULL; - int status = -1, baseonly = 0; + int status = -1; + size_t i; + bool baseonly = false; + bool path_provided; /* Process arguments */ - while (n--) - switch(opts[n].type) { + i = n; + while (i--) + switch(opts[i].type) {case
SELABEL_OPT_PATH:
- path = opts[n].value;
+ num_paths++;
break;
caseSELABEL_OPT_SUBSET:
- prefix = opts[n].value;
+ prefix = opts[i].value;
break;
caseSELABEL_OPT_BASEONLY:
- baseonly = !!opts[n].value;
+ baseonly = !!opts[i].value;
break;
}+ if (!num_paths) {
+ num_paths = 1;
+ path_provided = false;
+ } else {
+ path_provided = true;
+ }libselinux/src/label_file.c +3
-0+ + path = calloc(num_paths, sizeof(*path)); + if (path == NULL) {libselinux/src/label_file.c static int init(struct selabel_handle *rec, const struct selinux_opt *opts, · line 753
Quoted 10 lines
libselinux/src/label_file.c +10
-0+ goto finish; + } + rec->spec_files = path; + rec->spec_files_len = num_paths; + + if (path_provided) { + for (i = 0; i < n; i++) { + switch(opts[i].type) { + case SELABEL_OPT_PATH: + *path = strdup(opts[i].value);Perhaps surprisingly, opts[i].value can be NULL here and some callers
rely on that. After applying your patch, coreutils,selabel_lookup,
and other userspace programs all seg fault as a result. The use case
is programs where theselinux_optstructure is declared with aSELABEL_OPT_PATHentry whose value is subsequently filled in, and left
NULL if they want to use the default path forfile_contexts. Internally, libselinux also does this from thematchpathcon_init_prefix()function.Quoted 7 lines
Quoted 2 lines
In any event, you need to handle this case.
Poor Stephen has turned into your test bot :-)
Does any of the test suite exercise this? Maybe make a PR on github first to get Travis to test these?
Unfortunately the travis-ci tests don't catch this one currently.
Quoted 8 lines
Quoted 2 lines
Stephen can you share with how your testing this so Dan can follow suit?
In this case, you could build and install to a private directory as per
the README, then set your LD_LIBRARY_PATH=~/obj/lib or whatever and runselabel_lookup.
Ala:
$ make DESTDIR=~/obj install $ LD_LIBRARY_PATH=~/obj/lib selabel_lookup -b file -k /etc Segmentation fault (core dumped)
BTW, I guess this exposes another issue with the patch; there is no support for exercising the new code included with it, e.g. an extension to utils/selabel_lookup.c to permit specifying multiple input files via multiple -f options. Otherwise, we have no upstream way of testing it.
Quoted 217 lines
Quoted 217 lines
Quoted 217 lines
Quoted 217 lines
+ if (*path == NULL)
+ goto finish; + path++;
+ break;
+ default:
+ break;
+ }
+ }
+ }
#if !defined(BUILD_HOST) && !defined(ANDROID)
char subs_file[PATH_MAX + 1];
/* Process local and distribution substitution files */ - if (!path) {
+ if (!path_provided) {
status =selabel_subs_init( selinux_file_context_subs_dist_path(), rec->digest, &data->dist_subs);libselinux/src/label_file.c +6
-3@@ -740,43 +773,52 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, rec->digest, &data->subs); if (status) goto finish; - path = selinux_file_context_path(); + rec->spec_files[0] =strdup (selinux_file_context_path ()); + if (rec->spec_files[0] == NULL) + goto finish; } else { - snprintf(subs_file, sizeof(subs_file), "%s.subs_dist", path); - status = selabel_subs_init(subs_file, rec- >digest, + for (i = 0; i < num_paths; i++) { + snprintf(subs_file, sizeof(subs_file),"%s.subs_dist" , rec->spec_files[i]); + status = selabel_subs_init(subs_file,rec-
Quoted 1 line
digest,
&data->dist_subs); - if (status)
- goto finish;
- snprintf(subs_file,sizeof(subs_file), "%s.subs", path);
- status = selabel_subs_init(subs_file, rec-Quoted 1 line
digest,
+ if (status)
+ goto finish;
+ snprintf(subs_file,sizeof(subs_file), "%s.subs", rec->spec_files[i]);
+ status = selabel_subs_init(subs_file, rec-Quoted 1 line
digest,
&data->subs); - if (status)
- goto finish;
+ if (status)
+ goto finish;
+ }
+ }
+#else
+ if (!path_provided) {
+ selinux_log(SELINUX_ERROR, "No path given to file labeling backend\n");
+ goto finish;
}
-
#endif
- rec->spec_file = strdup(path);/*
- * The do detailed validation of the input and fill the spec
array
+ * Do detailed validation of the input and fill the spec array
*/
- status = process_file(path, NULL, rec, prefix, rec-Quoted 1 line
digest);
- if (status)
- goto finish;
-
- if (rec->validating) {
- status =nodups_specs(data, path);
+ for (i = 0; i <num_paths; i++) {
+ status = process_file(rec->spec_files[i], NULL, rec,
prefix,rec->digest);
if (status)
goto finish;
+
+ if (rec->validating) {
+ status = nodups_specs(data, rec-Quoted 1 line
spec_files[i]);
+ if (status)
+ goto finish;
+ }
}if (!baseonly) {
- status = process_file(path, "homedirs", rec, prefix,
+ status = process_file(rec->spec_files[0], "homedirs", rec, prefix,
rec-Quoted 1 line
digest);
if (status && errno != ENOENT)
goto finish;- status = process_file(path, "local", rec, prefix, + status = process_file(rec->spec_files[0], "local",
rec, prefix,
rec-Quoted 1 line
digest);
if (status && errno != ENOENT)
goto finish;libselinux/src/label_file.c +6
-0@@ -804,6 +846,12 @@ static void closef(struct selabel_handle *rec) struct stem *stem; unsigned int i; + if (!data) + return; + + /* make sure successive ->func_close() calls are harmless */ + rec->data = NULL; + selabel_subs_fini(data->subs);selabel_subs_fini(data->dist_subs);
libselinux/src/label_internal.h +0
-1@@ -98,10 +98,11 @@ struct selabel_handle { void *data; /* -* The main spec file used. Note for file contexts the local
and/or
+ * The main spec file(s) used. Note for file contexts the local and/or
* homedirs could also have been used to resolve a context.
*/
- char*spec_file;
+ size_tspec_files_len;
+ char **spec_files;/* ptr to SHA1 hash information if
SELABEL_OPT_DIGESTset */
structselabel_digest*digest;libselinux/src/label_media.c +9
-1@@ -100,7 +100,15 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, errno = EINVAL; return -1; } - rec->spec_file = strdup(path); + rec->spec_files_len = 1; + rec->spec_files = calloc(rec->spec_files_len, sizeof(rec- > spec_files[0])); + if (!rec->spec_files) + return -1; + rec->spec_files[0] = strdup(path); + if (!rec->spec_files[0]) { + free(rec->spec_files); + return -1; + }/*
* Perform two passes over the specification file.libselinux/src/label_x.c +9
-1@@ -127,7 +127,15 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, errno = EINVAL; return -1; } - rec->spec_file = strdup(path); + rec->spec_files_len = 1; + rec->spec_files = calloc(rec->spec_files_len, sizeof(rec- > spec_files[0])); + if (!rec->spec_files) + return -1; + rec->spec_files[0] = strdup(path); + if (!rec->spec_files[0]) { + free(rec->spec_files); + return -1; + }/*
* Perform two passes over the specification file.
Quoted 101 lines · Stephen Smalley
Quoted 90 lines · Stephen Smalley
Quoted 79 lines · William Roberts
On Thu, Oct 19, 2017 at 7:26 AM, Stephen Smalley <[email protected]
Quoted 1 line
Quoted 76 lines · wrote:
Quoted 51 lines · Daniel Cashman
[...]
libselinux/src/label_file.c +21
-7@@ -709,28 +709,61 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, unsigned n) {struct saved_data *data = (struct saved_data *)rec->data; -const char *path =NULL ; +size_t num_paths =0 ; +char **path =NULL ;const char *prefix =NULL ; -int status =-1 , baseonly =0 ; +int status =-1 ; +size_t i; +bool baseonly = false; +bool path_provided;/* Process arguments */ -while (n--) -switch (opts[n].type) { + i = n; +while (i--) +switch (opts[i].type) {case SELABEL_OPT_PATH : - path = opts[n].value; + num_paths++;break ;case SELABEL_OPT_SUBSET : - prefix = opts[n].value; + prefix = opts[i].value;break ;case SELABEL_OPT_BASEONLY : - baseonly = !!opts[n].value; + baseonly = !!opts[i].value;break ; } +if (!num_paths) { + num_paths =1 ; + path_provided = false; + }else { + path_provided = true; + } + + path =calloc (num_paths,sizeof (*path)); +if (path ==NULL ) {libselinux/src/label_file.c static int init(struct selabel_handle · line 755
Quoted 10 lines
libselinux/src/label_file.c +10
-0+ goto finish; + } + rec->spec_files = path; + rec->spec_files_len = num_paths; + +if (path_provided) { +for (i =0 ; i < n; i++) { +switch (opts[i].type) { +case SELABEL_OPT_PATH : + *path =strdup (opts[i].value);Perhaps surprisingly, opts[i].value can be NULL here and some callers
rely on that. After applying your patch, coreutils,selabel_lookup,
and other userspace programs all seg fault as a result. The use case
is programs where theselinux_optstructure is declared with aSELABEL_OPT_PATHentry whose value is subsequently filled in, and left
NULL if they want to use the default path forfile_contexts. Internally, libselinux also does this from thematchpathcon_init_prefix()function.Quoted 7 lines
Quoted 2 lines
In any event, you need to handle this case.
Poor Stephen has turned into your test bot :-)
Does any of the test suite exercise this? Maybe make a PR on github first to get Travis to test these?
Unfortunately the travis-ci tests don't catch this one currently.
Quoted 8 lines
Quoted 2 lines
Stephen can you share with how your testing this so Dan can follow suit?
In this case, you could build and install to a private directory as per
the README, then set your LD_LIBRARY_PATH=~/obj/lib or whatever and runselabel_lookup.Ala:
Quoted 8 lines
$ make DESTDIR=~/obj install $ LD_LIBRARY_PATH=~/obj/lib selabel_lookup -b file -k /etc Segmentation fault (core dumped)BTW, I guess this exposes another issue with the patch; there is no support for exercising the new code included with it, e.g. an extension to
utils/selabel_lookup.cto permit specifying multiple input files via multiple -f options. Otherwise, we have no upstream way of testing it.
For selabel_lookup, it would be possible to write a test script (or a CUnit-based test program like libsepol and libsemanage have) which does not need an SELinux policy to be installed on the system, thanks to option -f.
Nevertheless other parts of libselinux (and programs) can not currently be tested on a system without SELinux, as /etc/selinux and /sys/fs/selinux are needed. If we want to automatize the testing of such parts in a CI like Travis-CI, I suppose we would need to run a virtual machine with its own filesystem, a kernel with SELinux enabled, and some programs like coreutils compiled with the tested version of libselinux/libsepol/... As implementing such an idea may take quite a long time, would there be simpler ways to perform automatic testing of libselinux? Perhaps using another continuous integration system?
Cheers,
Nicolas
Quoted 111 lines · Nicolas Iooss
Quoted 101 lines · Stephen Smalley
Quoted 90 lines · Stephen Smalley
Quoted 79 lines · William Roberts
On Thu, Oct 19, 2017 at 7:26 AM, Stephen Smalley <[email protected]
Quoted 1 line
Quoted 76 lines · wrote:
Quoted 51 lines · Daniel Cashman
[...]
libselinux/src/label_file.c +21
-7@@ -709,28 +709,61 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, unsigned n) {struct saved_data *data = (struct saved_data *)rec->data; -const char *path =NULL ; +size_t num_paths =0 ; +char **path =NULL ;const char *prefix =NULL ; -int status =-1 , baseonly =0 ; +int status =-1 ; +size_t i; +bool baseonly = false; +bool path_provided;/* Process arguments */ -while (n--) -switch (opts[n].type) { + i = n; +while (i--) +switch (opts[i].type) {case SELABEL_OPT_PATH : - path = opts[n].value; + num_paths++;break ;case SELABEL_OPT_SUBSET : - prefix = opts[n].value; + prefix = opts[i].value;break ;case SELABEL_OPT_BASEONLY : - baseonly = !!opts[n].value; + baseonly = !!opts[i].value;break ; } +if (!num_paths) { + num_paths =1 ; + path_provided = false; + }else { + path_provided = true; + } + + path =calloc (num_paths,sizeof (*path)); +if (path ==NULL ) {libselinux/src/label_file.c static int init(struct selabel_handle · line 755
Quoted 10 lines
libselinux/src/label_file.c +10
-0+ goto finish; + } + rec->spec_files = path; + rec->spec_files_len = num_paths; + +if (path_provided) { +for (i =0 ; i < n; i++) { +switch (opts[i].type) { +case SELABEL_OPT_PATH : + *path =strdup (opts[i].value);Perhaps surprisingly, opts[i].value can be NULL here and some callers
rely on that. After applying your patch, coreutils,selabel_lookup,
and other userspace programs all seg fault as a result. The use case
is programs where theselinux_optstructure is declared with aSELABEL_OPT_PATHentry whose value is subsequently filled in, and left
NULL if they want to use the default path forfile_contexts. Internally, libselinux also does this from thematchpathcon_init_prefix()function.Quoted 7 lines
Quoted 2 lines
In any event, you need to handle this case.
Poor Stephen has turned into your test bot :-)
Does any of the test suite exercise this? Maybe make a PR on github first to get Travis to test these?
Unfortunately the travis-ci tests don't catch this one currently.
Quoted 8 lines
Quoted 2 lines
Stephen can you share with how your testing this so Dan can follow suit?
In this case, you could build and install to a private directory as per
the README, then set your LD_LIBRARY_PATH=~/obj/lib or whatever and runselabel_lookup.Ala:
Quoted 8 lines
$ make DESTDIR=~/obj install $ LD_LIBRARY_PATH=~/obj/lib selabel_lookup -b file -k /etc Segmentation fault (core dumped)BTW, I guess this exposes another issue with the patch; there is no support for exercising the new code included with it, e.g. an extension to
utils/selabel_lookup.cto permit specifying multiple input files via multiple -f options. Otherwise, we have no upstream way of testing it.
Quoted 14 lines
For
selabel_lookup, it would be possible to write a test script (or a CUnit-based test program like libsepol and libsemanage have) which does not need an SELinux policy to be installed on the system, thanks to option -f.
Nevertheless other parts of libselinux (and programs) can not currently be tested on a system without SELinux, as/etc/selinuxand/sys/fs/selinuxare needed. If we want to automatize the testing of such parts in a CI like Travis-CI, I suppose we would need to run a virtual machine with its own filesystem, a kernel with SELinux enabled, and some programs like coreutils compiled with the tested version of libselinux/libsepol/... As implementing such an idea may take quite a long time, would there be simpler ways to perform automatic testing of libselinux? Perhaps using another continuous integration system?
The free travis just provides an Ubuntu image.... if you use sudo: true you get a full VM if you don't, you get a docker container.
With that said, we could try to hack up the VM image to support what we need, or perhaps we can use CMockaand mock out the filesystem calls to proc/selinuxfs with something that replicates it.(not sure if that's how mock objects work)
Quoted 4 lines
Cheers,
Nicolas
Quoted 127 lines · William Roberts
Quoted 111 lines · Nicolas Iooss
Quoted 101 lines · Stephen Smalley
Quoted 90 lines · Stephen Smalley
Quoted 79 lines · William Roberts
On Thu, Oct 19, 2017 at 7:26 AM, Stephen Smalley <[email protected]
Quoted 1 line
Quoted 76 lines · wrote:
Quoted 51 lines · Daniel Cashman
[...]
libselinux/src/label_file.c +21
-7@@ -709,28 +709,61 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, unsigned n) {struct saved_data *data = (struct saved_data *)rec->data; -const char *path =NULL ; +size_t num_paths =0 ; +char **path =NULL ;const char *prefix =NULL ; -int status =-1 , baseonly =0 ; +int status =-1 ; +size_t i; +bool baseonly = false; +bool path_provided;/* Process arguments */ -while (n--) -switch (opts[n].type) { + i = n; +while (i--) +switch (opts[i].type) {case SELABEL_OPT_PATH : - path = opts[n].value; + num_paths++;break ;case SELABEL_OPT_SUBSET : - prefix = opts[n].value; + prefix = opts[i].value;break ;case SELABEL_OPT_BASEONLY : - baseonly = !!opts[n].value; + baseonly = !!opts[i].value;break ; } +if (!num_paths) { + num_paths =1 ; + path_provided = false; + }else { + path_provided = true; + } + + path =calloc (num_paths,sizeof (*path)); +if (path ==NULL ) {libselinux/src/label_file.c static int init(struct selabel_handle · line 755
Quoted 10 lines
libselinux/src/label_file.c +10
-0+ goto finish; + } + rec->spec_files = path; + rec->spec_files_len = num_paths; + +if (path_provided) { +for (i =0 ; i < n; i++) { +switch (opts[i].type) { +case SELABEL_OPT_PATH : + *path =strdup (opts[i].value);Perhaps surprisingly, opts[i].value can be NULL here and some callers
rely on that. After applying your patch, coreutils,selabel_lookup,
and other userspace programs all seg fault as a result. The use case
is programs where theselinux_optstructure is declared with aSELABEL_OPT_PATHentry whose value is subsequently filled in, and left
NULL if they want to use the default path forfile_contexts. Internally, libselinux also does this from thematchpathcon_init_prefix()function.Quoted 7 lines
Quoted 2 lines
In any event, you need to handle this case.
Poor Stephen has turned into your test bot :-)
Does any of the test suite exercise this? Maybe make a PR on github first to get Travis to test these?
Unfortunately the travis-ci tests don't catch this one currently.
Quoted 8 lines
Quoted 2 lines
Stephen can you share with how your testing this so Dan can follow suit?
In this case, you could build and install to a private directory as per
the README, then set your LD_LIBRARY_PATH=~/obj/lib or whatever and runselabel_lookup.Ala:
Quoted 8 lines
$ make DESTDIR=~/obj install $ LD_LIBRARY_PATH=~/obj/lib selabel_lookup -b file -k /etc Segmentation fault (core dumped)BTW, I guess this exposes another issue with the patch; there is no support for exercising the new code included with it, e.g. an extension to
utils/selabel_lookup.cto permit specifying multiple input files via multiple -f options. Otherwise, we have no upstream way of testing it.Quoted 14 lines
For
selabel_lookup, it would be possible to write a test script (or a CUnit-based test program like libsepol and libsemanage have) which does not need an SELinux policy to be installed on the system, thanks to option -f.
Nevertheless other parts of libselinux (and programs) can not currently be tested on a system without SELinux, as/etc/selinuxand/sys/fs/selinuxare needed. If we want to automatize the testing of such parts in a CI like Travis-CI, I suppose we would need to run a virtual machine with its own filesystem, a kernel with SELinux enabled, and some programs like coreutils compiled with the tested version of libselinux/libsepol/... As implementing such an idea may take quite a long time, would there be simpler ways to perform automatic testing of libselinux? Perhaps using another continuous integration system?
Quoted 13 lines
The free travis just provides an Ubuntu image.... if you use sudo: true you get a full VM if you don't, you get a docker container.
With that said, we could try to hack up the VM image to support what we need, or perhaps we can use CMockaand mock out the filesystem calls to proc/selinuxfs with something that replicates it.(not sure if that's how mock objects work)
Quoted 4 lines
Cheers,
Nicolas
Is there selinux-specific travis documentation, or a pointer to appropriate travis documetnation in general? I'd obviously like to cut down on bad patch submissions, so having a test suite batter a patch before wasting reveiwer time would be great. We could/should probably add this to the README as well if a proper standard workflow is developed.
Thanks,
Dan
Quoted 128 lines · Dan Cashman
Quoted 127 lines · William Roberts
Quoted 124 lines · Nicolas Iooss
Quoted 119 lines · Stephen Smalley
Quoted 101 lines · Stephen Smalley
Quoted 84 lines · William Roberts
On Thu, Oct 19, 2017 at 7:26 AM, Stephen Smalley <[email protected]
Quoted 2 lines
Quoted 79 lines · wrote:
Quoted 52 lines · Daniel Cashman
[...]
libselinux/src/label_file.c +21
-7@@ -709,28 +709,61 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, unsigned n) {struct saved_data *data = (struct saved_data *)rec->data; -const char *path =NULL ; +size_t num_paths =0 ; +char **path =NULL ;const char *prefix =NULL ; -int status =-1 , baseonly =0 ; +int status =-1 ; +size_t i; +bool baseonly = false; +bool path_provided;/* Process arguments */ -while (n--) -switch (opts[n].type) { + i = n; +while (i--) +switch (opts[i].type) {case SELABEL_OPT_PATH : - path = opts[n].value; + num_paths++;break ;case SELABEL_OPT_SUBSET : - prefix = opts[n].value; + prefix = opts[i].value;break ;case SELABEL_OPT_BASEONLY : - baseonly = !!opts[n].value; + baseonly = !!opts[i].value;break ; } +if (!num_paths) { + num_paths =1 ; + path_provided = false; + }else { + path_provided = true; + } + + path =calloc (num_paths,sizeof (*path)); +if (path ==NULL ) {libselinux/src/label_file.c static int init(struct selabel_handle · line 755
Quoted 10 lines
libselinux/src/label_file.c +10
-0+ goto finish; + } + rec->spec_files = path; + rec->spec_files_len = num_paths; + +if (path_provided) { +for (i =0 ; i < n; i++) { +switch (opts[i].type) { +case SELABEL_OPT_PATH : + *path =strdup (opts[i].value);Perhaps surprisingly, opts[i].value can be NULL here and some callers
rely on that. After applying your patch, coreutils,selabel_lookup,
and other userspace programs all seg fault as a result. The use case
is programs where theselinux_optstructure is declared with aSELABEL_OPT_PATHentry whose value is subsequently filled in, and left
NULL if they want to use the default path forfile_contexts. Internally, libselinux also does this from thematchpathcon_init_prefix()function.Quoted 7 lines
Quoted 2 lines
In any event, you need to handle this case.
Poor Stephen has turned into your test bot :-)
Does any of the test suite exercise this? Maybe make a PR on github first to get Travis to test these?
Unfortunately the travis-ci tests don't catch this one currently.
Quoted 2 lines
Stephen can you share with how your testing this so Dan can follow suit?
Quoted 5 lines
In this case, you could build and install to a private directory as per
the README, then set your LD_LIBRARY_PATH=~/obj/lib or whatever and runselabel_lookup.Ala:
$ make DESTDIR=~/obj install $ LD_LIBRARY_PATH=~/obj/lib selabel_lookup -b file -k /etc Segmentation fault (core dumped)BTW, I guess this exposes another issue with the patch; there is no support for exercising the new code included with it, e.g. an extension to
utils/selabel_lookup.cto permit specifying multiple input files via multiple -f options. Otherwise, we have no upstream way of testing it.
Quoted 14 lines
Quoted 6 lines
Quoted 6 lines
For
selabel_lookup, it would be possible to write a test script (or a CUnit-based test program like libsepol and libsemanage have) which does not need an SELinux policy to be installed on the system, thanks to option -f.
Nevertheless other parts of libselinux (and programs) can not currently be tested on a system without SELinux, as/etc/selinuxandQuoted 8 lines
Quoted 8 lines
/sys/fs/selinuxare needed. If we want to automatize the testing of such parts in a CI like Travis-CI, I suppose we would need to run a virtual machine with its own filesystem, a kernel with SELinux enabled, and some programs like coreutils compiled with the tested version of libselinux/libsepol/... As implementing such an idea may take quite a long time, would there be simpler ways to perform automatic testing of libselinux? Perhaps using another continuous integration system?
We have travis up and running, but it's not excising this code path. Any PR submitted to the selinux project on githib has travis "test" it via the .travis.yml
file in the root of the tree.
If you have a fork of selinux on github you could set up travis to run it. It's pretty simple:
https://docs.travis-ci.com/user/getting-started/
You could also just submit a PR for testing and publish a patch to the list when you're ready for review...
Quoted 28 lines
Quoted 5 lines
The free travis just provides an Ubuntu image.... if you use sudo: true you get a full VM if you don't, you get a docker container.
Quoted 13 lines
With that said, we could try to hack up the VM image to support what we need,
or perhaps we can use CMockaand mock out the filesystem calls to proc/selinuxfs
with something that replicates it.(not sure if that's how mock objects work)Quoted 4 lines
Cheers,
NicolasIs there selinux-specific travis documentation, or a pointer to appropriate travis documetnation in general? I'd obviously like to cut down on bad patch submissions, so having a test suite batter a patch before wasting reveiwer time would be great. We could/should probably add this to the README as well if a proper standard workflow is developed.
Thanks,
Dan
Quoted 149 lines · William Roberts
Quoted 128 lines · Dan Cashman
Quoted 127 lines · William Roberts
Quoted 124 lines · Nicolas Iooss
Quoted 119 lines · Stephen Smalley
Quoted 101 lines · Stephen Smalley
Quoted 84 lines · William Roberts
On Thu, Oct 19, 2017 at 7:26 AM, Stephen Smalley <[email protected]
Quoted 2 lines
Quoted 79 lines · wrote:
Quoted 52 lines · Daniel Cashman
[...]
libselinux/src/label_file.c +21
-7@@ -709,28 +709,61 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, unsigned n) {struct saved_data *data = (struct saved_data *)rec->data; -const char *path =NULL ; +size_t num_paths =0 ; +char **path =NULL ;const char *prefix =NULL ; -int status =-1 , baseonly =0 ; +int status =-1 ; +size_t i; +bool baseonly = false; +bool path_provided;/* Process arguments */ -while (n--) -switch (opts[n].type) { + i = n; +while (i--) +switch (opts[i].type) {case SELABEL_OPT_PATH : - path = opts[n].value; + num_paths++;break ;case SELABEL_OPT_SUBSET : - prefix = opts[n].value; + prefix = opts[i].value;break ;case SELABEL_OPT_BASEONLY : - baseonly = !!opts[n].value; + baseonly = !!opts[i].value;break ; } +if (!num_paths) { + num_paths =1 ; + path_provided = false; + }else { + path_provided = true; + } + + path =calloc (num_paths,sizeof (*path)); +if (path ==NULL ) {libselinux/src/label_file.c static int init(struct selabel_handle · line 755
Quoted 10 lines
libselinux/src/label_file.c +10
-0+ goto finish; + } + rec->spec_files = path; + rec->spec_files_len = num_paths; + +if (path_provided) { +for (i =0 ; i < n; i++) { +switch (opts[i].type) { +case SELABEL_OPT_PATH : + *path =strdup (opts[i].value);Perhaps surprisingly, opts[i].value can be NULL here and some callers
rely on that. After applying your patch, coreutils,selabel_lookup,
and other userspace programs all seg fault as a result. The use case
is programs where theselinux_optstructure is declared with aSELABEL_OPT_PATHentry whose value is subsequently filled in, and left
NULL if they want to use the default path forfile_contexts. Internally, libselinux also does this from thematchpathcon_init_prefix()function.Quoted 7 lines
Quoted 2 lines
In any event, you need to handle this case.
Poor Stephen has turned into your test bot :-)
Does any of the test suite exercise this? Maybe make a PR on github first to get Travis to test these?
Unfortunately the travis-ci tests don't catch this one currently.
Quoted 2 lines
Stephen can you share with how your testing this so Dan can follow suit?
Quoted 5 lines
In this case, you could build and install to a private directory as per
the README, then set your LD_LIBRARY_PATH=~/obj/lib or whatever and runselabel_lookup.Ala:
$ make DESTDIR=~/obj install $ LD_LIBRARY_PATH=~/obj/lib selabel_lookup -b file -k /etc Segmentation fault (core dumped)BTW, I guess this exposes another issue with the patch; there is no support for exercising the new code included with it, e.g. an extension to
utils/selabel_lookup.cto permit specifying multiple input files via multiple -f options. Otherwise, we have no upstream way of testing it.Quoted 14 lines
Quoted 6 lines
Quoted 6 lines
For
selabel_lookup, it would be possible to write a test script (or a CUnit-based test program like libsepol and libsemanage have) which does not need an SELinux policy to be installed on the system, thanks to option -f.
Nevertheless other parts of libselinux (and programs) can not currently be tested on a system without SELinux, as/etc/selinuxandQuoted 8 lines
Quoted 8 lines
/sys/fs/selinuxare needed. If we want to automatize the testing of such parts in a CI like Travis-CI, I suppose we would need to run a virtual machine with its own filesystem, a kernel with SELinux enabled, and some programs like coreutils compiled with the tested version of libselinux/libsepol/... As implementing such an idea may take quite a long time, would there be simpler ways to perform automatic testing of libselinux? Perhaps using another continuous integration system?We have travis up and running, but it's not excising this code path. Any PR submitted to the selinux project on githib has travis "test" it via the .travis.yml
file in the root of the tree.
Quoted 6 lines
If you have a fork of selinux on github you could set up travis to run it. It's pretty simple:
https://docs.travis-ci.com/user/getting-started/You could also just submit a PR for testing and publish a patch to the list when you're ready for review...
My response might not be very clear, there are three things you need to do here:
1. Add a test as outlined previously in this thread
2. Test your change:
Test your change with travis (optional):
1. Setting it up on your fork of selinux project on github (hardest)
2. Submitting a PR to the selinux github project (easiest)
If its part of the test suite, you can run that by hand....
3. Submit patch to mailing list for review.
I know you tested all these on Android, but I think we need it as part of the test framework.,,
Quoted 29 lines
Quoted 29 lines
Quoted 6 lines
The free travis just provides an Ubuntu image.... if you use sudo: true you get a full VM if you don't, you get a docker container.
Quoted 13 lines
With that said, we could try to hack up the VM image to support what we need,
or perhaps we can use CMockaand mock out the filesystem calls to proc/selinuxfs
with something that replicates it.(not sure if that's how mock objects work)Quoted 4 lines
Cheers,
NicolasIs there selinux-specific travis documentation, or a pointer to appropriate travis documetnation in general? I'd obviously like to cut down on bad patch submissions, so having a test suite batter a patch before wasting reveiwer time would be great. We could/should probably add this to the README as well if a proper standard workflow is developed.
Thanks,
Dan
Signature · 3 lines
Respectfully, William C Roberts
Quoted 161 lines · William Roberts
Quoted 149 lines · William Roberts
Quoted 128 lines · Dan Cashman
Quoted 127 lines · William Roberts
Quoted 124 lines · Nicolas Iooss
Quoted 119 lines · Stephen Smalley
Quoted 101 lines · Stephen Smalley
Quoted 84 lines · William Roberts
On Thu, Oct 19, 2017 at 7:26 AM, Stephen Smalley <[email protected]
Quoted 2 lines
Quoted 79 lines · wrote:
Quoted 52 lines · Daniel Cashman
[...]
libselinux/src/label_file.c +21
-7@@ -709,28 +709,61 @@ static int init(struct selabel_handle *rec, const struct selinux_opt *opts, unsigned n) {struct saved_data *data = (struct saved_data *)rec->data; -const char *path =NULL ; +size_t num_paths =0 ; +char **path =NULL ;const char *prefix =NULL ; -int status =-1 , baseonly =0 ; +int status =-1 ; +size_t i; +bool baseonly = false; +bool path_provided;/* Process arguments */ -while (n--) -switch (opts[n].type) { + i = n; +while (i--) +switch (opts[i].type) {case SELABEL_OPT_PATH : - path = opts[n].value; + num_paths++;break ;case SELABEL_OPT_SUBSET : - prefix = opts[n].value; + prefix = opts[i].value;break ;case SELABEL_OPT_BASEONLY : - baseonly = !!opts[n].value; + baseonly = !!opts[i].value;break ; } +if (!num_paths) { + num_paths =1 ; + path_provided = false; + }else { + path_provided = true; + } + + path =calloc (num_paths,sizeof (*path)); +if (path ==NULL ) {libselinux/src/label_file.c static int init(struct selabel_handle · line 755
Quoted 10 lines
libselinux/src/label_file.c +10
-0+ goto finish; + } + rec->spec_files = path; + rec->spec_files_len = num_paths; + +if (path_provided) { +for (i =0 ; i < n; i++) { +switch (opts[i].type) { +case SELABEL_OPT_PATH : + *path =strdup (opts[i].value);Perhaps surprisingly, opts[i].value can be NULL here and some callers
rely on that. After applying your patch, coreutils,selabel_lookup,
and other userspace programs all seg fault as a result. The use case
is programs where theselinux_optstructure is declared with aSELABEL_OPT_PATHentry whose value is subsequently filled in, and left
NULL if they want to use the default path forfile_contexts. Internally, libselinux also does this from thematchpathcon_init_prefix()function.Quoted 7 lines
Quoted 2 lines
In any event, you need to handle this case.
Poor Stephen has turned into your test bot :-)
Does any of the test suite exercise this? Maybe make a PR on github first to get Travis to test these?
Unfortunately the travis-ci tests don't catch this one currently.
Quoted 2 lines
Stephen can you share with how your testing this so Dan can follow suit?
Quoted 5 lines
In this case, you could build and install to a private directory as per
the README, then set your LD_LIBRARY_PATH=~/obj/lib or whatever and runselabel_lookup.Ala:
$ make DESTDIR=~/obj install $ LD_LIBRARY_PATH=~/obj/lib selabel_lookup -b file -k /etc Segmentation fault (core dumped)BTW, I guess this exposes another issue with the patch; there is no support for exercising the new code included with it, e.g. an extension to
utils/selabel_lookup.cto permit specifying multiple input files via multiple -f options. Otherwise, we have no upstream way of testing it.Quoted 14 lines
Quoted 6 lines
Quoted 6 lines
For
selabel_lookup, it would be possible to write a test script (or a CUnit-based test program like libsepol and libsemanage have) which does not need an SELinux policy to be installed on the system, thanks to option -f.
Nevertheless other parts of libselinux (and programs) can not currently be tested on a system without SELinux, as/etc/selinuxandQuoted 8 lines
Quoted 8 lines
/sys/fs/selinuxare needed. If we want to automatize the testing of such parts in a CI like Travis-CI, I suppose we would need to run a virtual machine with its own filesystem, a kernel with SELinux enabled, and some programs like coreutils compiled with the tested version of libselinux/libsepol/... As implementing such an idea may take quite a long time, would there be simpler ways to perform automatic testing of libselinux? Perhaps using another continuous integration system?We have travis up and running, but it's not excising this code path. Any PR submitted to the selinux project on githib has travis "test" it via the .travis.yml
file in the root of the tree.Quoted 6 lines
If you have a fork of selinux on github you could set up travis to run it. It's pretty simple:
https://docs.travis-ci.com/user/getting-started/You could also just submit a PR for testing and publish a patch to the list when you're ready for review...
My response might not be very clear, there are three things you need to do here:
1. Add a test as outlined previously in this thread
Quoted 8 lines
2. Test your change:
Test your change with travis (optional):
1. Setting it up on your fork of selinux project on github (hardest)
2. Submitting a PR to the selinux github project (easiest)If its part of the test suite, you can run that by hand....
3. Submit patch to mailing list for review.
Ok, I'll look into that a bit when I get around to attempting v4. Option 2 should suffice.
Quoted 4 lines
I know you tested all these on Android, but I think we need it as part of the test framework.,,
Yes, this patch has clearly illustrated that android testing is insufficient for upstream use-cases. That's why I'd like to eventually have the upstream tests running as part of android builds and also why I'm hoping to establish some sort of upstreaming workflow beyond "fire away and let sds@ catch issues."
Thanks!
Dan
Quoted 32 lines
Quoted 29 lines
Quoted 29 lines
Quoted 6 lines
The free travis just provides an Ubuntu image.... if you use sudo: true you get a full VM if you don't, you get a docker container.
Quoted 13 lines
With that said, we could try to hack up the VM image to support what we need,
or perhaps we can use CMockaand mock out the filesystem calls to proc/selinuxfs
with something that replicates it.(not sure if that's how mock objects work)Quoted 4 lines
Cheers,
NicolasIs there selinux-specific travis documentation, or a pointer to appropriate travis documetnation in general? I'd obviously like to cut down on bad patch submissions, so having a test suite batter a patch before wasting reveiwer time would be great. We could/should probably add this to the README as well if a proper standard workflow is developed.
Thanks,
Dan