Re: [PATCH v3] libselinux: improve performance with pcre matches

On Thu, Jul 23, 2026 at 8:54 PM Inseob Kim <[email protected]> wrote:
>
> From: Carlo Marcelo Arenas Belón <[email protected]>
>
> Since 30b3e9d2 (libselinux: Workaround for heap overhead of pcre,
> 2023-01-12), performance of PCRE2 matches has been affected due to
> excessive recreation of the match_data in an attempt to reduce memory
> utilization; instead of a workaround, it would be better to address
> the problem and maybe even improve performance in the process.
>
> The issue is that currently the structure that holds PCRE state has
> both a pcre2_code (which is per pattern) and a pcre2_match_data (which
> is per match), forcing us to add a mutex to prevent multiple matches to
> step on each other.
>
> Lets remove the match_data and the mutex and instead allocate one once
> in a thread independent way that could be used and reused, by extending
> our pthread interface to not only store TLS variables but also retrieve
> them, and then use one of those.
>
> Since we are not interested on the capture groups (if any) lets only
> allocate 1 pair which is all that will be needed and change the logic
> so that a return of 0 (which means the pattern matched but there were
> not enough capture spots) is also considered a match.
>
> This will ensure that the memory use would be bound to the number of
> concurrent matches instead of the number of patterns and therefore
> reduce the impact that recent changes on the way that the frames used
> for matching are allocated have brought since PCRE2 10.41 was released.
>
> For cases where threads are not available, just keep it working in slow
> mode as done before the workaround was reverted.
>
> Signed-off-by: Carlo Marcelo Arenas Belón <[email protected]>
> Signed-off-by: Inseob Kim <[email protected]>

Acked-by: Stephen Smalley <[email protected]>

