stable.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf v2] libbpf: Fix buffer overflow in bpf_object__init_prog
@ 2025-04-10  9:55 Viktor Malik
  2025-04-11 16:22 ` Andrii Nakryiko
  0 siblings, 1 reply; 7+ messages in thread
From: Viktor Malik @ 2025-04-10  9:55 UTC (permalink / raw)
  To: bpf
  Cc: Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	Viktor Malik, lmarch2, stable, Shung-Hsi Yu

As reported by CVE-2025-29481 [1], it is possible to corrupt a BPF ELF
file such that arbitrary BPF instructions are loaded by libbpf. This can
be done by setting a symbol (BPF program) section offset to a large
(unsigned) number such that <section start + symbol offset> overflows
and points before the section data in the memory.

Consider the situation below where:
- prog_start = sec_start + symbol_offset    <-- size_t overflow here
- prog_end   = prog_start + prog_size

    prog_start        sec_start        prog_end        sec_end
        |                |                 |              |
        v                v                 v              v
    .....................|################################|............

The CVE report in [1] also provides a corrupted BPF ELF which can be
used as a reproducer:

    $ readelf -S crash
    Section Headers:
      [Nr] Name              Type             Address           Offset
           Size              EntSize          Flags  Link  Info  Align
    ...
      [ 2] uretprobe.mu[...] PROGBITS         0000000000000000  00000040
           0000000000000068  0000000000000000  AX       0     0     8

    $ readelf -s crash
    Symbol table '.symtab' contains 8 entries:
       Num:    Value          Size Type    Bind   Vis      Ndx Name
    ...
         6: ffffffffffffffb8   104 FUNC    GLOBAL DEFAULT    2 handle_tp

Here, the handle_tp prog has section offset ffffffffffffffb8, i.e. will
point before the actual memory where section 2 is allocated.

This is also reported by AddressSanitizer:

    =================================================================
    ==1232==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7c7302fe0000 at pc 0x7fc3046e4b77 bp 0x7ffe64677cd0 sp 0x7ffe64677490
    READ of size 104 at 0x7c7302fe0000 thread T0
        #0 0x7fc3046e4b76 in memcpy (/lib64/libasan.so.8+0xe4b76)
        #1 0x00000040df3e in bpf_object__init_prog /src/libbpf/src/libbpf.c:856
        #2 0x00000040df3e in bpf_object__add_programs /src/libbpf/src/libbpf.c:928
        #3 0x00000040df3e in bpf_object__elf_collect /src/libbpf/src/libbpf.c:3930
        #4 0x00000040df3e in bpf_object_open /src/libbpf/src/libbpf.c:8067
        #5 0x00000040f176 in bpf_object__open_file /src/libbpf/src/libbpf.c:8090
        #6 0x000000400c16 in main /poc/poc.c:8
        #7 0x7fc3043d25b4 in __libc_start_call_main (/lib64/libc.so.6+0x35b4)
        #8 0x7fc3043d2667 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x3667)
        #9 0x000000400b34 in _start (/poc/poc+0x400b34)

    0x7c7302fe0000 is located 64 bytes before 104-byte region [0x7c7302fe0040,0x7c7302fe00a8)
    allocated by thread T0 here:
        #0 0x7fc3046e716b in malloc (/lib64/libasan.so.8+0xe716b)
        #1 0x7fc3045ee600 in __libelf_set_rawdata_wrlock (/lib64/libelf.so.1+0xb600)
        #2 0x7fc3045ef018 in __elf_getdata_rdlock (/lib64/libelf.so.1+0xc018)
        #3 0x00000040642f in elf_sec_data /src/libbpf/src/libbpf.c:3740

The problem here is that currently, libbpf only checks that the program
end is within the section bounds. There used to be a check
`while (sec_off < sec_sz)` in bpf_object__add_programs, however, it was
removed by commit 6245947c1b3c ("libbpf: Allow gaps in BPF program
sections to support overriden weak functions").

Put the above condition back to bpf_object__init_prog to make sure that
the program start is also within the bounds of the section to avoid the
potential buffer overflow.

[1] https://github.com/lmarch2/poc/blob/main/libbpf/libbpf.md

Reported-by: lmarch2 <2524158037@qq.com>
Cc: stable@vger.kernel.org
Fixes: 6245947c1b3c ("libbpf: Allow gaps in BPF program sections to support overriden weak functions")
Link: https://github.com/lmarch2/poc/blob/main/libbpf/libbpf.md
Link: https://www.cve.org/CVERecord?id=CVE-2025-29481
Signed-off-by: Viktor Malik <vmalik@redhat.com>
Reviewed-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
---
 tools/lib/bpf/libbpf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 6b85060f07b3..d0ece3c9618e 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -896,7 +896,7 @@ bpf_object__add_programs(struct bpf_object *obj, Elf_Data *sec_data,
 			return -LIBBPF_ERRNO__FORMAT;
 		}
 
-		if (sec_off + prog_sz > sec_sz) {
+		if (sec_off >= sec_sz || sec_off + prog_sz > sec_sz) {
 			pr_warn("sec '%s': program at offset %zu crosses section boundary\n",
 				sec_name, sec_off);
 			return -LIBBPF_ERRNO__FORMAT;
-- 
2.49.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf v2] libbpf: Fix buffer overflow in bpf_object__init_prog
  2025-04-10  9:55 [PATCH bpf v2] libbpf: Fix buffer overflow in bpf_object__init_prog Viktor Malik
@ 2025-04-11 16:22 ` Andrii Nakryiko
  2025-04-12  6:24   ` Greg KH
  2025-04-14  4:59   ` Viktor Malik
  0 siblings, 2 replies; 7+ messages in thread
