[PATCH] qsort: Fix a typo causing unnecessary malloc/free

[H.J. Lu]

On Mon, Jan 22, 2024 at 11:32 AM Xi Ruoyao <xry111@xry111.site> wrote:
>
> In qsort_r we allocate a buffer sized QSORT_STACK_SIZE (1024) on stack
> and we intend to use it if all elements can fit into it.  But there is a
> typo:
>
>     if (total_size < sizeof buf)
>       buf = tmp;
>     else
>       /* allocate a buffer on heap and use it ... */
>
> Here "buf" is a pointer, thus sizeof buf is just 4 or 8, instead of
> 1024.

This sounds like a real bug.  A glibc bug report is needed to track it.

> This bug is detected debugging some strange heap corruption running the
> Ruby-3.3.0 test suite (on an experimental Linux From Scratch build using
> Binutils-2.41.90 and Glibc trunk, and also Fedora Rawhide [1]).  It
> seems Ruby is doing some wild "optimization" by jumping into somewhere
> in qsort_r instead of calling it normally, resulting in a double free of
> buf if we allocate it on heap.  The issue can be reproduced
> deterministically with:
>
>     LD_PRELOAD=/usr/lib/libc_malloc_debug.so MALLOC_CHECK_=3 \
>     LD_LIBRARY_PATH=. ./ruby test/runner.rb test/ruby/test_enum.rb
>
> in Ruby-3.3.0 tree after building it.  This change would hide the issue
> for Ruby, but Ruby is likely still buggy (if using this "optimization"
> sorting larger arrays).
>
> [1]:https://kojipkgs.fedoraproject.org//work/tasks/9729/111889729/build.log
>
> Signed-off-by: Xi Ruoyao <xry111@xry111.site>
> ---
>  stdlib/qsort.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/stdlib/qsort.c b/stdlib/qsort.c
> index 7f5a00fb33..1c1505e7b2 100644
> --- a/stdlib/qsort.c
> +++ b/stdlib/qsort.c
> @@ -353,7 +353,7 @@ __qsort_r (void *const pbase, size_t total_elems, size_t size,
>    if (size > INDIRECT_SORT_SIZE_THRES)
>      total_size = 2 * total_elems * sizeof (void *) + size;
>
> -  if (total_size < sizeof buf)
> +  if (total_size < sizeof tmp)
>      buf = tmp;
>    else
>      {
> --
> 2.43.0
>


-- 
H.J.