Re: [PATCH] conf: fix memory leak on the error path in parse_args()

[Mark Hills <mark@xwax.org>]

On Wed, 17 Mar 2021, Takashi Iwai wrote:

> On Wed, 17 Mar 2021 16:44:20 +0100,
> Mark Hills wrote:
> > 
> > Having a little trouble which bisected to this patch.
> > 
> > First noticed it's causing Chromium to crash out one of its subprocesses 
> > (stack trace below)
> > 
> > Can actually be replicated with a simple "aplay -L":
> > 
> > aplay: conf.c:2207: snd_config_delete: Assertion `config' failed.
> > Aborted (core dumped)
> 
> That patch seems to have a few flaws.
> Could you check the patch below covers it?

Thanks. Yes, the patch builds ok and resolves my two test cases (aplay and 
chromium). That's the only testing I've done so far.

> --- a/src/conf.c
> +++ b/src/conf.c
> @@ -5080,6 +5080,8 @@ static int parse_args(snd_config_t *subs, const char *str, snd_config_t *defs)
>  		const char *new = str;
>  		const char *tmp;
>  		char *val = NULL;
> +
> +		sub = NULL;
>  		err = parse_arg(&new, &varlen, &val);
>  		if (err < 0)
>  			goto _err;
> @@ -5104,6 +5106,7 @@ static int parse_args(snd_config_t *subs, const char *str, snd_config_t *defs)
>  		err = snd_config_search(subs, var, &sub);
>  		if (err >= 0)
>  			snd_config_delete(sub);
> +		sub = NULL;
>  		err = snd_config_search(def, "type", &typ);
>  		if (err < 0) {
>  		_invalid_type:
> @@ -5169,7 +5172,8 @@ static int parse_args(snd_config_t *subs, const char *str, snd_config_t *defs)
>  		err = snd_config_add(subs, sub);
>  		if (err < 0) {
>  		_err:
> -			snd_config_delete(sub);
> +			if (sub)
> +				snd_config_delete(sub);
>  			free(val);
>  			return err;
>  		}
> 
> 

-- 
Mark