From: Stefan Sperling <stsp@stsp.name>
Subject: Re: plug leak in got_fileindex_read error path
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Fri, 7 Apr 2023 15:31:26 +0200

On Fri, Apr 07, 2023 at 12:40:28PM +0200, Omar Polo wrote:
> Was playing with Otto' malloc diff to find leaks and spotted this.
> It's not what the malloc diff complains about (the recallocarray()
> call in lib/fileindex.c:/^read_fileindex_path around line 591 that I
> just can't see -- maybe it's an error in how malloc marks leaks.)
> 
> add_entry doesn't release the fileindex entry on error, the callers do
> (grep for how got_fileindex_entry_add() is used for reference) and
> this is the noly instance where we fail to do so.  Admittedly, it's
> very unlikely to leak here, got should have produced an invalid
> fileindex and we fail immediately anyway, but it still may be worth,
> if not only for reference.  ok?

ok

> diff /home/op/w/got
> commit - 6be067cef84c15f7e8623ec8fccaf955d98d006b
> path + /home/op/w/got
> blob - 19b8418d8d3b6252476d3f2048056a0c997b1377
> file + lib/fileindex.c
> --- lib/fileindex.c
> +++ lib/fileindex.c
> @@ -752,8 +752,10 @@ got_fileindex_read(struct got_fileindex *fileindex, FI
>  		if (err)
>  			return err;
>  		err = add_entry(fileindex, ie);
> -		if (err)
> +		if (err) {
> +			got_fileindex_entry_free(ie);
>  			return err;
> +		}
>  	}
>  
>  	n = fread(sha1_expected, 1, sizeof(sha1_expected), infile);
> 
>