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

Re: [PATCH 4/4] resolve_ref_unsafe(): close race condition reading loose refs

From
Michael Haggerty <mhagger@alum.mit.edu>
Date
Jun 14, 2013, 07:17 UTC
Message-ID
<51BAC39D.5030201@alum.mit.edu>
In-Reply-To
<87d2rqs9ma.fsf@linux-k42r.v.cablecom.net>
On 06/13/2013 10:22 AM, Thomas Rast wrote:
Show 19 quoted lines
> Michael Haggerty <mhagger@alum.mit.edu> writes:
> 
>> One race is still possible and undetected: another process could
>> change the file from a regular file into a symlink between the call to
>> lstat and the call to open().  The open() call would silently follow
>> the symlink and not know that something is wrong.  I don't see a way
>> to detect this situation without the use of the O_NOFOLLOW option,
>> which is not portable and is not used elsewhere in our code base.
>>
>> However, we don't use symlinks anymore, so this situation is unlikely.
>> And it doesn't appear that treating a symlink as a regular file would
>> have grave consequences; after all, this is exactly how the code
>> handles non-relative symlinks.
> 
> You could fstat() the fd you got from open(), and verify that it is
> still the same inode/device.  That's wasting one syscall per ref for
> pretty much everyone, but perhaps if we really cared about this (and I
> gather from the above that we don't), we could conditionally use
> O_NOFOLLOW if available, otherwise do that fstat().

Yes, that would work. For now I think I will not worry about it, but I'll keep your trick in mind.

Thanks, Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/
Previous: Thomas RastNext: Junio C Hamano
Message 28 of 45 in “another packed-refs race”
  1. Jeff KingMay 3, 2013
  2. Johan HerlandMay 3, 2013
  3. Jeff KingMay 3, 2013
  4. Jeff KingMay 3, 2013
  5. Johan HerlandMay 3, 2013
  6. Michael HaggertyMay 6, 2013
  7. Jeff KingMay 6, 2013
  8. Jeff KingMay 3, 2013
  9. Michael HaggertyMay 6, 2013
  10. Jeff KingMay 6, 2013
  11. Jeff KingMay 6, 2013
  12. Michael HaggertyMay 7, 2013
  13. Jeff KingMay 7, 2013
  14. Michael HaggertyMay 7, 2013
  15. 0/4 fix packed-refs racesJeff King, May 7, 2013
  16. 1/4 resolve_ref: close race condition for packed refsJeff King, May 7, 2013
  17. Michael HaggertyMay 12, 2013
  18. Jeff KingMay 16, 2013
  19. Michael HaggertyMay 16, 2013
  20. Michael HaggertyMay 12, 2013
  21. 0/4 Fix a race condition when reading loose refsMichael Haggerty, Jun 11, 2013
  22. 1/4 resolve_ref_unsafe(): extract function handle_missing_loose_ref()Michael Haggerty, Jun 11, 2013
  23. 2/4 resolve_ref_unsafe(): handle the case of an SHA-1 within loopMichael Haggerty, Jun 11, 2013
  24. 3/4 resolve_ref_unsafe(): nest reference-reading code in an infinite loopMichael Haggerty, Jun 11, 2013
  25. 4/4 resolve_ref_unsafe(): close race condition reading loose refsMichael Haggerty, Jun 11, 2013
  26. Jeff KingJun 12, 2013
  27. Thomas RastJun 13, 2013
  28. Michael HaggertyJun 14, 2013
  29. Junio C HamanoJun 11, 2013
  30. 2/4 add a stat_validity structJeff King, May 7, 2013
  31. Michael HaggertyMay 13, 2013
  32. 0/2 Separate stat_data from cache_entryMichael Haggerty, May 13, 2013
  33. 1/2 Extract a struct stat_data from cache_entryMichael Haggerty, May 13, 2013
  34. 2/2 add a stat_validity structMichael Haggerty, May 13, 2013
  35. Junio C HamanoMay 13, 2013
  36. Jeff KingMay 16, 2013
  37. 3/4 get_packed_refs: reload packed-refs file when it changesJeff King, May 7, 2013
  38. 0/2 peel_ref cleanups changesJeff King, May 7, 2013
  39. 1/2 peel_ref: rename "sha1" argument to "peeled"Jeff King, May 7, 2013
  40. 2/2 peel_ref: refactor for safety with simultaneous updateJeff King, May 7, 2013
  41. Eric SunshineMay 9, 2013
  42. Michael HaggertyMay 13, 2013
  43. 4/4 for_each_ref: load all loose refs before packed refsJeff King, May 7, 2013
  44. Junio C HamanoMay 7, 2013
  45. Jeff KingMay 7, 2013

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.