Re: [PATCH 7/9] submodule: remove validate_submodule_git_dir()

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Sep 8, 2025, 14:23 UTC
Message-ID: <c64c8c73-13db-49c8-a3d6-a4ce8b554867@gmail.com>
In-Reply-To: <20250816213642.3517822-8-adrian.ratiu@collabora.com>

Hi Adrian

On 16/08/2025 22:36, Adrian Ratiu wrote:

>   
> +	/* Trigger a BUG if these invariants do not hold */
> +	p = buf->buf + buf->len - encoded_len;
> +	if (buf->len <= encoded_len || p[-1] != '/' || strcmp(p, encoded_sub_name.buf))

if buf->len is less than encoded_len then the pointer p is invalid. As a valid program cannot create an invalid pointer the compiler may assume that buf->len >= encoded_len. We should check the lengths before creating the pointer as the original code in validate_submodule_git_dir() did which looked like

 > if (len <= suffix_len || (p = git_dir + len - suffix_len)[-1] != '/' ||
 > 	    strcmp(p, submodule_name))

Thanks

Phillip

Show 21 quoted lines

> +		BUG("encoded submodule name '%s' is not a suffix of git dir '%s'",
> +		    encoded_sub_name.buf, buf->buf);
> +
>   	strbuf_release(&encoded_sub_name);
>   }
> diff --git a/submodule.h b/submodule.h
> index b10e16e6c0..0b7692bc20 100644
> --- a/submodule.h
> +++ b/submodule.h
> @@ -137,11 +137,6 @@ int submodule_to_gitdir(struct repository *repo,
>   void submodule_name_to_gitdir(struct strbuf *buf, struct repository *r,
>   			      const char *submodule_name);
>   
> -/*
> - * Make sure that no submodule's git dir is nested in a sibling submodule's.
> - */
> -int validate_submodule_git_dir(char *git_dir, const char *submodule_name);
> -
>   /*
>    * Make sure that the given submodule path does not follow symlinks.
>    */