From 37ed36fc8b6e766824c7a4aab20b866c5b7a3ff1 Mon Sep 17 00:00:00 2001 From: Nick Alcock Date: Fri, 26 Apr 2024 18:19:15 +0100 Subject: libctf: fix leak of entire dict when dict opening fails Ever since commit 1fa7a0c24e78e7f ("libctf: sort out potential refcount loops") ctf_dict_close has only freed anything if the refcount on entry to the function is precisely 1. >1 obviously just decrements the refcount, but the linker machinery can sometimes cause freeing to recurse from a dict to another dict and then back to the first dict again, so we interpret a refcount of 0 as an indication that this is a recursive call and we should just return, because a caller is already freeing this dict. Unfortunately there is one situation in which this is not true: the bad: codepath in ctf_bufopen entered when opening fails. Because the refcount is bumped only at the very end of ctf_bufopen, any failure causes ctf_dict_close to be entered with a refcount of zero, and it frees nothing and we leak the entire dict. The solution is to bump the refcount to 1 right before freeing... but this codepath is clearly delicate enough that we need to properly validate it, so we add a test that uses malloc interposition to count allocations and frees, creates a dict, writes it out, intentionally corrupts it (by setting a bunch of bytes after the header to a value high enough that it is definitely not a valid CTF type kind), then tries to open it again and counts the malloc/free pairs to make sure they're matched. (Test run only on *-linux-gnu, because malloc interposition is not a thing you can rely upon working everywhere, and this test is not arch-dependent so if it passes on one arch it can be assumed to pass on all of them.) libctf/ * ctf-open.c (ctf_bufopen): Bump the refcount on failure. * testsuite/libctf-regression/open-error-free.*: New test. --- libctf/ctf-open.c | 4 + .../testsuite/libctf-regression/open-error-free.c | 185 +++++++++++++++++++++ .../testsuite/libctf-regression/open-error-free.lk | 5 + 3 files changed, 194 insertions(+) create mode 100644 libctf/testsuite/libctf-regression/open-error-free.c create mode 100644 libctf/testsuite/libctf-regression/open-error-free.lk (limited to 'libctf') diff --git a/libctf/ctf-open.c b/libctf/ctf-open.c index 03faf2d..59c6ed0 100644 --- a/libctf/ctf-open.c +++ b/libctf/ctf-open.c @@ -1724,6 +1724,10 @@ ctf_bufopen (const ctf_sect_t *ctfsect, const ctf_sect_t *symsect, bad: ctf_set_open_errno (errp, err); ctf_err_warn_to_open (fp); + /* Without this, the refcnt is zero on entry and ctf_dict_close() won't + actually do anything on the grounds that this is a recursive call via + another dict being closed. */ + fp->ctf_refcnt = 1; ctf_dict_close (fp); return NULL; } diff --git a/libctf/testsuite/libctf-regression/open-error-free.c b/libctf/testsuite/libctf-regression/open-error-free.c new file mode 100644 index 0000000..8319a09 --- /dev/null +++ b/libctf/testsuite/libctf-regression/open-error-free.c @@ -0,0 +1,185 @@ +/* Make sure that, on error, an opened dict is properly freed. */ + +#define _GNU_SOURCE 1 +#include +#include +#include +#include +#include +#include + +static unsigned long long malloc_count; +static unsigned long long free_count; + +static void *(*real_malloc) (size_t size); +static void (*real_free) (void *ptr); +static void *(*real_realloc) (void *ptr, size_t size); +static void *(*real_calloc) (size_t nmemb, size_t size); + +/* Interpose malloc/free functionss and count calls to spot unbalanced ones. + Extra complexity to deal with dlsym() calling dlerror() and thus calloc() -- + luckily it handles malloc failure fine, so we can just always fail before the + hooks are installed. */ + +static int in_hooks; + +static void hook_init (void) +{ + if (!real_calloc) + { + in_hooks = 1; + real_calloc = (void *(*) (size_t, size_t)) dlsym (RTLD_NEXT, "calloc"); + real_malloc = (void *(*) (size_t)) dlsym (RTLD_NEXT, "malloc"); + real_free = (void (*) (void *)) dlsym (RTLD_NEXT, "free"); + real_realloc = (void *(*) (void *, size_t)) dlsym (RTLD_NEXT, "realloc"); + if (!real_malloc || !real_free || !real_realloc || !real_calloc) + { + fprintf (stderr, "Cannot hook malloc\n"); + exit(1); + } + in_hooks = 0; + } +} + +void *malloc (size_t size) +{ + if (in_hooks) + return NULL; + + hook_init(); + malloc_count++; + return real_malloc (size); +} + +void *realloc (void *ptr, size_t size) +{ + void *new_ptr; + + if (in_hooks) + return NULL; + + hook_init(); + new_ptr = real_realloc (ptr, size); + + if (!ptr) + malloc_count++; + + if (size == 0) + free_count++; + + return new_ptr; +} + +void *calloc (size_t nmemb, size_t size) +{ + void *ptr; + + if (in_hooks) + return NULL; + + hook_init(); + ptr = real_calloc (nmemb, size); + + if (ptr) + malloc_count++; + return ptr; +} + +void free (void *ptr) +{ + hook_init(); + + if (in_hooks) + return; + + if (ptr != NULL) + free_count++; + + return real_free (ptr); +} + +int main (void) +{ + ctf_dict_t *fp; + ctf_encoding_t e = { CTF_INT_SIGNED, 0, sizeof (long) }; + int err; + ctf_id_t type; + size_t i; + char *written; + size_t written_size; + char *foo; + ctf_next_t *it = NULL; + unsigned long long frozen_malloc_count, frozen_free_count; + + if ((fp = ctf_create (&err)) == NULL) + goto open_err; + + /* Define an integer, then a pile of unconnected pointers to it, just to + use up space.. */ + + if ((type = ctf_add_integer (fp, CTF_ADD_ROOT, "long", &e)) == CTF_ERR) + goto err; + + for (i = 0; i < 100; i++) + { + if (ctf_add_pointer (fp, CTF_ADD_ROOT, type) == CTF_ERR) + goto err; + } + + /* Write the dict out, uncompressed (to stop it failing to open due to decompression + failure after we corrupt it: the leak is only observable if the dict gets + malloced, which only happens after that point.) */ + + if ((written = ctf_write_mem (fp, &written_size, (size_t) -1)) == NULL) + goto write_err; + + ctf_dict_close (fp); + + /* Corrupt the dict. */ + + memset (written + sizeof (ctf_header_t), 64, 64); + + /* Reset the counters: we are interested only in leaks at open + time. */ + malloc_count = 0; + free_count = 0; + + if ((ctf_simple_open (written, written_size, NULL, 0, 0, NULL, 0, &err)) != NULL) + { + fprintf (stderr, "wildly corrupted dict still opened OK?!\n"); + exit (1); + } + + /* The error log will have accumulated errors which need to be + consumed and freed if they are not to appear as a spurious leak. */ + + while ((foo = ctf_errwarning_next (NULL, &it, NULL, NULL)) != NULL) + free (foo); + + frozen_malloc_count = malloc_count; + frozen_free_count = free_count; + + if (frozen_malloc_count == 0) + fprintf (stderr, "Allocation count after failed open is zero: likely hook failure.\n"); + else if (frozen_malloc_count > frozen_free_count) + fprintf (stderr, "Memory leak is present: %lli allocations (%lli allocations, %lli frees).\n", + frozen_malloc_count - frozen_free_count, frozen_malloc_count, frozen_free_count); + else if (frozen_malloc_count < frozen_free_count) + fprintf (stderr, "Possible double-free: %li allocations, %li frees.\n", + frozen_malloc_count, frozen_free_count); + + printf ("All OK.\n"); + exit (0); + + open_err: + fprintf (stderr, "Cannot open/create: %s\n", ctf_errmsg (err)); + exit (1); + + err: + fprintf (stderr, "Cannot add: %s\n", ctf_errmsg (ctf_errno (fp))); + exit (1); + + write_err: + fprintf (stderr, "Cannot write: %s\n", ctf_errmsg (ctf_errno (fp))); + exit (1); +} diff --git a/libctf/testsuite/libctf-regression/open-error-free.lk b/libctf/testsuite/libctf-regression/open-error-free.lk new file mode 100644 index 0000000..383cfff2 --- /dev/null +++ b/libctf/testsuite/libctf-regression/open-error-free.lk @@ -0,0 +1,5 @@ +# source: pptrtab-a.c +# source: pptrtab-b.c +# host: *-linux-gnu +# lookup-link: -ldl +All OK. -- cgit v1.1