{"thread":{"id":"65316","subject":"[PATCH] path-walk: fix NULL pointer dereference in error message","startedAt":"2026-03-20T11:46:17Z","lastAt":"2026-03-21T04:05:36Z","messageCount":5,"participants":["Yuvraj Singh Chauhan","Tian Yuchen","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"539512","messageId":"20260320114556.3151040-1-ysinghcin@gmail.com","threadId":"65316","inReplyTo":null,"subject":"[PATCH] path-walk: fix NULL pointer dereference in error message","fromName":"Yuvraj Singh Chauhan","fromEmail":"ysinghcin@gmail.com","sentAt":"2026-03-20T11:45:56Z","receivedAt":"2026-03-20T11:46:17Z","isPatch":true,"sender":{"key":"ysinghcin@gmail.com","avatar":"https://avatars.githubusercontent.com/u/133221941?v=4"},"body":"When lookup_tree() or lookup_blob() cannot find a tree entry's object,\n'o' is set to NULL via:\n\n    o = child ? &child->object : NULL;\n\nThe subsequent null-check catches this correctly, but then dereferences\n'o' to format the error message:\n\n    error(_(\"failed to find object %s\"), oid_to_hex(&o->oid));\n\nThis causes a segfault instead of the intended diagnostic output.\nFix this by using &entry.oid instead. 'entry' is the struct name_entry\npopulated by tree_entry() on each loop iteration and holds the OID of\nthe failing lookup\n---\n path-walk.c | 2 +-\n 1 file changed, 1 insertion(+), 1 deletion(-)\n\ndiff --git a/path-walk.c b/path-walk.c\nindex 364e4cfa19..839582380c 100644\n--- a/path-walk.c\n+++ b/path-walk.c\n@@ -171,7 +171,7 @@ static int add_tree_entries(struct path_walk_context *ctx,\n \n \t\tif (!o) {\n \t\t\terror(_(\"failed to find object %s\"),\n-\t\t\t      oid_to_hex(&o->oid));\n+\t\t\t      oid_to_hex(&entry.oid));\n \t\t\treturn -1;\n \t\t}\n \n-- \n2.53.0.582.gca1db8a0f7\n\n"},{"id":"539571","messageId":"eca1a469-2e15-4466-ae58-978ffc23c177@gmail.com","threadId":"65316","inReplyTo":"20260320114556.3151040-1-ysinghcin@gmail.com","subject":"Re: [PATCH] path-walk: fix NULL pointer dereference in error message","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-03-20T17:18:36Z","receivedAt":"2026-03-20T17:18:42Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Hello,\n\nThanks for the patch!\n\nOn 3/20/26 19:45, Yuvraj Singh Chauhan wrote:\n> When lookup_tree() or lookup_blob() cannot find a tree entry's object,\n> 'o' is set to NULL via:\n> \n>      o = child ? &child->object : NULL;\n> \n> The subsequent null-check catches this correctly, but then dereferences\n> 'o' to format the error message:\n> \n>      error(_(\"failed to find object %s\"), oid_to_hex(&o->oid));\n> \n> This causes a segfault instead of the intended diagnostic output.\n> Fix this by using &entry.oid instead. 'entry' is the struct name_entry\n> populated by tree_entry() on each loop iteration and holds the OID of\n> the failing lookup\n\nChecking for NULL and then dereference the pointer, a segfault is bound \nto occur. I believe this is indeed a bug.\n\n> ---\n>   path-walk.c | 2 +-\n>   1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/path-walk.c b/path-walk.c\n> index 364e4cfa19..839582380c 100644\n> --- a/path-walk.c\n> +++ b/path-walk.c\n> @@ -171,7 +171,7 @@ static int add_tree_entries(struct path_walk_context *ctx,\n>   \n>   \t\tif (!o) {\n>   \t\t\terror(_(\"failed to find object %s\"),\n> -\t\t\t      oid_to_hex(&o->oid));\n> +\t\t\t      oid_to_hex(&entry.oid));\n>   \t\t\treturn -1;\n>   \t\t}\n>   \n\nThe change itself looks good to me.\n\nHowever, I have a slight concern about the original code implementation:\n\n >      o = child ? &child->object : NULL;\n\nThis means that 'child = NULL' is the expected failure path. But why is \n'NULL' returned? Does the object truly not exist, or was it simply not \nparsed? Is 'failed to find object' sufficient to describe the cause of \nthe failure? I think that's debatable. (I’m not 'suggesting' that you \nmake this change; I just hope we can all think about it together ;)\n\nAlso, it looks like you forgot to include 'Signed-off-by' line when you \ncommitted the changes. When you've gathered enough feedback to commit \nv2, please remember to include it.\n\nRegards,\n\nYuchen\n\n"},{"id":"539574","messageId":"9d0746d8-2194-4a13-812b-9b46d04c189a@gmail.com","threadId":"65316","inReplyTo":"eca1a469-2e15-4466-ae58-978ffc23c177@gmail.com","subject":"Re: [PATCH] path-walk: fix NULL pointer dereference in error message","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-03-20T17:32:25Z","receivedAt":"2026-03-20T17:32:29Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"Although this isn’t covered in this patch, I’d still like to add a few \nmore comments. Please allow me to elaborate a bit further:\n\nThis is the implementation of object_as_type() in object.c:\n\n> void *object_as_type(struct object *obj, enum object_type type, int quiet)\n> {\n> \tif (obj->type == type)\n> \t\treturn obj;\n> \telse if (obj->type == OBJ_NONE) {\n> \t\tif (type == OBJ_COMMIT)\n> \t\t\tinit_commit_node((struct commit *) obj);\n> \t\telse\n> \t\t\tobj->type = type;\n> \t\treturn obj;\n> \t}\n> \telse {\n> \t\tif (!quiet)\n> \t\t\terror(_(\"object %s is a %s, not a %s\"),\n> \t\t\t      oid_to_hex(&obj->oid),\n> \t\t\t      type_name(obj->type), type_name(type));\n> \t\treturn NULL;\n> \t}\n> }\n\nThere are at least two possible scenarios: the object doesn't exist, or \nthe type doesn't match, right?\n\nThen the message 'failed to find object' is misleading when user \nencounters the second scenario. Wouldn’t it be confusing if a user saw \nthis message, checked the object using 'git cat-file -t', and discovered \nthat the object actually exists?\n\nRegards,\n\nYuchen\n\n\n"},{"id":"539575","messageId":"xmqqpl4ye6ll.fsf@gitster.g","threadId":"65316","inReplyTo":"9d0746d8-2194-4a13-812b-9b46d04c189a@gmail.com","subject":"Re: [PATCH] path-walk: fix NULL pointer dereference in error message","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2026-03-20T17:47:02Z","receivedAt":"2026-03-20T17:47:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Tian Yuchen <a3205153416@gmail.com> writes:\n\n> Then the message 'failed to find object' is misleading when user \n> encounters the second scenario. Wouldn’t it be confusing if a user saw \n> this message, checked the object using 'git cat-file -t', and discovered \n> that the object actually exists?\n\nFor that, you'd need to remove the block in question and duplicate\nthe message generation, perhaps like so, to allow the message\nproperly localized.\n\nBut that is way outside the scope of the patch that was posted,\nwhich was a surgical fix for a reference to an incorrect pointer, I\nwould have to say.\n\n path-walk.c | 16 ++++++++--------\n 1 file changed, 8 insertions(+), 8 deletions(-)\n\ndiff --git c/path-walk.c w/path-walk.c\nindex 364e4cfa19..bd2c47079e 100644\n--- c/path-walk.c\n+++ w/path-walk.c\n@@ -161,20 +161,20 @@ static int add_tree_entries(struct path_walk_context *ctx,\n \n \t\tif (type == OBJ_TREE) {\n \t\t\tstruct tree *child = lookup_tree(ctx->repo, &entry.oid);\n-\t\t\to = child ? &child->object : NULL;\n+\t\t\tif (!child)\n+\t\t\t\treturn error(_(\"failed to find tree object %s\"),\n+\t\t\t\t\t     oid_to_hex(&entry.oid));\n+\t\t\to = &child->object;\n \t\t} else if (type == OBJ_BLOB) {\n \t\t\tstruct blob *child = lookup_blob(ctx->repo, &entry.oid);\n-\t\t\to = child ? &child->object : NULL;\n+\t\t\tif (!child)\n+\t\t\t\treturn error(_(\"failed to find blob object %s\"),\n+\t\t\t\t\t     oid_to_hex(&entry.oid));\n+\t\t\to = &child->object;\n \t\t} else {\n \t\t\tBUG(\"invalid type for tree entry: %d\", type);\n \t\t}\n \n-\t\tif (!o) {\n-\t\t\terror(_(\"failed to find object %s\"),\n-\t\t\t      oid_to_hex(&o->oid));\n-\t\t\treturn -1;\n-\t\t}\n-\n \t\t/* Skip this object if already seen. */\n \t\tif (o->flags & SEEN)\n \t\t\tcontinue;\n\n"},{"id":"539587","messageId":"448b07eb-b274-4111-bd55-4ad25ffb94c3@gmail.com","threadId":"65316","inReplyTo":"xmqqpl4ye6ll.fsf@gitster.g","subject":"Re: [PATCH] path-walk: fix NULL pointer dereference in error message","fromName":"Tian Yuchen","fromEmail":"a3205153416@gmail.com","sentAt":"2026-03-21T04:05:30Z","receivedAt":"2026-03-21T04:05:36Z","isPatch":true,"sender":{"key":"cat@malon.dev","avatar":"https://avatars.githubusercontent.com/u/232002048?v=4"},"body":"On 3/21/26 01:47, Junio C Hamano wrote:\n\n> For that, you'd need to remove the block in question and duplicate\n> the message generation, perhaps like so, to allow the message\n> properly localized.\n> \n> But that is way outside the scope of the patch that was posted,\n> which was a surgical fix for a reference to an incorrect pointer, I\n> would have to say.\n\nThank you for pointing that out, but I believe I already mentioned that \nthis is not what the patch is intended to address, and I did not suggest \nmaking that change.\n\nYuchen\n\n"}]}