Re: [BUG] git crashes with a SIGBUS on sparc64 during pull

From: Koakuma <koachan@protonmail.com>
Date: Jan 17, 2025, 15:55 UTC
Message-ID: <nwg0r52UT4d_kBbQa7V7xCWKxE1xtHiwXKtBvzEwxtJa-3nKpXcWG2DRGUENDSJQ9QOkk4jrEaXYjAbiSAZds2ynH1TZPcX3x6T_t4zm8s4=@protonmail.com>
In-Reply-To: <20250117125207.GB2356599@coredump.intra.peff.net>

Jeff King <peff@peff.net> wrote:

Show 13 quoted lines

> Perhaps:
> 
> git init repo
> cd repo
> git commit --allow-empty -m foo
> git repack -ad
> pack=$(ls .git/objects/pack/*.pack)
> dd if=$pack of=no-header.pack bs=1 skip=12
> # don't bother parsing the first 12 bytes; we know it is
> # a version 2 pack with 2 objects
> git unpack-objects --pack_header=2,2 <no-header.pack
> 
> would be a minimal reproduction?

Thank you! This reliably triggers it every time.

Show 34 quoted lines

> diff --git a/builtin/unpack-objects.c b/builtin/unpack-objects.c
> index 2197d6d933..288cecf98f 100644
> --- a/builtin/unpack-objects.c
> +++ b/builtin/unpack-objects.c
> @@ -645,18 +645,20 @@ int cmd_unpack_objects(int argc,
> continue;
> }
> if (starts_with(arg, "--pack_header=")) {
> - struct pack_header *hdr;
> + unsigned char *hdr = buffer;
> char *c;
> 
> - hdr = (struct pack_header *)buffer;
> - hdr->hdr_signature = htonl(PACK_SIGNATURE);
> 
> - hdr->hdr_version = htonl(strtoul(arg + 14, &c, 10));
> 
> + put_be32(hdr, PACK_SIGNATURE);
> + hdr += 4;
> + put_be32(hdr, strtoul(arg + 14, &c, 10));
> + hdr += 4;
> if (*c != ',')
> die("bad %s", arg);
> - hdr->hdr_entries = htonl(strtoul(c + 1, &c, 10));
> 
> + put_be32(hdr, strtoul(c + 1, &c, 10));
> + hdr += 4;
> if (*c)
> die("bad %s", arg);
> - len = sizeof(*hdr);
> + len = hdr - buffer;
> continue;
> }
> if (skip_prefix(arg, "--max-input-size=", &arg)) {

This diff does fix the issue in `cmd_unpack_objects`, however...

> I'm curious if it's enough. After we write to this unaligned buffer,
> naturally the next thing we'll do is read from it, and the reading
> routines will do the same cast (see unpack_all() in unpack-objects).

It crashes in `unpack_all`, just as you guessed:

#0 unpack_all () at builtin/unpack-objects.c:583 583 nr_objects = ntohl(hdr->hdr_entries);

So I suppose the reading part needs to be adjusted as well?

> But maybe your platform allows unaligned reads but not writes? Probably
> I am being too optimistic. :)

As far as I understand, sparc64 traps on any unaligned accesses, be it reads or writes.