[PATCH v3] libselinux: improve performance with pcre matches

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