From: Andrii Nakryiko @ 2025-04-11 16:22 UTC (permalink / raw)
  To: Viktor Malik
  Cc: bpf, Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	lmarch2, stable, Shung-Hsi Yu

On Thu, Apr 10, 2025 at 2:55 AM Viktor Malik <vmalik@redhat.com> wrote:
>
> As reported by CVE-2025-29481 [1], it is possible to corrupt a BPF ELF
> file such that arbitrary BPF instructions are loaded by libbpf. This can
> be done by setting a symbol (BPF program) section offset to a large
> (unsigned) number such that <section start + symbol offset> overflows
> and points before the section data in the memory.
>
> Consider the situation below where:
> - prog_start = sec_start + symbol_offset    <-- size_t overflow here
> - prog_end   = prog_start + prog_size
>
>     prog_start        sec_start        prog_end        sec_end
>         |                |                 |              |
>         v                v                 v              v
>     .....................|################################|............
>
> The CVE report in [1] also provides a corrupted BPF ELF which can be
> used as a reproducer:
>
>     $ readelf -S crash
>     Section Headers:
>       [Nr] Name              Type             Address           Offset
>            Size              EntSize          Flags  Link  Info  Align
>     ...
>       [ 2] uretprobe.mu[...] PROGBITS         0000000000000000  00000040
>            0000000000000068  0000000000000000  AX       0     0     8
>
>     $ readelf -s crash
>     Symbol table '.symtab' contains 8 entries:
>        Num:    Value          Size Type    Bind   Vis      Ndx Name
>     ...
>          6: ffffffffffffffb8   104 FUNC    GLOBAL DEFAULT    2 handle_tp
>
> Here, the handle_tp prog has section offset ffffffffffffffb8, i.e. will
> point before the actual memory where section 2 is allocated.
>
> This is also reported by AddressSanitizer:
>
>     =================================================================
>     ==1232==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7c7302fe0000 at pc 0x7fc3046e4b77 bp 0x7ffe64677cd0 sp 0x7ffe64677490
>     READ of size 104 at 0x7c7302fe0000 thread T0
>         #0 0x7fc3046e4b76 in memcpy (/lib64/libasan.so.8+0xe4b76)
>         #1 0x00000040df3e in bpf_object__init_prog /src/libbpf/src/libbpf.c:856
>         #2 0x00000040df3e in bpf_object__add_programs /src/libbpf/src/libbpf.c:928
>         #3 0x00000040df3e in bpf_object__elf_collect /src/libbpf/src/libbpf.c:3930
>         #4 0x00000040df3e in bpf_object_open /src/libbpf/src/libbpf.c:8067
>         #5 0x00000040f176 in bpf_object__open_file /src/libbpf/src/libbpf.c:8090
>         #6 0x000000400c16 in main /poc/poc.c:8
>         #7 0x7fc3043d25b4 in __libc_start_call_main (/lib64/libc.so.6+0x35b4)
>         #8 0x7fc3043d2667 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x3667)
>         #9 0x000000400b34 in _start (/poc/poc+0x400b34)
>
>     0x7c7302fe0000 is located 64 bytes before 104-byte region [0x7c7302fe0040,0x7c7302fe00a8)
>     allocated by thread T0 here:
>         #0 0x7fc3046e716b in malloc (/lib64/libasan.so.8+0xe716b)
>         #1 0x7fc3045ee600 in __libelf_set_rawdata_wrlock (/lib64/libelf.so.1+0xb600)
>         #2 0x7fc3045ef018 in __elf_getdata_rdlock (/lib64/libelf.so.1+0xc018)
>         #3 0x00000040642f in elf_sec_data /src/libbpf/src/libbpf.c:3740
>
> The problem here is that currently, libbpf only checks that the program
> end is within the section bounds. There used to be a check
> `while (sec_off < sec_sz)` in bpf_object__add_programs, however, it was
> removed by commit 6245947c1b3c ("libbpf: Allow gaps in BPF program
> sections to support overriden weak functions").
>
> Put the above condition back to bpf_object__init_prog to make sure that
> the program start is also within the bounds of the section to avoid the
> potential buffer overflow.
>
> [1] https://github.com/lmarch2/poc/blob/main/libbpf/libbpf.md
>
> Reported-by: lmarch2 <2524158037@qq.com>
> Cc: stable@vger.kernel.org

Libbpf is packaged and consumed from Github mirror, which is produced
from latest bpf-next and bpf trees, so there is no point in
backporting fixes like this to stable kernel branches. Please drop the
CC: stable in the next revision.

> Fixes: 6245947c1b3c ("libbpf: Allow gaps in BPF program sections to support overriden weak functions")
> Link: https://github.com/lmarch2/poc/blob/main/libbpf/libbpf.md
> Link: https://www.cve.org/CVERecord?id=CVE-2025-29481

libbpf is meant to load BPF programs under root. It's a
highly-privileged operation, and libbpf is not meant, designed, and
actually explicitly discouraged from loading untrusted ELF files. As
such, this is just a normal bug fix, like lots of others. So let's
drop the CVE link as well.

Again, no one in their sane mind should be passing untrusted ELF files
into libbpf while running under root. Period.

All production use cases load ELF that they generated and control
(usually embedded into their memory through BPF skeleton header). And
if that ELF file is corrupted, you have problems somewhere else,
libbpf is not a culprit.

> Signed-off-by: Viktor Malik <vmalik@redhat.com>
> Reviewed-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
> ---
>  tools/lib/bpf/libbpf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
> index 6b85060f07b3..d0ece3c9618e 100644
> --- a/tools/lib/bpf/libbpf.c
> +++ b/tools/lib/bpf/libbpf.c
> @@ -896,7 +896,7 @@ bpf_object__add_programs(struct bpf_object *obj, Elf_Data *sec_data,
>                         return -LIBBPF_ERRNO__FORMAT;
>                 }
>
> -               if (sec_off + prog_sz > sec_sz) {
> +               if (sec_off >= sec_sz || sec_off + prog_sz > sec_sz) {

So the thing we are protecting against is that sec_off + prog_sz can
overflow and turn out to be < sec_sz (even though the sum is actually
bigger), right?

If my understanding is correct, then I'd find it much more obviously
expressed as:


if (sec_off + prog_sz > sec_sz || sec_off + prog_sz < sec_off)

We have such an overflow detection checking pattern used in a few
places already, I believe. WDYT?

pw-bot: cr

>                         pr_warn("sec '%s': program at offset %zu crosses section boundary\n",
>                                 sec_name, sec_off);
>                         return -LIBBPF_ERRNO__FORMAT;
> --
> 2.49.0
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf v2] libbpf: Fix buffer overflow in bpf_object__init_prog
  2025-04-11 16:22 ` Andrii Nakryiko
@ 2025-04-12  6:24   ` Greg KH
  2025-04-14  5:05     ` Viktor Malik
  2025-04-14  4:59   ` Viktor Malik
  1 sibling, 1 reply; 7+ messages in thread
From: Greg KH @ 2025-04-12  6:24 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: Viktor Malik, bpf, Andrii Nakryiko, Eduard Zingerman,
	Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, lmarch2, stable, Shung-Hsi Yu

On Fri, Apr 11, 2025 at 09:22:37AM -0700, Andrii Nakryiko wrote:
> On Thu, Apr 10, 2025 at 2:55 AM Viktor Malik <vmalik@redhat.com> wrote:
> >
> > As reported by CVE-2025-29481 [1], it is possible to corrupt a BPF ELF
> > file such that arbitrary BPF instructions are loaded by libbpf. This can
> > be done by setting a symbol (BPF program) section offset to a large
> > (unsigned) number such that <section start + symbol offset> overflows
> > and points before the section data in the memory.
> >
> > Consider the situation below where:
> > - prog_start = sec_start + symbol_offset    <-- size_t overflow here
> > - prog_end   = prog_start + prog_size
> >
> >     prog_start        sec_start        prog_end        sec_end
> >         |                |                 |              |
> >         v                v                 v              v
> >     .....................|################################|............
> >
> > The CVE report in [1] also provides a corrupted BPF ELF which can be
> > used as a reproducer:
> >
> >     $ readelf -S crash
> >     Section Headers:
> >       [Nr] Name              Type             Address           Offset
> >            Size              EntSize          Flags  Link  Info  Align
> >     ...
> >       [ 2] uretprobe.mu[...] PROGBITS         0000000000000000  00000040
> >            0000000000000068  0000000000000000  AX       0     0     8
> >
> >     $ readelf -s crash
> >     Symbol table '.symtab' contains 8 entries:
> >        Num:    Value          Size Type    Bind   Vis      Ndx Name
> >     ...
> >          6: ffffffffffffffb8   104 FUNC    GLOBAL DEFAULT    2 handle_tp
> >
> > Here, the handle_tp prog has section offset ffffffffffffffb8, i.e. will
> > point before the actual memory where section 2 is allocated.
> >
> > This is also reported by AddressSanitizer:
> >
> >     =================================================================
> >     ==1232==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7c7302fe0000 at pc 0x7fc3046e4b77 bp 0x7ffe64677cd0 sp 0x7ffe64677490
> >     READ of size 104 at 0x7c7302fe0000 thread T0
> >         #0 0x7fc3046e4b76 in memcpy (/lib64/libasan.so.8+0xe4b76)
> >         #1 0x00000040df3e in bpf_object__init_prog /src/libbpf/src/libbpf.c:856
> >         #2 0x00000040df3e in bpf_object__add_programs /src/libbpf/src/libbpf.c:928
> >         #3 0x00000040df3e in bpf_object__elf_collect /src/libbpf/src/libbpf.c:3930
> >         #4 0x00000040df3e in bpf_object_open /src/libbpf/src/libbpf.c:8067
> >         #5 0x00000040f176 in bpf_object__open_file /src/libbpf/src/libbpf.c:8090
> >         #6 0x000000400c16 in main /poc/poc.c:8
> >         #7 0x7fc3043d25b4 in __libc_start_call_main (/lib64/libc.so.6+0x35b4)
> >         #8 0x7fc3043d2667 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x3667)
> >         #9 0x000000400b34 in _start (/poc/poc+0x400b34)
> >
> >     0x7c7302fe0000 is located 64 bytes before 104-byte region [0x7c7302fe0040,0x7c7302fe00a8)
> >     allocated by thread T0 here:
> >         #0 0x7fc3046e716b in malloc (/lib64/libasan.so.8+0xe716b)
> >         #1 0x7fc3045ee600 in __libelf_set_rawdata_wrlock (/lib64/libelf.so.1+0xb600)
> >         #2 0x7fc3045ef018 in __elf_getdata_rdlock (/lib64/libelf.so.1+0xc018)
> >         #3 0x00000040642f in elf_sec_data /src/libbpf/src/libbpf.c:3740
> >
> > The problem here is that currently, libbpf only checks that the program
> > end is within the section bounds. There used to be a check
> > `while (sec_off < sec_sz)` in bpf_object__add_programs, however, it was
> > removed by commit 6245947c1b3c ("libbpf: Allow gaps in BPF program
> > sections to support overriden weak functions").
> >
> > Put the above condition back to bpf_object__init_prog to make sure that
> > the program start is also within the bounds of the section to avoid the
> > potential buffer overflow.
> >
> > [1] https://github.com/lmarch2/poc/blob/main/libbpf/libbpf.md
> >
> > Reported-by: lmarch2 <2524158037@qq.com>
> > Cc: stable@vger.kernel.org
> 
> Libbpf is packaged and consumed from Github mirror, which is produced
> from latest bpf-next and bpf trees, so there is no point in
> backporting fixes like this to stable kernel branches. Please drop the
> CC: stable in the next revision.
> 
> > Fixes: 6245947c1b3c ("libbpf: Allow gaps in BPF program sections to support overriden weak functions")
> > Link: https://github.com/lmarch2/poc/blob/main/libbpf/libbpf.md
> > Link: https://www.cve.org/CVERecord?id=CVE-2025-29481
> 
> libbpf is meant to load BPF programs under root. It's a
> highly-privileged operation, and libbpf is not meant, designed, and
> actually explicitly discouraged from loading untrusted ELF files. As
> such, this is just a normal bug fix, like lots of others. So let's
> drop the CVE link as well.
> 
> Again, no one in their sane mind should be passing untrusted ELF files
> into libbpf while running under root. Period.
> 
> All production use cases load ELF that they generated and control
> (usually embedded into their memory through BPF skeleton header). And
> if that ELF file is corrupted, you have problems somewhere else,
> libbpf is not a culprit.

Should that context-less CVE be revoked as well?  Who asked for it to be
issued?

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf v2] libbpf: Fix buffer overflow in bpf_object__init_prog
  2025-04-11 16:22 ` Andrii Nakryiko
  2025-04-12  6:24   ` Greg KH
@ 2025-04-14  4:59   ` Viktor Malik
  2025-04-15  9:30     ` Shung-Hsi Yu
  1 sibling, 1 reply; 7+ messages in thread
From: Viktor Malik @ 2025-04-14  4:59 UTC (permalink / raw)
  To: Andrii Nakryiko
  Cc: bpf, Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	lmarch2, stable, Shung-Hsi Yu

On 4/11/25 18:22, Andrii Nakryiko wrote:
> On Thu, Apr 10, 2025 at 2:55 AM Viktor Malik <vmalik@redhat.com> wrote:
>>
>> As reported by CVE-2025-29481 [1], it is possible to corrupt a BPF ELF
>> file such that arbitrary BPF instructions are loaded by libbpf. This can
>> be done by setting a symbol (BPF program) section offset to a large
>> (unsigned) number such that <section start + symbol offset> overflows
>> and points before the section data in the memory.
>>
>> Consider the situation below where:
>> - prog_start = sec_start + symbol_offset    <-- size_t overflow here
>> - prog_end   = prog_start + prog_size
>>
>>     prog_start        sec_start        prog_end        sec_end
>>         |                |                 |              |
>>         v                v                 v              v
>>     .....................|################################|............
>>
>> The CVE report in [1] also provides a corrupted BPF ELF which can be
>> used as a reproducer:
>>
>>     $ readelf -S crash
>>     Section Headers:
>>       [Nr] Name              Type             Address           Offset
>>            Size              EntSize          Flags  Link  Info  Align
>>     ...
>>       [ 2] uretprobe.mu[...] PROGBITS         0000000000000000  00000040
>>            0000000000000068  0000000000000000  AX       0     0     8
>>
>>     $ readelf -s crash
>>     Symbol table '.symtab' contains 8 entries:
>>        Num:    Value          Size Type    Bind   Vis      Ndx Name
>>     ...
>>          6: ffffffffffffffb8   104 FUNC    GLOBAL DEFAULT    2 handle_tp
>>
>> Here, the handle_tp prog has section offset ffffffffffffffb8, i.e. will
>> point before the actual memory where section 2 is allocated.
>>
>> This is also reported by AddressSanitizer:
>>
>>     =================================================================
>>     ==1232==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7c7302fe0000 at pc 0x7fc3046e4b77 bp 0x7ffe64677cd0 sp 0x7ffe64677490
>>     READ of size 104 at 0x7c7302fe0000 thread T0
>>         #0 0x7fc3046e4b76 in memcpy (/lib64/libasan.so.8+0xe4b76)
>>         #1 0x00000040df3e in bpf_object__init_prog /src/libbpf/src/libbpf.c:856
>>         #2 0x00000040df3e in bpf_object__add_programs /src/libbpf/src/libbpf.c:928
>>         #3 0x00000040df3e in bpf_object__elf_collect /src/libbpf/src/libbpf.c:3930
>>         #4 0x00000040df3e in bpf_object_open /src/libbpf/src/libbpf.c:8067
>>         #5 0x00000040f176 in bpf_object__open_file /src/libbpf/src/libbpf.c:8090
>>         #6 0x000000400c16 in main /poc/poc.c:8
>>         #7 0x7fc3043d25b4 in __libc_start_call_main (/lib64/libc.so.6+0x35b4)
>>         #8 0x7fc3043d2667 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x3667)
>>         #9 0x000000400b34 in _start (/poc/poc+0x400b34)
>>
>>     0x7c7302fe0000 is located 64 bytes before 104-byte region [0x7c7302fe0040,0x7c7302fe00a8)
>>     allocated by thread T0 here:
>>         #0 0x7fc3046e716b in malloc (/lib64/libasan.so.8+0xe716b)
>>         #1 0x7fc3045ee600 in __libelf_set_rawdata_wrlock (/lib64/libelf.so.1+0xb600)
>>         #2 0x7fc3045ef018 in __elf_getdata_rdlock (/lib64/libelf.so.1+0xc018)
>>         #3 0x00000040642f in elf_sec_data /src/libbpf/src/libbpf.c:3740
>>
>> The problem here is that currently, libbpf only checks that the program
>> end is within the section bounds. There used to be a check
>> `while (sec_off < sec_sz)` in bpf_object__add_programs, however, it was
>> removed by commit 6245947c1b3c ("libbpf: Allow gaps in BPF program
>> sections to support overriden weak functions").
>>
>> Put the above condition back to bpf_object__init_prog to make sure that
>> the program start is also within the bounds of the section to avoid the
>> potential buffer overflow.
>>
>> [1] https://github.com/lmarch2/poc/blob/main/libbpf/libbpf.md
>>
>> Reported-by: lmarch2 <2524158037@qq.com>
>> Cc: stable@vger.kernel.org
> 
> Libbpf is packaged and consumed from Github mirror, which is produced
> from latest bpf-next and bpf trees, so there is no point in
> backporting fixes like this to stable kernel branches. Please drop the
> CC: stable in the next revision.

Ack, will drop it.

> 
>> Fixes: 6245947c1b3c ("libbpf: Allow gaps in BPF program sections to support overriden weak functions")
>> Link: https://github.com/lmarch2/poc/blob/main/libbpf/libbpf.md
>> Link: https://www.cve.org/CVERecord?id=CVE-2025-29481
> 
> libbpf is meant to load BPF programs under root. It's a
> highly-privileged operation, and libbpf is not meant, designed, and
> actually explicitly discouraged from loading untrusted ELF files. As
> such, this is just a normal bug fix, like lots of others. So let's
> drop the CVE link as well.
> 
> Again, no one in their sane mind should be passing untrusted ELF files
> into libbpf while running under root. Period.
> 
> All production use cases load ELF that they generated and control
> (usually embedded into their memory through BPF skeleton header). And
> if that ELF file is corrupted, you have problems somewhere else,
> libbpf is not a culprit.

While I couldn't agree more, I'm a bit on the fence here. On one hand,
unless the CVE is revoked (see the other thread), people may still run
across it and, without sufficient knowledge of libbpf, think that they
are vulnerable. Having a CVE reference in the patch could help them
recognize that they are using a patched version of libbpf or at least
read an explanation why the vulnerability is not real.

On the other hand, since it's just a bug, I agree that it doesn't make
much sense to reference a CVE from it. So, I'm ok both ways. I can
reference the CVE and provide some better explanation why this should
not be considered a vulnerability.

>> Signed-off-by: Viktor Malik <vmalik@redhat.com>
>> Reviewed-by: Shung-Hsi Yu <shung-hsi.yu@suse.com>
>> ---
>>  tools/lib/bpf/libbpf.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 6b85060f07b3..d0ece3c9618e 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -896,7 +896,7 @@ bpf_object__add_programs(struct bpf_object *obj, Elf_Data *sec_data,
>>                         return -LIBBPF_ERRNO__FORMAT;
>>                 }
>>
>> -               if (sec_off + prog_sz > sec_sz) {
>> +               if (sec_off >= sec_sz || sec_off + prog_sz > sec_sz) {
> 
> So the thing we are protecting against is that sec_off + prog_sz can
> overflow and turn out to be < sec_sz (even though the sum is actually
> bigger), right?
> 
> If my understanding is correct, then I'd find it much more obviously
> expressed as:
> 
> 
> if (sec_off + prog_sz > sec_sz || sec_off + prog_sz < sec_off)
> 
> We have such an overflow detection checking pattern used in a few
> places already, I believe. WDYT?

Sure, since we're dealing with unsigned numbers, the above is an
equivalent condition. And you're right that it better expresses the
intent so let me use it.

Thanks!
Viktor

> 
> pw-bot: cr
> 
>>                         pr_warn("sec '%s': program at offset %zu crosses section boundary\n",
>>                                 sec_name, sec_off);
>>                         return -LIBBPF_ERRNO__FORMAT;
>> --
>> 2.49.0
>>
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf v2] libbpf: Fix buffer overflow in bpf_object__init_prog
  2025-04-12  6:24   ` Greg KH
@ 2025-04-14  5:05     ` Viktor Malik
  0 siblings, 0 replies; 7+ messages in thread
From: Viktor Malik @ 2025-04-14  5:05 UTC (permalink / raw)
  To: Greg KH, Andrii Nakryiko
  Cc: bpf, Andrii Nakryiko, Eduard Zingerman, Alexei Starovoitov,
	Daniel Borkmann, Martin KaFai Lau, Song Liu, Yonghong Song,
	John Fastabend, KP Singh, Stanislav Fomichev, Hao Luo, Jiri Olsa,
	lmarch2, stable, Shung-Hsi Yu

On 4/12/25 08:24, Greg KH wrote:
> On Fri, Apr 11, 2025 at 09:22:37AM -0700, Andrii Nakryiko wrote:
>> On Thu, Apr 10, 2025 at 2:55 AM Viktor Malik <vmalik@redhat.com> wrote:
>>>
>>> As reported by CVE-2025-29481 [1], it is possible to corrupt a BPF ELF
>>> file such that arbitrary BPF instructions are loaded by libbpf. This can
>>> be done by setting a symbol (BPF program) section offset to a large
>>> (unsigned) number such that <section start + symbol offset> overflows
>>> and points before the section data in the memory.
>>>
>>> Consider the situation below where:
>>> - prog_start = sec_start + symbol_offset    <-- size_t overflow here
>>> - prog_end   = prog_start + prog_size
>>>
>>>     prog_start        sec_start        prog_end        sec_end
>>>         |                |                 |              |
>>>         v                v                 v              v
>>>     .....................|################################|............
>>>
>>> The CVE report in [1] also provides a corrupted BPF ELF which can be
>>> used as a reproducer:
>>>
>>>     $ readelf -S crash
>>>     Section Headers:
>>>       [Nr] Name              Type             Address           Offset
>>>            Size              EntSize          Flags  Link  Info  Align
>>>     ...
>>>       [ 2] uretprobe.mu[...] PROGBITS         0000000000000000  00000040
>>>            0000000000000068  0000000000000000  AX       0     0     8
>>>
>>>     $ readelf -s crash
>>>     Symbol table '.symtab' contains 8 entries:
>>>        Num:    Value          Size Type    Bind   Vis      Ndx Name
>>>     ...
>>>          6: ffffffffffffffb8   104 FUNC    GLOBAL DEFAULT    2 handle_tp
>>>
>>> Here, the handle_tp prog has section offset ffffffffffffffb8, i.e. will
>>> point before the actual memory where section 2 is allocated.
>>>
>>> This is also reported by AddressSanitizer:
>>>
>>>     =================================================================
>>>     ==1232==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x7c7302fe0000 at pc 0x7fc3046e4b77 bp 0x7ffe64677cd0 sp 0x7ffe64677490
>>>     READ of size 104 at 0x7c7302fe0000 thread T0
>>>         #0 0x7fc3046e4b76 in memcpy (/lib64/libasan.so.8+0xe4b76)
>>>         #1 0x00000040df3e in bpf_object__init_prog /src/libbpf/src/libbpf.c:856
>>>         #2 0x00000040df3e in bpf_object__add_programs /src/libbpf/src/libbpf.c:928
>>>         #3 0x00000040df3e in bpf_object__elf_collect /src/libbpf/src/libbpf.c:3930
>>>         #4 0x00000040df3e in bpf_object_open /src/libbpf/src/libbpf.c:8067
>>>         #5 0x00000040f176 in bpf_object__open_file /src/libbpf/src/libbpf.c:8090
>>>         #6 0x000000400c16 in main /poc/poc.c:8
>>>         #7 0x7fc3043d25b4 in __libc_start_call_main (/lib64/libc.so.6+0x35b4)
>>>         #8 0x7fc3043d2667 in __libc_start_main@@GLIBC_2.34 (/lib64/libc.so.6+0x3667)
>>>         #9 0x000000400b34 in _start (/poc/poc+0x400b34)
>>>
>>>     0x7c7302fe0000 is located 64 bytes before 104-byte region [0x7c7302fe0040,0x7c7302fe00a8)
>>>     allocated by thread T0 here:
>>>         #0 0x7fc3046e716b in malloc (/lib64/libasan.so.8+0xe716b)
>>>         #1 0x7fc3045ee600 in __libelf_set_rawdata_wrlock (/lib64/libelf.so.1+0xb600)
>>>         #2 0x7fc3045ef018 in __elf_getdata_rdlock (/lib64/libelf.so.1+0xc018)
>>>         #3 0x00000040642f in elf_sec_data /src/libbpf/src/libbpf.c:3740
>>>
>>> The problem here is that currently, libbpf only checks that the program
>>> end is within the section bounds. There used to be a check
>>> `while (sec_off < sec_sz)` in bpf_object__add_programs, however, it was
>>> removed by commit 6245947c1b3c ("libbpf: Allow gaps in BPF program
>>> sections to support overriden weak functions").
>>>
>>> Put the above condition back to bpf_object__init_prog to make sure that
>>> the program start is also within the bounds of the section to avoid the
>>> potential buffer overflow.
>>>
>>> [1] https://github.com/lmarch2/poc/blob/main/libbpf/libbpf.md
>>>
>>> Reported-by: lmarch2 <2524158037@qq.com>
>>> Cc: stable@vger.kernel.org
>>
>> Libbpf is packaged and consumed from Github mirror, which is produced
>> from latest bpf-next and bpf trees, so there is no point in
>> backporting fixes like this to stable kernel branches. Please drop the
>> CC: stable in the next revision.
>>
>>> Fixes: 6245947c1b3c ("libbpf: Allow gaps in BPF program sections to support overriden weak functions")
>>> Link: https://github.com/lmarch2/poc/blob/main/libbpf/libbpf.md
>>> Link: https://www.cve.org/CVERecord?id=CVE-2025-29481
>>
>> libbpf is meant to load BPF programs under root. It's a
>> highly-privileged operation, and libbpf is not meant, designed, and
>> actually explicitly discouraged from loading untrusted ELF files. As
>> such, this is just a normal bug fix, like lots of others. So let's
>> drop the CVE link as well.
>>
>> Again, no one in their sane mind should be passing untrusted ELF files
>> into libbpf while running under root. Period.
>>
>> All production use cases load ELF that they generated and control
>> (usually embedded into their memory through BPF skeleton header). And
>> if that ELF file is corrupted, you have problems somewhere else,
>> libbpf is not a culprit.
> 
> Should that context-less CVE be revoked as well?  Who asked for it to be
> issued?

That would be ideal. It was filed by MITRE but the CVE report doesn't
contain more information than a link to the GitHub repo with the
reproducer [1]. Since the repo contains reproducers for other newly
filed CVEs, it's likely that they have been requested by the repo owner.

MITRE has a form [2] which apparently could be used for providing more
information on a CVE. Should we try to use it and request revoking it?
(I'm asking as I'm not much familiar with the overall CVE filing process).

Thanks!
Viktor

[1] https://github.com/lmarch2/poc/blob/main/libbpf/libbpf.md
[2] https://cveform.mitre.org/

> 
> thanks,
> 
> greg k-h
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf v2] libbpf: Fix buffer overflow in bpf_object__init_prog
  2025-04-14  4:59   ` Viktor Malik
@ 2025-04-15  9:30     ` Shung-Hsi Yu
  2025-04-15 15:32       ` Viktor Malik
  0 siblings, 1 reply; 7+ messages in thread
From: Shung-Hsi Yu @ 2025-04-15  9:30 UTC (permalink / raw)
  To: Viktor Malik
  Cc: Andrii Nakryiko, bpf, Andrii Nakryiko, Eduard Zingerman,
	Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, lmarch2, stable

On Mon, Apr 14, 2025 at 06:59:31AM +0200, Viktor Malik wrote:
> On 4/11/25 18:22, Andrii Nakryiko wrote:
> > On Thu, Apr 10, 2025 at 2:55 AM Viktor Malik <vmalik@redhat.com> wrote:
> >> As reported by CVE-2025-29481 [1], it is possible to corrupt a BPF ELF
> >> file such that arbitrary BPF instructions are loaded by libbpf. This can
> >> be done by setting a symbol (BPF program) section offset to a large
> >> (unsigned) number such that <section start + symbol offset> overflows
> >> and points before the section data in the memory.
...
> >> Cc: stable@vger.kernel.org
> > 
> > Libbpf is packaged and consumed from Github mirror, which is produced
> > from latest bpf-next and bpf trees, so there is no point in
> > backporting fixes like this to stable kernel branches. Please drop the
> > CC: stable in the next revision.
> 
> Ack, will drop it.

Sorry for blindly suggesting the CC. I'll keep that in mind.

> >> Fixes: 6245947c1b3c ("libbpf: Allow gaps in BPF program sections to support overriden weak functions")
> >> Link: https://github.com/lmarch2/poc/blob/main/libbpf/libbpf.md
> >> Link: https://www.cve.org/CVERecord?id=CVE-2025-29481
> > 
> > libbpf is meant to load BPF programs under root. It's a
> > highly-privileged operation, and libbpf is not meant, designed, and
> > actually explicitly discouraged from loading untrusted ELF files. As
> > such, this is just a normal bug fix, like lots of others. So let's
> > drop the CVE link as well.
> > 
> > Again, no one in their sane mind should be passing untrusted ELF files
> > into libbpf while running under root. Period.
> > 
> > All production use cases load ELF that they generated and control
> > (usually embedded into their memory through BPF skeleton header). And
> > if that ELF file is corrupted, you have problems somewhere else,
> > libbpf is not a culprit.
> 
> While I couldn't agree more, I'm a bit on the fence here. On one hand,
> unless the CVE is revoked (see the other thread), people may still run
> across it and, without sufficient knowledge of libbpf, think that they
> are vulnerable. Having a CVE reference in the patch could help them
> recognize that they are using a patched version of libbpf or at least
> read an explanation why the vulnerability is not real.
> 
> On the other hand, since it's just a bug, I agree that it doesn't make
> much sense to reference a CVE from it. So, I'm ok both ways. I can
> reference the CVE and provide some better explanation why this should
> not be considered a vulnerability.

While I also see other colleagues that reference CVE number in the
commit message in other subsystems, personally I would drop CVE
reference here. This CVE entry doesn't have techinical detail in itself
beside mentioning that the issue being buffer overflow, and is
disputed/on the way to being rejected as far as this thread is
concerned.

...

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH bpf v2] libbpf: Fix buffer overflow in bpf_object__init_prog
  2025-04-15  9:30     ` Shung-Hsi Yu
@ 2025-04-15 15:32       ` Viktor Malik
  0 siblings, 0 replies; 7+ messages in thread
From: Viktor Malik @ 2025-04-15 15:32 UTC (permalink / raw)
  To: Shung-Hsi Yu
  Cc: Andrii Nakryiko, bpf, Andrii Nakryiko, Eduard Zingerman,
	Alexei Starovoitov, Daniel Borkmann, Martin KaFai Lau, Song Liu,
	Yonghong Song, John Fastabend, KP Singh, Stanislav Fomichev,
	Hao Luo, Jiri Olsa, lmarch2, stable

On 4/15/25 11:30, Shung-Hsi Yu wrote:
> On Mon, Apr 14, 2025 at 06:59:31AM +0200, Viktor Malik wrote:
>> On 4/11/25 18:22, Andrii Nakryiko wrote:
>>> On Thu, Apr 10, 2025 at 2:55 AM Viktor Malik <vmalik@redhat.com> wrote:
>>>> As reported by CVE-2025-29481 [1], it is possible to corrupt a BPF ELF
>>>> file such that arbitrary BPF instructions are loaded by libbpf. This can
>>>> be done by setting a symbol (BPF program) section offset to a large
>>>> (unsigned) number such that <section start + symbol offset> overflows
>>>> and points before the section data in the memory.
> ...
>>>> Cc: stable@vger.kernel.org
>>>
>>> Libbpf is packaged and consumed from Github mirror, which is produced
>>> from latest bpf-next and bpf trees, so there is no point in
>>> backporting fixes like this to stable kernel branches. Please drop the
>>> CC: stable in the next revision.
>>
>> Ack, will drop it.
> 
> Sorry for blindly suggesting the CC. I'll keep that in mind.
> 
>>>> Fixes: 6245947c1b3c ("libbpf: Allow gaps in BPF program sections to support overriden weak functions")
>>>> Link: https://github.com/lmarch2/poc/blob/main/libbpf/libbpf.md
>>>> Link: https://www.cve.org/CVERecord?id=CVE-2025-29481
>>>
>>> libbpf is meant to load BPF programs under root. It's a
>>> highly-privileged operation, and libbpf is not meant, designed, and
>>> actually explicitly discouraged from loading untrusted ELF files. As
>>> such, this is just a normal bug fix, like lots of others. So let's
>>> drop the CVE link as well.
>>>
>>> Again, no one in their sane mind should be passing untrusted ELF files
>>> into libbpf while running under root. Period.
>>>
>>> All production use cases load ELF that they generated and control
>>> (usually embedded into their memory through BPF skeleton header). And
>>> if that ELF file is corrupted, you have problems somewhere else,
>>> libbpf is not a culprit.
>>
>> While I couldn't agree more, I'm a bit on the fence here. On one hand,
>> unless the CVE is revoked (see the other thread), people may still run
>> across it and, without sufficient knowledge of libbpf, think that they
>> are vulnerable. Having a CVE reference in the patch could help them
>> recognize that they are using a patched version of libbpf or at least
>> read an explanation why the vulnerability is not real.
>>
>> On the other hand, since it's just a bug, I agree that it doesn't make
>> much sense to reference a CVE from it. So, I'm ok both ways. I can
>> reference the CVE and provide some better explanation why this should
>> not be considered a vulnerability.
> 
> While I also see other colleagues that reference CVE number in the
> commit message in other subsystems, personally I would drop CVE
> reference here. This CVE entry doesn't have techinical detail in itself
> beside mentioning that the issue being buffer overflow, and is
> disputed/on the way to being rejected as far as this thread is
> concerned.

Good point, I agree that dropping the reference is probably the best
approach here. It will allow us to merge this fix while we discuss the
next steps wrt. the CVE in the other thread.

I'll send a new version.

Thanks.
Viktor

> 
> ...
> 


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-04-15 15:32 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-10  9:55 [PATCH bpf v2] libbpf: Fix buffer overflow in bpf_object__init_prog Viktor Malik
2025-04-11 16:22 ` Andrii Nakryiko
2025-04-12  6:24   ` Greg KH
2025-04-14  5:05     ` Viktor Malik
2025-04-14  4:59   ` Viktor Malik
2025-04-15  9:30     ` Shung-Hsi Yu
2025-04-15 15:32       ` Viktor Malik

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).