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

[PATCH 01/12] for_each_alternate_ref: handle failure from real_pathdup()

From
Jeff King <peff@peff.net>
Date
Jan 24, 2017, 00:38 UTC
Message-ID
<20170124003827.l2rimgitsyxsvtly@sigill.intra.peff.net>
In-Reply-To
<20170124003729.j4ygjcgypdq7hceg@sigill.intra.peff.net>

In older versions of git, if real_path() failed to resolve the alternate object store path, we would die() with an error. However, since 4ac9006f8 (real_path: have callers use real_pathdup and strbuf_realpath, 2016-12-12) we use the real_pathdup() function, which may return NULL. Since we don't check the return value, we can segfault.

This is hard to trigger in practice, since we check that the path is accessible before creating the alternate_object_database struct. But it could be removed racily, or we could see a transient filesystem error.

We could restore the original behavior by switching back to xstrdup(real_path()). However, dying is probably not the best option here. This whole function is best-effort already; there might not even be a repository around the shared objects at all. And if the alternate store has gone away, there are no objects to show.

So let's just quietly return, as we would if we failed to open "refs/", or if upload-pack failed to start, etc.

Signed-off-by: Jeff King <peff@peff.net>
---
 transport.c | 2 ++
 1 file changed, 2 insertions(+)
diff --git a/transport.c b/transport.c
index c86ba2eb8..74d0e45bd 100644
--- a/transport.c
+++ b/transport.c
@@ -1215,6 +1215,8 @@ static int refs_from_alternate_cb(struct alternate_object_database *e,
 	struct alternate_refs_data *cb = data;
 
 	other = real_pathdup(e->path);
+	if (!other)
+		return 0;
 	len = strlen(other);
 
 	while (other[len-1] == '/')
-- 
2.11.0.765.g454d2182f
Previous: Jeff KingNext: Junio C Hamano
Message 2 of 48 in “reducing resource usage of for_each_alternate_ref”
  1. 0/12 reducing resource usage of for_each_alternate_refJeff King, Jan 24, 2017
  2. 01/12 for_each_alternate_ref: handle failure from real_pathdup()Jeff King, Jan 24, 2017
  3. Junio C HamanoJan 25, 2017
  4. 02/12 for_each_alternate_ref: stop trimming trailing slashesJeff King, Jan 24, 2017
  5. 03/12 for_each_alternate_ref: use strbuf for path allocationJeff King, Jan 24, 2017
  6. Junio C HamanoJan 25, 2017
  7. Jeff KingJan 25, 2017
  8. 04/12 for_each_alternate_ref: pass name/oid instead of ref structJeff King, Jan 24, 2017
  9. 05/12 for_each_alternate_ref: replace transport code with for-each-refJeff King, Jan 24, 2017
  10. Junio C HamanoJan 25, 2017
  11. 06/12 clone: disable save_commit_bufferJeff King, Jan 24, 2017
  12. Junio C HamanoJan 25, 2017
  13. Jeff KingJan 25, 2017
  14. Jeff KingJan 25, 2017
  15. Jeff KingJan 25, 2017
  16. 07/12 fetch-pack: cache results of for_each_alternate_refJeff King, Jan 24, 2017
  17. Junio C HamanoJan 25, 2017
  18. Jeff KingJan 25, 2017
  19. 08/12 add oidset APIJeff King, Jan 24, 2017
  20. Ramsay JonesJan 24, 2017
  21. Jeff KingJan 24, 2017
  22. 10/12 receive-pack: fix misleading namespace/.have commentJeff King, Jan 24, 2017
  23. 09/12 receive-pack: use oidset to de-duplicate .have linesJeff King, Jan 24, 2017
  24. Junio C HamanoJan 25, 2017
  25. Jeff KingJan 25, 2017
  26. 12/12 receive-pack: avoid duplicates between our refs and alternatesJeff King, Jan 24, 2017
  27. Junio C HamanoJan 25, 2017
  28. Jeff KingJan 25, 2017
  29. 11/12 receive-pack: treat namespace .have lines like alternatesJeff King, Jan 24, 2017
  30. Junio C HamanoJan 25, 2017
  31. Jeff KingJan 25, 2017
  32. Lukas FleischerJan 27, 2017
  33. Jeff KingJan 27, 2017
  34. Junio C HamanoJan 27, 2017
  35. Brandon WilliamsJan 24, 2017
  36. Jeff KingJan 24, 2017
  37. 0/11 reducing resource usage of for_each_alternate_refJeff King, Feb 8, 2017
  38. 01/11 for_each_alternate_ref: handle failure from real_pathdup()Jeff King, Feb 8, 2017
  39. 02/11 for_each_alternate_ref: stop trimming trailing slashesJeff King, Feb 8, 2017
  40. 04/11 for_each_alternate_ref: pass name/oid instead of ref structJeff King, Feb 8, 2017
  41. 03/11 for_each_alternate_ref: use strbuf for path allocationJeff King, Feb 8, 2017
  42. 05/11 for_each_alternate_ref: replace transport code with for-each-refJeff King, Feb 8, 2017
  43. 08/11 receive-pack: use oidset to de-duplicate .have linesJeff King, Feb 8, 2017
  44. 07/11 add oidset APIJeff King, Feb 8, 2017
  45. 06/11 fetch-pack: cache results of for_each_alternate_refJeff King, Feb 8, 2017
  46. 09/11 receive-pack: fix misleading namespace/.have commentJeff King, Feb 8, 2017
  47. 10/11 receive-pack: treat namespace .have lines like alternatesJeff King, Feb 8, 2017
  48. 11/11 receive-pack: avoid duplicates between our refs and alternatesJeff King, Feb 8, 2017

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.