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

Re: [PATCH 6/6] hash-object: use fsck for object checks

From
Junio C Hamano <gitster@pobox.com>
Date
Feb 1, 2023, 20:41 UTC
Message-ID
<xmqqlelhyv81.fsf@gitster.g>
In-Reply-To
<Y9pgG10dAoQABGXG@coredump.intra.peff.net>
Jeff King <peff@peff.net> writes:
Show 5 quoted lines
>      ... So if that tree is fsck'd, and then
>      checks the blob during fsck_finish(), that should be enough.
>      Assuming that fsck complains when the pointed-to blob cannot be
>      accessed, which I think it should (because really, incremental
>      pushes face the same problem).
Yes.
Show 6 quoted lines
>   2. We're not checking fsck connectivity here, and that's intentional.
>      So you can "hash-object" a tree that points to blobs that we don't
>      actually have. But if you hash a tree that points a .gitmodules
>      entry at a blob that doesn't exist, then that will fail the fsck
>      (during the finish step). And respecting the fsck_finish() exit
>      code would break that.

That's tricky. An attack vector to sneak a bad .gitmodules file into history then becomes (1) hash a tree with a .gitmodules entry that points at a missing blob and then (2) after that fact is forgotten, hash a bad blob pointed to by the tree? We cannot afford to remember all trees with .gitmodules we didn't find the blob for forever, so one approach to solve it is to reject trees with missing blobs. Legitimate use cases should be able to build up trees bottle up to hash blobs before their containing trees.

If you hash a commit object, we would want to fsck its tree? Do we want to fsck its parent commit and its tree? Ideally we can stop when our "traversal" reaches objects that are known to be good, but how do we decide which objects are "known to be good"? Being reachable from our refs, as usual?

> So I dunno. The code above is doing (2), albeit with the inefficiency of
> checking blobs that we might not care about. I kind of think (1) is the
> right thing, though, and anybody who really wants to make trees that
> point to bogus .gitmodules can use --literally.
True.
Previous: Ævar Arnfjörð BjarmasonNext: Jeff King
Message 17 of 28 in “hash-object: use fsck to check objects”
  1. 0/6 hash-object: use fsck to check objectsJeff King, Jan 18, 2023
  2. 1/6 t1007: modernize malformed object testsJeff King, Jan 18, 2023
  3. Taylor BlauJan 18, 2023
  4. 2/6 t1006: stop using 0-padded timestampsJeff King, Jan 18, 2023
  5. 3/6 t7030: stop using invalid tag nameJeff King, Jan 18, 2023
  6. 4/6 t: use hash-object --literally when created malformed objectsJeff King, Jan 18, 2023
  7. Taylor BlauJan 18, 2023
  8. Jeff KingJan 19, 2023
  9. 5/6 fsck: provide a function to fsck buffer without object structJeff King, Jan 18, 2023
  10. Taylor BlauJan 18, 2023
  11. Jeff KingJan 19, 2023
  12. 6/6 hash-object: use fsck for object checksJeff King, Jan 18, 2023
  13. Taylor BlauJan 18, 2023
  14. Jeff KingJan 19, 2023
  15. Jeff KingFeb 1, 2023
  16. Ævar Arnfjörð BjarmasonFeb 1, 2023
  17. Junio C HamanoFeb 1, 2023
  18. Jeff KingJan 18, 2023
  19. Junio C HamanoJan 18, 2023
  20. Taylor BlauJan 18, 2023
  21. Jeff KingJan 19, 2023
  22. Jeff KingJan 19, 2023
  23. 7/6 fsck: do not assume NUL-termination of buffersJeff King, Jan 19, 2023
  24. Junio C HamanoJan 19, 2023
  25. René ScharfeJan 21, 2023
  26. Jeff KingJan 22, 2023
  27. René ScharfeJan 22, 2023
  28. Ævar Arnfjörð BjarmasonFeb 1, 2023

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.