From patchwork Fri Jan 17 12:54:59 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13943284 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 5889A1FCCEE for ; Fri, 17 Jan 2025 12:55:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737118503; cv=none; b=aGVhqIQk+U7HxIxev6EDeS/TlB0mrLja0cQw96rSIJF+zWVaZ9b01aUDizsoEIAJbo4vgOhkVfCLj+sAhIPKnRw3KvmkjoJAxDy4vxE7QFrSKQWIxCJ9UyIEH+lWOXO1ZsDRXl9meibsUBxXxktgWzu5JhPYUQhm/BF5ACLBVaI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737118503; c=relaxed/simple; bh=CQ85c7ci26jovyosP/S1sH4QhZ/kacsh/qWBZNm4xkM=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=FY+5UZG21xlBk43osyBihWPytXPC8Nt/o9F5lxMq9xM3V1qujZJXor8dnGXJY1mdfln+jpJownTZyMnaaiWg/GKbXZSZK9NuqrW+2J88yg+LMGHRzvlUd1f/LML7/4gGZbpikhQQAEsNzTiZZxCh/cBsTYI3HvorOqgMSrp1FnU= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b=CtNgxeR7; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b="CtNgxeR7" Received: (qmail 21917 invoked by uid 109); 17 Jan 2025 12:55:00 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=peff.net; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:in-reply-to; s=20240930; bh=CQ85c7ci26jovyosP/S1sH4QhZ/kacsh/qWBZNm4xkM=; b=CtNgxeR7ObasFyhjISHD6V7HznlY2H4HGmk91Thd532JFSfYSsZMcSMQqdifm9xNyNaZjNQRjYueIHZKk5pIIv7rQpWZ0asZOjvvnOfKBnebKSiAIrhDjjMiFz7ogEhqBbStUI5u0RyWYQGHRiUGVP5Su6H9AisknQVGvp7YahTPrY6Fh/CpPp2y9K3wVNH9aWuTOgPr9inhFNPdI8aigivd+z8b5Wlt8eRmOx+OWr0eTIsfMoK2jyCTrfe7nLuss5B8InMtox27DnljmAfOEA0pCdYYBz8DPjHQA6K5nyKElmaMAcX2fiKALsKM7guIY/2PLeCmiVH7nmIW7IdzUw== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Fri, 17 Jan 2025 12:55:00 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 8053 invoked by uid 111); 17 Jan 2025 12:54:59 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Fri, 17 Jan 2025 07:54:59 -0500 Authentication-Results: peff.net; auth=none Date: Fri, 17 Jan 2025 07:54:59 -0500 From: Jeff King To: Koakuma Cc: "git@vger.kernel.org" Subject: [PATCH 1/3] packfile: factor out --pack_header argument parsing Message-ID: <20250117125459.GA2893666@coredump.intra.peff.net> References: <20250117125207.GB2356599@coredump.intra.peff.net> 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: <20250117125207.GB2356599@coredump.intra.peff.net> Both index-pack and unpack-objects accept a --pack_header argument. This is an undocumented internal argument used by receive-pack and fetch to pass along information about the header of the pack, which they've already read from the incoming stream. In preparation for a bugfix, let's factor the duplicated code into a common helper. The callers are still responsible for identifying the option. While this could likewise be factored out, it is more flexible this way (e.g., if they ever started using parse-options and wanted to handle both the stuck and unstuck forms). Likewise, the callers are responsible for reporting errors, though they both just call die(). I've tweaked unpack-objects to match index-pack in marking the error for translation. Signed-off-by: Jeff King --- The generating side of this option is duplicate, too, between receive-pack and fetch-pack. But it's so much simpler that I didn't think it was worth factoring out. We could always do it later. builtin/index-pack.c | 14 +++----------- builtin/unpack-objects.c | 16 ++++------------ packfile.c | 17 +++++++++++++++++ packfile.h | 6 ++++++ 4 files changed, 30 insertions(+), 23 deletions(-) diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 0b62b2589f..75b84f78f4 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1955,18 +1955,10 @@ int cmd_index_pack(int argc, nr_threads = 1; } } else if (starts_with(arg, "--pack_header=")) { - struct pack_header *hdr; - char *c; - - hdr = (struct pack_header *)input_buffer; - hdr->hdr_signature = htonl(PACK_SIGNATURE); - hdr->hdr_version = htonl(strtoul(arg + 14, &c, 10)); - if (*c != ',') - die(_("bad %s"), arg); - hdr->hdr_entries = htonl(strtoul(c + 1, &c, 10)); - if (*c) + if (parse_pack_header_option(arg + 14, + input_buffer, + &input_len) < 0) die(_("bad %s"), arg); - input_len = sizeof(*hdr); } else if (!strcmp(arg, "-v")) { verbose = 1; } else if (!strcmp(arg, "--progress-title")) { diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index 2197d6d933..cf2bc5c531 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -18,6 +18,7 @@ #include "progress.h" #include "decorate.h" #include "fsck.h" +#include "packfile.h" static int dry_run, quiet, recover, has_errors, strict; static const char unpack_usage[] = "git unpack-objects [-n] [-q] [-r] [--strict]"; @@ -645,18 +646,9 @@ int cmd_unpack_objects(int argc, continue; } if (starts_with(arg, "--pack_header=")) { - struct pack_header *hdr; - char *c; - - hdr = (struct pack_header *)buffer; - hdr->hdr_signature = htonl(PACK_SIGNATURE); - hdr->hdr_version = htonl(strtoul(arg + 14, &c, 10)); - if (*c != ',') - die("bad %s", arg); - hdr->hdr_entries = htonl(strtoul(c + 1, &c, 10)); - if (*c) - die("bad %s", arg); - len = sizeof(*hdr); + if (parse_pack_header_option(arg + 14, + buffer, &len) < 0) + die(_("bad %s"), arg); continue; } if (skip_prefix(arg, "--max-input-size=", &arg)) { diff --git a/packfile.c b/packfile.c index cc7ab6403a..2bf9e57330 100644 --- a/packfile.c +++ b/packfile.c @@ -2315,3 +2315,20 @@ int is_promisor_object(struct repository *r, const struct object_id *oid) } return oidset_contains(&promisor_objects, oid); } + +int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *len) +{ + struct pack_header *hdr; + char *c; + + hdr = (struct pack_header *)out; + hdr->hdr_signature = htonl(PACK_SIGNATURE); + hdr->hdr_version = htonl(strtoul(in, &c, 10)); + if (*c != ',') + return -1; + hdr->hdr_entries = htonl(strtoul(c + 1, &c, 10)); + if (*c) + return -1; + *len = sizeof(*hdr); + return 0; +} diff --git a/packfile.h b/packfile.h index 58104fa009..00ada7a938 100644 --- a/packfile.h +++ b/packfile.h @@ -216,4 +216,10 @@ int is_promisor_object(struct repository *r, const struct object_id *oid); int load_idx(const char *path, const unsigned int hashsz, void *idx_map, size_t idx_size, struct packed_git *p); +/* + * Parse a --pack_header option as accepted by index-pack and unpack-objects, + * turning it into the matching bytes we'd find in a pack. + */ +int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *len); + #endif From patchwork Fri Jan 17 12:55:30 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13943285 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 CC2151FE477 for ; Fri, 17 Jan 2025 12:55:32 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737118534; cv=none; b=JMYLocX/MrkpSnCBi+spRP/QSqbqMA/IVi9RS9gka01jB2lHFQjtgMPdTJ3fHeY5JWro9vtmjIc8kVe01qOUXNX6oD/da2rbUkAqGH7ycB/G4X/1iMCe1sJjadA0reqIfx99cdky/mvLbEw7Im51rhOhQSqlJTgJLaS5FBihzwY= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737118534; c=relaxed/simple; bh=zDEUXzXJJiK9j4cKo5qDBYp7uG1Mdb1tuyyOiQMcvQE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=EyvZR/ZsU6hoAfM/hTZYXtb+nFpQB5nDKETyQyYbAerlbOAydRNjwgGqvZrzVEOr74Xqf28bt7XjNegQYO5zqFE05ZBHix1YQCCdztlwGZ2rIyW/68e/tJ+iip2Lk8F/pPN6RkQ91rR8Wmyns6m8jrxfzChbFmCI+f/dW0D73NA= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b=HS23juK9; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b="HS23juK9" Received: (qmail 21926 invoked by uid 109); 17 Jan 2025 12:55:32 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=peff.net; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:in-reply-to; s=20240930; bh=zDEUXzXJJiK9j4cKo5qDBYp7uG1Mdb1tuyyOiQMcvQE=; b=HS23juK9/99eQGw/Svq0wbJsSANxCS0qQ+edkBqNJJxkSq0KcO5gfD1Fe4l+opLA+LwszwFzYZhb33bC/2JkwbwoOS+iJ168SmV01Oc27RfSIDCjcHr5jC4UvzPZaL8NTnlydw+VkvN5dBDHHK+gXj0ikMMGZcAgYPvtWop5DolCbP0HYcCUUu0tS+etGIWCdpt6MBXmD8CASUWX2YR/Ip91ewe2ySa6dVy1p9oP0zCAfsqb0rzBCbAGcL6vaIzjdF0DMTVzlCo3nB6cxFMhqvvduM7Xh65qXpts/CK7WLqvPgOVNG0eKEoeHVYBF6GMRkhx5c1SeHmuRs42SUYapg== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Fri, 17 Jan 2025 12:55:32 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 8088 invoked by uid 111); 17 Jan 2025 12:55:31 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Fri, 17 Jan 2025 07:55:31 -0500 Authentication-Results: peff.net; auth=none Date: Fri, 17 Jan 2025 07:55:30 -0500 From: Jeff King To: Koakuma Cc: "git@vger.kernel.org" Subject: [PATCH 2/3] parse_pack_header_option(): avoid unaligned memory writes Message-ID: <20250117125530.GB2893666@coredump.intra.peff.net> References: <20250117125207.GB2356599@coredump.intra.peff.net> 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: <20250117125207.GB2356599@coredump.intra.peff.net> In order to recreate a pack header in our in-memory buffer, we cast the buffer to a "struct pack_header" and assign the individual fields. This is reported to cause SIGBUS on sparc64 due to alignment issues. We can work around this by using put_be32() which will write individual bytes into the buffer. Reported-by: Koakuma Signed-off-by: Jeff King --- Fingers crossed that this is sufficient, and we don't have more alignment problems. packfile.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/packfile.c b/packfile.c index 2bf9e57330..2d80d80cb3 100644 --- a/packfile.c +++ b/packfile.c @@ -2318,17 +2318,20 @@ int is_promisor_object(struct repository *r, const struct object_id *oid) int parse_pack_header_option(const char *in, unsigned char *out, unsigned int *len) { - struct pack_header *hdr; + unsigned char *hdr; char *c; - hdr = (struct pack_header *)out; - hdr->hdr_signature = htonl(PACK_SIGNATURE); - hdr->hdr_version = htonl(strtoul(in, &c, 10)); + hdr = out; + put_be32(hdr, PACK_SIGNATURE); + hdr += 4; + put_be32(hdr, strtoul(in, &c, 10)); + hdr += 4; if (*c != ',') return -1; - hdr->hdr_entries = htonl(strtoul(c + 1, &c, 10)); + put_be32(hdr, strtoul(c + 1, &c, 10)); + hdr += 4; if (*c) return -1; - *len = sizeof(*hdr); + *len = hdr - out; return 0; } From patchwork Fri Jan 17 12:56:47 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13943286 Received: from cloud.peff.net (cloud.peff.net [104.130.231.41]) (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 526B7197A92 for ; Fri, 17 Jan 2025 12:56:48 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=104.130.231.41 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737118610; cv=none; b=J7/KRM/FA/DlaRT6AwEMNX3MnlvTLl89c9D3/12vbByrv9J00rbtl1JvqwhrGm6G49jDfMlT//HWnwoX9D4XOg0FrN0QPEc77c+F+fweYfQWM0pxG97PuNNg58Hi1nF7y8bp/FPXsM4f/uieRMisjyBm9Yjn72LTA/LgNc/5P2w= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737118610; c=relaxed/simple; bh=gukDzbMnIbjMbkiE0lj4VjIPoLk8XV0HgUTGc1EXer0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=WnlFsRY3F3D4XmFVVcdAJlxXaLTnjz3EN62ApOb0QXmupbDKaMEAHyK/2nSJ2C0BJDsbDM0XVxjSn8baGIsQNi+fO3tQlhfxPdkNr2UwCsPFJ7ifiuu2OuiBqOHT2U0ZBQLkbjhIq4PIBWvw50SyL+tGokjz3FJD35EHuMsSVJI= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net; spf=pass smtp.mailfrom=peff.net; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b=BHC56SOJ; arc=none smtp.client-ip=104.130.231.41 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=peff.net Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=peff.net Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=peff.net header.i=@peff.net header.b="BHC56SOJ" Received: (qmail 21939 invoked by uid 109); 17 Jan 2025 12:56:48 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed; d=peff.net; h=date:from:to:cc:subject:message-id:references:mime-version:content-type:in-reply-to; s=20240930; bh=gukDzbMnIbjMbkiE0lj4VjIPoLk8XV0HgUTGc1EXer0=; b=BHC56SOJY6NWE05dRRmCRNUrnZNpzVyFmKUXp+ECnOSrXgRK8yRrapnFfOov3yqc5oOOWQrb+D1knRUWbuGkcGbmOxCuoExbAW1zkpjyFyWecWf7GeDtXyl8RGsQsmYdBv6in0MKRSq2QG807CWZVfGhR7FHhAdokJWk8UsxHWT7FSmsORgRL+fE00uoxt00zv93tIy08+oiuRDTilZU0jkV/LTbG3/CMb4Op19bP/bAOC32HxWJURn2/IMcuRA5PGH59sLHLKPK3nlxAczafLkLp0/jS6fnqzsiKMZfC3qCkb2MK75v8AGAeyQl4+StUF9ZF+MYCbxn8tJjn6xiOg== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Fri, 17 Jan 2025 12:56:48 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 8136 invoked by uid 111); 17 Jan 2025 12:56:48 -0000 Received: from coredump.intra.peff.net (HELO coredump.intra.peff.net) (10.0.0.2) by peff.net (qpsmtpd/0.94) with (TLS_AES_256_GCM_SHA384 encrypted) ESMTPS; Fri, 17 Jan 2025 07:56:48 -0500 Authentication-Results: peff.net; auth=none Date: Fri, 17 Jan 2025 07:56:47 -0500 From: Jeff King To: Koakuma Cc: "git@vger.kernel.org" Subject: [PATCH 3/3] index-pack, unpack-objects: use skip_prefix to avoid magic number Message-ID: <20250117125647.GC2893666@coredump.intra.peff.net> References: <20250117125207.GB2356599@coredump.intra.peff.net> 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: <20250117125207.GB2356599@coredump.intra.peff.net> When parsing --pack_header=, we manually skip 14 bytes to the data. Let's use skip_prefix() to do this automatically. Note that we overwrite our pointer to the front of the string, so we have to add more context to the error message. We could avoid this by declaring an extra pointer to hold the value, but I think the modified message is actually preferable. It should give translators a bit more context. Signed-off-by: Jeff King --- Just a cleanup I noticed in the area. It's possible that I'm overly annoyed by manual string counting and this is just churn, though. ;) builtin/index-pack.c | 6 +++--- builtin/unpack-objects.c | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 75b84f78f4..0561eec00a 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1954,11 +1954,11 @@ int cmd_index_pack(int argc, warning(_("no threads support, ignoring %s"), arg); nr_threads = 1; } - } else if (starts_with(arg, "--pack_header=")) { - if (parse_pack_header_option(arg + 14, + } else if (skip_prefix(arg, "--pack_header=", &arg)) { + if (parse_pack_header_option(arg, input_buffer, &input_len) < 0) - die(_("bad %s"), arg); + die(_("bad --pack_header: %s"), arg); } else if (!strcmp(arg, "-v")) { verbose = 1; } else if (!strcmp(arg, "--progress-title")) { diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index cf2bc5c531..06d517dfb6 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -645,10 +645,10 @@ int cmd_unpack_objects(int argc, fsck_set_msg_types(&fsck_options, arg); continue; } - if (starts_with(arg, "--pack_header=")) { - if (parse_pack_header_option(arg + 14, + if (skip_prefix(arg, "--pack_header=", &arg)) { + if (parse_pack_header_option(arg, buffer, &len) < 0) - die(_("bad %s"), arg); + die(_("bad --pack_header: %s"), arg); continue; } if (skip_prefix(arg, "--max-input-size=", &arg)) {