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.
_______________________________________________
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.
