* [PATCH] libbpf: check for empty BTF data section in btf_parse_elf @ 2025-04-08 18:41 Ihor Solodrai 2025-04-09 12:09 ` Mykyta Yatsenko 2025-04-09 23:14 ` Andrii Nakryiko 0 siblings, 2 replies; 6+ messages in thread From: Ihor Solodrai @ 2025-04-08 18:41 UTC (permalink / raw) To: andrii, ast, daniel, eddyz87; +Cc: bpf, mykolal, kernel-team A valid ELF file may contain a SHT_NOBITS .BTF section. This case is not handled correctly in btf_parse_elf, which leads to a segfault. Add a null check for a buffer returned by elf_getdata() before proceeding with its processing. Bug report: https://github.com/libbpf/libbpf/issues/894 Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> --- tools/lib/bpf/btf.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c index 38bc6b14b066..90599f0311bd 100644 --- a/tools/lib/bpf/btf.c +++ b/tools/lib/bpf/btf.c @@ -1201,6 +1201,12 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, goto done; } + if (!secs.btf_data->d_buf) { + pr_warn("BTF data is empty in %s\n", path); + err = -ENODATA; + goto done; + } + if (secs.btf_base_data) { dist_base_btf = btf_new(secs.btf_base_data->d_buf, secs.btf_base_data->d_size, NULL); -- 2.49.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] libbpf: check for empty BTF data section in btf_parse_elf 2025-04-08 18:41 [PATCH] libbpf: check for empty BTF data section in btf_parse_elf Ihor Solodrai @ 2025-04-09 12:09 ` Mykyta Yatsenko 2025-04-09 15:44 ` Ihor Solodrai 2025-04-09 23:14 ` Andrii Nakryiko 1 sibling, 1 reply; 6+ messages in thread From: Mykyta Yatsenko @ 2025-04-09 12:09 UTC (permalink / raw) To: Ihor Solodrai, andrii, ast, daniel, eddyz87; +Cc: bpf, mykolal, kernel-team On 08/04/2025 19:41, Ihor Solodrai wrote: > A valid ELF file may contain a SHT_NOBITS .BTF section. This case is > not handled correctly in btf_parse_elf, which leads to a segfault. > > Add a null check for a buffer returned by elf_getdata() before > proceeding with its processing. > > Bug report: https://github.com/libbpf/libbpf/issues/894 > > Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> > --- > tools/lib/bpf/btf.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > index 38bc6b14b066..90599f0311bd 100644 > --- a/tools/lib/bpf/btf.c > +++ b/tools/lib/bpf/btf.c > @@ -1201,6 +1201,12 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, > goto done; > } > > + if (!secs.btf_data->d_buf) { > + pr_warn("BTF data is empty in %s\n", path); > + err = -ENODATA; > + goto done; > + } > + > if (secs.btf_base_data) { > dist_base_btf = btf_new(secs.btf_base_data->d_buf, secs.btf_base_data->d_size, > NULL); is `secs.btf_data->d_size` non-zero in this case, making it access `secs.btf_data->d_buf`? Acked-by: Mykyta Yatsenko <mykyta.yatsenko5@gmail.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libbpf: check for empty BTF data section in btf_parse_elf 2025-04-09 12:09 ` Mykyta Yatsenko @ 2025-04-09 15:44 ` Ihor Solodrai 0 siblings, 0 replies; 6+ messages in thread From: Ihor Solodrai @ 2025-04-09 15:44 UTC (permalink / raw) To: Mykyta Yatsenko, andrii, ast, daniel, eddyz87; +Cc: bpf, mykolal, kernel-team On 4/9/25 5:09 AM, Mykyta Yatsenko wrote: > On 08/04/2025 19:41, Ihor Solodrai wrote: >> A valid ELF file may contain a SHT_NOBITS .BTF section. This case is >> not handled correctly in btf_parse_elf, which leads to a segfault. >> >> Add a null check for a buffer returned by elf_getdata() before >> proceeding with its processing. >> >> Bug report: https://github.com/libbpf/libbpf/issues/894 >> >> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> >> --- >> tools/lib/bpf/btf.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c >> index 38bc6b14b066..90599f0311bd 100644 >> --- a/tools/lib/bpf/btf.c >> +++ b/tools/lib/bpf/btf.c >> @@ -1201,6 +1201,12 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, >> goto done; >> } >> + if (!secs.btf_data->d_buf) { >> + pr_warn("BTF data is empty in %s\n", path); >> + err = -ENODATA; >> + goto done; >> + } >> + >> if (secs.btf_base_data) { >> dist_base_btf = btf_new(secs.btf_base_data->d_buf, secs.btf_base_data->d_size, >> NULL); > > is `secs.btf_data->d_size` non-zero in this case, making it access `secs.btf_data->d_buf`? Yes, as it turns out. This is also described in the spec in relation to Elf32_Shdr [1]: sh_size This member gives the section's size in bytes. Unless the section type is SHT_NOBITS, the section occupies sh_size bytes in the file. A section of type SHT_NOBITS may have a non-zero size, but it occupies no space in the file. [1] https://refspecs.linuxfoundation.org/elf/elf.pdf > > Acked-by: Mykyta Yatsenko <mykyta.yatsenko5@gmail.com> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libbpf: check for empty BTF data section in btf_parse_elf 2025-04-08 18:41 [PATCH] libbpf: check for empty BTF data section in btf_parse_elf Ihor Solodrai 2025-04-09 12:09 ` Mykyta Yatsenko @ 2025-04-09 23:14 ` Andrii Nakryiko 2025-04-10 17:34 ` Ihor Solodrai 1 sibling, 1 reply; 6+ messages in thread From: Andrii Nakryiko @ 2025-04-09 23:14 UTC (permalink / raw) To: Ihor Solodrai; +Cc: andrii, ast, daniel, eddyz87, bpf, mykolal, kernel-team On Tue, Apr 8, 2025 at 11:41 AM Ihor Solodrai <ihor.solodrai@linux.dev> wrote: > > A valid ELF file may contain a SHT_NOBITS .BTF section. This case is > not handled correctly in btf_parse_elf, which leads to a segfault. > > Add a null check for a buffer returned by elf_getdata() before > proceeding with its processing. > > Bug report: https://github.com/libbpf/libbpf/issues/894 > > Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> > --- > tools/lib/bpf/btf.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > index 38bc6b14b066..90599f0311bd 100644 > --- a/tools/lib/bpf/btf.c > +++ b/tools/lib/bpf/btf.c > @@ -1201,6 +1201,12 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, > goto done; > } > > + if (!secs.btf_data->d_buf) { > + pr_warn("BTF data is empty in %s\n", path); > + err = -ENODATA; > + goto done; > + } > + let's handle this more generally for all BTF data sections in btf_find_elf_sections()? let's also use similar style of warning messaging to others, maybe something like "failed to get section(%s, %d) data from %s\n" ? pw-bot: cr > if (secs.btf_base_data) { > dist_base_btf = btf_new(secs.btf_base_data->d_buf, secs.btf_base_data->d_size, > NULL); > -- > 2.49.0 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libbpf: check for empty BTF data section in btf_parse_elf 2025-04-09 23:14 ` Andrii Nakryiko @ 2025-04-10 17:34 ` Ihor Solodrai 2025-04-10 17:39 ` Andrii Nakryiko 0 siblings, 1 reply; 6+ messages in thread From: Ihor Solodrai @ 2025-04-10 17:34 UTC (permalink / raw) To: Andrii Nakryiko; +Cc: andrii, ast, daniel, eddyz87, bpf, mykolal, kernel-team On 4/9/25 4:14 PM, Andrii Nakryiko wrote: > On Tue, Apr 8, 2025 at 11:41 AM Ihor Solodrai <ihor.solodrai@linux.dev> wrote: >> >> A valid ELF file may contain a SHT_NOBITS .BTF section. This case is >> not handled correctly in btf_parse_elf, which leads to a segfault. >> >> Add a null check for a buffer returned by elf_getdata() before >> proceeding with its processing. >> >> Bug report: https://github.com/libbpf/libbpf/issues/894 >> >> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> >> --- >> tools/lib/bpf/btf.c | 6 ++++++ >> 1 file changed, 6 insertions(+) >> >> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c >> index 38bc6b14b066..90599f0311bd 100644 >> --- a/tools/lib/bpf/btf.c >> +++ b/tools/lib/bpf/btf.c >> @@ -1201,6 +1201,12 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, >> goto done; >> } >> >> + if (!secs.btf_data->d_buf) { >> + pr_warn("BTF data is empty in %s\n", path); >> + err = -ENODATA; >> + goto done; >> + } >> + > > let's handle this more generally for all BTF data sections in > btf_find_elf_sections()? Sure. I think it makes sense to check for the section type before attempting to load a buffer like this: @@ -1148,6 +1148,12 @@ static int btf_find_elf_sections(Elf *elf, const char *path, struct btf_elf_secs else continue; + if (sh.sh_type == SHT_NOBITS) { + pr_warn("failed to get section(%d, %s) data from %s\n", + idx, name, path); + goto err; + } + But then we might as well test for the expected section type, which is supposed to be SHT_PROGBITS, if I understand correctly. What I don't know is whether this is *the only* possible expected type (for ".BTF", ".BTF.ext" and ".BTF.base"), or are there others? Andrii, do you know if that's the case? > > let's also use similar style of warning messaging to others, maybe > something like > > "failed to get section(%s, %d) data from %s\n" ? > > > pw-bot: cr > >> if (secs.btf_base_data) { >> dist_base_btf = btf_new(secs.btf_base_data->d_buf, secs.btf_base_data->d_size, >> NULL); >> -- >> 2.49.0 >> >> ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] libbpf: check for empty BTF data section in btf_parse_elf 2025-04-10 17:34 ` Ihor Solodrai @ 2025-04-10 17:39 ` Andrii Nakryiko 0 siblings, 0 replies; 6+ messages in thread From: Andrii Nakryiko @ 2025-04-10 17:39 UTC (permalink / raw) To: Ihor Solodrai; +Cc: andrii, ast, daniel, eddyz87, bpf, mykolal, kernel-team On Thu, Apr 10, 2025 at 10:34 AM Ihor Solodrai <ihor.solodrai@linux.dev> wrote: > > On 4/9/25 4:14 PM, Andrii Nakryiko wrote: > > On Tue, Apr 8, 2025 at 11:41 AM Ihor Solodrai <ihor.solodrai@linux.dev> wrote: > >> > >> A valid ELF file may contain a SHT_NOBITS .BTF section. This case is > >> not handled correctly in btf_parse_elf, which leads to a segfault. > >> > >> Add a null check for a buffer returned by elf_getdata() before > >> proceeding with its processing. > >> > >> Bug report: https://github.com/libbpf/libbpf/issues/894 > >> > >> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev> > >> --- > >> tools/lib/bpf/btf.c | 6 ++++++ > >> 1 file changed, 6 insertions(+) > >> > >> diff --git a/tools/lib/bpf/btf.c b/tools/lib/bpf/btf.c > >> index 38bc6b14b066..90599f0311bd 100644 > >> --- a/tools/lib/bpf/btf.c > >> +++ b/tools/lib/bpf/btf.c > >> @@ -1201,6 +1201,12 @@ static struct btf *btf_parse_elf(const char *path, struct btf *base_btf, > >> goto done; > >> } > >> > >> + if (!secs.btf_data->d_buf) { > >> + pr_warn("BTF data is empty in %s\n", path); > >> + err = -ENODATA; > >> + goto done; > >> + } > >> + > > > > let's handle this more generally for all BTF data sections in > > btf_find_elf_sections()? > > Sure. I think it makes sense to check for the section type before > attempting to load a buffer like this: > > @@ -1148,6 +1148,12 @@ static int btf_find_elf_sections(Elf *elf, const char *path, struct btf_elf_secs > else > continue; > > + if (sh.sh_type == SHT_NOBITS) { > + pr_warn("failed to get section(%d, %s) data from %s\n", > + idx, name, path); > + goto err; > + } > + > > But then we might as well test for the expected section type, which is > supposed to be SHT_PROGBITS, if I understand correctly. > > What I don't know is whether this is *the only* possible expected type > (for ".BTF", ".BTF.ext" and ".BTF.base"), or are there others? > > Andrii, do you know if that's the case? yeah, I think it has to be SHT_PROGBITS, everything else is either zero-initialized section (SHT_NOBITS), which is useless for BTF data. Or it's some other special type of section. > > > > > let's also use similar style of warning messaging to others, maybe > > something like > > > > "failed to get section(%s, %d) data from %s\n" ? > > > > > > pw-bot: cr > > > >> if (secs.btf_base_data) { > >> dist_base_btf = btf_new(secs.btf_base_data->d_buf, secs.btf_base_data->d_size, > >> NULL); > >> -- > >> 2.49.0 > >> > >> ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-04-10 17:40 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-04-08 18:41 [PATCH] libbpf: check for empty BTF data section in btf_parse_elf Ihor Solodrai 2025-04-09 12:09 ` Mykyta Yatsenko 2025-04-09 15:44 ` Ihor Solodrai 2025-04-09 23:14 ` Andrii Nakryiko 2025-04-10 17:34 ` Ihor Solodrai 2025-04-10 17:39 ` Andrii Nakryiko
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox