diff mbox series

[bpf,v2] libbpf: Fix buffer overflow in bpf_object__init_prog

Message ID 20250410095517.141271-1-vmalik@redhat.com (mailing list archive)
State Superseded
Delegated to: BPF
Headers show
Series [bpf,v2] libbpf: Fix buffer overflow in bpf_object__init_prog | expand

Checks

Context Check Description
bpf/vmtest-bpf-PR success PR summary
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for bpf
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag present in non-next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/build_tools success Errors and warnings before: 26 (+2) this patch: 26 (+2)
netdev/cc_maintainers success CCed 13 of 13 maintainers
netdev/build_clang success Errors and warnings before: 0 this patch: 0
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/checkpatch warning WARNING: 'overriden' may be misspelled - perhaps 'overridden'?
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0
bpf/vmtest-bpf-VM_Test-0 success Logs for Lint
bpf/vmtest-bpf-VM_Test-1 success Logs for ShellCheck
bpf/vmtest-bpf-VM_Test-2 success Logs for Unittests
bpf/vmtest-bpf-VM_Test-3 success Logs for Validate matrix.py
bpf/vmtest-bpf-VM_Test-5 success Logs for aarch64-gcc / build / build for aarch64 with gcc
bpf/vmtest-bpf-VM_Test-4 success Logs for aarch64-gcc / GCC BPF
bpf/vmtest-bpf-VM_Test-7 success Logs for aarch64-gcc / test (test_maps, false, 360) / test_maps on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-10 success Logs for aarch64-gcc / test (test_verifier, false, 360) / test_verifier on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-13 success Logs for s390x-gcc / GCC BPF
bpf/vmtest-bpf-VM_Test-6 success Logs for aarch64-gcc / build-release
bpf/vmtest-bpf-VM_Test-11 success Logs for aarch64-gcc / veristat-kernel
bpf/vmtest-bpf-VM_Test-15 success Logs for s390x-gcc / build-release
bpf/vmtest-bpf-VM_Test-12 success Logs for aarch64-gcc / veristat-meta
bpf/vmtest-bpf-VM_Test-14 success Logs for s390x-gcc / build / build for s390x with gcc
bpf/vmtest-bpf-VM_Test-20 success Logs for s390x-gcc / veristat-meta
bpf/vmtest-bpf-VM_Test-19 success Logs for s390x-gcc / veristat-kernel
bpf/vmtest-bpf-VM_Test-18 success Logs for s390x-gcc / test (test_verifier, false, 360) / test_verifier on s390x with gcc
bpf/vmtest-bpf-VM_Test-21 success Logs for set-matrix
bpf/vmtest-bpf-VM_Test-22 success Logs for x86_64-gcc / GCC BPF / GCC BPF
bpf/vmtest-bpf-VM_Test-23 success Logs for x86_64-gcc / build / build for x86_64 with gcc
bpf/vmtest-bpf-VM_Test-25 success Logs for x86_64-gcc / test (test_maps, false, 360) / test_maps on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-24 success Logs for x86_64-gcc / build-release
bpf/vmtest-bpf-VM_Test-28 success Logs for x86_64-gcc / test (test_progs_no_alu32_parallel, true, 30) / test_progs_no_alu32_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-29 success Logs for x86_64-gcc / test (test_progs_parallel, true, 30) / test_progs_parallel on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-30 success Logs for x86_64-gcc / test (test_verifier, false, 360) / test_verifier on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-32 success Logs for x86_64-gcc / veristat-meta / x86_64-gcc veristat_meta
bpf/vmtest-bpf-VM_Test-33 success Logs for x86_64-llvm-17 / GCC BPF / GCC BPF
bpf/vmtest-bpf-VM_Test-31 success Logs for x86_64-gcc / veristat-kernel / x86_64-gcc veristat_kernel
bpf/vmtest-bpf-VM_Test-35 success Logs for x86_64-llvm-17 / build-release / build for x86_64 with llvm-17-O2
bpf/vmtest-bpf-VM_Test-36 success Logs for x86_64-llvm-17 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-39 success Logs for x86_64-llvm-17 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-41 success Logs for x86_64-llvm-17 / veristat-meta
bpf/vmtest-bpf-VM_Test-34 success Logs for x86_64-llvm-17 / build / build for x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-43 success Logs for x86_64-llvm-18 / build / build for x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-44 success Logs for x86_64-llvm-18 / build-release / build for x86_64 with llvm-18-O2
bpf/vmtest-bpf-VM_Test-40 success Logs for x86_64-llvm-17 / veristat-kernel
bpf/vmtest-bpf-VM_Test-42 success Logs for x86_64-llvm-18 / GCC BPF / GCC BPF
bpf/vmtest-bpf-VM_Test-45 success Logs for x86_64-llvm-18 / test (test_maps, false, 360) / test_maps on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-50 success Logs for x86_64-llvm-18 / veristat-kernel
bpf/vmtest-bpf-VM_Test-51 success Logs for x86_64-llvm-18 / veristat-meta
bpf/vmtest-bpf-VM_Test-49 success Logs for x86_64-llvm-18 / test (test_verifier, false, 360) / test_verifier on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-8 success Logs for aarch64-gcc / test (test_progs, false, 360) / test_progs on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-9 success Logs for aarch64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on aarch64 with gcc
bpf/vmtest-bpf-VM_Test-27 success Logs for x86_64-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-26 success Logs for x86_64-gcc / test (test_progs, false, 360) / test_progs on x86_64 with gcc
bpf/vmtest-bpf-VM_Test-38 success Logs for x86_64-llvm-17 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-37 success Logs for x86_64-llvm-17 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-17
bpf/vmtest-bpf-VM_Test-46 success Logs for x86_64-llvm-18 / test (test_progs, false, 360) / test_progs on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-48 success Logs for x86_64-llvm-18 / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-47 success Logs for x86_64-llvm-18 / test (test_progs_cpuv4, false, 360) / test_progs_cpuv4 on x86_64 with llvm-18
bpf/vmtest-bpf-VM_Test-17 success Logs for s390x-gcc / test (test_progs_no_alu32, false, 360) / test_progs_no_alu32 on s390x with gcc
bpf/vmtest-bpf-VM_Test-16 success Logs for s390x-gcc / test (test_progs, false, 360) / test_progs on s390x with gcc

Commit Message

Viktor Malik April 10, 2025, 9:55 a.m. UTC 
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(-)

Comments

Andrii Nakryiko April 11, 2025, 4:22 p.m. UTC | #1 
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
>
Greg Kroah-Hartman April 12, 2025, 6:24 a.m. UTC | #2 
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
Viktor Malik April 14, 2025, 4:59 a.m. UTC | #3 
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
>>
>
Viktor Malik April 14, 2025, 5:05 a.m. UTC | #4 
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
>
Shung-Hsi Yu April 15, 2025, 9:30 a.m. UTC | #5 
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.

...
Viktor Malik April 15, 2025, 3:32 p.m. UTC | #6 
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

> 
> ...
>
diff mbox series

Patch

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;