[PATCH] libbpf: check for empty BTF data section in btf_parse_elf

[PATCH] libbpf: check for empty BTF data section in btf_parse_elf

[Ihor Solodrai]

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

Re: [PATCH] libbpf: check for empty BTF data section in btf_parse_elf

[Mykyta Yatsenko]

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>

Re: [PATCH] libbpf: check for empty BTF data section in btf_parse_elf

[Ihor Solodrai]

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>

Re: [PATCH] libbpf: check for empty BTF data section in btf_parse_elf

[Andrii Nakryiko]

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
>
>

Re: [PATCH] libbpf: check for empty BTF data section in btf_parse_elf

[Ihor Solodrai]

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
>>
>>

Re: [PATCH] libbpf: check for empty BTF data section in btf_parse_elf

[Andrii Nakryiko]

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
> >>
> >>