Re: [PATCH 3/3] index-helper: take extra care with readlink

[David Turner <dturner@twopensource.com>]

On Mon, 2016-04-11 at 00:03 +0100, Ramsay Jones wrote:
> It took me a few minutes to convince myself that, if index-helper is
> the only writer for the symlink, that the call to readlink would
> result in a properly NULL terminated string. This relies on the
> initialization of the address variable setting each byte of the
> 'struct sockaddr_un' to zero and the contents of the symlink being
> no more than 'sizeof(address.sun_path) - 1' bytes long.
> 
> In order to make the call easier to reason about, introduce a wrapper
> function, read_link(), which copes with overlong symlinks and ensures
> correct NULL termination.
> 
> Signed-off-by: Ramsay Jones <ramsay@ramsayjones.plus.com>
> ---
>  read-cache.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/read-cache.c b/read-cache.c
> index c7053d8..39330a1 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -1722,12 +1722,23 @@ static void post_read_index_from(struct
> index_state *istate)
>  	tweak_untracked_cache(istate);
>  }
>  
> +static int read_link(const char *path, char *buf, size_t bufsize)
> +{
> +	int len;
> +
> +	len = readlink(path, buf, bufsize);
> +	if (len < 0 || len >= bufsize)
> +		return -1;
> +	buf[len] = '\0';
> +	return 0;
> +}
> +
>  int connect_to_index_helper(void)
>  {
>  	struct sockaddr_un address = {0};
>  	int fd;
>  
> -	if (readlink(git_path("index-helper.path"),
> address.sun_path,
> +	if (read_link(git_path("index-helper.path"),
> address.sun_path,
>  		     sizeof(address.sun_path)) < 0)
>  		return -1;
>  


Will squash.