From patchwork Thu Jun 6 18:40:52 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13688888 Received: from mail-qv1-f52.google.com (mail-qv1-f52.google.com [209.85.219.52]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id F12993BBEA for ; Thu, 6 Jun 2024 18:41:00 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.219.52 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717699263; cv=none; b=MFWhLYmMyyvvoa7FQDYllAFnYQf2JbmlibVz9NiGTBM4fIj05e2E22D5nsqgCntDdKKGRbGZwC7HT385MHAUDRwVPv13ZhYAqIQvrddICWlY4REEsL6pVpY/ozusTel352BWyoZQtLHPuQn2fq5G/okAcOJIE43NFSyIlWKpuIw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717699263; c=relaxed/simple; bh=ZvVm55J8OBlCSLEOBk9jtny6d0QjRwaU0S0zGjxlXZ0=; h=Date:From:To:Cc:Subject:Message-ID:MIME-Version:Content-Type: Content-Disposition; b=Ix5AHoPQurY1MkveIriSMymjfVSBI+jX6O5tjopml6Z3S8B9JaljzzWCt6tDJsEJucReR/UrbdCICyhdlmuDnymEMympL0cj1tO+PZrs6lSP7zY/SDxV2c4R3L2/hIE/iAkgdQlj1AF1FwfgHIuxrweRvXytAfqSLiCfFwK1Fcc= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ttaylorr.com; spf=none smtp.mailfrom=ttaylorr.com; dkim=pass (2048-bit key) header.d=ttaylorr-com.20230601.gappssmtp.com header.i=@ttaylorr-com.20230601.gappssmtp.com header.b=GjTbkuyr; arc=none smtp.client-ip=209.85.219.52 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ttaylorr.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=ttaylorr.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=ttaylorr-com.20230601.gappssmtp.com header.i=@ttaylorr-com.20230601.gappssmtp.com header.b="GjTbkuyr" Received: by mail-qv1-f52.google.com with SMTP id 6a1803df08f44-6ad86f3cc34so7295136d6.1 for ; Thu, 06 Jun 2024 11:41:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20230601.gappssmtp.com; s=20230601; t=1717699259; x=1718304059; darn=vger.kernel.org; h=content-disposition:mime-version:message-id:subject:cc:to:from:date :from:to:cc:subject:date:message-id:reply-to; bh=CXwMpGDE9tQyW7crVZLyWDujYP5ONZ2CYWH0zJaqjLo=; b=GjTbkuyrv/8NMN2hWDxAQclYMKRKYLjxBQYIbFudXjYBzzIJHax1REgmfRUV8yG5BD jLb/do8jh8ZnSDwfZLwSO0SWlXrBa6KNIXzU8iWSkZNNIlofDgqBa3Ndzpw6QRQt7ES0 8vDjaoXNjME1TdACIi1GMyJW0hAminksia6qXjv5gOphdjyYW7xOw9IeRPFdQAfIAZ8R 0rm7OXI5KgTU/nt2DmoPzgXiigTJIyiCsofjD+PVlJNfPpmnHh76t7pnvt6MLpqVeSKY nOH3bypGJN9ijETk86Hae9K2kKbby0urwcq3d5I+LaXN/qIqL1nXh6MWmOJhk0bZgLpO fswA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717699259; x=1718304059; h=content-disposition:mime-version:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=CXwMpGDE9tQyW7crVZLyWDujYP5ONZ2CYWH0zJaqjLo=; b=KAGW0CJzNFv5O6uHPOlwb6O1f6a5L34wwa64UU7etUT9jPo7JjYBE9xa3Kn/JEGguc GYFBOAFpc6CsuVshgSpvKVnHKfvNTsRZUsOzihL1w2CJZZrP/pdWZG+h2g+UdBhYQijK 4eyv+yHX6rJ5M1lYs932MrYck8lzbwEq4K1UhorAlZbrNCQxWqSyct5hssFT5vp7dTND svAco5sLu1O+AmkEgq+22BwH/UDA/YUeoQeeG7FT9o3RSFVuzFPn89deqAU2+Cr+dU7t 5KXpOq3jfaopveGNjviB3oH+IeKZ7vz+IbAY6sQL7k4Ae8HJMFa+QHTbyiHV9UZJWhMI zGFw== X-Gm-Message-State: AOJu0YxWTnFhVR9PVLKqgYWqA9U5mRVEjHZ1WkQw4YhqXSPNhdJaOXLy jxBPn9dYDvqFRjRQXrGQ1B1wX/yijyYsGHMaTYmnytRhr4KahaV71SFLBHSvnLh9Fk0g3IF9BzO YuGo= X-Google-Smtp-Source: AGHT+IGeAtXxkjfQSxzBdcQyStZyDwE4VPIiPvoz25TVoshDikALxlzxNnP1iczq6m2sp6Fdjtrdmw== X-Received: by 2002:a05:6214:5982:b0:6ae:497:8738 with SMTP id 6a1803df08f44-6b059bd6c2emr5175556d6.29.1717699258925; Thu, 06 Jun 2024 11:40:58 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6b04f6213basm8720046d6.7.2024.06.06.11.40.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Jun 2024 11:40:58 -0700 (PDT) Date: Thu, 6 Jun 2024 14:40:52 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , Patrick Steinhardt , Junio C Hamano Subject: [PATCH 1/2] Documentation/technical/bitmap-format.txt: add missing position table Message-ID: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline While investigating a benign Coverity warning on the new pseudo-merge implementation, I was struggling to understand the (paraphrased) below: ofs = index_end - 24 - (index->pseudo_merges.nr * sizeof(uint64_t)); for (i = 0; i < index->pseudo_merges.nr; i++) { index->pseudo_merges.v[i].at = get_be64(ofs); ofs += sizeof(uint64_t); } , in pack-bitmap.c::load_bitmap_header(). Looking at the documentation, the diagram describing the on-disk format (prior to this patch) suggested that the optional extended lookup table immediately preceded the trailing metadata portion. If that were the case, that would make the above code from load_bitmap_header() incorrect, as we'd be blindly reading into the extended offset table. But later on in the documentation there is a description of the pseudo-merge position table as immediately preceding the trailing metadata portion of the extension. And indeed, we do write the position table in pack-bitmap-write.c: /* write positions for all pseudo merges */ for (i = 0; i < writer->pseudo_merges_nr; i++) hashwrite_be64(f, pseudo_merge_ofs[i]); hashwrite_be32(f, writer->pseudo_merges_nr); hashwrite_be32(f, kh_size(writer->pseudo_merge_commits)); hashwrite_be64(f, table_start - start); hashwrite_be64(f, hashfile_total(f) - start + sizeof(uint64_t)); So this is purely a case of the diagram being out of sync with the textual description and actual implementation of the format specification. Add the missing component back to the format diagram to avoid further confusion in this area. Signed-off-by: Taylor Blau --- Documentation/technical/bitmap-format.txt | 9 +++++++++ 1 file changed, 9 insertions(+) base-commit: 0b7500dc66ffcb6b1ccc3332715936a59c6b5ce4 diff --git a/Documentation/technical/bitmap-format.txt b/Documentation/technical/bitmap-format.txt index ee7775a258..bfb0ec7beb 100644 --- a/Documentation/technical/bitmap-format.txt +++ b/Documentation/technical/bitmap-format.txt @@ -312,6 +312,15 @@ the end of a `.bitmap` file. The format is as follows: | | +-------------------------------------------+ | | +| Pseudo-merge position table | +| +----+----------+----------+----------+ | +| | N | Offset 1 | .... | Offset N | | +| +----+----------+----------+----------+ | +| | | 8 bytes | .... | 8 bytes | | +| +----+----------+----------+----------+ | +| | ++-------------------------------------------+ +| | | Pseudo-merge Metadata | | +-----------------------------------+ | | | # pseudo-merges (4 bytes) | | From patchwork Thu Jun 6 18:41:00 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Taylor Blau X-Patchwork-Id: 13688889 Received: from mail-oi1-f179.google.com (mail-oi1-f179.google.com [209.85.167.179]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 385BE47A53 for ; Thu, 6 Jun 2024 18:41:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.167.179 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717699266; cv=none; b=PPI6Nc4d/85v4YKniaeM66KKlNjgJKU//qcQ96xOrkckvOhlIdkVXr+UDJ3SZQ+l9aWA3w0wUI1jvJ3PvDHFAiJj8wu+agwWLjr01l70aWl2hjgejxy2lKIt4T2oMj5PEJXqiH5fqzd0joPcaWcdUhHqAC84PWEugblS9v+w3DQ= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1717699266; c=relaxed/simple; bh=Vc+EKRmmGYIVg+C0TrorUx33a8i63l/mg9RNWJqXTQ4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=eLTvjwdFaw8mwG6voDP19gELBeXL9tpMjVG635jKIV/vmbBAFakGIAZLL06wtvqNpuN29NhVErYm0co1uw/k/gFWdgsa1QtsT/bRg7b+h0mM6rvOGGMOnKR86VThtGTAGaACbKJX1VwB4TSbgqjf17WvXNF8rBwKZQ+UnxX86Hk= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ttaylorr.com; spf=none smtp.mailfrom=ttaylorr.com; dkim=pass (2048-bit key) header.d=ttaylorr-com.20230601.gappssmtp.com header.i=@ttaylorr-com.20230601.gappssmtp.com header.b=c6Ga8FG7; arc=none smtp.client-ip=209.85.167.179 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=ttaylorr.com Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=ttaylorr.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=ttaylorr-com.20230601.gappssmtp.com header.i=@ttaylorr-com.20230601.gappssmtp.com header.b="c6Ga8FG7" Received: by mail-oi1-f179.google.com with SMTP id 5614622812f47-3d20f97463aso223223b6e.1 for ; Thu, 06 Jun 2024 11:41:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ttaylorr-com.20230601.gappssmtp.com; s=20230601; t=1717699263; x=1718304063; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=1hjENXSJm+1v3r/8IQdJmUOsdwUkpa/ZpTjR0I5jEks=; b=c6Ga8FG7yoBx+blk4GBHH5HeLVqXrTOGUcLT6HcENDqyuCU3o23MXoD+hSx8q5uGWp EYSg2SmIB7BLUKR0pdsP2S1CBzrgeRkDK1adWoZXhMNxuUuRX/Ap/fc2nd8VD6NsLNzx 9/BVFLTvPvxy4ttJRZY3zsKhNWMmGARRQ2nR4wUWz7oNHznIW8D3nUyLHQuF2S8xo8RJ d5Mqrrg+3UlzpBtGZ/w+9UUOEoAeWLkr+HXht8HgEZjbQ/VlkCRcX3HWFKvcaiKxdnGn 4xJWr0a9ih4LAKMhnl4qk/68rOHY9V3BMKe4/c8zZ1+0FXUVBu0wnxhZp4J5FVPvuczk I9PA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1717699263; x=1718304063; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=1hjENXSJm+1v3r/8IQdJmUOsdwUkpa/ZpTjR0I5jEks=; b=YXIRH3M9nGKTsqVxD5nVrcWJOhBECzGdiv4RcPDkNWLocjObQYt3lnJctxgx87qMK4 3qU+JLzuU2V1pHf5L2d05Mpzz5isHQRPcbhqalDbiMC63t+xRqSLeyVGbxjhhVoF6FRU rqMhtYEE2sQHutWPPx16SF4GUCUrBGFmiQUepnyf03ZoAIcXtR+SBsf9yE9ceRXZOs1J nGcLfaanXPuKCgfuvaF32OeWj5DLXua0UUOG1qGHaiGeHwkoHuCducR9cLRv1VPOdZ3C sPt0za0RiwCfLmsgYGv9ZzteJAfGLIayTZ/6pL6MnO+gzv6iSensKJ1o69FhHdBDgfcs dIXg== X-Gm-Message-State: AOJu0YxjAN3q0LUDo00v0tcWj/EMCphVq/2hXcjnWj4HJEp09RbXlTC/ cL146DZRr4Yvq6F595BMVlrTccNsZawTMAy+G4rPXzZNz9HaHf2Py4ZevjSV7GgRx3sHEFTLSlI dpZQ= X-Google-Smtp-Source: AGHT+IGnBCU0x4aD7AtOP1kDdW0FhSbQm39nZWUXiVYVAMojdrcENy6XMhxVcs1clCHYpfEBYcJ3Qw== X-Received: by 2002:a05:6808:60f:b0:3d2:395:b84 with SMTP id 5614622812f47-3d210f7e201mr233410b6e.59.1717699262224; Thu, 06 Jun 2024 11:41:02 -0700 (PDT) Received: from localhost (104-178-186-189.lightspeed.milwwi.sbcglobal.net. [104.178.186.189]) by smtp.gmail.com with ESMTPSA id 6a1803df08f44-6b04f6c3a68sm8741296d6.52.2024.06.06.11.41.01 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 06 Jun 2024 11:41:01 -0700 (PDT) Date: Thu, 6 Jun 2024 14:41:00 -0400 From: Taylor Blau To: git@vger.kernel.org Cc: Jeff King , Elijah Newren , Patrick Steinhardt , Junio C Hamano Subject: [PATCH 2/2] pack-bitmap.c: ensure pseudo-merge offset reads are bounded Message-ID: <0a16399d14afd527f4db63f2a4a3b0a3cbf112f1.1717699237.git.me@ttaylorr.com> References: Precedence: bulk X-Mailing-List: git@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: After reading the pseudo-merge extension's metadata table, we allocate an array to store information about each pseudo-merge, including its byte offset within the .bitmap file itself. This is done like so: pseudo_merge_ofs = index_end - 24 - (index->pseudo_merges.nr * sizeof(uint64_t)); for (i = 0; i < index->pseudo_merges.nr; i++) { index->pseudo_merges.v[i].at = get_be64(pseudo_merge_ofs); pseudo_merge_ofs += sizeof(uint64_t); } But if the pseudo-merge table is corrupt, we'll keep calling get_be64() past the end of the pseudo-merge extension, potentially reading off the end of the mmap'd region. Prevent this by ensuring that we have at least `table_size - 24` many bytes available to read (subtracting 24 as the length of the metadata component). This is sufficient to prevent us from reading off the end of the pseudo-merge extension, and ensures that all of the get_be64() calls below are in bounds. Helped-by: Jeff King Signed-off-by: Taylor Blau --- pack-bitmap.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/pack-bitmap.c b/pack-bitmap.c index 70230e2647..ad2635c025 100644 --- a/pack-bitmap.c +++ b/pack-bitmap.c @@ -238,6 +238,9 @@ static int load_bitmap_header(struct bitmap_index *index) index->pseudo_merges.commits_nr = get_be32(index_end - 20); index->pseudo_merges.nr = get_be32(index_end - 24); + if (st_mult(index->pseudo_merges.nr, sizeof(uint64_t)) > table_size - 24) + return error(_("corrupted bitmap index file, pseudo-merge table too short")); + CALLOC_ARRAY(index->pseudo_merges.v, index->pseudo_merges.nr);