[PATCH bpf v2 1/4] selftests/bpf: Fix btf leak on new btf alloc failure in btf_distill test

[PATCH bpf v2 1/4] selftests/bpf: Fix btf leak on new btf alloc failure in btf_distill test

[Pu Lehui]

From: Pu Lehui <pulehui@huawei.com>

Fix btf leak on new btf alloc failure in btf_distill test.

Fixes: affdeb50616b ("selftests/bpf: Extend distilled BTF tests to cover BTF relocation")
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
v2: newly discovered bugfix.

 tools/testing/selftests/bpf/prog_tests/btf_distill.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_distill.c b/tools/testing/selftests/bpf/prog_tests/btf_distill.c
index ca84726d5ac1..b72b966df77b 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_distill.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_distill.c
@@ -385,7 +385,7 @@ static void test_distilled_base_missing_err(void)
 		"[2] INT 'int' size=8 bits_offset=0 nr_bits=64 encoding=SIGNED");
 	btf5 = btf__new_empty();
 	if (!ASSERT_OK_PTR(btf5, "empty_reloc_btf"))
-		return;
+		goto cleanup;
 	btf__add_int(btf5, "int", 4, BTF_INT_SIGNED);   /* [1] int */
 	VALIDATE_RAW_BTF(
 		btf5,
@@ -478,7 +478,7 @@ static void test_distilled_base_multi_err2(void)
 		"[1] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED");
 	btf5 = btf__new_empty();
 	if (!ASSERT_OK_PTR(btf5, "empty_reloc_btf"))