> ---
> v2:
>  - Fix the dual destructor antipattern by separating the thread-exit
>    destructor (match_data_thread_free) from the library-unload key cleanup
>    (match_data_key_destroy, annotated with __attribute__((destructor))).
>  - Eliminate the data race on match_data_key_initialized by using pthread_once
>    (via __selinux_once) for safe, race-free initialization of match_data_key.
>  - Remove redundant match_data_initialized thread-local variable, making
>    lookups purely based on __selinux_getspecific().
> v3:
>  - Fix typos in the commit message, along with "PCRE2 10.41"
>    clarification
> ---
>  libselinux/src/regex.c            | 105 +++++++++++++++---------------
>  libselinux/src/selinux_internal.h |   4 ++
>  2 files changed, 56 insertions(+), 53 deletions(-)
>
> diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
> index 80b01692..b132b558 100644
> --- a/libselinux/src/regex.c
> +++ b/libselinux/src/regex.c
> @@ -36,6 +36,10 @@
>  static struct regex_data *regex_data_create(void);
>
>  #ifdef USE_PCRE2
> +static pthread_key_t match_data_key;
> +static pthread_once_t match_data_key_once = PTHREAD_ONCE_INIT;
> +static int match_data_key_alloc_failed = 0;
> +static int match_data_key_created = 0;
>  static pthread_once_t once = PTHREAD_ONCE_INIT;
>  static char arch_string_buffer[32];
>
> @@ -71,14 +75,6 @@ const char *regex_arch_string(void)
>
>  struct regex_data {
>         pcre2_code *regex; /* compiled regular expression */
> -#ifndef AGGRESSIVE_FREE_AFTER_REGEX_MATCH
> -       /*
> -        * match data block required for the compiled
> -        * pattern in pcre2
> -        */
> -       pcre2_match_data *match_data;
> -#endif
> -       pthread_mutex_t match_mutex;
>  };
>
>  int regex_prepare_data(struct regex_data **regex, char const *pattern_string,
> @@ -98,13 +94,6 @@ int regex_prepare_data(struct regex_data **regex, char const *pattern_string,
>                 goto err;
>         }
>
> -#ifndef AGGRESSIVE_FREE_AFTER_REGEX_MATCH
> -       (*regex)->match_data =
> -               pcre2_match_data_create_from_pattern((*regex)->regex, NULL);
> -       if (!(*regex)->match_data) {
> -               goto err;
> -       }
> -#endif
>         return 0;
>
>  err:
> @@ -159,13 +148,6 @@ int regex_load_mmap(struct mmap_area *mmap_area, struct regex_data **regex,
>                 if (rc != 1)
>                         goto err;
>
> -#ifndef AGGRESSIVE_FREE_AFTER_REGEX_MATCH
> -               (*regex)->match_data = pcre2_match_data_create_from_pattern(
> -                       (*regex)->regex, NULL);
> -               if (!(*regex)->match_data)
> -                       goto err;
> -#endif
> -
>                 *regex_compiled = true;
>         }
>
> @@ -223,18 +205,32 @@ out:
>         return rc;
>  }
>
> +static void match_data_thread_free(void *ptr)
> +{
> +       pcre2_match_data_free(ptr);
> +}
> +
> +static void match_data_key_init(void)
> +{
> +       if (__selinux_key_create(&match_data_key, match_data_thread_free) == 0)
> +               match_data_key_created = 1;
> +       else
> +               match_data_key_alloc_failed = 1;
> +}
> +
> +static void __attribute__((destructor)) match_data_key_destroy(void)
> +{
> +       if (match_data_key_created) {
> +               __selinux_key_delete(match_data_key);
> +               match_data_key_created = 0;
> +       }
> +}
> +
>  void regex_data_free(struct regex_data *regex)
>  {
>         if (regex) {
>                 if (regex->regex)
>                         pcre2_code_free(regex->regex);
> -
> -#ifndef AGGRESSIVE_FREE_AFTER_REGEX_MATCH
> -               if (regex->match_data)
> -                       pcre2_match_data_free(regex->match_data);
> -#endif
> -
> -               __pthread_mutex_destroy(&regex->match_mutex);
>                 free(regex);
>         }
>  }
> @@ -242,31 +238,38 @@ void regex_data_free(struct regex_data *regex)
>  int regex_match(struct regex_data *regex, char const *subject, int partial)
>  {
>         int rc;
> -       pcre2_match_data *match_data;
> -       __pthread_mutex_lock(&regex->match_mutex);
> +       bool slow;
> +       pcre2_match_data *match_data = NULL;
> +
> +       __selinux_once(match_data_key_once, match_data_key_init);
> +
> +       if (!match_data_key_alloc_failed) {
> +               match_data = __selinux_getspecific(match_data_key);
> +               if (!match_data) {
> +                       match_data = pcre2_match_data_create(1, NULL);
> +                       if (match_data) {
> +                               __selinux_setspecific(match_data_key,
> +                                                       match_data);
> +                       }
> +               }
> +       }
>
> -#ifdef AGGRESSIVE_FREE_AFTER_REGEX_MATCH
> -       match_data = pcre2_match_data_create_from_pattern(regex->regex, NULL);
> -       if (match_data == NULL) {
> -               __pthread_mutex_unlock(&regex->match_mutex);
> -               return REGEX_ERROR;
> +       slow = (match_data_key_alloc_failed || match_data == NULL);
> +       if (slow) {
> +               match_data = pcre2_match_data_create_from_pattern(regex->regex,
> +                                                                       NULL);
> +               if (!match_data)
> +                       return REGEX_ERROR;
>         }
> -#else
> -       match_data = regex->match_data;
> -#endif
>
> -       rc = pcre2_match(regex->regex, (PCRE2_SPTR)subject,
> -                        PCRE2_ZERO_TERMINATED, 0,
> -                        partial ? PCRE2_PARTIAL_SOFT : 0, match_data, NULL);
> +       rc = pcre2_match(
> +           regex->regex, (PCRE2_SPTR)subject, PCRE2_ZERO_TERMINATED, 0,
> +           partial ? PCRE2_PARTIAL_SOFT : 0, match_data, NULL);
>
> -#ifdef AGGRESSIVE_FREE_AFTER_REGEX_MATCH
> -       // pcre2_match allocates heap and it won't be freed until
> -       // pcre2_match_data_free, resulting in heap overhead.
> -       pcre2_match_data_free(match_data);
> -#endif
> +       if (slow)
> +               pcre2_match_data_free(match_data);
>
> -       __pthread_mutex_unlock(&regex->match_mutex);
> -       if (rc > 0)
> +       if (rc >= 0)
>                 return REGEX_MATCH;
>         switch (rc) {
>         case PCRE2_ERROR_PARTIAL:
> @@ -305,10 +308,6 @@ static struct regex_data *regex_data_create(void)
>  {
>         struct regex_data *regex_data =
>                 (struct regex_data *)calloc(1, sizeof(struct regex_data));
> -       if (!regex_data)
> -               return NULL;
> -
> -       __pthread_mutex_init(&regex_data->match_mutex, NULL);
>         return regex_data;
>  }
>
> diff --git a/libselinux/src/selinux_internal.h b/libselinux/src/selinux_internal.h
> index b0ef5a75..5feb5d97 100644
> --- a/libselinux/src/selinux_internal.h
> +++ b/libselinux/src/selinux_internal.h
> @@ -14,6 +14,7 @@ extern size_t selinux_page_size;
>  #pragma weak pthread_key_create
>  #pragma weak pthread_key_delete
>  #pragma weak pthread_setspecific
> +#pragma weak pthread_getspecific
>
>  /* Call handler iff the first call.  */
>  #define __selinux_once(ONCE_CONTROL, INIT_FUNCTION)                     \
> @@ -42,6 +43,9 @@ extern size_t selinux_page_size;
>                         pthread_setspecific(KEY, VALUE); \
>         } while (0)
>
> +#define __selinux_getspecific(KEY) \
> +       (pthread_getspecific != NULL ? pthread_getspecific(KEY) : NULL)
> +
>  /* selabel_lookup() is only thread safe if we're compiled with pthreads */
>
>  #pragma weak pthread_mutex_init
> --
> 2.55.0.229.g6434b31f56-goog
>