Re: [PATCH 0/2] midx: prevent bitmap corruption when permuting pack order

On 12/8/2021 2:26 PM, Taylor Blau wrote:

This series demonstrates and fixes a problem (which has existed since the
inception of MIDX bitmaps) which made it possible to reliably introduce bitmap
corruption into a repository.

The recipe (which is explained in detail in the first patch) roughly looks like
creating a MIDX with some packs. Then re-generating the MIDX and its bitmap but
under a different object order (e.g., by changing the `--preferred-pack`).

Because we used to link the new .rev file into place, and because the object
order is not encoded into the MIDX itself, we would inadvertently generate a
.rev file with the same name as the existing one that contains different
contents, and then fail to move it into place. That causes us to read the
.bitmap under the old object order, producing incorrect results.

This series applies a minimal fix (in the second patch), which is to include the
pack ordering in the MIDX itself. Some small thoughts here:

  - We could alternatively have included the entire .rev file's contents in the
    MIDX. But this ends up being kind of awkward (and is also discussed in the
    second patch).

  - We could cache the identity of the preferred pack (which we currently
    discover by looking up the pack for the object in the 0th position of the
    MIDX's pseudo-pack order) by just reading the first value of the new
    optional chunk.

I'm certainly interested in pursuing the latter, but in a different series,
since I'd like to keep this fix as minimal as possible.

Taylor Blau (2):
  t5326: demonstrate bitmap corruption after permutation
  midx.c: make changing the preferred pack safe

Just chiming in to say that I reviewed an earlier version of this series
and the version in this submission looks good to me.

Thanks,
-Stolee