-		return;
+		goto cleanup;
 	btf__add_int(btf5, "int", 4, BTF_INT_SIGNED);   /* [1] int */
 	btf__add_int(btf5, "int", 4, BTF_INT_SIGNED);   /* [2] int */
 	VALIDATE_RAW_BTF(
-- 
2.34.1

[PATCH bpf v2 2/4] libbpf: Fix return zero when elf_begin failed

[Pu Lehui]

From: Pu Lehui <pulehui@huawei.com>

The error number of elf_begin is omitted when encapsulating the
btf_find_elf_sections function.

Fixes: c86f180ffc99 ("libbpf: Make btf_parse_elf process .BTF.base transparently")
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
 tools/lib/bpf/btf.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c
index 12468ae0d573..7e810fa468ea 100644
--- a/tools/lib/bpf/btf.c
+++ b/tools/lib/bpf/btf.c
@@ -1186,6 +1186,7 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf,
 
 	elf = elf_begin(fd, ELF_C_READ, NULL);
 	if (!elf) {
+		err = -LIBBPF_ERRNO__FORMAT;
 		pr_warn("failed to open %s as ELF file\n", path);
 		goto done;
 	}
-- 
2.34.1

[PATCH bpf v2 3/4] libbpf: Fix incorrect traversal end type ID when marking BTF_IS_EMBEDDED

[Pu Lehui]

From: Pu Lehui <pulehui@huawei.com>

When redirecting the split BTF to the vmlinux base BTF, we need to mark
the distilled base struct/union members of split BTF structs/unions in
id_map with BTF_IS_EMBEDDED. This indicates that these types must match
both name and size later. Therefore, we need to traverse the entire
split BTF, which involves traversing type IDs from nr_dist_base_types to
nr_types. However, the current implementation uses an incorrect
traversal end type ID, so let's correct it.

Fixes: 19e00c897d50 ("libbpf: Split BTF relocation")
Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
v2: make commit description more sense.

 tools/lib/bpf/btf_relocate.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/btf_relocate.c b/tools/lib/bpf/btf_relocate.c
index b72f83e15156..53d1f3541bce 100644
--- a/tools/lib/bpf/btf_relocate.c
+++ b/tools/lib/bpf/btf_relocate.c
@@ -212,7 +212,7 @@ static int btf_relocate_map_distilled_base(struct btf_relocate *r)
 	 * need to match both name and size, otherwise embedding the base
 	 * struct/union in the split type is invalid.
 	 */
-	for (id = r->nr_dist_base_types; id < r->nr_split_types; id++) {
+	for (id = r->nr_dist_base_types; id < r->nr_dist_base_types + r->nr_split_types; id++) {
 		err = btf_mark_embedded_composite_type_ids(r, id);
 		if (err)
 			goto done;
-- 
2.34.1

[PATCH bpf v2 4/4] selftests/bpf: Add distilled BTF test about marking BTF_IS_EMBEDDED

[Pu Lehui]

From: Pu Lehui <pulehui@huawei.com>

When redirecting the split BTF to the vmlinux base BTF, we need to mark
the distilled base struct/union members of split BTF structs/unions in
id_map with BTF_IS_EMBEDDED. This indicates that these types must match
both name and size later. So if a needed composite type, which is the
member of composite type in the split BTF, has a different size in the
base BTF we wish to relocate with, btf__relocate() should error out.

Signed-off-by: Pu Lehui <pulehui@huawei.com>
---
v2: Add test about marking BTF_IS_EMBEDDED.

 .../selftests/bpf/prog_tests/btf_distill.c    | 72 +++++++++++++++++++
 1 file changed, 72 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/btf_distill.c b/tools/testing/selftests/bpf/prog_tests/btf_distill.c
index b72b966df77b..fb67ae195a73 100644
--- a/tools/testing/selftests/bpf/prog_tests/btf_distill.c
+++ b/tools/testing/selftests/bpf/prog_tests/btf_distill.c
@@ -601,6 +601,76 @@ static void test_distilled_endianness(void)
 	btf__free(base);
 }
 
+/* If a needed composite type, which is the member of composite type
+ * in the split BTF, has a different size in the base BTF we wish to
+ * relocate with, btf__relocate() should error out.
+ */
+static void test_distilled_base_embedded_err(void)
+{
+	struct btf *btf1 = NULL, *btf2 = NULL, *btf3 = NULL, *btf4 = NULL, *btf5 = NULL;
+
+	btf1 = btf__new_empty();
+	if (!ASSERT_OK_PTR(btf1, "empty_main_btf"))
+		return;
+
+	btf__add_int(btf1, "int", 4, BTF_INT_SIGNED);   /* [1] int */
+	btf__add_struct(btf1, "s1", 4);                 /* [2] struct s1 { */
+	btf__add_field(btf1, "f1", 1, 0, 0);            /*      int f1; */
+							/* } */
+	VALIDATE_RAW_BTF(
+		btf1,
+		"[1] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED",
+		"[2] STRUCT 's1' size=4 vlen=1\n"
+		"\t'f1' type_id=1 bits_offset=0");
+
+	btf2 = btf__new_empty_split(btf1);
+	if (!ASSERT_OK_PTR(btf2, "empty_split_btf"))
+		goto cleanup;
+
+	btf__add_struct(btf2, "with_embedded", 8);      /* [3] struct with_embedded { */
+	btf__add_field(btf2, "e1", 2, 0, 0);		/*      struct s1 e1; */
+							/* } */
+
+	VALIDATE_RAW_BTF(
+		btf2,
+		"[1] INT 'int' size=4 bits_offset=0 nr_bits=32 encoding=SIGNED",
+		"[2] STRUCT 's1' size=4 vlen=1\n"
+		"\t'f1' type_id=1 bits_offset=0",
+		"[3] STRUCT 'with_embedded' size=8 vlen=1\n"
+		"\t'e1' type_id=2 bits_offset=0");
+
+	if (!ASSERT_EQ(0, btf__distill_base(btf2, &btf3, &btf4),
+		       "distilled_base") ||
+	    !ASSERT_OK_PTR(btf3, "distilled_base") ||
+	    !ASSERT_OK_PTR(btf4, "distilled_split") ||
+	    !ASSERT_EQ(2, btf__type_cnt(btf3), "distilled_base_type_cnt"))
+		goto cleanup;
+
+	VALIDATE_RAW_BTF(
+		btf4,
+		"[1] STRUCT 's1' size=4 vlen=0",
+		"[2] STRUCT 'with_embedded' size=8 vlen=1\n"
+		"\t'e1' type_id=1 bits_offset=0");
+
+	btf5 = btf__new_empty();
+	if (!ASSERT_OK_PTR(btf5, "empty_reloc_btf"))
+		goto cleanup;
+
+	btf__add_int(btf5, "int", 4, BTF_INT_SIGNED);   /* [1] int */
+	/* struct with the same name but different size */
+	btf__add_struct(btf5, "s1", 8);                 /* [2] struct s1 { */
+	btf__add_field(btf5, "f1", 1, 0, 0);            /*      int f1; */
+							/* } */
+
+	ASSERT_EQ(btf__relocate(btf4, btf5), -EINVAL, "relocate_split");
+cleanup:
+	btf__free(btf5);
+	btf__free(btf4);
+	btf__free(btf3);
+	btf__free(btf2);
+	btf__free(btf1);
+}
+
 void test_btf_distill(void)
 {
 	if (test__start_subtest("distilled_base"))
@@ -613,6 +683,8 @@ void test_btf_distill(void)
 		test_distilled_base_multi_err();
 	if (test__start_subtest("distilled_base_multi_err2"))
 		test_distilled_base_multi_err2();
+	if (test__start_subtest("distilled_base_embedded_err"))
+		test_distilled_base_embedded_err();
 	if (test__start_subtest("distilled_base_vmlinux"))
 		test_distilled_base_vmlinux();
 	if (test__start_subtest("distilled_endianness"))
-- 
2.34.1

Re: [PATCH bpf v2 4/4] selftests/bpf: Add distilled BTF test about marking BTF_IS_EMBEDDED

[Andrii Nakryiko]

On Wed, Jan 15, 2025 at 2:00 AM Pu Lehui <pulehui@huaweicloud.com> wrote:
>
> From: Pu Lehui <pulehui@huawei.com>
>
> When redirecting the split BTF to the vmlinux base BTF, we need to mark
> the distilled base struct/union members of split BTF structs/unions in
> id_map with BTF_IS_EMBEDDED. This indicates that these types must match
> both name and size later. So if a needed composite type, which is the
> member of composite type in the split BTF, has a different size in the
> base BTF we wish to relocate with, btf__relocate() should error out.
>
> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> ---
> v2: Add test about marking BTF_IS_EMBEDDED.
>
>  .../selftests/bpf/prog_tests/btf_distill.c    | 72 +++++++++++++++++++
>  1 file changed, 72 insertions(+)
>

Nice test, thanks! Applied the series to bpf-next.

> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_distill.c b/tools/testing/selftests/bpf/prog_tests/btf_distill.c
> index b72b966df77b..fb67ae195a73 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf_distill.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf_distill.c
> @@ -601,6 +601,76 @@ static void test_distilled_endianness(void)
>         btf__free(base);
>  }
>

[...]

Re: [PATCH bpf v2 4/4] selftests/bpf: Add distilled BTF test about marking BTF_IS_EMBEDDED

[Pu Lehui]

On 2025/1/17 7:34, Andrii Nakryiko wrote:
> On Wed, Jan 15, 2025 at 2:00 AM Pu Lehui <pulehui@huaweicloud.com> wrote:
>>
>> From: Pu Lehui <pulehui@huawei.com>
>>
>> When redirecting the split BTF to the vmlinux base BTF, we need to mark
>> the distilled base struct/union members of split BTF structs/unions in
>> id_map with BTF_IS_EMBEDDED. This indicates that these types must match
>> both name and size later. So if a needed composite type, which is the
>> member of composite type in the split BTF, has a different size in the
>> base BTF we wish to relocate with, btf__relocate() should error out.
>>
>> Signed-off-by: Pu Lehui <pulehui@huawei.com>
>> ---
>> v2: Add test about marking BTF_IS_EMBEDDED.
>>
>>   .../selftests/bpf/prog_tests/btf_distill.c    | 72 +++++++++++++++++++
>>   1 file changed, 72 insertions(+)
>>
> 
> Nice test, thanks! Applied the series to bpf-next.

Curious, resilient split BTF is currently supported, shall we deprecate 
MODULE_ALLOW_BTF_MISMATCH?

> 
>> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_distill.c b/tools/testing/selftests/bpf/prog_tests/btf_distill.c
>> index b72b966df77b..fb67ae195a73 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/btf_distill.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/btf_distill.c
>> @@ -601,6 +601,76 @@ static void test_distilled_endianness(void)
>>          btf__free(base);
>>   }
>>
> 
> [...]

Re: [PATCH bpf v2 4/4] selftests/bpf: Add distilled BTF test about marking BTF_IS_EMBEDDED

[Andrii Nakryiko]

On Thu, Jan 16, 2025 at 11:15 PM Pu Lehui <pulehui@huaweicloud.com> wrote:
>
>
>
> On 2025/1/17 7:34, Andrii Nakryiko wrote:
> > On Wed, Jan 15, 2025 at 2:00 AM Pu Lehui <pulehui@huaweicloud.com> wrote:
> >>
> >> From: Pu Lehui <pulehui@huawei.com>
> >>
> >> When redirecting the split BTF to the vmlinux base BTF, we need to mark
> >> the distilled base struct/union members of split BTF structs/unions in
> >> id_map with BTF_IS_EMBEDDED. This indicates that these types must match
> >> both name and size later. So if a needed composite type, which is the
> >> member of composite type in the split BTF, has a different size in the
> >> base BTF we wish to relocate with, btf__relocate() should error out.
> >>
> >> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> >> ---
> >> v2: Add test about marking BTF_IS_EMBEDDED.
> >>
> >>   .../selftests/bpf/prog_tests/btf_distill.c    | 72 +++++++++++++++++++
> >>   1 file changed, 72 insertions(+)
> >>
> >
> > Nice test, thanks! Applied the series to bpf-next.
>
> Curious, resilient split BTF is currently supported, shall we deprecate
> MODULE_ALLOW_BTF_MISMATCH?

I'd first give it a bit more time for distilled BTFs to be used widely
in practice to work out all the kinks.

>
> >
> >> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_distill.c b/tools/testing/selftests/bpf/prog_tests/btf_distill.c
> >> index b72b966df77b..fb67ae195a73 100644
> >> --- a/tools/testing/selftests/bpf/prog_tests/btf_distill.c
> >> +++ b/tools/testing/selftests/bpf/prog_tests/btf_distill.c
> >> @@ -601,6 +601,76 @@ static void test_distilled_endianness(void)
> >>          btf__free(base);
> >>   }
> >>
> >
> > [...]
>

