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

Re: [PATCH] fix segv with corrupt tag object

From
René Scharfe <l.s.r@web.de>
Date
Aug 29, 2019, 19:06 UTC
Message-ID
<c6601cca-7de0-ba82-2e18-916a2e9048d3@web.de>
In-Reply-To
<xmqqo90bhmi3.fsf@gitster-ct.c.googlers.com>
Am 26.08.19 um 19:20 schrieb Junio C Hamano:
Show 17 quoted lines
> Stefan Sperling <stsp@stsp.name> writes:
>
>> The root cause of this bug seems to be that the valid assumption
>> that obj->parsed implies a successfully parsed object is broken by
>> parse_tag_buffer() because this function sets the 'parsed' flag even
>> if errors occur during parsing.
>
> I am mildly negative about that approach.  obj->parsed is about
> "we've done all we need to do to attempt parsing this object" (so
> that next person who gets hold of the object knows that fact---one
> of the reasons why may be that the caller who wants to ensure that
> the fields are ready to be accessed does not have to spend extra
> cycles, but that is not the only one).  Those that want to look at
> various fields in the object (e.g. the tagged object of a tag, the
> tagger identity of a tag, etc.) should be prepared to see and react
> to NULL in there so that they can gracefully handle "slightly"
> corrupt objects.

Not sure how this could happen under normal circumstances, but how about this here?

-- >8 --
Subject: [PATCH] tree: simplify parse_tree_indirect()

Reduce code duplication by turning parse_tree_indirect() into a wrapper of repo_peel_to_type(). This avoids a segfault when handling a broken tag where ->tagged is NULL. The new version also checks the return value of parse_object() that was ignored by the old one.

Initial-patch-by: Stefan Sperling <stsp@stsp.name>
Signed-off-by: René Scharfe <l.s.r@web.de>
---
 tree.c | 18 +++---------------
 1 file changed, 3 insertions(+), 15 deletions(-)
diff --git a/tree.c b/tree.c
index 4720945e6a..1466bcc6a8 100644
--- a/tree.c
+++ b/tree.c
@@ -244,19 +244,7 @@ void free_tree_buffer(struct tree *tree)

 struct tree *parse_tree_indirect(const struct object_id *oid)
 {
-	struct object *obj = parse_object(the_repository, oid);
-	do {
-		if (!obj)
-			return NULL;
-		if (obj->type == OBJ_TREE)
-			return (struct tree *) obj;
-		else if (obj->type == OBJ_COMMIT)
-			obj = &(get_commit_tree(((struct commit *)obj))->object);
-		else if (obj->type == OBJ_TAG)
-			obj = ((struct tag *) obj)->tagged;
-		else
-			return NULL;
-		if (!obj->parsed)
-			parse_object(the_repository, &obj->oid);
-	} while (1);
+	struct repository *r = the_repository;
+	struct object *obj = parse_object(r, oid);
+	return (struct tree *)repo_peel_to_type(r, NULL, 0, obj, OBJ_TREE);
 }
--
2.23.0
Previous: Jeff KingNext: Junio C Hamano
Message 7 of 8 in “fix segv with corrupt tag object”
  1. fix segv with corrupt tag objectStefan Sperling, Aug 24, 2019
  2. René ScharfeAug 25, 2019
  3. Stefan SperlingAug 26, 2019
  4. Junio C HamanoAug 26, 2019
  5. Stefan SperlingAug 26, 2019
  6. Jeff KingAug 26, 2019
  7. René ScharfeAug 29, 2019
  8. Junio C HamanoAug 30, 2019

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.