Re: [PATCHv6 bpf-next 21/29] selftests/bpf: Add tracing multi skel/pattern/ids attach tests

[Jiri Olsa <olsajiri@gmail.com>]

On Wed, May 27, 2026 at 12:01:53PM +0000, sashiko-bot@kernel.org wrote:

SNIP

> > +static __u32 *get_ids(const char * const funcs[], int funcs_cnt, const char *mod)
> > +{
> 
> [ ... ]
> 
> > +	ids = calloc(funcs_cnt, sizeof(ids[0]));
> > +	if (!ids)
> > +		goto out;
> > +
> > +	/*
> > +	 * We sort function names by name and search them
> > +	 * below for each function.
> > +	 */
> > +	for (i = 0; i < funcs_cnt; i++) {
> > +		if (!tsearch(&funcs[i], &root, compare)) {
> > +			ASSERT_FAIL("tsearch failed");
> > +			err = -1;
> > +			goto out;
> > +		}
> > +	}
> 
> [ ... ]
> 
> > +	if (err) {
> > +		free(ids);
> > +		ids = NULL;
> > +	}
> > +
> > +out:
> > +	tdestroy(root, tdestroy_free_nop);
> > +	btf__free(vmlinux_btf);
> > +	btf__free(btf);
> > +	return ids;
> > +}
> 
> [Severity: Medium]
> Does this error path leak the ids array and return an invalid pointer?
> 
> If tsearch() fails, the code sets err = -1 and jumps to the out label.
> However, the out label is placed after the if (err) cleanup block that is
> supposed to free ids and set it to NULL. 
> 
> Because the cleanup is bypassed, ids is leaked. Furthermore, the caller
> will receive a non-NULL pointer, which will incorrectly pass the
> ASSERT_OK_PTR check and cause the test to proceed using an array of zeros.

yep, it should land few lines above and do that err check, will fix

jirka

> 
> -- 
> Sashiko AI review · https://sashiko.dev/#/patchset/20260527113951.46265-22-jolsa@kernel.org?part=1