* [PATCH v2 bpf-next 0/2] Multi-split BTF fixes and test @ 2025-10-28 15:57 Alan Maguire 2025-10-28 15:57 ` [PATCH v2 bpf-next 1/2] libbpf: Fix parsing of multi-split BTF Alan Maguire 2025-10-28 15:57 ` [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF Alan Maguire 0 siblings, 2 replies; 8+ messages in thread From: Alan Maguire @ 2025-10-28 15:57 UTC (permalink / raw) To: andrii Cc: eddyz87, ast, daniel, martin.lau, acme, ttreyer, yonghong.song, song, john.fastabend, kpsingh, sdf, haoluo, jolsa, ihor.solodrai, bpf, Alan Maguire This small series consists of a fix to multi-split BTF parsing (patch 1) and a test which exercises (multi-)split BTF parsing (patch 2). Changes since v1 [1] - BPF code-review bot spotted another place that the string offset needed to be adjusted based upon base start string offset + header string offset. - added selftests to extend split BTF testing to parsing [1] https://lore.kernel.org/bpf/20251023142812.258870-1-alan.maguire@oracle.com/ Alan Maguire (2): libbpf: Fix parsing of multi-split BTF selftests/bpf: Test parsing of (multi-)split BTF tools/lib/bpf/btf.c | 4 +- .../selftests/bpf/prog_tests/btf_split.c | 71 ++++++++++++++++++- 2 files changed, 72 insertions(+), 3 deletions(-) -- 2.39.3 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 bpf-next 1/2] libbpf: Fix parsing of multi-split BTF 2025-10-28 15:57 [PATCH v2 bpf-next 0/2] Multi-split BTF fixes and test Alan Maguire @ 2025-10-28 15:57 ` Alan Maguire 2025-10-28 20:01 ` Andrii Nakryiko 2025-10-28 15:57 ` [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF Alan Maguire 1 sibling, 1 reply; 8+ messages in thread From: Alan Maguire @ 2025-10-28 15:57 UTC (permalink / raw) To: andrii Cc: eddyz87, ast, daniel, martin.lau, acme, ttreyer, yonghong.song, song, john.fastabend, kpsingh, sdf, haoluo, jolsa, ihor.solodrai, bpf, Alan Maguire When creating multi-split BTF we correctly set the start string offset to be the size of the base string section plus the base BTF start string offset; the latter is needed for multi-split BTF since the offset is non-zero there. Unfortunately the BTF parsing case needed that logic and it was missed. Fixes: 4e29128a9ace ("libbpf/btf: Fix string handling to support multi-split BTF") Signed-off-by: Alan Maguire <alan.maguire@oracle.com> --- tools/lib/bpf/btf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c index 18907f0fcf9f..9f141395c074 100644 --- a/tools/lib/bpf/btf.c +++ b/tools/lib/bpf/btf.c @@ -1061,7 +1061,7 @@ static struct btf *btf_new(const void *data, __u32 size, struct btf *base_btf, b if (base_btf) { btf->base_btf = base_btf; btf->start_id = btf__type_cnt(base_btf); - btf->start_str_off = base_btf->hdr->str_len; + btf->start_str_off = base_btf->hdr->str_len + base_btf->start_str_off; } if (is_mmap) { @@ -5818,7 +5818,7 @@ void btf_set_base_btf(struct btf *btf, const struct btf *base_btf) { btf->base_btf = (struct btf *)base_btf; btf->start_id = btf__type_cnt(base_btf); - btf->start_str_off = base_btf->hdr->str_len; + btf->start_str_off = base_btf->hdr->str_len + base_btf->start_str_off; } int btf__relocate(struct btf *btf, const struct btf *base_btf) -- 2.39.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 bpf-next 1/2] libbpf: Fix parsing of multi-split BTF 2025-10-28 15:57 ` [PATCH v2 bpf-next 1/2] libbpf: Fix parsing of multi-split BTF Alan Maguire @ 2025-10-28 20:01 ` Andrii Nakryiko 0 siblings, 0 replies; 8+ messages in thread From: Andrii Nakryiko @ 2025-10-28 20:01 UTC (permalink / raw) To: Alan Maguire Cc: andrii, eddyz87, ast, daniel, martin.lau, acme, ttreyer, yonghong.song, song, john.fastabend, kpsingh, sdf, haoluo, jolsa, ihor.solodrai, bpf On Tue, Oct 28, 2025 at 8:57 AM Alan Maguire <alan.maguire@oracle.com> wrote: > > When creating multi-split BTF we correctly set the start string offset > to be the size of the base string section plus the base BTF start > string offset; the latter is needed for multi-split BTF since the > offset is non-zero there. > > Unfortunately the BTF parsing case needed that logic and it was > missed. > > Fixes: 4e29128a9ace ("libbpf/btf: Fix string handling to support > multi-split BTF") please make sure commit references are not wrapped when used as part of Fixes: tag, I had to manually fix it > Signed-off-by: Alan Maguire <alan.maguire@oracle.com> > --- > tools/lib/bpf/btf.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > index 18907f0fcf9f..9f141395c074 100644 > --- a/tools/lib/bpf/btf.c > +++ b/tools/lib/bpf/btf.c > @@ -1061,7 +1061,7 @@ static struct btf *btf_new(const void *data, __u32 size, struct btf *base_btf, b > if (base_btf) { > btf->base_btf = base_btf; > btf->start_id = btf__type_cnt(base_btf); > - btf->start_str_off = base_btf->hdr->str_len; > + btf->start_str_off = base_btf->hdr->str_len + base_btf->start_str_off; > } > > if (is_mmap) { > @@ -5818,7 +5818,7 @@ void btf_set_base_btf(struct btf *btf, const struct btf *base_btf) > { > btf->base_btf = (struct btf *)base_btf; > btf->start_id = btf__type_cnt(base_btf); > - btf->start_str_off = base_btf->hdr->str_len; > + btf->start_str_off = base_btf->hdr->str_len + base_btf->start_str_off; > } > > int btf__relocate(struct btf *btf, const struct btf *base_btf) > -- > 2.39.3 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF 2025-10-28 15:57 [PATCH v2 bpf-next 0/2] Multi-split BTF fixes and test Alan Maguire 2025-10-28 15:57 ` [PATCH v2 bpf-next 1/2] libbpf: Fix parsing of multi-split BTF Alan Maguire @ 2025-10-28 15:57 ` Alan Maguire 2025-10-28 16:45 ` bot+bpf-ci 1 sibling, 1 reply; 8+ messages in thread From: Alan Maguire @ 2025-10-28 15:57 UTC (permalink / raw) To: andrii Cc: eddyz87, ast, daniel, martin.lau, acme, ttreyer, yonghong.song, song, john.fastabend, kpsingh, sdf, haoluo, jolsa, ihor.solodrai, bpf, Alan Maguire Write raw BTF to files, parse it and compare to original; this allows us to test parsing of (multi-)split BTF code. Signed-off-by: Alan Maguire <alan.maguire@oracle.com> --- .../selftests/bpf/prog_tests/btf_split.c | 71 ++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/prog_tests/btf_split.c b/tools/testing/selftests/bpf/prog_tests/btf_split.c index 3696fb9a05ed..ee1481c5fe27 100644 --- a/tools/testing/selftests/bpf/prog_tests/btf_split.c +++ b/tools/testing/selftests/bpf/prog_tests/btf_split.c @@ -12,11 +12,43 @@ static void btf_dump_printf(void *ctx, const char *fmt, va_list args) vfprintf(ctx, fmt, args); } +static int btf_raw_write(struct btf *btf, char *file) +{ + ssize_t written = 0; + const void *data; + __u32 size = 0; + int fd, ret; + + fd = mkstemp(file); + if (!ASSERT_GE(fd, 0, "create_file")) + return -errno; + + data = btf__raw_data(btf, &size); + if (!ASSERT_OK_PTR(data, "btf__raw_data")) { + close(fd); + return -EINVAL; + } + while (written < size) { + ret = write(fd, data + written, size - written); + if (!ASSERT_GE(ret, 0, "write succeeded")) { + close(fd); + return -errno; + } + written += ret; + } + close(fd); + return 0; +} + static void __test_btf_split(bool multi) { + char multisplit_btf_file[] = "/tmp/test_btf_multisplit.XXXXXX"; + char split_btf_file[] = "/tmp/test_btf_split.XXXXXX"; + char base_btf_file[] = "/tmp/test_btf_base.XXXXXX"; struct btf_dump *d = NULL; - const struct btf_type *t; + const struct btf_type *t, *ot; struct btf *btf1, *btf2, *btf3 = NULL; + struct btf *btf4, *btf5, *btf6 = NULL; int str_off, i, err; btf1 = btf__new_empty(); @@ -123,6 +155,35 @@ static void __test_btf_split(bool multi) " int uf2;\n" "};\n\n", "c_dump"); + /* write base, split BTFs to files and ensure parsing succeeds */ + if (btf_raw_write(btf1, base_btf_file) != 0) + goto cleanup; + if (btf_raw_write(btf2, split_btf_file) != 0) + goto cleanup; + btf4 = btf__parse(base_btf_file, NULL); + if (!ASSERT_OK_PTR(btf4, "parse_base")) + goto cleanup; + btf5 = btf__parse_split(split_btf_file, btf4); + if (!ASSERT_OK_PTR(btf5, "parse_split")) + goto cleanup; + if (multi) { + if (btf_raw_write(btf3, multisplit_btf_file) != 0) + goto cleanup; + btf6 = btf__parse_split(multisplit_btf_file, btf5); + if (!ASSERT_OK_PTR(btf5, "parse_multisplit")) + goto cleanup; + } else { + btf6 = btf5; + } + + /* compare parsed to original BTF */ + for (i = 1; i < btf__type_cnt(btf6); i++) { + t = btf__type_by_id(btf6, i); + ot = btf__type_by_id(btf3, i); + if (!ASSERT_EQ(memcmp(t, ot, sizeof(*ot)), 0, "cmp_parsed_orig_btf")) + goto cleanup; + } + cleanup: if (dump_buf_file) fclose(dump_buf_file); @@ -132,6 +193,14 @@ static void __test_btf_split(bool multi) btf__free(btf2); if (btf2 != btf3) btf__free(btf3); + btf__free(btf4); + btf__free(btf5); + if (btf5 != btf6) + btf__free(btf6); + unlink(base_btf_file); + unlink(split_btf_file); + if (multi) + unlink(multisplit_btf_file); } void test_btf_split(void) -- 2.39.3 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF 2025-10-28 15:57 ` [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF Alan Maguire @ 2025-10-28 16:45 ` bot+bpf-ci 2025-10-28 20:01 ` Andrii Nakryiko 0 siblings, 1 reply; 8+ messages in thread From: bot+bpf-ci @ 2025-10-28 16:45 UTC (permalink / raw) To: alan.maguire, andrii Cc: eddyz87, ast, daniel, martin.lau, acme, ttreyer, yonghong.song, song, john.fastabend, kpsingh, sdf, haoluo, jolsa, ihor.solodrai, bpf, alan.maguire, ast, andrii, daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai [-- Attachment #1: Type: text/plain, Size: 2082 bytes --] ``` commit 8509f4500fb2dfaec7a383a802dffaf84995c434 Author: Alan Maguire <alan.maguire@oracle.com> selftests/bpf: Test parsing of (multi-)split BTF This commit adds test coverage for parsing split BTF data that has been written to files. It writes raw BTF data to temporary files, parses them back, and compares the parsed results to the original in-memory BTF. > diff --git a/tools/testing/selftests/bpf/prog_tests/btf_split.c b/tools/testing/selftests/bpf/prog_tests/btf_split.c > index 3696fb9a0..ee1481c5f 100644 > --- a/tools/testing/selftests/bpf/prog_tests/btf_split.c > +++ b/tools/testing/selftests/bpf/prog_tests/btf_split.c [ ... ] > @@ -123,6 +155,35 @@ static void __test_btf_split(bool multi) > " int uf2;\n" > "};\n\n", "c_dump"); > > + /* write base, split BTFs to files and ensure parsing succeeds */ > + if (btf_raw_write(btf1, base_btf_file) != 0) > + goto cleanup; > + if (btf_raw_write(btf2, split_btf_file) != 0) > + goto cleanup; > + btf4 = btf__parse(base_btf_file, NULL); > + if (!ASSERT_OK_PTR(btf4, "parse_base")) > + goto cleanup; > + btf5 = btf__parse_split(split_btf_file, btf4); > + if (!ASSERT_OK_PTR(btf5, "parse_split")) > + goto cleanup; > + if (multi) { > + if (btf_raw_write(btf3, multisplit_btf_file) != 0) > + goto cleanup; > + btf6 = btf__parse_split(multisplit_btf_file, btf5); > + if (!ASSERT_OK_PTR(btf5, "parse_multisplit")) ^^^^^ Should this check btf6 instead of btf5? The line above assigns to btf6, but this assertion checks btf5 which was already validated earlier. Following the pattern from the btf4 and btf5 checks above, it looks like this should be checking the newly assigned btf6 pointer. > + goto cleanup; > + } else { > + btf6 = btf5; > + } [ ... ] ``` --- AI reviewed your patch. Please fix the bug or email reply why it's not a bug. See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md In-Reply-To-Subject: `selftests/bpf: Test parsing of (multi-)split BTF` CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18881352510 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF 2025-10-28 16:45 ` bot+bpf-ci @ 2025-10-28 20:01 ` Andrii Nakryiko 2025-10-28 20:11 ` Andrii Nakryiko 0 siblings, 1 reply; 8+ messages in thread From: Andrii Nakryiko @ 2025-10-28 20:01 UTC (permalink / raw) To: bot+bpf-ci Cc: alan.maguire, andrii, eddyz87, ast, daniel, martin.lau, acme, ttreyer, yonghong.song, song, john.fastabend, kpsingh, sdf, haoluo, jolsa, ihor.solodrai, bpf, martin.lau, clm On Tue, Oct 28, 2025 at 9:45 AM <bot+bpf-ci@kernel.org> wrote: > > > ``` > commit 8509f4500fb2dfaec7a383a802dffaf84995c434 > Author: Alan Maguire <alan.maguire@oracle.com> > > selftests/bpf: Test parsing of (multi-)split BTF > > This commit adds test coverage for parsing split BTF data that has been > written to files. It writes raw BTF data to temporary files, parses them > back, and compares the parsed results to the original in-memory BTF. > > > diff --git a/tools/testing/selftests/bpf/prog_tests/btf_split.c b/tools/testing/selftests/bpf/prog_tests/btf_split.c > > index 3696fb9a0..ee1481c5f 100644 > > --- a/tools/testing/selftests/bpf/prog_tests/btf_split.c > > +++ b/tools/testing/selftests/bpf/prog_tests/btf_split.c > > [ ... ] > > > @@ -123,6 +155,35 @@ static void __test_btf_split(bool multi) > > " int uf2;\n" > > "};\n\n", "c_dump"); > > > > + /* write base, split BTFs to files and ensure parsing succeeds */ > > + if (btf_raw_write(btf1, base_btf_file) != 0) > > + goto cleanup; > > + if (btf_raw_write(btf2, split_btf_file) != 0) > > + goto cleanup; > > + btf4 = btf__parse(base_btf_file, NULL); > > + if (!ASSERT_OK_PTR(btf4, "parse_base")) > > + goto cleanup; > > + btf5 = btf__parse_split(split_btf_file, btf4); > > + if (!ASSERT_OK_PTR(btf5, "parse_split")) > > + goto cleanup; > > + if (multi) { > > + if (btf_raw_write(btf3, multisplit_btf_file) != 0) > > + goto cleanup; > > + btf6 = btf__parse_split(multisplit_btf_file, btf5); > > + if (!ASSERT_OK_PTR(btf5, "parse_multisplit")) > ^^^^^ > > Should this check btf6 instead of btf5? The line above assigns to btf6, > but this assertion checks btf5 which was already validated earlier. > Following the pattern from the btf4 and btf5 checks above, it looks like > this should be checking the newly assigned btf6 pointer. > AI has a good point, I fixed it up while applying. Alan, while fixing this up, I noticed that __test_btf_split clean up path can dereference a bunch of uninitialized btfs (btf4 and btf5). Also, we can goto cleanup before we create {base,split,multisplit}_btf_file, so it would be nice if you could send a follow up fixing all that. Thanks! > > + goto cleanup; > > + } else { > > + btf6 = btf5; > > + } > > [ ... ] > > > ``` > > --- > AI reviewed your patch. Please fix the bug or email reply why it's not a bug. > See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md > > In-Reply-To-Subject: `selftests/bpf: Test parsing of (multi-)split BTF` > CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18881352510 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF 2025-10-28 20:01 ` Andrii Nakryiko @ 2025-10-28 20:11 ` Andrii Nakryiko 2025-10-28 22:58 ` Alan Maguire 0 siblings, 1 reply; 8+ messages in thread From: Andrii Nakryiko @ 2025-10-28 20:11 UTC (permalink / raw) To: bot+bpf-ci Cc: alan.maguire, andrii, eddyz87, ast, daniel, martin.lau, acme, ttreyer, yonghong.song, song, john.fastabend, kpsingh, sdf, haoluo, jolsa, ihor.solodrai, bpf, martin.lau, clm On Tue, Oct 28, 2025 at 1:01 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Tue, Oct 28, 2025 at 9:45 AM <bot+bpf-ci@kernel.org> wrote: > > > > > > ``` > > commit 8509f4500fb2dfaec7a383a802dffaf84995c434 > > Author: Alan Maguire <alan.maguire@oracle.com> > > > > selftests/bpf: Test parsing of (multi-)split BTF > > > > This commit adds test coverage for parsing split BTF data that has been > > written to files. It writes raw BTF data to temporary files, parses them > > back, and compares the parsed results to the original in-memory BTF. > > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/btf_split.c b/tools/testing/selftests/bpf/prog_tests/btf_split.c > > > index 3696fb9a0..ee1481c5f 100644 > > > --- a/tools/testing/selftests/bpf/prog_tests/btf_split.c > > > +++ b/tools/testing/selftests/bpf/prog_tests/btf_split.c > > > > [ ... ] > > > > > @@ -123,6 +155,35 @@ static void __test_btf_split(bool multi) > > > " int uf2;\n" > > > "};\n\n", "c_dump"); > > > > > > + /* write base, split BTFs to files and ensure parsing succeeds */ > > > + if (btf_raw_write(btf1, base_btf_file) != 0) > > > + goto cleanup; > > > + if (btf_raw_write(btf2, split_btf_file) != 0) > > > + goto cleanup; > > > + btf4 = btf__parse(base_btf_file, NULL); > > > + if (!ASSERT_OK_PTR(btf4, "parse_base")) > > > + goto cleanup; > > > + btf5 = btf__parse_split(split_btf_file, btf4); > > > + if (!ASSERT_OK_PTR(btf5, "parse_split")) > > > + goto cleanup; > > > + if (multi) { > > > + if (btf_raw_write(btf3, multisplit_btf_file) != 0) > > > + goto cleanup; > > > + btf6 = btf__parse_split(multisplit_btf_file, btf5); > > > + if (!ASSERT_OK_PTR(btf5, "parse_multisplit")) > > ^^^^^ > > > > Should this check btf6 instead of btf5? The line above assigns to btf6, > > but this assertion checks btf5 which was already validated earlier. > > Following the pattern from the btf4 and btf5 checks above, it looks like > > this should be checking the newly assigned btf6 pointer. > > > > AI has a good point, I fixed it up while applying. > > > Alan, while fixing this up, I noticed that __test_btf_split clean up > path can dereference a bunch of uninitialized btfs (btf4 and btf5). > Also, we can goto cleanup before we create > {base,split,multisplit}_btf_file, so it would be nice if you could > send a follow up fixing all that. Thanks! Ok, so BPF CI noticed this as well. I ended up not pushing, please fix the clean up path (unlink is pre-existing bug, but it doesn't cause compiler to complain) > > > > + goto cleanup; > > > + } else { > > > + btf6 = btf5; > > > + } > > > > [ ... ] > > > > > > ``` > > > > --- > > AI reviewed your patch. Please fix the bug or email reply why it's not a bug. > > See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md > > > > In-Reply-To-Subject: `selftests/bpf: Test parsing of (multi-)split BTF` > > CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18881352510 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF 2025-10-28 20:11 ` Andrii Nakryiko @ 2025-10-28 22:58 ` Alan Maguire 0 siblings, 0 replies; 8+ messages in thread From: Alan Maguire @ 2025-10-28 22:58 UTC (permalink / raw) To: Andrii Nakryiko, bot+bpf-ci Cc: andrii, eddyz87, ast, daniel, martin.lau, acme, ttreyer, yonghong.song, song, john.fastabend, kpsingh, sdf, haoluo, jolsa, ihor.solodrai, bpf, martin.lau, clm On 28/10/2025 20:11, Andrii Nakryiko wrote: > On Tue, Oct 28, 2025 at 1:01 PM Andrii Nakryiko > <andrii.nakryiko@gmail.com> wrote: >> >> On Tue, Oct 28, 2025 at 9:45 AM <bot+bpf-ci@kernel.org> wrote: >>> >>> >>> ``` >>> commit 8509f4500fb2dfaec7a383a802dffaf84995c434 >>> Author: Alan Maguire <alan.maguire@oracle.com> >>> >>> selftests/bpf: Test parsing of (multi-)split BTF >>> >>> This commit adds test coverage for parsing split BTF data that has been >>> written to files. It writes raw BTF data to temporary files, parses them >>> back, and compares the parsed results to the original in-memory BTF. >>> >>>> diff --git a/tools/testing/selftests/bpf/prog_tests/btf_split.c b/tools/testing/selftests/bpf/prog_tests/btf_split.c >>>> index 3696fb9a0..ee1481c5f 100644 >>>> --- a/tools/testing/selftests/bpf/prog_tests/btf_split.c >>>> +++ b/tools/testing/selftests/bpf/prog_tests/btf_split.c >>> >>> [ ... ] >>> >>>> @@ -123,6 +155,35 @@ static void __test_btf_split(bool multi) >>>> " int uf2;\n" >>>> "};\n\n", "c_dump"); >>>> >>>> + /* write base, split BTFs to files and ensure parsing succeeds */ >>>> + if (btf_raw_write(btf1, base_btf_file) != 0) >>>> + goto cleanup; >>>> + if (btf_raw_write(btf2, split_btf_file) != 0) >>>> + goto cleanup; >>>> + btf4 = btf__parse(base_btf_file, NULL); >>>> + if (!ASSERT_OK_PTR(btf4, "parse_base")) >>>> + goto cleanup; >>>> + btf5 = btf__parse_split(split_btf_file, btf4); >>>> + if (!ASSERT_OK_PTR(btf5, "parse_split")) >>>> + goto cleanup; >>>> + if (multi) { >>>> + if (btf_raw_write(btf3, multisplit_btf_file) != 0) >>>> + goto cleanup; >>>> + btf6 = btf__parse_split(multisplit_btf_file, btf5); >>>> + if (!ASSERT_OK_PTR(btf5, "parse_multisplit")) >>> ^^^^^ >>> >>> Should this check btf6 instead of btf5? The line above assigns to btf6, >>> but this assertion checks btf5 which was already validated earlier. >>> Following the pattern from the btf4 and btf5 checks above, it looks like >>> this should be checking the newly assigned btf6 pointer. >>> >> >> AI has a good point, I fixed it up while applying. >> >> >> Alan, while fixing this up, I noticed that __test_btf_split clean up >> path can dereference a bunch of uninitialized btfs (btf4 and btf5). >> Also, we can goto cleanup before we create >> {base,split,multisplit}_btf_file, so it would be nice if you could >> send a follow up fixing all that. Thanks! > > Ok, so BPF CI noticed this as well. I ended up not pushing, please fix > the clean up path (unlink is pre-existing bug, but it doesn't cause > compiler to complain) > sure, I've sent v3 which I think has all the cleanups needed now, along with fixed Fixes: tag and ASSERT_OK_PTR() on right btf. Thanks! Alan ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2025-10-28 22:59 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-10-28 15:57 [PATCH v2 bpf-next 0/2] Multi-split BTF fixes and test Alan Maguire 2025-10-28 15:57 ` [PATCH v2 bpf-next 1/2] libbpf: Fix parsing of multi-split BTF Alan Maguire 2025-10-28 20:01 ` Andrii Nakryiko 2025-10-28 15:57 ` [PATCH v2 bpf-next 2/2] selftests/bpf: Test parsing of (multi-)split BTF Alan Maguire 2025-10-28 16:45 ` bot+bpf-ci 2025-10-28 20:01 ` Andrii Nakryiko 2025-10-28 20:11 ` Andrii Nakryiko 2025-10-28 22:58 ` Alan Maguire
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox