From patchwork Mon Jan 22 20:29:18 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Xi Ruoyao X-Patchwork-Id: 1889384 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=xry111.site header.i=@xry111.site header.a=rsa-sha256 header.s=default header.b=a9+qqqLb; dkim-atps=neutral Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=sourceware.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=server2.sourceware.org; envelope-from=libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org; receiver=patchwork.ozlabs.org) Received: from server2.sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (secp384r1) server-digest SHA384) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4TJhg53WX3z1yS7 for ; Tue, 23 Jan 2024 07:30:25 +1100 (AEDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 21A083858431 for ; Mon, 22 Jan 2024 20:30:23 +0000 (GMT) X-Original-To: libc-alpha@sourceware.org Delivered-To: libc-alpha@sourceware.org Received: from xry111.site (xry111.site [89.208.246.23]) by sourceware.org (Postfix) with ESMTPS id 25E3C3858D20 for ; Mon, 22 Jan 2024 20:30:08 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 25E3C3858D20 Authentication-Results: sourceware.org; dmarc=pass (p=reject dis=none) header.from=xry111.site Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=xry111.site ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 25E3C3858D20 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=89.208.246.23 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705955409; cv=none; b=OUXfTDCi4xp7Pzg+wLopGkkuYilpABrHWS2G8he/vLscBElYgCX9YG8T5rqaDuja3SRu078tjEWZ6oMgJmw4ccLnNCIZEaGelPPUCZr4Ebq6r4xrparW/3e0eW7ZMDv1JvxE/pIoDllwlD51cXvlpy1FLdR8sfmfO4DVXE1usxA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1705955409; c=relaxed/simple; bh=vaQ07XHI6c+b/xnPQKsTnrG1ksPla41W1sWUk7q8U1M=; h=DKIM-Signature:From:To:Subject:Date:Message-ID:MIME-Version; b=vLHF15p5fSMLwy6VC9pxuSL+dcI2H3NzqqaOqOpivpFXAjZhGAdYaDWWHT+KP4tABrdSQ6LFTVakCY1TiRT8TyuizAzO4M5hYpIE7ISWXgX+BcgdCIPsOzWYkWTfQcnnSf4QrMAqx0VJBN8plsEsvWb8QvGyCX5hrPA7we8xMn0= ARC-Authentication-Results: i=1; server2.sourceware.org DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=xry111.site; s=default; t=1705955406; bh=vaQ07XHI6c+b/xnPQKsTnrG1ksPla41W1sWUk7q8U1M=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=a9+qqqLbMCGnpq3Y40Jct0VYKUDHfBT2vLEg+U6fgdbXVCgkHVobn7gATW4Q5f5pz G0Z781igYtjue2I8LODJ0bj9NTijB80Abam9a/I+ujdac4t+O/QVLItVAxIzzx7xPb RWDaSEo/7cinz8YtjVLbzqny8UPAEm1Sc4ntj/20= Received: from stargazer.. (unknown [IPv6:240e:358:119a:ce00:dc73:854d:832e:2]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (P-384) server-digest SHA384) (Client did not present a certificate) (Authenticated sender: xry111@xry111.site) by xry111.site (Postfix) with ESMTPSA id 125A466F4E; Mon, 22 Jan 2024 15:30:03 -0500 (EST) From: Xi Ruoyao To: libc-alpha@sourceware.org Cc: Adhemerval Zanella Netto , "Andreas K . Huettel" , "H . J . Lu" , Xi Ruoyao Subject: [PATCH v2] qsort: Fix a typo causing unnecessary malloc/free (BZ 31276) Date: Tue, 23 Jan 2024 04:29:18 +0800 Message-ID: <20240122202945.1408304-1-xry111@xry111.site> X-Mailer: git-send-email 2.43.0 In-Reply-To: <0bd78d613891454421309b0651ec693ef8a1e439.camel@xry111.site> References: <0bd78d613891454421309b0651ec693ef8a1e439.camel@xry111.site> MIME-Version: 1.0 X-Spam-Status: No, score=-9.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, LIKELY_SPAM_FROM, SPF_HELO_PASS, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libc-alpha-bounces+incoming=patchwork.ozlabs.org@sourceware.org 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 --- v1 -> v2: - Use `<=` instead of `<`. - Add BZ number. 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 {