public inbox for glibc-cvs@sourceware.org
help / color / mirror / Atom feed
* [glibc] qsort: Fix a typo causing unnecessary malloc/free (BZ 31276)
@ 2024-01-23 13:17 H.J. Lu
  0 siblings, 0 replies; only message in thread
From: H.J. Lu @ 2024-01-23 13:17 UTC (permalink / raw)
  To: glibc-cvs

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

commit dfa3394a605c8f6f25e4f827789bc89eca1d206c
Author: Xi Ruoyao <xry111@xry111.site>
Date:   Tue Jan 23 04:29:18 2024 +0800

    qsort: Fix a typo causing unnecessary malloc/free (BZ 31276)
    
    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.  There is also a minor issue that we should use "<=" instead of
    "<".
    
    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>

Diff:
---
 stdlib/qsort.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/stdlib/qsort.c b/stdlib/qsort.c
index 7f5a00fb33..be47aebbe0 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
     {

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2024-01-23 13:17 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-23 13:17 [glibc] qsort: Fix a typo causing unnecessary malloc/free (BZ 31276) H.J. Lu

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).