From patchwork Sun Jan 19 13:23:08 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13944458 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 70FA12913 for ; Sun, 19 Jan 2025 13:23:10 +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=1737292993; cv=none; b=Z/OwCEx1lblGKq2aCNjKU1UIDRkT2xSBfksNW0BNaiCqjrmzLNjDn1e4TyaGZVOzX6U7h1lh5ctfltPqUJkDhMZtgtSht8VbtMA4plgPVYXHYVDUuSVZWEW8cjuMBbZhkeQCTkK6NK/0clUrwlY+j8IqLtRUZ7kmvJ38o2+NQdA= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737292993; c=relaxed/simple; bh=lld4gzhICbA6zpIv2SHx+SqkuomgJcI2nvmXVB0gQS8=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=i9n955l5pk9GwmCuDRKEAXYvDbXSeOBqXE+2sKm1VfExEiFivhd3I7WCi4bu7T9MyJbyEAtizLCXXwc20B8OPYc7EAq0uYRzjpZfQ5AtOBXN6DHePr0cqaIFHdlqO2+2Z+RL5q+kPpAOExV8BQl42+cSrN7WXo7wLwZl/Pp/1LA= 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=hnpJqUEr; 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="hnpJqUEr" Received: (qmail 7965 invoked by uid 109); 19 Jan 2025 13:23:09 -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=lld4gzhICbA6zpIv2SHx+SqkuomgJcI2nvmXVB0gQS8=; b=hnpJqUErNz2HPvmUri74MoFjXLZQgZJVwiIppvTSswi4ftelrxxwqAx0ZEQUyWvH8JEYUJlWXXECUriHxQ05nfLX8iWeK+fDeChSqJyb0LLuaTjmC+V6Zody/P265Sy2SGtNxUgDWf/fqCTKK5xW2NF1uL2KQCtmkKfQfy89QfXcnRt7lw7n4Nqy22Hv02TQpswST9u5nteP96sF/djHJCJaN87rWj4juYowoxOmILoZn1tWSmIDJAUaMpJQVe2UEQf/sOVzpMUpJbTvsXV80SGBJ1tkgH2fuV+q6cy3dO/XZ4qkC2IOcTILP+C74H3jb+/E2NYaRkrECRJdRgDmcQ== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sun, 19 Jan 2025 13:23:09 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 734 invoked by uid 111); 19 Jan 2025 13:23:09 -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; Sun, 19 Jan 2025 08:23:09 -0500 Authentication-Results: peff.net; auth=none Date: Sun, 19 Jan 2025 08:23:08 -0500 From: Jeff King To: Koakuma Cc: Junio C Hamano , "git@vger.kernel.org" Subject: [PATCH v2 1/5] bswap.h: squelch potential sparse -Wcast-truncate warnings Message-ID: <20250119132308.GA1552263@coredump.intra.peff.net> References: <20250119131224.GA1541095@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: <20250119131224.GA1541095@coredump.intra.peff.net> From: Junio C Hamano In put_be32(), we right-shift a uint32_t value various amounts and then assign the low 8-bits to individual "unsigned char" bytes, throwing away the high bits. For shifts smaller than 24 bits, those thrown away bits will be arbitrary bits from the original uint32_t. This works exactly as we want, but if you feed a constant, then sparse complains. For example if we write this (which we plan to do in a future patch): put_be32(hdr, PACK_SIGNATURE); then "make sparse" produces: compat/bswap.h:175:22: error: cast truncates bits from constant value (5041 becomes 41) compat/bswap.h:176:22: error: cast truncates bits from constant value (504143 becomes 43) compat/bswap.h:177:22: error: cast truncates bits from constant value (5041434b becomes 4b) And the same issue exists in the other put_be*() functions, when used with a constant. We can silence this warning by explicitly masking off the truncated bits. The compiler is smart enough to know the result is the same, and the asm generated by gcc (with both -O0 and -O2) is identical. Curiously this line already exists: put_be32(&hdr_version, INDEX_EXTENSION_VERSION2); in the fsmonitor.c file, but it does not get flagged because the CPP macro expands to a small integer (2). Signed-off-by: Jeff King --- I tweaked the commit message to make more sense in context. Probably should get Junio's signoff. :) I noticed that reftable/basics.c has its own version of these macros, and it also does the masking. compat/bswap.h | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/compat/bswap.h b/compat/bswap.h index 512f6f4b99..b34054f2bd 100644 --- a/compat/bswap.h +++ b/compat/bswap.h @@ -171,23 +171,23 @@ static inline uint64_t get_be64(const void *ptr) static inline void put_be32(void *ptr, uint32_t value) { unsigned char *p = ptr; - p[0] = value >> 24; - p[1] = value >> 16; - p[2] = value >> 8; - p[3] = value >> 0; + p[0] = (value >> 24) & 0xff; + p[1] = (value >> 16) & 0xff; + p[2] = (value >> 8) & 0xff; + p[3] = (value >> 0) & 0xff; } static inline void put_be64(void *ptr, uint64_t value) { unsigned char *p = ptr; - p[0] = value >> 56; - p[1] = value >> 48; - p[2] = value >> 40; - p[3] = value >> 32; - p[4] = value >> 24; - p[5] = value >> 16; - p[6] = value >> 8; - p[7] = value >> 0; + p[0] = (value >> 56) & 0xff; + p[1] = (value >> 48) & 0xff; + p[2] = (value >> 40) & 0xff; + p[3] = (value >> 32) & 0xff; + p[4] = (value >> 24) & 0xff; + p[5] = (value >> 16) & 0xff; + p[6] = (value >> 8) & 0xff; + p[7] = (value >> 0) & 0xff; } #endif /* COMPAT_BSWAP_H */ From patchwork Sun Jan 19 13:23:37 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13944459 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 4EDED2913 for ; Sun, 19 Jan 2025 13:23:38 +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=1737293021; cv=none; b=N6E6lMZJpXgRyHlsxSGnRsMLdm1A7vWYJaV8pB0Vt2C6NfDSnghfTz8uVnMDkRZaAOAEfLddlfu3zQvqiK+j0j04lsa+pUFd3cRhHQtavlVM4R46O9EwfngMVhM2EaWRwxknGOxLZBEhRvVQOx6O3pH8yhtSayvxN9/qyM5ucbw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737293021; c=relaxed/simple; bh=M601vB9sRbik5FJIH0h1AYpNfUThGTn4Pn4XlJbl9mA=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=lC+7CnOfouu5vuJw7EZzb8/VCRHjckD8zCmToxUHhMdXdhyyE8cZ+G/0HV6kDznnrsMkgoTrn81bL9W37DpgZBjWo8gzOKMWWy65YTIpOrWyIpggImw0T7KF1DXOuNltvtKh7YkUVSZMFnFMrc9iS0S2xzTbdb4fbgoptljfwAI= 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=S5vJQxyQ; 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="S5vJQxyQ" Received: (qmail 7973 invoked by uid 109); 19 Jan 2025 13:23:38 -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=M601vB9sRbik5FJIH0h1AYpNfUThGTn4Pn4XlJbl9mA=; b=S5vJQxyQzHEUh7Ot1DARojslyqqGIEJrQakv3Wlcd/9HGP54gaIHzeR/dl0tCZxCjg1XjgWoWRjS+lvUEfmSlcXbRwcnnHvEzsiG+rvZ37VBU/fLwsGbMo6qTb+E7ffW02VH+5Dehzg0lEom2rFcWbVg9jSFIMIEn1M4fWvEbyxFQkaZTshZ3V1GLoJsTSFvTPykfwVQkeBkML0v3SfLHLXGG491GePjuZdP/OETWncSxB3+M756PhGKZ5jWR1fFzTgjm8LtSo6opehpI1VRH0Uap22ZdXylStG8SRGnfWmiP08f0KSST0LwEIN04TqzjZHUCELHwIgQ4yfiBAD7Mg== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sun, 19 Jan 2025 13:23:38 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 742 invoked by uid 111); 19 Jan 2025 13:23:38 -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; Sun, 19 Jan 2025 08:23:38 -0500 Authentication-Results: peff.net; auth=none Date: Sun, 19 Jan 2025 08:23:37 -0500 From: Jeff King To: Koakuma Cc: Junio C Hamano , "git@vger.kernel.org" Subject: [PATCH v2 2/5] packfile: factor out --pack_header argument parsing Message-ID: <20250119132337.GB1552263@coredump.intra.peff.net> References: <20250119131224.GA1541095@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: <20250119131224.GA1541095@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 --- Same as before. 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 Sun Jan 19 13:23:44 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13944460 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 84A521DEFCD for ; Sun, 19 Jan 2025 13:23:46 +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=1737293028; cv=none; b=IRIPiIl/T3and2UAo5fB35Dc1/EsBIV3G5MpHg9ss+DCZkgc3dTAgZMwxj05zWGApIZ/GbgyJrOlgixutYQ6Q8EbsyyphUuMbkiiGapvGYjzSvWqAdqk3yIQ41XoRg4W8e6mAp6WG26X3LxJ2lQJGygKJjobewoP9CUuQ7qQzmI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737293028; c=relaxed/simple; bh=ANFyt+e7D6m7+Lf5Rtw4I1uNNKtKznzTJXeKhtM/2r4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=n8ADieY6M0f1IjqoX5sNgHrgEhR/XVjhWNQhytdN2HdqoAt5koe25H44mbIsFWW/fu/IVPF/Y8JZAgi0gIbXBxJIdLdF04OqQjLvrYyYav3k9ixFbxiViwnFlzGcUEIjPVUIYLswzOgGY+7Usw2sq1QdPmuShHc3WILdM6RBX88= 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=T4iyTfhI; 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="T4iyTfhI" Received: (qmail 7984 invoked by uid 109); 19 Jan 2025 13:23:45 -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=ANFyt+e7D6m7+Lf5Rtw4I1uNNKtKznzTJXeKhtM/2r4=; b=T4iyTfhIggb0eoXe9JJQ3Y259XKuqg6Pz73XWAqwxWE1xRUvK59/uU/gZDHlUVfwEO9tkrdDJ3u0t8xYJeA0FONcPVel22WPrVgVi0z3aczhhludsYsGuBqgmDoNl7yVYYGz+qa8k3uFNlFV0DMSwxNPQXgU7JiuA6CQ1B1fvix4EC3Z3Dxkw0QV7P2rQgk5YSzNVxE/dg2fZMRJ+fotFerthklJSV8683Mij1zmBXBehxMwNlmxDGjGTMpbiO1gzvIMiKPAOzBYGVBfxHWFw5s+d//1ZoFfC+ctVc+wydU/10mU3g1MvsEyH0qJEyKs+B6dslTZdkQoq2OP1nX4Lw== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sun, 19 Jan 2025 13:23:45 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 747 invoked by uid 111); 19 Jan 2025 13:23:45 -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; Sun, 19 Jan 2025 08:23:45 -0500 Authentication-Results: peff.net; auth=none Date: Sun, 19 Jan 2025 08:23:44 -0500 From: Jeff King To: Koakuma Cc: Junio C Hamano , "git@vger.kernel.org" Subject: [PATCH v2 3/5] parse_pack_header_option(): avoid unaligned memory writes Message-ID: <20250119132344.GC1552263@coredump.intra.peff.net> References: <20250119131224.GA1541095@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: <20250119131224.GA1541095@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 --- Same as before. 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 Sun Jan 19 13:25: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: 13944461 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 4A9FA1DA3D for ; Sun, 19 Jan 2025 13:25: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=1737293151; cv=none; b=XAr8vDxJnqJ/hen4/DsR5xUbhuS/7JPwR7AeYFprCvql64AyLTtsO1V4lkLUpNDJ8r/02827BtBKEaSSjUyqD83z0/0rDD4j72kipAP8lYeSrD9p6psf+WdFnoMl42tkjQGNZ5a3SCqzNfst3vTLE4UtuS4KhLU8ZUuMoP5B+6o= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737293151; c=relaxed/simple; bh=A113ll2crsf8XC3UOLfcyOZi2Ch4MBypyXRHvPzlOvI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=npAgR6wyOkWFR4xOL7ccRvXFdJ+gMd6zosaPP9BL7oUkUkFq5488bWd4u9Qt9oszFqBqT+MbefBqt3ID5+li3aUGOcpg28+jAQ4eeKOQ1Be8mxUniTbG73oCriu76fBJI0BZBQ9JePHQItEj8PCTfjHQoGl2nlgt/jyvl4cIKsc= 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=hhmmYmiq; 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="hhmmYmiq" Received: (qmail 7996 invoked by uid 109); 19 Jan 2025 13:25: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=A113ll2crsf8XC3UOLfcyOZi2Ch4MBypyXRHvPzlOvI=; b=hhmmYmiqA2HKBwqS9IU51b7I2UPKW9gkE5+812boTyz16CyHU2adbimhpYn7KWj+KeonX7ZYPp6cmyELCDB1l5eRrX02F+8BiOrvesfw6CqWl95+EbrtnwPKlBmDVXvFhlL3skJMtBkOrjwYwfr9VkiEusQJl37anHFmI6mDLkPV/nLR0jmaRiaolYf56FVZGZsssJBhrOW+0coQVtl/6JF1ivHJbyRYT0GIUWPphbP75AaQQUN0SUCSYsR33jc20o25fzu5LpuTq16zc5W1Zzds3pLgpxvsFVRz8qfUcBWRqA7f0nkPWTuWSn44PE+37LMWs9ALJm18AdSfQm0EuQ== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sun, 19 Jan 2025 13:25:48 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 786 invoked by uid 111); 19 Jan 2025 13:25: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; Sun, 19 Jan 2025 08:25:47 -0500 Authentication-Results: peff.net; auth=none Date: Sun, 19 Jan 2025 08:25:47 -0500 From: Jeff King To: Koakuma Cc: Junio C Hamano , "git@vger.kernel.org" Subject: [PATCH v2 4/5] index-pack, unpack-objects: use get_be32() for reading pack header Message-ID: <20250119132547.GD1552263@coredump.intra.peff.net> References: <20250119131224.GA1541095@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: <20250119131224.GA1541095@coredump.intra.peff.net> Both of these commands read the incoming pack into a static unsigned char buffer in BSS, and then parse it by casting the start of the buffer to a struct pack_header. This can result in SIGBUS on some platforms if the compiler doesn't place the buffer in a position that is properly aligned for 4-byte integers. This reportedly happens with unpack-objects (but not index-pack) on sparc64 when compiled with clang (but not gcc). But we are definitely in the wrong in both spots; since the buffer's type is unsigned char, we can't depend on larger alignment. When it works it is only because we are lucky. We'll fix this by switching to get_be32() to read the headers (just like the last few commits similarly switched us to put_be32() for writing into the same buffer). It would be nice to factor this out into a common helper function, but the interface ends up quite awkward. Either the caller needs to hardcode how many bytes we'll need, or it needs to pass us its fill()/use() functions as pointers. So I've just fixed both spots in the same way; this is not code that is likely to be repeated a third time (most of the pack reading code uses an mmap'd buffer, which should be properly aligned). I did make one tweak to the shared code: our pack_version_ok() macro expects us to pass the big-endian value we'd get by casting. We can introduce a "native" variant which uses the host integer ordering. Reported-by: Koakuma Signed-off-by: Jeff King --- New in v2. I did write the function-pointer version, and it's not even _too_ bad to read. But while trying to write a comment documenting the helper, it was just too gross to justify. builtin/index-pack.c | 12 +++++++----- builtin/unpack-objects.c | 13 +++++++------ pack.h | 3 ++- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/builtin/index-pack.c b/builtin/index-pack.c index 75b84f78f4..d6fd4bbde6 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -379,16 +379,18 @@ static const char *open_pack_file(const char *pack_name) static void parse_pack_header(void) { - struct pack_header *hdr = fill(sizeof(struct pack_header)); + unsigned char *hdr = fill(sizeof(struct pack_header)); /* Header consistency check */ - if (hdr->hdr_signature != htonl(PACK_SIGNATURE)) + if (get_be32(hdr) != PACK_SIGNATURE) die(_("pack signature mismatch")); - if (!pack_version_ok(hdr->hdr_version)) + hdr += 4; + if (!pack_version_ok_native(get_be32(hdr))) die(_("pack version %"PRIu32" unsupported"), - ntohl(hdr->hdr_version)); + get_be32(hdr)); + hdr += 4; - nr_objects = ntohl(hdr->hdr_entries); + nr_objects = get_be32(hdr); use(sizeof(struct pack_header)); } diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c index cf2bc5c531..76c6a9031b 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -579,15 +579,16 @@ static void unpack_one(unsigned nr) static void unpack_all(void) { int i; - struct pack_header *hdr = fill(sizeof(struct pack_header)); + unsigned char *hdr = fill(sizeof(struct pack_header)); - nr_objects = ntohl(hdr->hdr_entries); - - if (ntohl(hdr->hdr_signature) != PACK_SIGNATURE) + if (get_be32(hdr) != PACK_SIGNATURE) die("bad pack file"); - if (!pack_version_ok(hdr->hdr_version)) + hdr += 4; + if (!pack_version_ok_native(get_be32(hdr))) die("unknown pack file version %"PRIu32, - ntohl(hdr->hdr_version)); + get_be32(hdr)); + hdr += 4; + nr_objects = get_be32(hdr); use(sizeof(struct pack_header)); if (!quiet) diff --git a/pack.h b/pack.h index a8da040629..78f8d8b213 100644 --- a/pack.h +++ b/pack.h @@ -13,7 +13,8 @@ struct repository; */ #define PACK_SIGNATURE 0x5041434b /* "PACK" */ #define PACK_VERSION 2 -#define pack_version_ok(v) ((v) == htonl(2) || (v) == htonl(3)) +#define pack_version_ok(v) pack_version_ok_native(ntohl(v)) +#define pack_version_ok_native(v) ((v) == 2 || (v) == 3) struct pack_header { uint32_t hdr_signature; uint32_t hdr_version; From patchwork Sun Jan 19 13:25:53 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff King X-Patchwork-Id: 13944462 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 7215D1DA3D for ; Sun, 19 Jan 2025 13:25:55 +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=1737293157; cv=none; b=RxmcF9iFKfOFwaWQmbpuUTebS7zY18aLjGgVY4R57yM1zrNsHhsJZTwP3LtYQrtV494Iv0xFbGuB5WV2LBbsUuVld+eRmqly3OP186nIF7SyZBhjwYTUEYJM5juA44DjlADkZ00DzkU97KxjppjcknHu3s0DRrkeg+RBxorRRu4= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1737293157; c=relaxed/simple; bh=PAxAqlbV0/EmRXxrrWIL0KmPpewo3kY+rgIi4Xyrkl0=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=N61wLsvwADI81MZzPLhKvkEpWI8wibZLqAYawQK5lWTIOKtuMNf5RcH/cf1bQp2Gi6uSBnarZbVeHkODD+Nje3n++E3iI64hrax0/O++eq/NsXrxWv7UAH97I2XFeN8apLUn8mibcdrFxbehismTLDJX1YzTVbXgUPLpX8VXDv0= 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=PWZU2BlU; 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="PWZU2BlU" Received: (qmail 8003 invoked by uid 109); 19 Jan 2025 13:25:54 -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=PAxAqlbV0/EmRXxrrWIL0KmPpewo3kY+rgIi4Xyrkl0=; b=PWZU2BlUMPb8qS5tLeIdyUXrErU4X9GH+RQLiZTIieV1aP4RKuYrT2wbvJzUYdM5/hR6UY872ywfwJ7zjpBtL4B6HW3zkNL+78OfeX4KapUY9Vps4Z6lFVqdlwnmYKXi8+KMPBXfCv/5zPj2Z+UwT1HN54sWmiA6m6yhBO0yOWVwUaR8qvH92Y+w9GBmu/qx/vOf5vpZA17k+/ZdUBHiXLEgdHpfXhXEWhWJ3DiIKexjq70qXp4O1ymM9fEqJHi45/2EQRN+8/JmDzEQf9gNPmumL1fUltlKPkJRtnU0Q6VcY2Z0uQIalkVX4YG7YhNYTJi55UISMnkf4KHQza8o3g== Received: from Unknown (HELO peff.net) (10.0.1.2) by cloud.peff.net (qpsmtpd/0.94) with ESMTP; Sun, 19 Jan 2025 13:25:54 +0000 Authentication-Results: cloud.peff.net; auth=none Received: (qmail 791 invoked by uid 111); 19 Jan 2025 13:25:54 -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; Sun, 19 Jan 2025 08:25:54 -0500 Authentication-Results: peff.net; auth=none Date: Sun, 19 Jan 2025 08:25:53 -0500 From: Jeff King To: Koakuma Cc: Junio C Hamano , "git@vger.kernel.org" Subject: [PATCH v2 5/5] index-pack, unpack-objects: use skip_prefix to avoid magic number Message-ID: <20250119132553.GE1552263@coredump.intra.peff.net> References: <20250119131224.GA1541095@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: <20250119131224.GA1541095@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 --- 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 d6fd4bbde6..c632d9f88b 100644 --- a/builtin/index-pack.c +++ b/builtin/index-pack.c @@ -1956,11 +1956,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 76c6a9031b..51a856d823 100644 --- a/builtin/unpack-objects.c +++ b/builtin/unpack-objects.c @@ -646,10 +646,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)) {