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

[PATCH 07/15] get_sha1_with_context: always initialize oc->symlink_path

From
Jeff King <peff@peff.net>
Date
May 19, 2017, 12:52 UTC
Message-ID
<20170519125224.gnfifzcyly2n5te6@sigill.intra.peff.net>
In-Reply-To
<20170519124651.4q7waz75rmzfopgn@sigill.intra.peff.net>

The get_sha1_with_context() function zeroes out the oc->symlink_path strbuf, but doesn't use strbuf_init() to set up the usual invariants (like pointing to the slopbuf). We don't actually write to the oc->symlink_path strbuf unless we call get_tree_entry_follow_symlinks(), and that function does initialize it. However, readers may still look at the zero'd strbuf.

In practice this isn't a triggerable bug. The only caller that looks at it only does so when the mode we found is 0. This doesn't happen for non-tree-entries (where we return S_IFINVALID). A broken tree entry could have a mode of 0, but canon_mode() quietly rewrites that into S_IFGITLINK. So the "0" mode should only come up when we did indeed find a symlink.

This is mostly just an accident of how the code happens to work, though. Let's future-proof ourselves to make sure the strbuf is properly initialized for all calls (it's only a few struct member assignments, not a heap allocation).

Signed-off-by: Jeff King <peff@peff.net>
---
 sha1_name.c | 1 +
 tree-walk.c | 1 -
 2 files changed, 1 insertion(+), 1 deletion(-)
diff --git a/sha1_name.c b/sha1_name.c
index a11d08dd8..35b16efc6 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -1511,6 +1511,7 @@ static int get_sha1_with_context_1(const char *name,
 
 	memset(oc, 0, sizeof(*oc));
 	oc->mode = S_IFINVALID;
+	strbuf_init(&oc->symlink_path, 0);
 	ret = get_sha1_1(name, namelen, sha1, flags);
 	if (!ret)
 		return ret;
diff --git a/tree-walk.c b/tree-walk.c
index ff7760568..c7ecfc856 100644
--- a/tree-walk.c
+++ b/tree-walk.c
@@ -589,7 +589,6 @@ enum follow_symlinks_result get_tree_entry_follow_symlinks(unsigned char *tree_s
 	int i;
 
 	init_tree_desc(&t, NULL, 0UL);
-	strbuf_init(result_path, 0);
 	strbuf_addstr(&namebuf, name);
 	hashcpy(current_tree_sha1, tree_sha1);
 
-- 
2.13.0.219.g63f6bc368
Previous: Jeff KingNext: Jeff King
Message 23 of 33 in “Demonstrate and partially work around a gitattributes problem”
  1. 0/2 Demonstrate and partially work around a gitattributes problemJohannes Schindelin, May 15, 2017
  2. 1/2 gitattributes: demonstrate that Git tries to read a bogus fileJohannes Schindelin, May 15, 2017
  3. 2/2 mingw: Suppress warning that <commit>:.gitattributes does not existJohannes Schindelin, May 15, 2017
  4. Jeff KingMay 16, 2017
  5. Jeff KingMay 16, 2017
  6. Junio C HamanoMay 17, 2017
  7. Jeff KingMay 17, 2017
  8. Johannes SchindelinMay 18, 2017
  9. Jeff KingMay 19, 2017
  10. 0/15 retain blob info for git diff HEAD:foo HEAD:barJeff King, May 19, 2017
  11. 01/15 handle_revision_arg: reset "dotdot" consistentlyJeff King, May 19, 2017
  12. Philip OakleyMay 20, 2017
  13. Jeff KingMay 23, 2017
  14. Philip OakleyMay 23, 2017
  15. 02/15 handle_revision_arg: simplify commit reference lookupsJeff King, May 19, 2017
  16. 03/15 handle_revision_arg: stop using "dotdot" as a generic pointerJeff King, May 19, 2017
  17. Junio C HamanoMay 24, 2017
  18. Jeff KingMay 24, 2017
  19. 04/15 handle_revision_arg: hoist ".." check out of range parsingJeff King, May 19, 2017
  20. 05/15 handle_revision_arg: add handle_dotdot() helperJeff King, May 19, 2017
  21. Junio C HamanoMay 24, 2017
  22. 06/15 sha1_name: consistently refer to object_context as "oc"Jeff King, May 19, 2017
  23. 07/15 get_sha1_with_context: always initialize oc->symlink_pathJeff King, May 19, 2017
  24. 08/15 get_sha1_with_context: dynamically allocate oc->pathJeff King, May 19, 2017
  25. 09/15 t4063: add tests of direct blob diffsJeff King, May 19, 2017
  26. 10/15 handle_revision_arg: record modes for "a..b" endpointsJeff King, May 19, 2017
  27. 11/15 handle_revision_arg: record paths for pending objectsJeff King, May 19, 2017
  28. 12/15 diff: pass whole pending entry in blobinfoJeff King, May 19, 2017
  29. 13/15 diff: use the word "path" instead of "name" for blobsJeff King, May 19, 2017
  30. 14/15 diff: use pending "path" if it is availableJeff King, May 19, 2017
  31. 15/15 diff: use blob path for blob/file diffsJeff King, May 19, 2017
  32. Junio C HamanoMay 24, 2017
  33. Jeff KingMay 24, 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.