Re: MSan failures in pack-bitmap

[Taylor Blau <me@ttaylorr.com>]

On Sun, Jun 09, 2024 at 11:55:07AM -0700, Junio C Hamano wrote:
> 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