public inbox for glibc-cvs@sourceware.org
help / color / mirror / Atom feed
* [glibc/arm/morello/main] Fix malloc/tst-scratch_buffer OOB access
@ 2022-11-23 14:39 Szabolcs Nagy
  0 siblings, 0 replies; 2+ messages in thread
From: Szabolcs Nagy @ 2022-11-23 14:39 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=2a287534c18a37536141e94dc98685a4ce10f89f

commit 2a287534c18a37536141e94dc98685a4ce10f89f
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Tue Oct 11 13:23:25 2022 +0100

    Fix malloc/tst-scratch_buffer OOB access
    
    The test used scratch_buffer_dupfree incorrectly:
    
    - The passed in size must be <= buf.length.
    - Must be called at most once on a buf object since it frees it.
    - After it is called buf.data and buf.length must not be accessed.
    
    All of these were violated, the test happened to work because the
    buffer was on the stack, which meant the test copied out-of-bounds
    bytes from the stack into a new buffer and then compared those bytes.
    
    Run one test and avoid the issues above.

Diff:
---
 malloc/tst-scratch_buffer.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/malloc/tst-scratch_buffer.c b/malloc/tst-scratch_buffer.c
index 9fcb11ba2c..60a513ccc6 100644
--- a/malloc/tst-scratch_buffer.c
+++ b/malloc/tst-scratch_buffer.c
@@ -155,21 +155,13 @@ do_test (void)
     struct scratch_buffer buf;
     scratch_buffer_init (&buf);
     memset (buf.data, '@', buf.length);
-
-    size_t sizes[] = { 16, buf.length, buf.length + 16 };
-    for (int i = 0; i < array_length (sizes); i++)
-      {
-        /* The extra size is unitialized through realloc.  */
-        size_t l = sizes[i] > buf.length ? sizes[i] : buf.length;
-        void *r = scratch_buffer_dupfree (&buf, l);
-        void *c = xmalloc (l);
-        memset (c, '@', l);
-        TEST_COMPARE_BLOB (r, l, buf.data, l);
-        free (r);
-        free (c);
-      }
-
-    scratch_buffer_free (&buf);
+    size_t l = 16 <= buf.length ? 16 : buf.length;
+    void *r = scratch_buffer_dupfree (&buf, l);
+    void *c = xmalloc (l);
+    memset (c, '@', l);
+    TEST_COMPARE_BLOB (r, l, c, l);
+    free (r);
+    free (c);
   }
   return 0;
 }

^ permalink raw reply	[flat|nested] 2+ messages in thread

* [glibc/arm/morello/main] Fix malloc/tst-scratch_buffer OOB access
@ 2022-10-27 13:49 Szabolcs Nagy
  0 siblings, 0 replies; 2+ messages in thread
From: Szabolcs Nagy @ 2022-10-27 13:49 UTC (permalink / raw)
  To: glibc-cvs

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=1dc3098e2c316254d6a9cb50797f2eca9be92b9f

commit 1dc3098e2c316254d6a9cb50797f2eca9be92b9f
Author: Szabolcs Nagy <szabolcs.nagy@arm.com>
Date:   Tue Oct 11 13:23:25 2022 +0100

    Fix malloc/tst-scratch_buffer OOB access
    
    The test used scratch_buffer_dupfree incorrectly:
    
    - The passed in size must be <= buf.length.
    - Must be called at most once on a buf object since it frees it.
    - After it is called buf.data and buf.length must not be accessed.
    
    All of these were violated, the test happened to work because the
    buffer was on the stack, which meant the test copied out-of-bounds
    bytes from the stack into a new buffer and then compared those bytes.
    
    Run one test and avoid the issues above.

Diff:
---
 malloc/tst-scratch_buffer.c | 22 +++++++---------------
 1 file changed, 7 insertions(+), 15 deletions(-)

diff --git a/malloc/tst-scratch_buffer.c b/malloc/tst-scratch_buffer.c
index 9fcb11ba2c..60a513ccc6 100644
--- a/malloc/tst-scratch_buffer.c
+++ b/malloc/tst-scratch_buffer.c
@@ -155,21 +155,13 @@ do_test (void)
     struct scratch_buffer buf;
     scratch_buffer_init (&buf);
     memset (buf.data, '@', buf.length);
-
-    size_t sizes[] = { 16, buf.length, buf.length + 16 };
-    for (int i = 0; i < array_length (sizes); i++)
-      {
-        /* The extra size is unitialized through realloc.  */
-        size_t l = sizes[i] > buf.length ? sizes[i] : buf.length;
-        void *r = scratch_buffer_dupfree (&buf, l);
-        void *c = xmalloc (l);
-        memset (c, '@', l);
-        TEST_COMPARE_BLOB (r, l, buf.data, l);
-        free (r);
-        free (c);
-      }
-
-    scratch_buffer_free (&buf);
+    size_t l = 16 <= buf.length ? 16 : buf.length;
+    void *r = scratch_buffer_dupfree (&buf, l);
+    void *c = xmalloc (l);
+    memset (c, '@', l);
+    TEST_COMPARE_BLOB (r, l, c, l);
+    free (r);
+    free (c);
   }
   return 0;
 }

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2022-11-23 14:39 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-23 14:39 [glibc/arm/morello/main] Fix malloc/tst-scratch_buffer OOB access Szabolcs Nagy
  -- strict thread matches above, loose matches on Subject: below --
2022-10-27 13:49 Szabolcs Nagy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).