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

[PATCH] reflog-walk: don't segfault on non-commit sha1's in the reflog

From
Dennis Kaarsemaker <dennis@kaarsemaker.net>
Date
Dec 30, 2015, 15:22 UTC
Message-ID
<20151230152245.GA30549@spirit>
In-Reply-To
<20151230131914.GA27241@lanh>

Use lookup_commit instead of parse_object to look up commits mentioned in the reflog. This avoids a segfault in save_parents if somehow a sha1 for something other than a commit ends up in the reflog.

Signed-off-by: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Helped-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Duy Nguyen wrote:
> I would go with something like this. The typecasting to "struct commit
> *" is the bug because parse_object() can return any object type.
Yeah, that's much better. Here it is as a patch with a test. 
 reflog-walk.c     | 4 ++--
 t/t1410-reflog.sh | 6 ++++++
 2 files changed, 8 insertions(+), 2 deletions(-)
diff --git a/reflog-walk.c b/reflog-walk.c
index 85b8a54..b85c8e8 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -236,8 +236,8 @@ void fake_reflog_parent(struct reflog_walk_info *info, struct commit *commit)
 	reflog = &commit_reflog->reflogs->items[commit_reflog->recno];
 	info->last_commit_reflog = commit_reflog;
 	commit_reflog->recno--;
-	commit_info->commit = (struct commit *)parse_object(reflog->osha1);
-	if (!commit_info->commit) {
+	commit_info->commit = lookup_commit(reflog->osha1);
+	if (!commit_info->commit || parse_commit(commit_info->commit)) {
 		commit->parents = NULL;
 		return;
 	}
diff --git a/t/t1410-reflog.sh b/t/t1410-reflog.sh
index b79049f..76ccbe5 100755
--- a/t/t1410-reflog.sh
+++ b/t/t1410-reflog.sh
@@ -325,4 +325,10 @@ test_expect_success 'parsing reverse reflogs at BUFSIZ boundaries' '
 	test_cmp expect actual
 '
 
+test_expect_success 'reflog containing non-commit sha1s' '
+	git checkout -b broken-reflog &&
+	echo "$(git rev-parse HEAD^{tree}) $(git rev-parse HEAD) abc <xyz> 0000000001 +0000" >> .git/logs/refs/heads/broken-reflog &&
+	git reflog broken-reflog
+'
+
 test_done
-- 
2.7.0-rc1-207-ga35084c


-- 
Dennis Kaarsemaker <dennis@kaarsemaker.net>
http://twitter.com/seveas
Previous: Duy NguyenNext: Junio C Hamano
Message 8 of 25 in “Segfault in git reflog”
  1. Dennis KaarsemakerDec 30, 2015
  2. Duy NguyenDec 30, 2015
  3. Dennis KaarsemakerDec 30, 2015
  4. Duy NguyenDec 30, 2015
  5. Duy NguyenDec 30, 2015
  6. Dennis KaarsemakerDec 30, 2015
  7. Duy NguyenDec 30, 2015
  8. reflog-walk: don't segfault on non-commit sha1's in the reflogDennis Kaarsemaker, Dec 30, 2015
  9. Junio C HamanoDec 30, 2015
  10. Dennis KaarsemakerDec 30, 2015
  11. Junio C HamanoDec 30, 2015
  12. Dennis KaarsemakerDec 30, 2015
  13. reflog-walk: don't segfault on non-commit sha1's in the reflogDennis Kaarsemaker, Dec 30, 2015
  14. Junio C HamanoDec 30, 2015
  15. reflog-walk: don't segfault on non-commit sha1's in the reflogDennis Kaarsemaker, Dec 30, 2015
  16. Junio C HamanoDec 31, 2015
  17. Dennis KaarsemakerDec 31, 2015
  18. Dennis KaarsemakerDec 31, 2015
  19. reflog-walk: don't segfault on non-commit sha1's in the reflogDennis Kaarsemaker, Jan 5, 2016
  20. Eric SunshineJan 6, 2016
  21. Dennis KaarsemakerJan 6, 2016
  22. Eric SunshineJan 6, 2016
  23. Eric SunshineJan 6, 2016
  24. Dennis KaarsemakerJan 6, 2016
  25. Duy NguyenJan 6, 2016

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.