Re: MSan failures in pack-bitmap

From: Taylor Blau <me@ttaylorr.com>
Date: Jun 9, 2024, 20:00 UTC
Message-ID: <ZmYJ+d3+j1E08Ms/@nand.local>
In-Reply-To: <xmqqmsnuaqus.fsf@gitster.g>

On Sun, Jun 09, 2024 at 11:55:07AM -0700, Junio C Hamano wrote:

Show 14 quoted lines

> Taylor Blau <me@ttaylorr.com> writes:
>
> > Unfortunately, the regression happened in 795006fff4, so this is in a
> > released version of Git. But this is all behind a configuration option,
> > so affected users can reasonably work around this issue.
>
> Can you elaborate a bit about "reasonably work around"?
>
> I am imagining that you are saying that the user can say "do not use
> bitmap, because it may be corrupted due to an earlier regression" in
> some configuration, so that a corrupt bitmap data is not used to
> further spread the damage, but how does a user find out that on-disc
> midx is broken and a workaround needs to be employed in the first
> place?

I don't think the issue here is a corrupt on-disk bitmap or MIDX. The regression that Kyle reported was a logic issue reading a ordinary/non-corrupt MIDX file, not a result of some on-disk corruption.

I think it is possible to generate a corrupt pack as a result of this bug which would incorrectly "reuse" parts of an existing MIDX's preferred pack, but the result would be far worse than a corrupt MIDX (which, again, I don't think is what's happening here).

But I do not think that repository corruption is a likely outcome here, since we only do pack reuse when packing to stdout (i.e. by passing the `--stdout` option to `pack-objects`). So repacks (which would otherwise be the thing I'm most worried about) are safe: repacks trigger pack-objects to write a tempfile directly, and so `pack_to_stdout` is zero'd, and we do not allow verbatim pack reuse to take place.

So a doomsday scenario whereby we'd write a corrupt pack based on this bug and then delete a non-corrupt pack is not directly possible via 'repack' alone. The lesser-used 'git multi-pack-index repack' is also not affected, since it similarly does not use `pack-objects --stdout`.

A more likely outcome would be if a Git client who was using MIDX bitmaps, but only doing single-pack reuse tried to generate a pack to push to some remote (or vice-versa, replacing client and server/remote) which ended up being corrupt in some fashion. I haven't had enough of a chance to determine if this possible, though I suspect it is.

Thanks, Taylor