From: Stefan Sperling <stsp@stsp.name>
Subject: Re: gotwebd: leaks in blame page
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Sat, 3 Sep 2022 14:06:56 +0200

On Sat, Sep 03, 2022 at 01:21:30PM +0200, Omar Polo wrote:
> diff below fixes a leak in got_repo_pack_fds_open where we forget to
> release the memory for the temp array.
> 
> I see two ways to fix it.  The first one is the smallest one and less
> invasive: 

> The second one is to just get rid of the temp array altogether.  a bit
> more invasive but i like it a bit more:

I prefer the second one also. ok stsp

> 
> diff /home/op/w/got
> commit - 06edcc519b27fc4831d2583a8dd10c7d2ab61e10
> path + /home/op/w/got
> blob - 9d9dfd0586d7bfc07846c61a82e2f1778968d95c
> file + lib/repository.c
> --- lib/repository.c
> +++ lib/repository.c
> @@ -252,16 +252,11 @@ const struct got_error *
>  got_repo_pack_fds_open(int **pack_fds)
>  {
>  	const struct got_error *err = NULL;
> -	int i, *pack_fds_tmp;
> +	int i;
>  
> -	pack_fds_tmp = calloc(GOT_PACK_NUM_TEMPFILES, sizeof(int));
> -	if (pack_fds_tmp == NULL)
> -		return got_error_from_errno("calloc");
>  	*pack_fds = calloc(GOT_PACK_NUM_TEMPFILES, sizeof(**pack_fds));
> -	if (*pack_fds == NULL) {
> -		free(pack_fds_tmp);
> +	if (*pack_fds == NULL)
>  		return got_error_from_errno("calloc");
> -	}
>  
>  	/*
>  	 * got_repo_pack_fds_close will try to close all of the
> @@ -270,18 +265,19 @@ got_repo_pack_fds_open(int **pack_fds)
>  	 * we do not initialize to -1 here.
>  	 */
>  	for (i = 0; i < GOT_PACK_NUM_TEMPFILES; i++)
> -		pack_fds_tmp[i] = -1;
> +		(*pack_fds)[i] = -1;
>  
>  	for (i = 0; i < GOT_PACK_NUM_TEMPFILES; i++) {
> -		pack_fds_tmp[i] = got_opentempfd();
> -		if (pack_fds_tmp[i] == -1) {
> +		(*pack_fds)[i] = got_opentempfd();
> +		if ((*pack_fds)[i] == -1) {
>  			err = got_error_from_errno("got_opentempfd");
> -			got_repo_pack_fds_close(pack_fds_tmp);
> +			got_repo_pack_fds_close(*pack_fds);
> +			*pack_fds = NULL;
>  			return err;
>  		}
>  	}
> -	memcpy(*pack_fds, pack_fds_tmp, GOT_PACK_NUM_TEMPFILES * sizeof(int));
> -	return err;
> +
> +	return NULL;
>  }
>  
>  const struct got_error *
>