[v3,5/5] avoid -Wuse-after-free [BZ #26779]
Commit Message
On 1/25/22 10:49, Carlos O'Donell wrote:
> On 1/24/22 19:58, Martin Sebor via Libc-alpha wrote:
>> On 1/24/22 17:52, Martin Sebor wrote:
>>> This is a repost of the original patch but broken down by source
>>> file and with some suppression done by #pragma GCC diagnostic
>>> instead of conversion to intptr_t. It also adds fixes for
>>> the same problem in the test suite that I overlooked before.
>>
>> The attached patch suppresses the -Wuse-after-free instance in
>> the testsuite.
>>
>>>
>>> On 1/15/22 17:21, Martin Sebor wrote:
>>>> GCC 12 features a couple of new warnings designed to detect uses
>>>> of pointers made invalid by the pointees lifetimes having ended.
>>>> Building Glibc with the enhanced GCC exposes a few such uses,
>>>> mostly after successful calls to realloc. The attached patch
>>>> avoids the new warnings by converting the pointers to uintptr_t
>>>> first and using the converted integers instead.
>>>>
>>>> The patch suppresses all instances of the warning at the strictest
>>>> setting (-Wuse-after-free=3), which includes even uses in equality
>>>> expressions. The default setting approved for GCC 12 is
>>>> -Wuse-after-free=2, which doesn't warn on such uses to accommodate
>>>> the pointer-adjustment-after-realloc idiom. At the default setting,
>>>> the changes to ldconfig.c and setenv are not necessary.
>>>>
>>>> Martin
>>>
>
> This patch is not ready.
>
> Some tests are going to do invalid things to test specific behaviour and we need
> to possibly suppress those warnings. The malloc tests look correct.
>
> The support/tst-support-open-dev-null-range.c doesn't look correct, please send v3
> of just this *whole* patch as a new patch. I'll review again.
Okay, I've moved the free() call after the FAIL_EXIT. I've also
suppressed the same warning in a few more tests that I missed
before.
Martin
>
>> diff --git a/malloc/tst-malloc-backtrace.c b/malloc/tst-malloc-backtrace.c
>> index ea66da23ef..8a3f4a0b55 100644
>> --- a/malloc/tst-malloc-backtrace.c
>> +++ b/malloc/tst-malloc-backtrace.c
>> @@ -20,6 +20,7 @@
>> #include <stdlib.h>
>>
>> #include <support/support.h>
>> +#include <libc-diag.h>
>
> OK. Add header required for DIAG_* macros.
>
>>
>> #define SIZE 4096
>>
>> @@ -29,7 +30,15 @@ __attribute__((noinline))
>> call_free (void *ptr)
>> {
>> free (ptr);
>> +#if __GNUC_PREREQ (12, 0)
>> + /* Ignore a valid warning about using a pointer made indeterminate
>> + by a prior call to malloc(). */
>> + DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
>> +#endif
>> *(size_t *)(ptr - sizeof (size_t)) = 1;
>> +#if __GNUC_PREREQ (12, 0)
>> + DIAG_POP_NEEDS_COMMENT;
>> +#endif
>
> OK. Specifically testing use-after-free write to chunk to corrupt memory.
>
>> }
>>
>> int
>> diff --git a/malloc/tst-malloc-check.c b/malloc/tst-malloc-check.c
>> index 46938c0dbb..eb46cf3bbb 100644
>> --- a/malloc/tst-malloc-check.c
>> +++ b/malloc/tst-malloc-check.c
>
> OK. Already includes libc-diag.h.
>
>> @@ -86,7 +86,15 @@ do_test (void)
>> merror ("errno is not set correctly.");
>> DIAG_POP_NEEDS_COMMENT;
>>
>> +#if __GNUC_PREREQ (12, 0)
>> + /* Ignore a valid warning about using a pointer made indeterminate
>> + by a prior call to realloc(). */
>> + DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
>> +#endif
>> free (p);
>> +#if __GNUC_PREREQ (12, 0)
>> + DIAG_POP_NEEDS_COMMENT;
>> +#endif
>
> OK. Previous realloc made p indeterminate.
>
>>
>> p = malloc (512);
>> if (p == NULL)
>> @@ -104,7 +112,15 @@ do_test (void)
>> merror ("errno is not set correctly.");
>> DIAG_POP_NEEDS_COMMENT;
>>
>> +#if __GNUC_PREREQ (12, 0)
>> + /* Ignore a valid warning about using a pointer made indeterminate
>> + by a prior call to realloc(). */
>> + DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
>> +#endif
>> free (p);
>> +#if __GNUC_PREREQ (12, 0)
>> + DIAG_POP_NEEDS_COMMENT;
>> +#endif
>
> OK. Likewise.
>
>> free (q);
>>
>> return errors != 0;
>> diff --git a/malloc/tst-malloc-too-large.c b/malloc/tst-malloc-too-large.c
>> index e23aa08e4f..dac3c8086c 100644
>> --- a/malloc/tst-malloc-too-large.c
>> +++ b/malloc/tst-malloc-too-large.c
>
> OK. Already includes libc-diag.h.
>
>> @@ -95,7 +95,15 @@ test_large_allocations (size_t size)
>> DIAG_POP_NEEDS_COMMENT;
>> #endif
>> TEST_VERIFY (errno == ENOMEM);
>> +#if __GNUC_PREREQ (12, 0)
>> + /* Ignore a warning about using a pointer made indeterminate by
>> + a prior call to realloc(). */
>> + DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
>> +#endif
>> free (ptr_to_realloc);
>> +#if __GNUC_PREREQ (12, 0)
>> + DIAG_POP_NEEDS_COMMENT;
>> +#endif
>
> OK.
>
>>
>> for (size_t nmemb = 1; nmemb <= 8; nmemb *= 2)
>> if ((size % nmemb) == 0)
>> @@ -113,14 +121,30 @@ test_large_allocations (size_t size)
>> test_setup ();
>> TEST_VERIFY (reallocarray (ptr_to_realloc, nmemb, size / nmemb) == NULL);
>> TEST_VERIFY (errno == ENOMEM);
>> +#if __GNUC_PREREQ (12, 0)
>> + /* Ignore a warning about using a pointer made indeterminate by
>> + a prior call to realloc(). */
>> + DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
>> +#endif
>> free (ptr_to_realloc);
>> +#if __GNUC_PREREQ (12, 0)
>> + DIAG_POP_NEEDS_COMMENT;
>> +#endif
>
> OK.
>
>>
>> ptr_to_realloc = malloc (16);
>> TEST_VERIFY_EXIT (ptr_to_realloc != NULL);
>> test_setup ();
>> TEST_VERIFY (reallocarray (ptr_to_realloc, size / nmemb, nmemb) == NULL);
>> TEST_VERIFY (errno == ENOMEM);
>> +#if __GNUC_PREREQ (12, 0)
>> + /* Ignore a warning about using a pointer made indeterminate by
>> + a prior call to realloc(). */
>> + DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
>> +#endif
>> free (ptr_to_realloc);
>> +#if __GNUC_PREREQ (12, 0)
>> + DIAG_POP_NEEDS_COMMENT;
>> +#endif
>
> OK.
>
>> }
>> else
>> break;
>> diff --git a/support/tst-support-open-dev-null-range.c b/support/tst-support-open-dev-null-range.c
>> index 3ed3177d57..e7526597ce 100644
>> --- a/support/tst-support-open-dev-null-range.c
>> +++ b/support/tst-support-open-dev-null-range.c
>> @@ -26,6 +26,8 @@
>> #include <sys/resource.h>
>> #include <stdlib.h>
>>
>> +#include <libc-diag.h>
>
> OK. New macros required.
>
>> +
>> #ifndef PATH_MAX
>> # define PATH_MAX 1024
>> #endif
>> @@ -41,8 +43,18 @@ check_path (int fd)
>> = readlink (proc_fd_path, file_path, sizeof (file_path));
>> free (proc_fd_path);
>> if (file_path_length < 0)
>> - FAIL_EXIT1 ("readlink (%s, %p, %zu)", proc_fd_path, file_path,
>> - sizeof (file_path));
>> + {
>> +#if __GNUC_PREREQ (12, 0)
>> + /* Ignore a valid warning about using a pointer made indeterminate
>> + by a prior call to free(). */
>> + DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
>> +#endif
>> + FAIL_EXIT1 ("readlink (%s, %p, %zu)", proc_fd_path, file_path,
>> + sizeof (file_path));
>> +#if __GNUC_PREREQ (12, 0)
>> + DIAG_POP_NEEDS_COMMENT;
>> +#endif
>> + }
>
> We should move free (proc_fd_path) to after the check to correct the use-after-free.
>
>> file_path[file_path_length] = '\0';
>> TEST_COMPARE_STRING (file_path, "/dev/null");
>> }
>
>
Comments
On 1/25/22 17:50, Martin Sebor wrote:
> On 1/25/22 10:49, Carlos O'Donell wrote:
>> On 1/24/22 19:58, Martin Sebor via Libc-alpha wrote:
>>> On 1/24/22 17:52, Martin Sebor wrote:
>>>> This is a repost of the original patch but broken down by source
>>>> file and with some suppression done by #pragma GCC diagnostic
>>>> instead of conversion to intptr_t. It also adds fixes for
>>>> the same problem in the test suite that I overlooked before.
>>>
>>> The attached patch suppresses the -Wuse-after-free instance in
>>> the testsuite.
>>>
>>>>
>>>> On 1/15/22 17:21, Martin Sebor wrote:
>>>>> GCC 12 features a couple of new warnings designed to detect uses
>>>>> of pointers made invalid by the pointees lifetimes having ended.
>>>>> Building Glibc with the enhanced GCC exposes a few such uses,
>>>>> mostly after successful calls to realloc. The attached patch
>>>>> avoids the new warnings by converting the pointers to uintptr_t
>>>>> first and using the converted integers instead.
>>>>>
>>>>> The patch suppresses all instances of the warning at the strictest
>>>>> setting (-Wuse-after-free=3), which includes even uses in equality
>>>>> expressions. The default setting approved for GCC 12 is
>>>>> -Wuse-after-free=2, which doesn't warn on such uses to accommodate
>>>>> the pointer-adjustment-after-realloc idiom. At the default setting,
>>>>> the changes to ldconfig.c and setenv are not necessary.
>>>>>
>>>>> Martin
>>>>
>>
>> This patch is not ready.
>>
>> Some tests are going to do invalid things to test specific behaviour and we need
>> to possibly suppress those warnings. The malloc tests look correct.
>>
>> The support/tst-support-open-dev-null-range.c doesn't look correct, please send v3
>> of just this *whole* patch as a new patch. I'll review again.
>
> Okay, I've moved the free() call after the FAIL_EXIT. I've also
> suppressed the same warning in a few more tests that I missed
> before.
OK for glibc 2.35 with commit message including bug #.
Please push.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> commit c23cf7ff784186b8094b97a47a8445808145a69c
> Author: Martin Sebor <msebor@redhat.com>
> Date: Tue Jan 25 15:39:38 2022 -0700
>
> Avoid -Wuse-after-free in tests.
>
Suggest:
Avoid -Wuse-after-free in tests [BZ #26779]
> diff --git a/malloc/tst-malloc-backtrace.c b/malloc/tst-malloc-backtrace.c
> index ea66da23ef..65e1a1ffbc 100644
> --- a/malloc/tst-malloc-backtrace.c
> +++ b/malloc/tst-malloc-backtrace.c
> @@ -20,6 +20,7 @@
> #include <stdlib.h>
>
> #include <support/support.h>
> +#include <libc-diag.h>
>
> #define SIZE 4096
>
> @@ -29,7 +30,15 @@ __attribute__((noinline))
> call_free (void *ptr)
> {
> free (ptr);
> +#if __GNUC_PREREQ (12, 0)
> + /* Ignore a valid warning about using a pointer made indeterminate
> + by a prior call to free(). */
> + DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
> +#endif
> *(size_t *)(ptr - sizeof (size_t)) = 1;
> +#if __GNUC_PREREQ (12, 0)
> + DIAG_POP_NEEDS_COMMENT;
> +#endif
OK.
> }
>
> int
> diff --git a/malloc/tst-malloc-check.c b/malloc/tst-malloc-check.c
> index 46938c0dbb..eb46cf3bbb 100644
> --- a/malloc/tst-malloc-check.c
> +++ b/malloc/tst-malloc-check.c
> @@ -86,7 +86,15 @@ do_test (void)
> merror ("errno is not set correctly.");
> DIAG_POP_NEEDS_COMMENT;
>
> +#if __GNUC_PREREQ (12, 0)
> + /* Ignore a valid warning about using a pointer made indeterminate
> + by a prior call to realloc(). */
> + DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
> +#endif
> free (p);
> +#if __GNUC_PREREQ (12, 0)
> + DIAG_POP_NEEDS_COMMENT;
> +#endif
OK.
>
> p = malloc (512);
> if (p == NULL)
> @@ -104,7 +112,15 @@ do_test (void)
> merror ("errno is not set correctly.");
> DIAG_POP_NEEDS_COMMENT;
>
> +#if __GNUC_PREREQ (12, 0)
> + /* Ignore a valid warning about using a pointer made indeterminate
> + by a prior call to realloc(). */
> + DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
> +#endif
> free (p);
> +#if __GNUC_PREREQ (12, 0)
> + DIAG_POP_NEEDS_COMMENT;
> +#endif
OK.
> free (q);
>
> return errors != 0;
> diff --git a/malloc/tst-malloc-too-large.c b/malloc/tst-malloc-too-large.c
> index e23aa08e4f..dac3c8086c 100644
> --- a/malloc/tst-malloc-too-large.c
> +++ b/malloc/tst-malloc-too-large.c
> @@ -95,7 +95,15 @@ test_large_allocations (size_t size)
> DIAG_POP_NEEDS_COMMENT;
> #endif
> TEST_VERIFY (errno == ENOMEM);
> +#if __GNUC_PREREQ (12, 0)
> + /* Ignore a warning about using a pointer made indeterminate by
> + a prior call to realloc(). */
> + DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
> +#endif
> free (ptr_to_realloc);
> +#if __GNUC_PREREQ (12, 0)
> + DIAG_POP_NEEDS_COMMENT;
> +#endif
OK.
>
> for (size_t nmemb = 1; nmemb <= 8; nmemb *= 2)
> if ((size % nmemb) == 0)
> @@ -113,14 +121,30 @@ test_large_allocations (size_t size)
> test_setup ();
> TEST_VERIFY (reallocarray (ptr_to_realloc, nmemb, size / nmemb) == NULL);
> TEST_VERIFY (errno == ENOMEM);
> +#if __GNUC_PREREQ (12, 0)
> + /* Ignore a warning about using a pointer made indeterminate by
> + a prior call to realloc(). */
> + DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
> +#endif
> free (ptr_to_realloc);
> +#if __GNUC_PREREQ (12, 0)
> + DIAG_POP_NEEDS_COMMENT;
> +#endif
OK.
>
> ptr_to_realloc = malloc (16);
> TEST_VERIFY_EXIT (ptr_to_realloc != NULL);
> test_setup ();
> TEST_VERIFY (reallocarray (ptr_to_realloc, size / nmemb, nmemb) == NULL);
> TEST_VERIFY (errno == ENOMEM);
> +#if __GNUC_PREREQ (12, 0)
> + /* Ignore a warning about using a pointer made indeterminate by
> + a prior call to realloc(). */
> + DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
> +#endif
> free (ptr_to_realloc);
> +#if __GNUC_PREREQ (12, 0)
> + DIAG_POP_NEEDS_COMMENT;
> +#endif
OK.
> }
> else
> break;
> diff --git a/malloc/tst-obstack.c b/malloc/tst-obstack.c
> index 18af8ea62f..d80f471fa0 100644
> --- a/malloc/tst-obstack.c
> +++ b/malloc/tst-obstack.c
> @@ -20,8 +20,8 @@ verbose_malloc (size_t size)
> static void
> verbose_free (void *buf)
> {
> - free (buf);
> printf ("free (%p)\n", buf);
> + free (buf);
OK. Correct fix. Thanks.
> }
>
> static int
> diff --git a/malloc/tst-realloc.c b/malloc/tst-realloc.c
> index 83f165516a..8ce59f2602 100644
> --- a/malloc/tst-realloc.c
> +++ b/malloc/tst-realloc.c
> @@ -138,8 +138,16 @@ do_test (void)
> if (ok == 0)
> merror ("first 16 bytes were not correct after failed realloc");
>
> +#if __GNUC_PREREQ (12, 0)
> + /* Ignore a valid warning about using a pointer made indeterminate
> + by a prior call to realloc(). */
> + DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
> +#endif
> /* realloc (p, 0) frees p (C89) and returns NULL (glibc). */
> p = realloc (p, 0);
> +#if __GNUC_PREREQ (12, 0)
> + DIAG_POP_NEEDS_COMMENT;
> +#endif
OK.
> if (p != NULL)
> merror ("realloc (p, 0) returned non-NULL.");
>
> diff --git a/support/tst-support-open-dev-null-range.c b/support/tst-support-open-dev-null-range.c
> index 3ed3177d57..690b9d30b7 100644
> --- a/support/tst-support-open-dev-null-range.c
> +++ b/support/tst-support-open-dev-null-range.c
> @@ -39,10 +39,11 @@ check_path (int fd)
> char file_path[PATH_MAX];
> ssize_t file_path_length
> = readlink (proc_fd_path, file_path, sizeof (file_path));
> - free (proc_fd_path);
> if (file_path_length < 0)
> FAIL_EXIT1 ("readlink (%s, %p, %zu)", proc_fd_path, file_path,
> sizeof (file_path));
> +
> + free (proc_fd_path);
OK. Correct fix. Thanks.
> file_path[file_path_length] = '\0';
> TEST_COMPARE_STRING (file_path, "/dev/null");
> }
Note that there are still -Wuse-after-free build failures in the testsuite
for many 32-bit platforms. E.g., on i686-linux-gnu:
tst-mallocalign1.c: In function 'do_test':
tst-mallocalign1.c:69:1: error: pointer 'p' used after 'free' [-Werror=use-after-free]
69 | }
| ^
tst-mallocalign1.c:42:3: note: call to 'free' here
42 | free (p);
| ^~~~~~~~
Also, the s390x-linux-gnu-O3 build-many-glibcs.py configuration shows
failures:
In function 'do_test',
inlined from 'legacy_test_function' at ../test-skeleton.c:55:10:
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
On 1/28/22 08:10, Joseph Myers wrote:
> Note that there are still -Wuse-after-free build failures in the testsuite
> for many 32-bit platforms. E.g., on i686-linux-gnu:
>
> tst-mallocalign1.c: In function 'do_test':
> tst-mallocalign1.c:69:1: error: pointer 'p' used after 'free' [-Werror=use-after-free]
> 69 | }
> | ^
> tst-mallocalign1.c:42:3: note: call to 'free' here
> 42 | free (p);
> | ^~~~~~~~
>
> Also, the s390x-linux-gnu-O3 build-many-glibcs.py configuration shows
> failures:
>
> In function 'do_test',
> inlined from 'legacy_test_function' at ../test-skeleton.c:55:10:
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
> tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
> 134 | if (c[i] != 0xff)
> | ~^~~
> tst-realloc.c:124:7: note: call to 'realloc' here
> 124 | c = realloc (p, -1);
> | ^~~~~~~~~~~~~~~
>
I'm going to look at these right now. I want clean gcc 12 glibc 2.35 results before
we release.
On Fri, 28 Jan 2022, Carlos O'Donell via Libc-alpha wrote:
> I'm going to look at these right now. I want clean gcc 12 glibc 2.35
> results before we release.
There are also three ICE bugs in GCC 12 preventing glibc from building, so
you won't get fully clean build-many-glibcs.py results with just glibc
fixes. GCC bug 103722 on sh4, bug 104153 on or1k and 104154 on arc (the
last two reported by Jeff based on ICEs building newlib, but the ICEs I
see building glibc are in the same place in the compiler, so probably the
same bugs).
On 1/28/2022 10:51 AM, Joseph Myers wrote:
> On Fri, 28 Jan 2022, Carlos O'Donell via Libc-alpha wrote:
>
>> I'm going to look at these right now. I want clean gcc 12 glibc 2.35
>> results before we release.
> There are also three ICE bugs in GCC 12 preventing glibc from building, so
> you won't get fully clean build-many-glibcs.py results with just glibc
> fixes. GCC bug 103722 on sh4, bug 104153 on or1k and 104154 on arc (the
> last two reported by Jeff based on ICEs building newlib, but the ICEs I
> see building glibc are in the same place in the compiler, so probably the
> same bugs).
FYI, I got a good build with the patch in 104153 + an unrelated newlib
fix on or1k-elf. So I'll take a deeper look at Robin's patch over the
weekend if I can.
Jeff
On 1/28/22 12:51, Joseph Myers wrote:
> On Fri, 28 Jan 2022, Carlos O'Donell via Libc-alpha wrote:
>
>> I'm going to look at these right now. I want clean gcc 12 glibc 2.35
>> results before we release.
>
> There are also three ICE bugs in GCC 12 preventing glibc from building, so
> you won't get fully clean build-many-glibcs.py results with just glibc
> fixes. GCC bug 103722 on sh4, bug 104153 on or1k and 104154 on arc (the
> last two reported by Jeff based on ICEs building newlib, but the ICEs I
> see building glibc are in the same place in the compiler, so probably the
> same bugs).
Thanks.
I just fixed the last -Wuse-after-free bug.
The ICEs are compiler-side fixes and I don't think we'd want to work around them
in glibc unless the workaround was trivial. I dislike putting -O0 or -O1 in clfags
just to build things, but that's what the downstream distributions need to do
sometimes :-)
On Mon, 31 Jan 2022, Carlos O'Donell via Libc-alpha wrote:
> On 1/28/22 12:51, Joseph Myers wrote:
> > On Fri, 28 Jan 2022, Carlos O'Donell via Libc-alpha wrote:
> >
> >> I'm going to look at these right now. I want clean gcc 12 glibc 2.35
> >> results before we release.
> >
> > There are also three ICE bugs in GCC 12 preventing glibc from building, so
> > you won't get fully clean build-many-glibcs.py results with just glibc
> > fixes. GCC bug 103722 on sh4, bug 104153 on or1k and 104154 on arc (the
> > last two reported by Jeff based on ICEs building newlib, but the ICEs I
> > see building glibc are in the same place in the compiler, so probably the
> > same bugs).
>
> Thanks.
>
> I just fixed the last -Wuse-after-free bug.
I think there are still some more -Wuse-after-free issues building the
testsuite.
https://sourceware.org/pipermail/libc-testresults/2022q1/009283.html
s390x-linux-gnu-O3 failures are as shown at
<https://sourceware.org/pipermail/libc-alpha/2022-January/135797.html>.
hppa-linux-gnu, microblaze-linux-gnu, microblazeel-linux-gnu,
glibcs-sparcv8-linux-gnu-leon3, sparcv9-linux-gnu,
sparcv9-linux-gnu-disable-multi-arch:
In function 'do_test',
inlined from 'legacy_test_function' at ../test-skeleton.c:55:10:tst-realloc.c:134:12: error: pointer 'p' may be used after 'realloc' [-Werror=use-after-free]
134 | if (c[i] != 0xff)
| ~^~~
tst-realloc.c:124:7: note: call to 'realloc' here
124 | c = realloc (p, -1);
| ^~~~~~~~~~~~~~~
commit c23cf7ff784186b8094b97a47a8445808145a69c
Author: Martin Sebor <msebor@redhat.com>
Date: Tue Jan 25 15:39:38 2022 -0700
Avoid -Wuse-after-free in tests.
@@ -20,6 +20,7 @@
#include <stdlib.h>
#include <support/support.h>
+#include <libc-diag.h>
#define SIZE 4096
@@ -29,7 +30,15 @@ __attribute__((noinline))
call_free (void *ptr)
{
free (ptr);
+#if __GNUC_PREREQ (12, 0)
+ /* Ignore a valid warning about using a pointer made indeterminate
+ by a prior call to free(). */
+ DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
+#endif
*(size_t *)(ptr - sizeof (size_t)) = 1;
+#if __GNUC_PREREQ (12, 0)
+ DIAG_POP_NEEDS_COMMENT;
+#endif
}
int
@@ -86,7 +86,15 @@ do_test (void)
merror ("errno is not set correctly.");
DIAG_POP_NEEDS_COMMENT;
+#if __GNUC_PREREQ (12, 0)
+ /* Ignore a valid warning about using a pointer made indeterminate
+ by a prior call to realloc(). */
+ DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
+#endif
free (p);
+#if __GNUC_PREREQ (12, 0)
+ DIAG_POP_NEEDS_COMMENT;
+#endif
p = malloc (512);
if (p == NULL)
@@ -104,7 +112,15 @@ do_test (void)
merror ("errno is not set correctly.");
DIAG_POP_NEEDS_COMMENT;
+#if __GNUC_PREREQ (12, 0)
+ /* Ignore a valid warning about using a pointer made indeterminate
+ by a prior call to realloc(). */
+ DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
+#endif
free (p);
+#if __GNUC_PREREQ (12, 0)
+ DIAG_POP_NEEDS_COMMENT;
+#endif
free (q);
return errors != 0;
@@ -95,7 +95,15 @@ test_large_allocations (size_t size)
DIAG_POP_NEEDS_COMMENT;
#endif
TEST_VERIFY (errno == ENOMEM);
+#if __GNUC_PREREQ (12, 0)
+ /* Ignore a warning about using a pointer made indeterminate by
+ a prior call to realloc(). */
+ DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
+#endif
free (ptr_to_realloc);
+#if __GNUC_PREREQ (12, 0)
+ DIAG_POP_NEEDS_COMMENT;
+#endif
for (size_t nmemb = 1; nmemb <= 8; nmemb *= 2)
if ((size % nmemb) == 0)
@@ -113,14 +121,30 @@ test_large_allocations (size_t size)
test_setup ();
TEST_VERIFY (reallocarray (ptr_to_realloc, nmemb, size / nmemb) == NULL);
TEST_VERIFY (errno == ENOMEM);
+#if __GNUC_PREREQ (12, 0)
+ /* Ignore a warning about using a pointer made indeterminate by
+ a prior call to realloc(). */
+ DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
+#endif
free (ptr_to_realloc);
+#if __GNUC_PREREQ (12, 0)
+ DIAG_POP_NEEDS_COMMENT;
+#endif
ptr_to_realloc = malloc (16);
TEST_VERIFY_EXIT (ptr_to_realloc != NULL);
test_setup ();
TEST_VERIFY (reallocarray (ptr_to_realloc, size / nmemb, nmemb) == NULL);
TEST_VERIFY (errno == ENOMEM);
+#if __GNUC_PREREQ (12, 0)
+ /* Ignore a warning about using a pointer made indeterminate by
+ a prior call to realloc(). */
+ DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
+#endif
free (ptr_to_realloc);
+#if __GNUC_PREREQ (12, 0)
+ DIAG_POP_NEEDS_COMMENT;
+#endif
}
else
break;
@@ -20,8 +20,8 @@ verbose_malloc (size_t size)
static void
verbose_free (void *buf)
{
- free (buf);
printf ("free (%p)\n", buf);
+ free (buf);
}
static int
@@ -138,8 +138,16 @@ do_test (void)
if (ok == 0)
merror ("first 16 bytes were not correct after failed realloc");
+#if __GNUC_PREREQ (12, 0)
+ /* Ignore a valid warning about using a pointer made indeterminate
+ by a prior call to realloc(). */
+ DIAG_IGNORE_NEEDS_COMMENT (12, "-Wuse-after-free");
+#endif
/* realloc (p, 0) frees p (C89) and returns NULL (glibc). */
p = realloc (p, 0);
+#if __GNUC_PREREQ (12, 0)
+ DIAG_POP_NEEDS_COMMENT;
+#endif
if (p != NULL)
merror ("realloc (p, 0) returned non-NULL.");
@@ -39,10 +39,11 @@ check_path (int fd)
char file_path[PATH_MAX];
ssize_t file_path_length
= readlink (proc_fd_path, file_path, sizeof (file_path));
- free (proc_fd_path);
if (file_path_length < 0)
FAIL_EXIT1 ("readlink (%s, %p, %zu)", proc_fd_path, file_path,
sizeof (file_path));
+
+ free (proc_fd_path);
file_path[file_path_length] = '\0';
TEST_COMPARE_STRING (file_path, "/dev/null");
}