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

On Fri, Jul 24, 2026 at 9:43 AM Stephen Smalley
<[email protected]> wrote:
>
> 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]>

Merged with code formatting fixed.

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