Re: [PATCH] sha1_file: don't malloc the whole compressed result when writing out objects
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Feb 22, 2010, 05:30 UTC
- Message-ID
- <7v635p4z26.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <alpine.LFD.2.00.1002211950250.1946@xanadu.home>
Nicolas Pitre <nico@fluxnic.net> writes:
> The whole point is to detect data incoherencyes.
Yes. We want to make sure that the SHA-1 we compute is over what we fed deflate().
Show 32 quoted lines
> So current sequence of events is as follows: > > T0 write_sha1_file_prepare() is called > T1 start initial SHA1 computation on data buffer > T2 in the middle of initial SHA1 computation > T3 end of initial SHA1 computation -> object name is determined > T4 write_loose_object() is called > ... enter the write loop > T5+n deflate() called on buffer n > T6+n git_SHA1_Update(() called on the same buffer n > T7+n deflated data written out > ... > Tend abort if result of T6+n doesn't match object name from T3 > > So... what can happen: > > 1) Data is externally modified before T5+n: deflated data and its CRC32 > will be coherent with the SHA1 computed in T6+n, but incoherent with > the SHA1 used for the object name. Wrong data is written to the > object even if it will inflate OK. We really want to prevent that > from happening. The test in Tend will fail. > > 2) Data is externally modified between T5+n and T6+n: the deflated data > and CRC32 will be coherent with the object name but incoherent with > the parano_sha1. Although written data will be OK, this is way too > close from being wrong, and the test in Tend will fail. If there is > more than one round into the loop and the external modifications are > large enough then this becomes the same as case 1 above. > > 3) Data is externally modified in T2: again the test in Tend will fail. > > So in all possible cases I can think of, the write will abort.
There is one pathological case.
Immediately before T5+n (or between T5+n and T6+n), the external process changes the data deflate() is working on, but before T6+n, the external process changes the data back. Two SHA-1's computed may match, but it is not a hash over what was deflated(); you won't be able to abort.