On Fri, Sep 16, 2016 at 3:13 AM, Janis Danisevskis <jdanis@xxxxxxxxxx> wrote:
> 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.roberts@xxxxxxxxx>
> wrote:
>>
>> On Thu, Sep 15, 2016 at 7:57 AM, Stephen Smalley <sds@xxxxxxxxxxxxx>
>> wrote:
>> > On 09/15/2016 10:04 AM, Janis Danisevskis wrote:
>> >> From: Janis Danisevskis <jdanis@xxxxxxxxxx>
>> >>
>> >> 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.
We probably would want to encode the architecture since this matters.This way
we can detect a mismatch and compile them. This,prevents the
why isn't my device booting issue for cross compilations that don't have -r.
The other thing is, this is only an issue on binary formated file_context files,
this is the ideal reason to move back to text files, since their is no speedup
gained if we have to compile them.
>
>>
>>
>> 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.
Once you dump the pre-compiled regex's from the binary format,
your back to the text files performance. Also, with PCRE, the binary format was
3x the size on disk.
>
> 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 <jdanis@xxxxxxxxxx>
>> >> ---
>> >
>> >> 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@xxxxxxxxxxxxx
>> > To unsubscribe, send email to Seandroid-list-leave@xxxxxxxxxxxxx.
>> > To get help, send an email containing "help" to
>> > Seandroid-list-request@xxxxxxxxxxxxx.
>>
>>
>>
>> --
>> Respectfully,
>>
>> William C Roberts
>> _______________________________________________
>> 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.
--
Respectfully,
William C Roberts
_______________________________________________
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.
