On Thu, Apr 10, 2025 at 10:34 AM Ihor Solodrai <ihor.solodrai@xxxxxxxxx> wrote: > > On 4/9/25 4:14 PM, Andrii Nakryiko wrote: > > On Tue, Apr 8, 2025 at 11:41 AM Ihor Solodrai <ihor.solodrai@xxxxxxxxx> 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@xxxxxxxxx> > >> --- > >> 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 > >> > >>
