* [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-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-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-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).