diff mbox series

malloc: Fix -Wuse-after-free warning in tst-mallocalign1 [BZ #26779]

Message ID 20220131052132.3854764-1-carlos@redhat.com
State Committed
Commit 3a7bed5f5a527dbd87412551f41e42e63aeef07a
Headers
Series malloc: Fix -Wuse-after-free warning in tst-mallocalign1 [BZ #26779] |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Carlos O'Donell Jan. 31, 2022, 5:21 a.m. UTC 
  The test leaks bits from the freed pointer via the return value
in ret, and the compiler correctly identifies this issue.
We switch the test to use TEST_VERIFY and terminate the test
if any of the pointers return an unexpected alignment.

This fixes another -Wuse-after-free error when compiling glibc
with gcc 12.

Tested on x86_64 and i686 without regression.
---
 malloc/tst-mallocalign1.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

Comments

Siddhesh Poyarekar Jan. 31, 2022, 5:32 a.m. UTC | #1 
On 31/01/2022 10:51, Carlos O'Donell via Libc-alpha wrote:
> The test leaks bits from the freed pointer via the return value
> in ret, and the compiler correctly identifies this issue.
> We switch the test to use TEST_VERIFY and terminate the test
> if any of the pointers return an unexpected alignment.
> 
> This fixes another -Wuse-after-free error when compiling glibc
> with gcc 12.
> 
> Tested on x86_64 and i686 without regression.

LGTM.

Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
Carlos O'Donell Jan. 31, 2022, 6 a.m. UTC | #2 
On 1/31/22 00:32, Siddhesh Poyarekar wrote:
> On 31/01/2022 10:51, Carlos O'Donell via Libc-alpha wrote:
>> The test leaks bits from the freed pointer via the return value
>> in ret, and the compiler correctly identifies this issue.
>> We switch the test to use TEST_VERIFY and terminate the test
>> if any of the pointers return an unexpected alignment.
>>
>> This fixes another -Wuse-after-free error when compiling glibc
>> with gcc 12.
>>
>> Tested on x86_64 and i686 without regression.
> 
> LGTM.
> 
> Reviewed-by: Siddhesh Poyarekar <siddhesh@sourceware.org>
> 

Thanks. Pushed. That fixes the last gcc 12 issue I'm seeing on x86 for
the glibc 2.35 release.
Andreas Schwab Jan. 31, 2022, 8:31 a.m. UTC | #3 
On Jan 31 2022, Carlos O'Donell via Libc-alpha wrote:

> +#define ALIGNED(p) (((uintptr_t )p & MALLOC_ALIGN_MASK) == 0)

Wrong spacing.
Carlos O'Donell Jan. 31, 2022, 3:20 p.m. UTC | #4 
On 1/31/22 03:31, Andreas Schwab wrote:
> On Jan 31 2022, Carlos O'Donell via Libc-alpha wrote:
> 
>> +#define ALIGNED(p) (((uintptr_t )p & MALLOC_ALIGN_MASK) == 0)
> 
> Wrong spacing.

Good catch. Thanks.
Carlos O'Donell Feb. 1, 2022, 5:08 p.m. UTC | #5 
On 1/31/22 10:20, Carlos O'Donell wrote:
> On 1/31/22 03:31, Andreas Schwab wrote:
>> On Jan 31 2022, Carlos O'Donell via Libc-alpha wrote:
>>
>>> +#define ALIGNED(p) (((uintptr_t )p & MALLOC_ALIGN_MASK) == 0)
>>
>> Wrong spacing.
> 
> Good catch. Thanks.
> 

Fixed and pushed as obvious fix.
diff mbox series

Patch

diff --git a/malloc/tst-mallocalign1.c b/malloc/tst-mallocalign1.c
index 8bfd50c468..3116748e7e 100644
--- a/malloc/tst-mallocalign1.c
+++ b/malloc/tst-mallocalign1.c
@@ -20,6 +20,7 @@ 
 #include <stdlib.h>
 #include <inttypes.h>
 #include <malloc-size.h>
+#include <support/check.h>
 
 static void *
 test (size_t s)
@@ -31,41 +32,42 @@  test (size_t s)
   return p;
 }
 
+#define ALIGNED(p) (((uintptr_t )p & MALLOC_ALIGN_MASK) == 0)
+
 static int
 do_test (void)
 {
   void *p;
-  int ret = 0;
 
   p = test (2);
-  ret |= (uintptr_t) p & MALLOC_ALIGN_MASK;
+  TEST_VERIFY (ALIGNED (p));
   free (p);
 
   p = test (8);
-  ret |= (uintptr_t) p & MALLOC_ALIGN_MASK;
+  TEST_VERIFY (ALIGNED (p));
   free (p);
 
   p = test (13);
-  ret |= (uintptr_t) p & MALLOC_ALIGN_MASK;
+  TEST_VERIFY (ALIGNED (p));
   free (p);
 
   p = test (16);
-  ret |= (uintptr_t) p & MALLOC_ALIGN_MASK;
+  TEST_VERIFY (ALIGNED (p));
   free (p);
 
   p = test (23);
-  ret |= (uintptr_t) p & MALLOC_ALIGN_MASK;
+  TEST_VERIFY (ALIGNED (p));
   free (p);
 
   p = test (43);
-  ret |= (uintptr_t) p & MALLOC_ALIGN_MASK;
+  TEST_VERIFY (ALIGNED (p));
   free (p);
 
   p = test (123);
-  ret |= (uintptr_t) p & MALLOC_ALIGN_MASK;
+  TEST_VERIFY (ALIGNED (p));
   free (p);
 
-  return ret;
+  return 0;
 }
 
 #include <support/test-driver.c>