From patchwork Wed Jun 19 05:14:21 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Kirill Smelkov X-Patchwork-Id: 13703402 Received: from mail180-6.suw31.mandrillapp.com (mail180-6.suw31.mandrillapp.com [198.2.180.6]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 02C5145977 for ; Wed, 19 Jun 2024 05:14:22 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.2.180.6 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718774065; cv=none; b=th0lpdi2jPrUTYI/E0fPNaco1hlanzl32IuQZH0opFs+SZUv9Nxz530CqsYvsqL3aq1D8Fjw5oS1Sy2+Teo43t88qrOCqrLIks79p8guvrnJX+ClTJRgP95dpUPipmJTtN+HYJqxF+dKUnlZp4oy23jCpBwmX3/AGJnilsJEfF4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1718774065; c=relaxed/simple; bh=PUB5l0gYDRxoPZGcitGVHJ2o/8+F5DJo1EhqOu4wxmY=; h=From:Subject:To:Cc:Message-Id:References:In-Reply-To:Date: MIME-Version:Content-Type; b=DzMt7uCcy8N0fMTvK/j8tpAFAV76R/F5YZQqMP0SyRVbFMWUyzBDlbhnyTYxFtr51nD70kshMziGbAOpjZY3lb81aI7tXWmIXHeAJYRdr47nGbGrPvReyj2NH9AIykHyctpexB3TqiqsKOUPxUKa2B3KhR8zCBiv5PuIJ+nwGaU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=nexedi.com; spf=pass smtp.mailfrom=mandrillapp.mail.nexedi.com; dkim=pass (2048-bit key) header.d=mandrillapp.com header.i=@mandrillapp.com header.b=OExzLBBN; dkim=pass (2048-bit key) header.d=nexedi.com header.i=kirr@nexedi.com header.b=jdbrWEWN; arc=none smtp.client-ip=198.2.180.6 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=nexedi.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=mandrillapp.mail.nexedi.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=mandrillapp.com header.i=@mandrillapp.com header.b="OExzLBBN"; dkim=pass (2048-bit key) header.d=nexedi.com header.i=kirr@nexedi.com header.b="jdbrWEWN" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mandrillapp.com; s=mte1; t=1718774061; x=1719034561; bh=hcwuenlOugxejhElA3v6zk9yEskT+H2LwEeQKvLtMng=; h=From:Subject:To:Cc:Message-Id:References:In-Reply-To:Feedback-ID: Date:MIME-Version:Content-Type:Content-Transfer-Encoding:CC:Date: Subject:From; b=OExzLBBNLtBarDFpZ+O/8qDOSzUFRPHaRO5PuBt7zxCMAL0ldeH60eiyOfD4LKxRH jYQGjePAGu5POAa0082wzkkIEqHnXRraVzTdlDuiNMvrP9Nmtxzfmtl29yv6AKYLyn P0Ly9CHnmX2l0e70PGlbDS6+U0XGIowkkPDCGEOI5BPJU1pIzi1xnm9YOBhZYvkeHS qXr8acGWFGmDN//KZGi/HHGaktLOfhiDWlj5VRODyRc6JaBvLYjbh2/WCwqlwBpZsN 4aObCXdQTDcOTI4S1sOyRuEDo8nuXXXta5mWqFc5kpQxyuiWa25tf0mKXUErvsW9Et lH+ao3LyxqmOQ== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nexedi.com; s=mte1; t=1718774061; x=1719034561; i=kirr@nexedi.com; bh=hcwuenlOugxejhElA3v6zk9yEskT+H2LwEeQKvLtMng=; h=From:Subject:To:Cc:Message-Id:References:In-Reply-To:Feedback-ID: Date:MIME-Version:Content-Type:Content-Transfer-Encoding:CC:Date: Subject:From; b=jdbrWEWNcBCIlQARoUa8WorEKfL8kJ8dsggg8f4Hp1lkSkL9paCQZkpwUVZVXpGn0 bymmlan42zYoUo0mdKw+oyrsmxV/axCfbps7hch8t84RUA++LVvn/hQasx0vMP7fHr /gCmugW+9TpKz1JeIpgZLOiAWt9GrrwToY0rNkckGw9YghPb18IHBnUteZsf7xGBy0 UunIIRl+pjkrsoyN8C6a3TV2WIhAD4q1gMPV1H7JX7YQHKo/j/ge10RCutoLkJVi7A rT2i/DuGabrnZjG1dddiGlaBk9HYQ1b6GfP0lBuAAFbJZxNv3Eaum02Byg9+zPiAlL sn/zNOT4/2Egg== Received: from pmta11.mandrill.prod.suw01.rsglab.com (localhost [127.0.0.1]) by mail180-6.suw31.mandrillapp.com (Mailchimp) with ESMTP id 4W3sHK63Sqz2K22Yr for ; Wed, 19 Jun 2024 05:14:21 +0000 (GMT) From: Kirill Smelkov Subject: =?utf-8?q?=5BRESEND=2C_BUG=2C_SIGSEGV_CRASH=5D_=28Re=3A_=5BPATCH=5D?= =?utf-8?q?_fetch-pack=3A_test=3A_demonstrate_segmentation_fault_when_run_wi?= =?utf-8?q?th_fsckObjects_but_without_--lock-pack=29?= Received: from [87.98.221.171] by mandrillapp.com id 0e8c3e57282b4dc09f72896411dabfef; Wed, 19 Jun 2024 05:14:21 +0000 To: Junio C Hamano Cc: git@vger.kernel.org, Jonathan Tan , Alain Takoudjou , =?utf-8?q?J=C3=A9rome_Perrin?= , Elijah Newren , Jeff King , Calvin Wan , Patrick Steinhardt , =?utf-8?b?w4Z2YXIgQXJuZmrDtnLDsCBCamFybWFzb24=?= Message-Id: References: <20240606133605.602276-1-kirr@nexedi.com> In-Reply-To: <20240606133605.602276-1-kirr@nexedi.com> X-Native-Encoded: 1 X-Report-Abuse: =?utf-8?q?Please_forward_a_copy_of_this_message=2C_including?= =?utf-8?q?_all_headers=2C_to_abuse=40mandrill=2Ecom=2E_You_can_also_report_?= =?utf-8?q?abuse_here=3A_https=3A//mandrillapp=2Ecom/contact/abuse=3Fid=3D31?= =?utf-8?q?050260=2E0e8c3e57282b4dc09f72896411dabfef?= X-Mandrill-User: md_31050260 Feedback-ID: 31050260:31050260.20240619:md Date: Wed, 19 Jun 2024 05:14:21 +0000 Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 + newren, peff, calvinwan, ps, avarab Hello everyone. This patch demonstrates *segmentation fault* crash of Git with, hopefully, properly written test. It was originally posted two weeks ago without getting any replies nor any traces in seen/todo: https://lore.kernel.org/git/20240606133605.602276-1-kirr@nexedi.com/T/#m9d454aadc8857b84f17d1a331739f7399cf1bbf8 I understand that there might be something wrong with my test, but could you please at least provide some feedback and acknowledge the problem? Resending the patch and adding more people to Cc who touched fetch-pack.c recently. Thanks beforehand for feedback, Kirill ---- 8< ---- From: Kirill Smelkov Date: Thu, 6 Jun 2024 16:03:44 +0300 Subject: [PATCH] fetch-pack: test: demonstrate segmentation fault when run with fsckObjects but without --lock-pack MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit When running git-backup[1] over lab.nexedi.com archive we noticed that Git started to crash on fetch-pack operation[2] after Git upgrade. We tracked this down to be a regression introduced by Git 2.31, more specifically by commit 5476e1efded5 (fetch-pack: print and use dangling .gitmodules), which started to index and lock packfiles on do_keep=y && fsck_objects=y even if pack_lockfiles=NULL, which leads to NULL-dereference when trying to append an entry to that pack_lockfiles stringlist. Please find attached a test that demonstrates this problem. When run with test_expect_failure changed to test_expect_success the test crashes with ./test-lib.sh: line 1063: 599675 Segmentation fault (core dumped) git -c fetch.fsckObjects=true -c fetch.unpackLimit=1 -C client fetch-pack ../server $(git -C server rev-parse refs/heads/main) and the backtrace is Program terminated with signal SIGSEGV, Segmentation fault. #0 0x0000558032c7ef3b in string_list_append_nodup (list=0x0, string=0x558033b695f0 ".git/objects/pack/pack-c1b2455a361bb50a0db087e7202db73d62938fa1.keep") at string-list.c:218 218 ALLOC_GROW(list->items, list->nr + 1, list->alloc); (gdb) bt #0 0x0000558032c7ef3b in string_list_append_nodup (list=0x0, string=0x558033b695f0 ".git/objects/pack/pack-c1b2455a361bb50a0db087e7202db73d62938fa1.keep") at string-list.c:218 #1 0x0000558032b5b83f in get_pack (args=0x7ffe680f3fa0, xd=0x7ffe680f4058, pack_lockfiles=0x0, index_pack_args=0x0, sought=0x558033b65e90, nr_sought=1, gitmodules_oids=0x558032e17b88 ) at fetch-pack.c:1042 #2 0x0000558032b5e0b3 in do_fetch_pack_v2 (args=0x7ffe680f3fa0, fd=0x7ffe680f4058, orig_ref=0x558033b67b90, sought=0x558033b65e90, nr_sought=1, shallows=0x7ffe680f3df0, si=0x7ffe680f3e10, pack_lockfiles=0x0) at fetch-pack.c:1781 #3 0x0000558032b5ef3c in fetch_pack (args=0x7ffe680f3fa0, fd=0x7ffe680f4058, ref=0x558033b67b90, sought=0x558033b65e90, nr_sought=1, shallow=0x7ffe680f3f80, pack_lockfiles=0x0, version=protocol_v2) at fetch-pack.c:2073 #4 0x0000558032a08851 in cmd_fetch_pack (argc=3, argv=0x7ffe680f4500, prefix=0x0) at builtin/fetch-pack.c:242 #5 0x00005580329b2be3 in run_builtin (p=0x558032e0bb30 , argc=3, argv=0x7ffe680f4500) at git.c:474 #6 0x00005580329b2ffe in handle_builtin (argc=3, argv=0x7ffe680f4500) at git.c:729 #7 0x00005580329b3222 in run_argv (argcp=0x7ffe680f433c, argv=0x7ffe680f4330) at git.c:793 #8 0x00005580329b3796 in cmd_main (argc=3, argv=0x7ffe680f4500) at git.c:928 #9 0x0000558032ab9002 in main (argc=10, argv=0x7ffe680f44c8) at common-main.c:62 A simple debug patch below --- a/fetch-pack.c +++ b/fetch-pack.c @@ -1032,7 +1032,7 @@ static int get_pack(struct fetch_pack_args *args, cmd.git_cmd = 1; if (start_command(&cmd)) die(_("fetch-pack: unable to fork off %s"), cmd_name); - if (do_keep && (pack_lockfiles || fsck_objects)) { + if (do_keep && (pack_lockfiles /*|| fsck_objects*/)) { int is_well_formed; char *pack_lockfile = index_pack_lockfile(cmd.out, &is_well_formed); makes the crash go away, but I did not investigated what should be the proper logic. Thanks beforehand for fixing this NULL-pointer dereference. Kirill [1] https://lab.nexedi.com/kirr/git-backup [2] https://lab.nexedi.com/kirr/git-backup/-/blob/3230197c/git-backup.go#L717-739 Cc: Jonathan Tan Cc: Elijah Newren Cc: Jeff King Cc: Calvin Wan Cc: Patrick Steinhardt Cc: Ævar Arnfjörð Bjarmason Cc: Alain Takoudjou Cc: Jérome Perrin Signed-off-by: Kirill Smelkov --- t/t5500-fetch-pack.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh index 1bc15a3f08..e3dbe79613 100755 --- a/t/t5500-fetch-pack.sh +++ b/t/t5500-fetch-pack.sh @@ -1056,6 +1056,20 @@ test_expect_success 'fetch with --filter=blob:limit=0' ' fetch_filter_blob_limit_zero server server ' +test_expect_failure 'fetch with fsckObjects but without --lock-pack does not segfault' ' + rm -rf server client && + git init server && + test_commit -C server 1 && + + git init client && + # unpackLimit=1 forces to keep received pack as pack instead of + # unpacking it to loose objects. The code is currently segfaulting when + # trying to index that kept pack. + git -c fetch.fsckObjects=true -c fetch.unpackLimit=1 \ + -C client fetch-pack ../server \ + $(git -C server rev-parse refs/heads/main) +' + . "$TEST_DIRECTORY"/lib-httpd.sh start_httpd