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

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [Medium] Double free of `vmlinux_btf` in the error path of `get_ids()`.
- [Medium] Missing error state assignment in `get_ids()` leads to returning a partially or fully zero-initialized array instead of failing.
--

commit 44b619347c03e0f87cfb994ba8b14346b293edcc
Author: Jiri Olsa <jolsa@kernel.org>

selftests/bpf: Add tracing multi skel/pattern/ids attach tests

Adding tests for tracing_multi link attachment via all possible
libbpf apis - skeleton, function pattern and btf ids.

> diff --git a/tools/testing/selftests/bpf/prog_tests/tracing_multi.c b/tools/testing/selftests/bpf/prog_tests/tracing_multi.c
> new file mode 100644
> index 0000000000000..52d3339b430a5
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/tracing_multi.c

[ ... ]

> +static __u32 *get_ids(const char * const funcs[], int funcs_cnt, const char *mod)
> +{
> +	struct btf *btf, *vmlinux_btf = NULL;
> +	__u32 nr, type_id, cnt = 0;
> +	void *root = NULL;
> +	__u32 *ids = NULL;
> +	int i, err = 0;
> +
> +	btf = btf__load_vmlinux_btf();
> +	if (!ASSERT_OK_PTR(btf, "btf__load_vmlinux_btf"))
> +		return NULL;
> +
> +	if (mod) {
> +		vmlinux_btf = btf;
> +		btf = btf__load_module_btf(mod, vmlinux_btf);
> +		if (!ASSERT_OK_PTR(btf, "btf__load_module_btf")) {
> +			btf__free(vmlinux_btf);
> +			goto out;

Can this result in a double free of vmlinux_btf? If btf__load_module_btf()
fails, we call btf__free(vmlinux_btf) here, and then goto out, where
btf__free(vmlinux_btf) is called again.

> +		}
> +	}
> +
> +	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");
> +			goto out;

Does this code return an invalid array if tsearch() fails? When tsearch()
fails, we jump to out, but err is still 0. Because err is 0, the cleanup
block below won't free ids and will instead return the partially or fully
zero-initialized array.

Also, if the BTF type loop that follows this completes without finding all
expected functions (cnt < funcs_cnt), err also remains 0.

Should err be set to a non-zero value before the goto and when
cnt < funcs_cnt?

> +		}
> +	}

[ ... ]

> +	if (err) {
> +		free(ids);
> +		ids = NULL;
> +	}
> +
> +out:
> +	tdestroy(root, tdestroy_free_nop);
> +	btf__free(vmlinux_btf);
> +	btf__free(btf);
> +	return ids;
> +}

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