Re: [PATCH v15 bpf-next 8/9] selftests/bpf: Test kind encoding/decoding

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The BTF object `btf` is leaked on early error paths.
--

--- Patch [8]: [PATCH v15 bpf-next 8/9] selftests/bpf: Test kind encoding/decoding ---
commit 2e5fe50dd1168fa6e0f69edd9f780d42539e7151
Author: Alan Maguire <[email protected]>

selftests/bpf: Test kind encoding/decoding

This commit adds tests to verify that btf__new_empty_opts() correctly adds
layouts for all supported kinds. It also ensures that parsing of a BTF
containing unrecognized kinds can still succeed.

> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_kind.c b/tools/testing/selftests/bpf/prog_tests/btf_kind.c
> new file mode 100644
> index 0000000000000..64ceac1d999e6
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_kind.c
> @@ -0,0 +1,226 @@
[ ... ]
> +static void test_btf_kind_encoding(void)
> +{
> +	LIBBPF_OPTS(btf_new_opts, opts);
> +	const struct btf_header *hdr;
> +	const void *raw_btf;
> +	struct btf *btf;
> +	__u32 raw_size;
> +
> +	opts.add_layout = true;
> +	btf = btf__new_empty_opts(&opts);
> +	if (!ASSERT_OK_PTR(btf, "btf_new"))
> +		return;
> +
> +	raw_btf = btf__raw_data(btf, &raw_size);
> +	if (!ASSERT_OK_PTR(raw_btf, "btf__raw_data"))
> +		return;

[Severity: Medium]
Does this early return leak the btf object allocated just above by 
btf__new_empty_opts()? It looks like btf__free(btf) is skipped if
btf__raw_data() fails.

[ ... ]
> +void test_btf_kind_decoding(void)
> +{
> +	char btf_kind_file1[] = "/tmp/test_btf_kind.XXXXXX";
> +	char btf_kind_file2[] = "/tmp/test_btf_kind.XXXXXX";
> +	char btf_kind_file3[] = "/tmp/test_btf_kind.XXXXXX";
> +	struct btf *btf = NULL, *new_btf = NULL;
> +	__s32 int_id, unrec_id, id, id2;
> +	LIBBPF_OPTS(btf_new_opts, opts);
> +	struct btf_layout *l;
> +	struct btf_header *hdr;
> +	const void *raw_btf;
> +	struct btf_type *t;
> +	void *new_raw_btf;
> +	void *str_data;
> +	__u32 raw_size;
> +
> +	opts.add_layout = true;
> +	btf = btf__new_empty_opts(&opts);
> +	if (!ASSERT_OK_PTR(btf, "btf_new"))
> +		return;
> +
> +	int_id = btf__add_int(btf, "test_char", 1, BTF_INT_CHAR);
> +	if (!ASSERT_GT(int_id, 0, "add_int_id"))
> +		return;

[Severity: Medium]
Will this direct return bypass the cleanup block at the end of the function
and leak the btf object? 

There are several other similar return statements following this one. Should 
these early failure paths use goto out to ensure btf__free(btf) is called?

> +
> +	/*
> +	 * Create our type with unrecognized kind by adding a typedef kind
> +	 * we will overwrite it with our unrecognized kind value.
> +	 */
> +	unrec_id = btf__add_typedef(btf, "unrec_kind", int_id);
> +	if (!ASSERT_GT(unrec_id, 0, "add_unrec_id"))
> +		return;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=8