Re: [PATCH] refs: fix corruption by not correctly syncing packed-refs to disk

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 25, 2022, 11:25 UTC
Message-ID: <xmqqtu1johbs.fsf@gitster.g>
In-Reply-To: <Y6HYTt58iecdBEdw@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 17 quoted lines

> On Tue, Dec 20, 2022 at 03:52:14PM +0100, Patrick Steinhardt wrote:
>
>> And while we do the dance when writing the `packed-refs` file, there is
>> indeed one gotcha: we use a `FILE *` stream to write the temporary file,
>> but don't flush it before synchronizing it to disk. As a consequence any
>> data that is still buffered will not get synchronized and a crash of the
>> machine may cause corruption.
>
> The problem description makes sense, and so does your fix.
>
> Grepping for other uses of fsync_component(), this looks like the only
> buggy case (loose refs use write() directly, and most other files go via
> finalize_hashfile(), which does likewise).
> ...
> So given that this is the only affected site, it makes sense to just fix
> it for now and worry about a more generalized solution if we run into it
> again.

Sounds good.

This came from bc22d845 (core.fsync: new option to harden references, 2022-03-11), before which we did not even fsync() the file, so let me apply directly on top of that commit. Those who are stuck on older versions of Git can choose to merge the result, even though I may probably not bother merging it down to anything older than 2.39 maintenance track.

Thanks.