git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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.

Previous: Nicolas PitreNext: Nicolas Pitre
Message 7 of 13 in “sha1_file: don't malloc the whole compressed result when writing out objects”
  1. sha1_file: don't malloc the whole compressed result when writing out objectsNicolas Pitre, Feb 21, 2010
  2. Junio C HamanoFeb 21, 2010
  3. Nicolas PitreFeb 21, 2010
  4. Junio C HamanoFeb 21, 2010
  5. Junio C HamanoFeb 21, 2010
  6. Nicolas PitreFeb 22, 2010
  7. Junio C HamanoFeb 22, 2010
  8. Nicolas PitreFeb 22, 2010
  9. Junio C HamanoFeb 22, 2010
  10. Junio C HamanoFeb 22, 2010
  11. Nicolas PitreFeb 22, 2010
  12. Junio C HamanoFeb 22, 2010
  13. Dmitry PotapovFeb 22, 2010

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.