Subject: Re: [PATCH 04/11] get_oid_with_context_1(): avoid use-after-free

From: Junio C Hamano <[email-masked]>
Date: Wed, 15 Jun 2022 21:29:43 -0700

"Johannes Schindelin via GitGitGadget" <[email-masked]>
writes:

>  			if (ce_stage(ce) == stage) {
> -				free(new_path);
>  				if (reject_tree_in_index(repo, only_to_die, ce,
> -							 stage, prefix, cp))
> +							 stage, prefix, cp)) {
> +					free(new_path);
>  					return -1;
> +				}
>  				oidcpy(oid, &ce->oid);
>  				oc->mode = ce->ce_mode;
> +				free(new_path);
>  				return 0;
>  			}

Hmph.  Without the "lets make sure we do not leak in the error code
path", it would have been no brainer to avoid this bug in the
original version.  Of course the postimage of the above hunk is
correct, but with extra free() sprinkled, it became ugly to the eye.

I wonder if the following is easier to follow.

Thanks.

 object-name.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git c/object-name.c w/object-name.c
index 4d2746574c..57188db7b0 100644
--- c/object-name.c
+++ w/object-name.c
@@ -1971,13 +1971,16 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,
 			    memcmp(ce->name, cp, namelen))
 				break;
 			if (ce_stage(ce) == stage) {
+				int ret = -1;
+
+				if (!reject_tree_in_index(repo, only_to_die, ce,
+							  stage, prefix, cp)) {
+					oidcpy(oid, &ce->oid);
+					oc->mode = ce->ce_mode;
+					ret = 0;
+				}
 				free(new_path);
-				if (reject_tree_in_index(repo, only_to_die, ce,
-							 stage, prefix, cp))
-					return -1;
-				oidcpy(oid, &ce->oid);
-				oc->mode = ce->ce_mode;
-				return 0;
+				return ret;
 			}
 			pos++;
 		}