Re: [PATCH 16/20] Fix malloc/tst-scratch_buffer OOB access

[Florian Weimer <fweimer@redhat.com>]

* Szabolcs Nagy via Libc-alpha:

 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.
> ---
>  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;
>  }

I think we should keep the test loop, but create a new scratch buffer on
each iteration.

Thanks,
Florian