"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: Plug leak in gitconfig.c
To:
Kyle Ackerman <kack@kyleackerman.net>
Cc:
"gameoftrees@openbsd.org" <gameoftrees@openbsd.org>
Date:
Mon, 5 May 2025 09:21:27 +0200

Download raw body.

Thread
On Sun, May 04, 2025 at 08:47:27PM +0000, Kyle Ackerman wrote:
> The diff below patches gitconfig.c.  When we are parsing a git config,
> conf_parse_line frees *section before we allocate a new
> section. This leaves the opportunity for *section to leak on the last
> iteration of conf_parse.

> diff /home/kyle/src/got
> path + /home/kyle/src/got
> commit - 4492e47bc914650ecd587fcc94010ae0373ab91b
> blob - 5af728df10a622bad125d0bc41ba804acfc9ccca
> file + lib/gitconfig.c
> --- lib/gitconfig.c
> +++ lib/gitconfig.c
> @@ -296,8 +296,10 @@ conf_parse(struct got_gitconfig *conf, int trans, char
>  				*cp = '\0';
>  				err = conf_parse_line(&section, conf, trans,
>  				    line, ln, cp - line);
> -				if (err)
> +				if (err) {
> +					free(section);
>  					return err;

Is this really a genuuine leak? I am asking because the only case
where conf_parse_line() returns an error is when the allocation of
*section has failed. 

> +				}
>  				line = cp + 1;
>  			}
>  			ln++;
> @@ -306,6 +308,7 @@ conf_parse(struct got_gitconfig *conf, int trans, char
>  	}
>  	if (cp != line)
>  		log_print("conf_parse: last line unterminated, ignored.");
> +	free(section);

This chunk looks correct to me and fixes an obvious leak. ok stsp@

>  	return NULL;
>  }
>  
> Thoughts/Comments/Suggestions/oks?
> 
>