From patchwork Thu Feb 16 10:29:48 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Patrick Steinhardt X-Patchwork-Id: 13142844 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5D835C636CC for ; Thu, 16 Feb 2023 10:30:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230018AbjBPKaR (ORCPT ); Thu, 16 Feb 2023 05:30:17 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36770 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229803AbjBPKaQ (ORCPT ); Thu, 16 Feb 2023 05:30:16 -0500 Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id D2BD4521F7 for ; Thu, 16 Feb 2023 02:29:58 -0800 (PST) Received: from compute2.internal (compute2.nyi.internal [10.202.2.46]) by mailout.nyi.internal (Postfix) with ESMTP id A635E5C023F for ; Thu, 16 Feb 2023 05:29:56 -0500 (EST) Received: from mailfrontend2 ([10.202.2.163]) by compute2.internal (MEProxy); Thu, 16 Feb 2023 05:29:56 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pks.im; h=cc :content-type:date:date:from:from:in-reply-to:message-id :mime-version:reply-to:sender:subject:subject:to:to; s=fm1; t= 1676543396; x=1676629796; bh=W2fc7Kl87R187HOWz9kCdbeZbQFLYjRG2jw Pg435crM=; b=VweVTBNQCJK1NNNft8FW5U0Zb4Fzo/LAhfXtsVYwTjx2+xK013v 6KP6VP110wcRqVPmgKfFy3Y82ATKA+BdZ2ilOnMSDgVmMSptR+FzxKNDuyxaae40 2fTv8cVQ/AxithoNaS9KMZoxaVKch7WAG/DCLhQbsReH5MuFYHTN4kCSrmG0rvAz KAOTnyssEr+k554DKUJM+aLOuUHgVZop+8W5ADsMv/iCJ7C6vHfHAXBNL9Ch3nSj FTwNTEolKjyO+Rz+BGSGi8tA90L60p/zRk9pj/MIBHzd3MU4lU/4ZnBQMCyBGj3A YgvUvWRdlW7uhYBPPeozrCLWhlIHox/c/Kg== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-type:date:date:feedback-id :feedback-id:from:from:in-reply-to:message-id:mime-version :reply-to:sender:subject:subject:to:to:x-me-proxy:x-me-proxy :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; t=1676543396; x= 1676629796; bh=W2fc7Kl87R187HOWz9kCdbeZbQFLYjRG2jwPg435crM=; b=T xrkMXQibCnGzllR4CXSz9kPYrkKLNXJ+B9YVFY0gN47dhCiMOAP9ZzP0zpDVIDsb e7JPmRDIVRd0f19E5vJHI0gxl80tieimwOIN6aVSZpjoV8ecKBCuCQ7VzWA+4wkM /pdmvnkbvPO49eeTElztjaD48Te1LMTfg6FyoIUnoYBqjtiIoYRW0IXcAhOVhW0C y7Xr0ngNPDVmGXlXrnCTEPcUczCgfC/6KE5rX3J1p9UgRE1O0ZbeIj/WkVzZD9E0 XDEMO0Pl97Tk+ywwdoSVsuVVzUFX3bN4sOoS3UjB99GHpn+PbY9/3EtbKZSWE2kk j7YXxjJxRnwTO5k1gKxsQ== X-ME-Sender: X-ME-Received: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedvhedrudeijedgudehucetufdoteggodetrfdotf fvucfrrhhofhhilhgvmecuhfgrshhtofgrihhlpdfqfgfvpdfurfetoffkrfgpnffqhgen uceurghilhhouhhtmecufedttdenucenucfjughrpeffhffvuffkgggtugesghdtreertd dtvdenucfhrhhomheprfgrthhrihgtkhcuufhtvghinhhhrghrughtuceophhssehpkhhs rdhimheqnecuggftrfgrthhtvghrnhepjeeifedvueelfffgjeduffdvgefhiefgjefgvd dvfeduvefffeevfffhgfekieffnecuvehluhhsthgvrhfuihiivgeptdenucfrrghrrghm pehmrghilhhfrhhomhepphhssehpkhhsrdhimh X-ME-Proxy: Feedback-ID: i197146af:Fastmail Received: by mail.messagingengine.com (Postfix) with ESMTPA for ; Thu, 16 Feb 2023 05:29:55 -0500 (EST) Received: by pks.im (OpenSMTPD) with ESMTPSA id f881798b (TLSv1.3:TLS_AES_256_GCM_SHA384:256:NO) for ; Thu, 16 Feb 2023 10:29:32 +0000 (UTC) Date: Thu, 16 Feb 2023 11:29:48 +0100 From: Patrick Steinhardt To: git@vger.kernel.org Subject: [PATCH] delta-islands: fix segfault when freeing island marks Message-ID: <61e490595b80b34c55fd640e093e021ff6fa9591.1676542973.git.ps@pks.im> MIME-Version: 1.0 Content-Disposition: inline Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org In 647982bb71 (delta-islands: free island_marks and bitmaps, 2023-02-03) we have introduced logic to free `island_marks` in order to reduce heap memory usage in git-pack-objects(1). This commit is causing segfaults in the case where this Git command does not load delta islands at all, e.g. when reading object IDs from standard input. One such crash can be hit when using repacking multi-pack-indices with delta islands enabled. The root cause of this bug is that we unconditionally dereference the `island_marks` variable even in the case where it is a `NULL` pointer, which is fixed by making it conditional. Note that we still leave the logic in place to set the pointer to `-1` to detect use-after-free bugs even when there are no loaded island marks at all. Tested-by: Eric Wong --- An easy way to reproduce the segfault is: $ git pack-objects .git/objects/pack/pack --delta-islands refcount) - free(bitmap); - }); - kh_destroy_oid_map(island_marks); + if (island_marks) { + kh_foreach_value(island_marks, bitmap, { + if (!--bitmap->refcount) + free(bitmap); + }); + kh_destroy_oid_map(island_marks); + } /* detect use-after-free with a an address which is never valid: */ island_marks = (void *)-1; diff --git a/t/t5319-multi-pack-index.sh b/t/t5319-multi-pack-index.sh index b5f9b10922..499d5d4c78 100755 --- a/t/t5319-multi-pack-index.sh +++ b/t/t5319-multi-pack-index.sh @@ -1015,4 +1015,20 @@ test_expect_success 'complains when run outside of a repository' ' grep "not a git repository" err ' +test_expect_success 'repack with delta islands' ' + git init repo && + test_when_finished "rm -fr repo" && + ( + cd repo && + + test_commit first && + git repack && + test_commit second && + git repack && + + git multi-pack-index write && + git -c repack.useDeltaIslands=true multi-pack-index repack + ) +' + test_done