Re: [PATCH bpf v2 1/4] selftests/bpf: Fix btf leak on new btf alloc failure in btf_distill test

[patchwork-bot+netdevbpf]

Hello:

This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Wed, 15 Jan 2025 10:02:38 +0000 you wrote:
> From: Pu Lehui <pulehui@huawei.com>
> 
> Fix btf leak on new btf alloc failure in btf_distill test.
> 
> Fixes: affdeb50616b ("selftests/bpf: Extend distilled BTF tests to cover BTF relocation")
> Signed-off-by: Pu Lehui <pulehui@huawei.com>
> 
> [...]

Here is the summary with links:
  - [bpf,v2,1/4] selftests/bpf: Fix btf leak on new btf alloc failure in btf_distill test
    https://git.kernel.org/bpf/bpf-next/c/4a04cb326a6c
  - [bpf,v2,2/4] libbpf: Fix return zero when elf_begin failed
    https://git.kernel.org/bpf/bpf-next/c/5436a54332c1
  - [bpf,v2,3/4] libbpf: Fix incorrect traversal end type ID when marking BTF_IS_EMBEDDED
    https://git.kernel.org/bpf/bpf-next/c/5ca681a86ef9
  - [bpf,v2,4/4] selftests/bpf: Add distilled BTF test about marking BTF_IS_EMBEDDED
    https://git.kernel.org/bpf/bpf-next/c/556a39940663

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html