First of all, I would like to thank you, Stephen and William, for your
patience and support.

On Thu, Sep 15, 2016 at 8:34 PM William Roberts <bill.c.robe...@gmail.com>
wrote:

On Thu, Sep 15, 2016 at 7:57 AM, Stephen Smalley <s...@tycho.nsa.gov> wrote:
> On 09/15/2016 10:04 AM, Janis Danisevskis wrote:
>> From: Janis Danisevskis <jda...@google.com>
>>
>> This patch moves all pcre1/2 dependencies into the new files regex.h
>> and regex.c implementing the common denominator of features needed
>> by libselinux. The compiler flag -DUSE_PCRE2 toggles between the
>> used implementations.
>>
>> As of this patch libselinux supports either pcre or pcre2 but not
>> both at the same time. The persistently stored file contexts
>> information differs. This means libselinux can only load file
>> context files generated by sefcontext_compile build with the
>> same pcre variant.
>>
>> Also, for pcre2 the persistent format is architecture dependent.
>> Stored precompiled regular expressions can only be used on the
>> same architecture they were generated on. If pcre2 is used,
>> sefcontext_compile now respects the "-r". This flag makes
>> sefcontext_compile include the precompiled regular expressions
>> in the output file. The default is to omit them, so that the
>> output remains portable at the cost of having to recompile
>> the regular expressions at load time, or rather on first use.
>
> Is that really the default behavior you want?

I would make the default include the pre-compiled regex's, if there
is an arch mismatch then ditch the pre-compiled ones and reload
them. I would make it an option to NOT generate pre-compiled
regexes. For the upstream consumers, who compile and deploy
on the same system, this means they don't need to update to add
-r. For android, we just need to update a makefile to include -r to
omit the precompiled regex's.


The arch is currently not encoded in the binary, so a mismatch cannot be
detected. I would sleep better if we left the -r option as is for now,
because it is kind of "use only if you know what you are doing". Also, this
way it does not break the android build process.



In the case of Android, where we cross compile the file_contexts.bin,
does using binary fc's even make sense now? The major reason is that
the binary format loads faster by skipping the regex compilation, we
can no longer skip that with PCRE2 and cross compilation. We should
likely move Android back to text based if this is the case. I have
speedups in the neighborhood of 30% for text file processing in
the pipeline.


I realize that the binary PCRE2 variant is currently much slower than the
PCRE stored regexes. But it still a little faster than the text file
processing. If you can make text file processing 30% faster, than this may
well be an option because serializing pcre2 regexes will probably never
beat pcre, even if we did stunts like compiling for the correct
architecture using qemu.
I'd be happy to help perform benchmarks and choose the best option.

Also I am exploring another option, which may sound a bit exotic. So I
would like to have a running POC first before I come forward. Maybe nothing
comes of it.



>
>> Signed-off-by: Janis Danisevskis <jda...@google.com>
>> ---
>
>> diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h
>> index 6d1e890..3df7972 100644
>> --- a/libselinux/src/label_file.h
>> +++ b/libselinux/src/label_file.h
>> @@ -453,12 +429,14 @@ static inline int process_line(struct
selabel_handle *rec,
>>        */
>>       data->nspec++;
>>
>> -     if (rec->validating &&
>> -                         compile_regex(data, &spec_arr[nspec],
&errbuf)) {
>> +     if (rec->validating
>> +                     && compile_regex(data, &spec_arr[nspec],
&error_data)) {
>> +             regex_format_error(&error_data, regex_error_format_buffer,
>> +                             sizeof(regex_error_format_buffer));
>>               COMPAT_LOG(SELINUX_ERROR,
>>                          "%s:  line %u has invalid regex %s:  %s\n",
>>                          path, lineno, regex,
>> -                        (errbuf ? errbuf : "out of memory"));
>> +                        regex_error_format_buffer);
>
> compile_regex() may fail on an out of memory condition before
> regex_error_format_buffer is initialized, which is why we previously
> passed errbuf ?: "out of memory" above.  I suppose you could initialize
> regex_error_format_buffer with "out of memory" prior to calling
> compile_regex().
>
>> diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
>> new file mode 100644
>> index 0000000..1c4a84d
>> --- /dev/null
>> +++ b/libselinux/src/regex.c
> <snip>
>> +int regex_writef(struct regex_data *regex, FILE *fp)
>
> This needs to be updated with the new do_write_precompregex argument,
> and either use the argument or mark it unused to permit compilation for
> the USE_PCRE2=n.
>
> _______________________________________________
> Seandroid-list mailing list
> Seandroid-list@tycho.nsa.gov
> To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
> To get help, send an email containing "help" to
seandroid-list-requ...@tycho.nsa.gov.



--
Respectfully,

William C Roberts
_______________________________________________
Selinux mailing list
seli...@tycho.nsa.gov
To unsubscribe, send email to selinux-le...@tycho.nsa.gov.
To get help, send an email containing "help" to
selinux-requ...@tycho.nsa.gov.
_______________________________________________
Seandroid-list mailing list
Seandroid-list@tycho.nsa.gov
To unsubscribe, send email to seandroid-list-le...@tycho.nsa.gov.
To get help, send an email containing "help" to 
seandroid-list-requ...@tycho.nsa.gov.

Reply via email to