Re: [PATCH bpf-next v1 4/4] selftests/bpf: Show the tag kflag in the raw BTF dump helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] bpf: NULL pointer dereference in test_btf_attrs on test failure
--

commit 551463268433576dd9cc5cc27a8ffb06a64ecabb
Author: Ihor Solodrai <ihor.solodrai@linux.dev>

selftests/bpf: Show the tag kflag in the raw BTF dump helper

This commit updates fprintf_btf_type_raw() to print the kflag for
BTF_KIND_DECL_TAG and BTF_KIND_TYPE_TAG types. It also adds a new subtest
test_btf_attrs to prog_tests/btf_write.c to verify this behavior.

> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_write.c b/tools/testing/selftests/bpf/prog_tests/btf_write.c
> index 5c84723cf2540..7ddda9348e4fe 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf_write.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_write.c
> @@ -606,6 +606,64 @@ static void test_btf_add_btf_split()
[ ... ]
> +static void test_btf_attrs(void)
> +{
> +	const struct btf_type *t;
> +	struct btf *btf;
> +	int id;
> +
> +	btf = btf__new_empty();
> +	if (!ASSERT_OK_PTR(btf, "new_empty"))
> +		return;
> +
> +	id = btf__add_int(btf, "int", 4, BTF_INT_SIGNED);
> +	ASSERT_EQ(id, 1, "int_id");
> +
> +	id = btf__add_struct(btf, "s", 4);
> +	ASSERT_EQ(id, 2, "struct_id");
> +	ASSERT_OK(btf__add_field(btf, "f", 1, 0, 0), "field_ok");
> +
> +	id = btf__add_decl_tag(btf, "tag", 2, -1);
> +	ASSERT_EQ(id, 3, "decl_tag_id");
> +	t = btf__type_by_id(btf, 3);
> +	ASSERT_EQ(btf_kind(t), BTF_KIND_DECL_TAG, "decl_tag_kind");

[Severity: Medium]
In BPF selftests, ASSERT_EQ() logs a failure but does not abort the execution
of the test block. If btf__add_decl_tag() fails, the subsequent call to
btf__type_by_id() will return a NULL pointer.

Will this cause a NULL pointer dereference when t is passed to btf_kind(),
which unconditionally dereferences it?

> +	ASSERT_EQ(btf_kflag(t), 0, "decl_tag_kflag");
> +
> +	id = btf__add_decl_attr(btf, "tag", 2, -1);
> +	ASSERT_EQ(id, 4, "decl_attr_id");
> +	t = btf__type_by_id(btf, 4);
> +	ASSERT_EQ(btf_kind(t), BTF_KIND_DECL_TAG, "decl_attr_kind");
> +	ASSERT_EQ(btf_kflag(t), 1, "decl_attr_kflag");
> +	ASSERT_EQ(t->type, 2, "decl_attr_type");
> +	ASSERT_EQ(btf_decl_tag(t)->component_idx, -1, "decl_attr_component_idx");
> +
> +	id = btf__add_decl_attr(btf, "member_attr", 2, 0);
> +	ASSERT_EQ(id, 5, "member_attr_id");
> +	ASSERT_EQ(btf_decl_tag(btf__type_by_id(btf, 5))->component_idx, 0,
> +		  "member_attr_component_idx");

[Severity: Medium]
Similarly, could chaining a potential NULL return from btf__type_by_id()
directly into btf_decl_tag() lead to an out-of-bounds dereference due to
pointer arithmetic on NULL?

> +
> +	id = btf__add_type_tag(btf, "tag", 1);
> +	ASSERT_EQ(id, 6, "type_tag_id");
> +	ASSERT_EQ(btf_kflag(btf__type_by_id(btf, 6)), 0, "type_tag_kflag");

[Severity: Medium]
Is it possible that calling btf_kflag() on a potential NULL pointer from
btf__type_by_id() results in a segmentation fault if the type addition failed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260917012037.1396254-1-ihor.solodrai@linux.dev?part=4