Re: [PATCH] path-walk: fix NULL pointer dereference in error message
- From
Tian Yuchen <a3205153416@gmail.com>
- Date
- Mar 20, 2026, 17:18 UTC
- Message-ID
- <eca1a469-2e15-4466-ae58-978ffc23c177@gmail.com>
- In-Reply-To
- <20260320114556.3151040-1-ysinghcin@gmail.com>
Hello,
Thanks for the patch!
On 3/20/26 19:45, Yuvraj Singh Chauhan wrote:
Show 14 quoted lines
> When lookup_tree() or lookup_blob() cannot find a tree entry's object,
> 'o' is set to NULL via:
>
> o = child ? &child->object : NULL;
>
> The subsequent null-check catches this correctly, but then dereferences
> 'o' to format the error message:
>
> error(_("failed to find object %s"), oid_to_hex(&o->oid));
>
> This causes a segfault instead of the intended diagnostic output.
> Fix this by using &entry.oid instead. 'entry' is the struct name_entry
> populated by tree_entry() on each loop iteration and holds the OID of
> the failing lookupChecking for NULL and then dereference the pointer, a segfault is bound to occur. I believe this is indeed a bug.
Show 17 quoted lines
> ---
> path-walk.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/path-walk.c b/path-walk.c
> index 364e4cfa19..839582380c 100644
> --- a/path-walk.c
> +++ b/path-walk.c
> @@ -171,7 +171,7 @@ static int add_tree_entries(struct path_walk_context *ctx,
>
> if (!o) {
> error(_("failed to find object %s"),
> - oid_to_hex(&o->oid));
> + oid_to_hex(&entry.oid));
> return -1;
> }
> The change itself looks good to me.
However, I have a slight concern about the original code implementation:
> o = child ? &child->object : NULL;
This means that 'child = NULL' is the expected failure path. But why is 'NULL' returned? Does the object truly not exist, or was it simply not parsed? Is 'failed to find object' sufficient to describe the cause of the failure? I think that's debatable. (I’m not 'suggesting' that you make this change; I just hope we can all think about it together ;)
Also, it looks like you forgot to include 'Signed-off-by' line when you committed the changes. When you've gathered enough feedback to commit v2, please remember to include it.
Regards,
Yuchen