[PATCH] posix: Fix double-free after allocation failure in regcomp (bug 33185)
Andreas K. Huettel
dilfridge@gentoo.org
Mon Jul 21 19:17:29 GMT 2025
Am Samstag, 19. Juli 2025, 17:23:38 Mitteleuropäische Sommerzeit schrieb Florian Weimer:
> If a memory allocation failure occurs during bracket expression
> parsing in regcomp, a double-free error may result.
>
> Reported-by: Anastasia Belova <abelova@astralinux.ru>
> Co-authored-by: Paul Eggert <eggert@cs.ucla.edu>
>
Reviewed-by: Andreas K. Huettel <dilfridge@gentoo.org>
> ---
> posix/Makefile | 1 +
> posix/regcomp.c | 4 +-
> posix/tst-regcomp-bracket-free.c | 176 +++++++++++++++++++++++++++++++++++++++
> 3 files changed, 180 insertions(+), 1 deletion(-)
>
> diff --git a/posix/Makefile b/posix/Makefile
> index 36b8b14c46..a36e5decd3 100644
> --- a/posix/Makefile
> +++ b/posix/Makefile
> @@ -304,6 +304,7 @@ tests := \
> tst-posix_spawn-setsid \
> tst-preadwrite \
> tst-preadwrite64 \
> + tst-regcomp-bracket-free \
> tst-regcomp-truncated \
> tst-regex \
> tst-regex2 \
OK, add a test to the makefile
> diff --git a/posix/regcomp.c b/posix/regcomp.c
> index 32043e9d37..f7278bb852 100644
> --- a/posix/regcomp.c
> +++ b/posix/regcomp.c
> @@ -3387,6 +3387,7 @@ parse_bracket_exp (re_string_t *regexp, re_dfa_t *dfa, re_token_t *token,
> {
> #ifdef RE_ENABLE_I18N
> free_charset (mbcset);
> + mbcset = NULL;
OK, set multibyte character set mbcset pointer to NULL after free
Above code is called when the condition
3347 if (mbcset->nmbchars || mbcset->ncoll_syms || mbcset->nequiv_classes
3348 || mbcset->nranges || (dfa->mb_cur_max > 1 && (mbcset->nchar_classes
3349 || mbcset->non_match)))
is NOT met, i.e. the charset does not contain anything?
> #endif
> /* Build a tree for simple bracket. */
> br_token.type = SIMPLE_BRACKET;
> @@ -3402,7 +3403,8 @@ parse_bracket_exp (re_string_t *regexp, re_dfa_t *dfa, re_token_t *token,
> parse_bracket_exp_free_return:
> re_free (sbcset);
> #ifdef RE_ENABLE_I18N
> - free_charset (mbcset);
> + if (__glibc_likely (mbcset != NULL))
> + free_charset (mbcset);
OK, ... and dont free it again if it's already NULL
> #endif /* RE_ENABLE_I18N */
> return NULL;
> }
> diff --git a/posix/tst-regcomp-bracket-free.c b/posix/tst-regcomp-bracket-free.c
> new file mode 100644
> index 0000000000..3c091d8c44
> --- /dev/null
> +++ b/posix/tst-regcomp-bracket-free.c
OK, add a test case
> @@ -0,0 +1,176 @@
> +/* Test regcomp bracket parsing with injected allocation failures (bug 33185).
> + Copyright (C) 2025 Free Software Foundation, Inc.
> + This file is part of the GNU C Library.
> +
> + The GNU C Library is free software; you can redistribute it and/or
> + modify it under the terms of the GNU Lesser General Public
> + License as published by the Free Software Foundation; either
> + version 2.1 of the License, or (at your option) any later version.
> +
> + The GNU C Library is distributed in the hope that it will be useful,
> + but WITHOUT ANY WARRANTY; without even the implied warranty of
> + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> + Lesser General Public License for more details.
> +
> + You should have received a copy of the GNU Lesser General Public
> + License along with the GNU C Library; if not, see
> + <https://www.gnu.org/licenses/>. */
> +
> +/* This test invokes regcomp multiple times, failing one memory
> + allocation in each call. The function call should fail with
> + REG_ESPACE (or succeed if it can recover from the allocation
> + failure). Previously, there was double-free bug. */
> +
> +#include <errno.h>
> +#include <regex.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <support/check.h>
> +#include <support/namespace.h>
> +#include <support/support.h>
> +
> +/* Data structure allocated via MAP_SHARED, so that writes from the
> + subprocess are visible. */
> +struct shared_data
> +{
> + /* Number of tracked allocations performed so far. */
> + volatile unsigned int allocation_count;
> +
> + /* If this number is reached, one allocation fails. */
> + volatile unsigned int failing_allocation;
> +
> + /* The subprocess stores the expected name here. */
> + char name[100];
> +};
> +
> +/* Allocation count in shared mapping. */
> +static struct shared_data *shared;
OK, keep track of allocations and when to fail across threads
> +
> +/* Returns true if a failure should be injected for this allocation. */
> +static bool
> +fail_this_allocation (void)
> +{
> + if (shared != NULL)
> + {
> + unsigned int count = shared->allocation_count;
> + shared->allocation_count = count + 1;
> + return count == shared->failing_allocation;
> + }
> + else
> + return false;
> +}
OK, access the shared struct, get and increase number of allocations,
return true if it reached the point where to fail
> +
> +/* Failure-injecting wrappers for allocation functions used by glibc. */
> +
> +void *
> +malloc (size_t size)
> +{
> + if (fail_this_allocation ())
> + {
> + errno = ENOMEM;
> + return NULL;
> + }
> + extern __typeof (malloc) __libc_malloc;
> + return __libc_malloc (size);
> +}
> +
> +void *
> +calloc (size_t a, size_t b)
> +{
> + if (fail_this_allocation ())
> + {
> + errno = ENOMEM;
> + return NULL;
> + }
> + extern __typeof (calloc) __libc_calloc;
> + return __libc_calloc (a, b);
> +}
> +
> +void *
> +realloc (void *ptr, size_t size)
> +{
> + if (fail_this_allocation ())
> + {
> + errno = ENOMEM;
> + return NULL;
> + }
> + extern __typeof (realloc) __libc_realloc;
> + return __libc_realloc (ptr, size);
> +}
OK, wrappers
> +
> +/* No-op subprocess to verify that support_isolate_in_subprocess does
> + not perform any heap allocations. */
> +static void
> +no_op (void *ignored)
> +{
> +}
OK, verifying that nothing does nothing
> +
> +/* Perform a regcomp call in a subprocess. Used to count its
> + allocations. */
> +static void
> +initialize (void *regexp1)
> +{
> + const char *regexp = regexp1;
> +
> + shared->allocation_count = 0;
> +
> + regex_t reg;
> + TEST_COMPARE (regcomp (®, regexp, 0), 0);
> +}
> +
> +/* Perform regcomp in a subprocess with fault injection. */
> +static void
> +test_in_subprocess (void *regexp1)
> +{
> + const char *regexp = regexp1;
> + unsigned int inject_at = shared->failing_allocation;
> +
> + regex_t reg;
> + int ret = regcomp (®, regexp, 0);
> +
> + if (ret != 0)
> + {
> + TEST_COMPARE (ret, REG_ESPACE);
> + printf ("info: allocation %u failure results in return value %d,"
> + " error %s (%d)\n",
> + inject_at, ret, strerrorname_np (errno), errno);
> + }
> +}
> +
OK, we make a lot of regcomp calls and ignore "proper errors"
> +static int
> +do_test (void)
> +{
> + char regexp[] = "[:alpha:]";
> +
> + shared = support_shared_allocate (sizeof (*shared));
> +
> + /* Disable fault injection. */
> + shared->failing_allocation = ~0U;
> +
> + support_isolate_in_subprocess (no_op, NULL);
> + TEST_COMPARE (shared->allocation_count, 0);
OK, ensure no surprises happen.
Dumb question, what ensures code outside this file calls the wrappers here?
I'm probably just missing a level of indirection.
> +
> + support_isolate_in_subprocess (initialize, regexp);
> +
> + /* The number of allocations in the successful case, plus some
> + slack. Once the number of expected allocations is exceeded,
> + injecting further failures does not make a difference. */
> + unsigned int maximum_allocation_count = shared->allocation_count;
> + printf ("info: successful call performs %u allocations\n",
> + maximum_allocation_count);
> + maximum_allocation_count += 10;
OK
> +
> + for (unsigned int inject_at = 0; inject_at <= maximum_allocation_count;
> + ++inject_at)
> + {
> + shared->allocation_count = 0;
> + shared->failing_allocation = inject_at;
> + support_isolate_in_subprocess (test_in_subprocess, regexp);
> + }
OK
on doublefree the subprocess aborts with __libc_message, which makes the test fail
> +
> + support_shared_free (shared);
> +
> + return 0;
> +}
> +
> +#include <support/test-driver.c>
>
> base-commit: 01196393c257c59f63e0e14fa1bfe8d2a699bf2d
>
>
--
Andreas K. Hüttel
dilfridge@gentoo.org
Gentoo Linux developer
(council, comrel, toolchain, base-system, perl, libreoffice)
https://wiki.gentoo.org/wiki/User:Dilfridge
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://sourceware.org/pipermail/libc-alpha/attachments/20250721/57b9e4fe/attachment.sig>
More information about the Libc-alpha
mailing list