On Oct 27, 2015 10:52 AM, "Stephen Smalley" <sds@xxxxxxxxxxxxx> wrote:
>
> On 10/26/2015 02:42 PM, Roberts, William C wrote:
>>
>> Shouldn't;
>> compat_validate(rec, &spec_arr[nspec].lr, path, lineno);
>>
>> in process_line() cause a failure? Right now the return code is being ignored.
>
>
> I think it is historical. Originally we had it bail on error. Red Hat had problems with that because they would ship a file_contexts file that had an invalid context or the user would have one bad entry in file_contexts.local or a local module (maybe due to a policy change that removed a type, so it might have been valid originally and then became invalid), and suddenly restorecon/setfiles/rpm/... couldn't label anything at all, leading to unlabeled files and possibly an unbootable system. So then we took the error handling to the validate callback, where setfiles or sefcontext_compile can make it a fatal error but you can still label most files (the ones that match valid entries) when they get installed via rpm or using restorecon.
Well it would be nice to reconcile this with Android. What's the plan there? Runtime option, ifdef, etc?
What's the plan on removing backends unused on Android?
>
>
>>
>> Bill
>>
>>> -----Original Message-----
>>> From: Roberts, William C
>>> Sent: Monday, October 26, 2015 11:33 AM
>>> To: selinux@xxxxxxxxxxxxx
>>> Cc: sds@xxxxxxxxxxxxx; Roberts, William C
>>> Subject: [PATCH] fix memory leaks and uninitialized jump
>>>
>>> From: William Roberts <william.c.roberts@xxxxxxxxx>
>>>
>>> Some error's were reported by valgrind (below) fix them. The test
>>> cases on which these leaks were detected:
>>>
>>> 1. properly formed file_contexts file.
>>> 2. malformed file_contexts file, unknown type.
>>> 3. malformed file_contexts file, type that fails on validate callback.
>>> 4. malformed file_contexts file, invalid regex.
>>> 5. malformed file_contexts file, invalid mode.
>>>
>>> ==3819== Conditional jump or move depends on uninitialised value(s)
>>> ==3819== at 0x12A682: closef (label_file.c:577)
>>> ==3819== by 0x12A196: selabel_close (label.c:163)
>>> ==3819== by 0x10A2FD: cleanup (checkfc.c:218)
>>> ==3819== by 0x5089258: __run_exit_handlers (exit.c:82)
>>> ==3819== by 0x50892A4: exit (exit.c:104)
>>> ==3819== by 0x10A231: main (checkfc.c:361)
>>> ==3819== Uninitialised value was created by a heap allocation
>>> ==3819== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-
>>> amd64-linux.so)
>>> ==3819== by 0x4C2CF1F: realloc (in /usr/lib/valgrind/vgpreload_memcheck-
>>> amd64-linux.so)
>>> ==3819== by 0x12BB31: process_file (label_file.h:273)
>>> ==3819== by 0x12A2BA: selabel_file_init (label_file.c:522)
>>> ==3819== by 0x12A0BB: selabel_open (label.c:88)
>>> ==3819== by 0x10A038: main (checkfc.c:292)
>>> ==3819==
>>> ==3819==
>>> ==3819== HEAP SUMMARY:
>>> ==3819== in use at exit: 729 bytes in 19 blocks
>>> ==3819== total heap usage: 21,126 allocs, 21,107 frees, 923,854 bytes allocated
>>> ==3819==
>>> ==3819== 81 bytes in 1 blocks are definitely lost in loss record 1 of 2
>>> ==3819== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-
>>> amd64-linux.so)
>>> ==3819== by 0x50D5839: strdup (strdup.c:42)
>>> ==3819== by 0x12A2A6: selabel_file_init (label_file.c:517)
>>> ==3819== by 0x12A0BB: selabel_open (label.c:88)
>>> ==3819== by 0x10A038: main (checkfc.c:292)
>>> ==3819==
>>>
>>> ==4238== 40 bytes in 1 blocks are definitely lost in loss record 1 of 6
>>> ==4238== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-
>>> amd64-linux.so)
>>> ==4238== by 0x12A1D2: selabel_file_init (label_file.c:886)
>>> ==4238== by 0x12A0BB: selabel_open (label.c:88)
>>> ==4238== by 0x10A038: main (checkfc.c:292)
>>> ==4238==
>>> ==4238== 81 bytes in 1 blocks are definitely lost in loss record 2 of 6
>>> ==4238== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-
>>> amd64-linux.so)
>>> ==4238== by 0x50D5839: strdup (strdup.c:42)
>>> ==4238== by 0x12A2A6: selabel_file_init (label_file.c:517)
>>> ==4238== by 0x12A0BB: selabel_open (label.c:88)
>>> ==4238== by 0x10A038: main (checkfc.c:292)
>>> ==4238==
>>> ==4238== 386 bytes in 24 blocks are definitely lost in loss record 3 of 6
>>> ==4238== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-
>>> amd64-linux.so)
>>> ==4238== by 0x50D5889: strndup (strndup.c:45)
>>> ==4238== by 0x12CDDF: read_spec_entries (label_support.c:37)
>>> ==4238== by 0x12B72D: process_file (label_file.h:392)
>>> ==4238== by 0x12A2BA: selabel_file_init (label_file.c:522)
>>> ==4238== by 0x12A0BB: selabel_open (label.c:88)
>>> ==4238== by 0x10A038: main (checkfc.c:292)
>>> ==4238==
>>> ==4238== 648 bytes in 18 blocks are definitely lost in loss record 4 of 6
>>> ==4238== at 0x4C2CC70: calloc (in /usr/lib/valgrind/vgpreload_memcheck-
>>> amd64-linux.so)
>>> ==4238== by 0x117C9B: avtab_insert_node (avtab.c:105)
>>> ==4238== by 0x117C10: avtab_insert (avtab.c:163)
>>> ==4238== by 0x11880A: avtab_read_item (avtab.c:566)
>>> ==4238== by 0x118BD3: avtab_read (avtab.c:600)
>>> ==4238== by 0x125BDD: policydb_read (policydb.c:3854)
>>> ==4238== by 0x109F87: main (checkfc.c:273)
>>> ==4238==
>>> ==4238== 1,095 bytes in 12 blocks are definitely lost in loss record 5 of 6
>>> ==4238== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-
>>> amd64-linux.so)
>>> ==4238== by 0x12D8D1: pcre_compile2 (pcre_compile.c:9217)
>>> ==4238== by 0x12B239: compile_regex (label_file.h:357)
>>> ==4238== by 0x12B9C7: process_file (label_file.h:429)
>>> ==4238== by 0x12A2BA: selabel_file_init (label_file.c:522)
>>> ==4238== by 0x12A0BB: selabel_open (label.c:88)
>>> ==4238== by 0x10A038: main (checkfc.c:292)
>>> ==4238==
>>> ==4238== 1,296 bytes in 12 blocks are definitely lost in loss record 6 of 6
>>> ==4238== at 0x4C2AB80: malloc (in /usr/lib/valgrind/vgpreload_memcheck-
>>> amd64-linux.so)
>>> ==4238== by 0x13EBE5: pcre_study (pcre_study.c:1565)
>>> ==4238== by 0x12B25D: compile_regex (label_file.h:366)
>>> ==4238== by 0x12B9C7: process_file (label_file.h:429)
>>> ==4238== by 0x12A2BA: selabel_file_init (label_file.c:522)
>>> ==4238== by 0x12A0BB: selabel_open (label.c:88)
>>> ==4238== by 0x10A038: main (checkfc.c:292)
>>>
>>> Signed-off-by: William Roberts <william.c.roberts@xxxxxxxxx>
>>> ---
>>> libselinux/src/label.c | 1 +
>>> libselinux/src/label_file.c | 5 ++++-
>>> libselinux/src/label_file.h | 22 ++++++++++++++--------
>>> 3 files changed, 19 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/libselinux/src/label.c b/libselinux/src/label.c index c656fda..963bfcb
>>> 100644
>>> --- a/libselinux/src/label.c
>>> +++ b/libselinux/src/label.c
>>> @@ -338,6 +338,7 @@ 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);
>>> rec = NULL;
>>> }
>>> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c index
>>> 1a0c15f..071d902 100644
>>> --- a/libselinux/src/label_file.c
>>> +++ b/libselinux/src/label_file.c
>>> @@ -507,6 +507,8 @@ out:
>>> return rc;
>>> }
>>>
>>> +static void closef(struct selabel_handle *rec);
>>> +
>>> static int init(struct selabel_handle *rec, const struct selinux_opt *opts,
>>> unsigned n)
>>> {
>>> @@ -580,7 +582,8 @@ static int init(struct selabel_handle *rec, const struct
>>> selinux_opt *opts,
>>>
>>> finish:
>>> if (status)
>>> - free(data->spec_arr);
>>> + closef(rec);
>>> +
>>> return status;
>>> }
>>>
>>> diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h index
>>> 2492826..a85b9bd 100644
>>> --- a/libselinux/src/label_file.h
>>> +++ b/libselinux/src/label_file.h
>>> @@ -278,6 +278,7 @@ static inline int store_stem(struct saved_data *data, char
>>> *buf, int stem_len)
>>> }
>>> data->stem_arr[num].len = stem_len;
>>> data->stem_arr[num].buf = buf;
>>> + data->stem_arr[num].from_mmap = 0;
>>> data->num_stems++;
>>>
>>> return num;
>>> @@ -425,6 +426,19 @@ static inline int process_line(struct selabel_handle *rec,
>>> /* process and store the specification in spec. */
>>> spec_arr[nspec].stem_id = find_stem_from_spec(data, regex);
>>> spec_arr[nspec].regex_str = regex;
>>> +
>>> + /* Convert the type string to a mode format */
>>> + spec_arr[nspec].type_str = type;
>>> + spec_arr[nspec].mode = 0;
>>> +
>>> + spec_arr[nspec].lr.ctx_raw = context;
>>> +
>>> + /*
>>> + * bump data->nspecs to cause closef() to cover it in its free
>>> + * but do not bump nspec since it's used below.
>>> + */
>>> + data->nspec++;
>>> +
>>> if (rec->validating &&
>>> compile_regex(data, &spec_arr[nspec], &errbuf)) {
>>> COMPAT_LOG(SELINUX_ERROR,
>>> @@ -435,9 +449,6 @@ static inline int process_line(struct selabel_handle *rec,
>>> return -1;
>>> }
>>>
>>> - /* Convert the type string to a mode format */
>>> - spec_arr[nspec].type_str = type;
>>> - spec_arr[nspec].mode = 0;
>>> if (type) {
>>> mode_t mode = string_to_mode(type);
>>>
>>> @@ -445,14 +456,11 @@ static inline int process_line(struct selabel_handle *rec,
>>> COMPAT_LOG(SELINUX_ERROR,
>>> "%s: line %u has invalid file type %s\n",
>>> path, lineno, type);
>>> - errno = EINVAL;
>>> return -1;
>>> }
>>> spec_arr[nspec].mode = mode;
>>> }
>>>
>>> - spec_arr[nspec].lr.ctx_raw = context;
>>> -
>>> /* Determine if specification has
>>> * any meta characters in the RE */
>>> spec_hasMetaChars(&spec_arr[nspec]);
>>> @@ -460,8 +468,6 @@ static inline int process_line(struct selabel_handle *rec,
>>> if (strcmp(context, "<<none>>") && rec->validating)
>>> compat_validate(rec, &spec_arr[nspec].lr, path, lineno);
>>>
>>> - data->nspec = ++nspec;
>>> -
>>> return 0;
>>> }
>>>
>>> --
>>> 1.9.1
>>
>>
>>
>> _______________________________________________
>> Selinux mailing list
>> Selinux@xxxxxxxxxxxxx
>> To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
>> To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
>>
>
> _______________________________________________
> Selinux mailing list
> Selinux@xxxxxxxxxxxxx
> To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx.
> To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
_______________________________________________ Selinux mailing list Selinux@xxxxxxxxxxxxx To unsubscribe, send email to Selinux-leave@xxxxxxxxxxxxx. To get help, send an email containing "help" to Selinux-request@xxxxxxxxxxxxx.
