On 09/07/2016 02:25 PM, Stephen Smalley wrote:
> On 09/07/2016 04:08 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 dependant.
>> Stored precompiled regular expressions can only be used on the
>> same architecture they were generated on. If pcre2 is used and
>> sefcontext_compile shall generate portable output, it and libselinux
>> must be compiled with -DNO_PERSISTENTLY_STORED_PATTERNS, at the
>> cost of having to recompile the regular expressions at load time.
>>
>> Signed-off-by: Janis Danisevskis <jdanis@xxxxxxxxxx>
>> ---
>> libselinux/Makefile | 13 ++
>> libselinux/src/Makefile | 4 +-
>> libselinux/src/label_file.c | 91 ++------
>> libselinux/src/label_file.h | 54 ++---
>> libselinux/src/regex.c | 405 ++++++++++++++++++++++++++++++++++
>> libselinux/src/regex.h | 168 ++++++++++++++
>> libselinux/utils/Makefile | 4 +-
>> libselinux/utils/sefcontext_compile.c | 53 +----
>> 8 files changed, 637 insertions(+), 155 deletions(-)
>> create mode 100644 libselinux/src/regex.c
>> create mode 100644 libselinux/src/regex.h
>>
>
>> diff --git a/libselinux/src/label_file.c b/libselinux/src/label_file.c
>> index c89bb35..6698624 100644
>> --- a/libselinux/src/label_file.c
>> +++ b/libselinux/src/label_file.c
>> @@ -278,7 +280,11 @@ static int load_mmap(struct selabel_handle *rec, const char *path,
>>
>> spec = &data->spec_arr[data->nspec];
>> spec->from_mmap = 1;
>> +#if defined USE_PCRE2 && defined NO_PERSISTENTLY_STORED_PATTERNS
>> + spec->regcomp = 0;
>> +#else
>> spec->regcomp = 1;
>> +#endif
>
> If we still need this, maybe regex_load_mmap() should take
> &spec->regcomp as an argument and set it internally so that we don't
> need to litter this file with #ifdefs?
>
>> diff --git a/libselinux/src/label_file.h b/libselinux/src/label_file.h
>> index 6d1e890..a2e30e5 100644
>> --- a/libselinux/src/label_file.h
>> +++ b/libselinux/src/label_file.h
>> @@ -394,7 +371,8 @@ static inline int process_line(struct selabel_handle *rec,
>> struct saved_data *data = (struct saved_data *)rec->data;
>> struct spec *spec_arr;
>> unsigned int nspec = data->nspec;
>> - const char *errbuf = NULL;
>> + char const *errbuf;
>> + struct regex_error_data error_data;
>>
>> items = read_spec_entries(line_buf, &errbuf, 3, ®ex, &type, &context);
>> if (items < 0) {
>> @@ -454,7 +432,7 @@ static inline int process_line(struct selabel_handle *rec,
>> data->nspec++;
>>
>> if (rec->validating &&
>> - compile_regex(data, &spec_arr[nspec], &errbuf)) {
>> + compile_regex(data, &spec_arr[nspec], &error_data)) {
>> COMPAT_LOG(SELINUX_ERROR,
>> "%s: line %u has invalid regex %s: %s\n",
>> path, lineno, regex,
>
> On the next line (omitted from the diff) we pass errbuf if set as the
> error string. But your error is hidden in error_data. Looks like we
> need to use regex_format_error() here?
>
>> diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
>> new file mode 100644
>> index 0000000..6b92b04
>> --- /dev/null
>> +++ b/libselinux/src/regex.c
>> +int regex_load_mmap(struct mmap_area * mmap_area, struct regex_data ** regex) {
>> + int rc;
>> + size_t entry_len;
>> +#ifndef USE_PCRE2
>> + size_t info_len;
>> +#endif
>> +
>> + rc = next_entry(&entry_len, mmap_area, sizeof(uint32_t));
>
> This and similar statements are the cause of your uninitialised variable
> use warnings. entry_len needs to be a uint32_t here. size_t is 64 bits
> on 64-bit architectures. Same for info_len.
>
>> +struct regex_data * regex_data_create(void) {
>> + struct regex_data * dummy = (struct regex_data*) malloc(
>> + sizeof(struct regex_data));
>> + if (dummy) {
>> + memset(dummy, 0, sizeof(struct regex_data));
>> + }
>> + return dummy;
>> +}
>> +
>> +void regex_data_free(struct regex_data * regex) {
>> + if (regex) {
>> +#ifdef USE_PCRE2
>> + if (regex->regex) {
>> + pcre2_code_free(regex->regex);
>> + }
>> + if (regex->match_data) {
>> + pcre2_match_data_free(regex->match_data);
>> + }
>> +#else
>> + if (regex->regex)
>> + pcre_free(regex->regex);
>> + if (regex->extra_owned && regex->sd) {
>> + pcre_free_study(regex->sd);
>> + }
>> +#endif
>> + free(regex);
>> + }
>> +}
>
> The reason you are leaking memory is that regex_data_free() is only ever
> called if !spec->from_mmap. The old code in closef() to free the
> compiled regexes was only necessary when the regexes were compiled at
> runtime, but you have introduced a memory allocation for regex_data even
> for the mmap'd file that needs to be freed.
And if you move your regex_data_free() call up, you need to make sure
you don't call any _free() functions on anything other than your
top-level data structure if it is from_mmap.
_______________________________________________
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.
