* [PATCH v2] BPF: Fix potential bad pointer dereference in bpf_sys_bpf()
@ 2022-07-29 20:17 Jinghao Jia
2022-08-02 23:50 ` Yonghong Song
2022-08-05 0:10 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Jinghao Jia @ 2022-07-29 20:17 UTC (permalink / raw)
To: ast, daniel, andrii, yhs
Cc: martin.lau, song, john.fastabend, kpsingh, sdf, haoluo, jolsa,
bpf, mvle, jinghao7, Jinghao Jia
The bpf_sys_bpf() helper function allows an eBPF program to load another
eBPF program from within the kernel. In this case the argument union
bpf_attr pointer (as well as the insns and license pointers inside) is a
kernel address instead of a userspace address (which is the case of a
usual bpf() syscall). To make the memory copying process in the syscall
work in both cases, bpfptr_t was introduced to wrap around the pointer
and distinguish its origin. Specifically, when copying memory contents
from a bpfptr_t, a copy_from_user() is performed in case of a userspace
address and a memcpy() is performed for a kernel address.
This can lead to problems because the in-kernel pointer is never checked
for validity. The problem happens when an eBPF syscall program tries to
call bpf_sys_bpf() to load a program but provides a bad insns pointer --
say 0xdeadbeef -- in the bpf_attr union. The helper calls __sys_bpf()
which would then call bpf_prog_load() to load the program.
bpf_prog_load() is responsible for copying the eBPF instructions to the
newly allocated memory for the program; it creates a kernel bpfptr_t for
insns and invokes copy_from_bpfptr(). Internally, all bpfptr_t
operations are backed by the corresponding sockptr_t operations, which
performs direct memcpy() on kernel pointers for copy_from/strncpy_from
operations. Therefore, the code is always happy to dereference the bad
pointer to trigger a un-handle-able page fault and in turn an oops.
However, this is not supposed to happen because at that point the eBPF
program is already verified and should not cause a memory error.
Sample KASAN trace:
[ 25.685056][ T228] ==================================================================
[ 25.685680][ T228] BUG: KASAN: user-memory-access in copy_from_bpfptr+0x21/0x30
[ 25.686210][ T228] Read of size 80 at addr 00000000deadbeef by task poc/228
[ 25.686732][ T228]
[ 25.686893][ T228] CPU: 3 PID: 228 Comm: poc Not tainted 5.19.0-rc7 #7
[ 25.687375][ T228] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS d55cb5a 04/01/2014
[ 25.687991][ T228] Call Trace:
[ 25.688223][ T228] <TASK>
[ 25.688429][ T228] dump_stack_lvl+0x73/0x9e
[ 25.688747][ T228] print_report+0xea/0x200
[ 25.689061][ T228] ? copy_from_bpfptr+0x21/0x30
[ 25.689401][ T228] ? _printk+0x54/0x6e
[ 25.689693][ T228] ? _raw_spin_lock_irqsave+0x70/0xd0
[ 25.690071][ T228] ? copy_from_bpfptr+0x21/0x30
[ 25.690412][ T228] kasan_report+0xb5/0xe0
[ 25.690716][ T228] ? copy_from_bpfptr+0x21/0x30
[ 25.691059][ T228] kasan_check_range+0x2bd/0x2e0
[ 25.691405][ T228] ? copy_from_bpfptr+0x21/0x30
[ 25.691734][ T228] memcpy+0x25/0x60
[ 25.692000][ T228] copy_from_bpfptr+0x21/0x30
[ 25.692328][ T228] bpf_prog_load+0x604/0x9e0
[ 25.692653][ T228] ? cap_capable+0xb4/0xe0
[ 25.692956][ T228] ? security_capable+0x4f/0x70
[ 25.693324][ T228] __sys_bpf+0x3af/0x580
[ 25.693635][ T228] bpf_sys_bpf+0x45/0x240
[ 25.693937][ T228] bpf_prog_f0ec79a5a3caca46_bpf_func1+0xa2/0xbd
[ 25.694394][ T228] bpf_prog_run_pin_on_cpu+0x2f/0xb0
[ 25.694756][ T228] bpf_prog_test_run_syscall+0x146/0x1c0
[ 25.695144][ T228] bpf_prog_test_run+0x172/0x190
[ 25.695487][ T228] __sys_bpf+0x2c5/0x580
[ 25.695776][ T228] __x64_sys_bpf+0x3a/0x50
[ 25.696084][ T228] do_syscall_64+0x60/0x90
[ 25.696393][ T228] ? fpregs_assert_state_consistent+0x50/0x60
[ 25.696815][ T228] ? exit_to_user_mode_prepare+0x36/0xa0
[ 25.697202][ T228] ? syscall_exit_to_user_mode+0x20/0x40
[ 25.697586][ T228] ? do_syscall_64+0x6e/0x90
[ 25.697899][ T228] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 25.698312][ T228] RIP: 0033:0x7f6d543fb759
[ 25.698624][ T228] Code: 08 5b 89 e8 5d c3 66 2e 0f 1f 84 00 00 00 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 97 a6 0e 00 f7 d8 64 89 01 48
[ 25.699946][ T228] RSP: 002b:00007ffc3df78468 EFLAGS: 00000287 ORIG_RAX: 0000000000000141
[ 25.700526][ T228] RAX: ffffffffffffffda RBX: 00007ffc3df78628 RCX: 00007f6d543fb759
[ 25.701071][ T228] RDX: 0000000000000090 RSI: 00007ffc3df78478 RDI: 000000000000000a
[ 25.701636][ T228] RBP: 00007ffc3df78510 R08: 0000000000000000 R09: 0000000000300000
[ 25.702191][ T228] R10: 0000000000000005 R11: 0000000000000287 R12: 0000000000000000
[ 25.702736][ T228] R13: 00007ffc3df78638 R14: 000055a1584aca68 R15: 00007f6d5456a000
[ 25.703282][ T228] </TASK>
[ 25.703490][ T228] ==================================================================
[ 25.704050][ T228] Disabling lock debugging due to kernel taint
Update copy_from_bpfptr() and strncpy_from_bpfptr() so that:
- for a kernel pointer, it uses the safe copy_from_kernel_nofault() and
strncpy_from_kernel_nofault() functions.
- for a userspace pointer, it performs copy_from_user() and
strncpy_from_user().
Fixes: af2ac3e13e45 ("bpf: Prepare bpf syscall to be used from kernel and user space.")
Link: https://lore.kernel.org/bpf/20220727132905.45166-1-jinghao@linux.ibm.com/
Signed-off-by: Jinghao Jia <jinghao@linux.ibm.com>
---
Changelog v2:
Applied Yonghong's comments and directly modified copy_from_bpfptr() and
strncpy_from_bpfptr() rather than the sockptr APIs.
include/linux/bpfptr.h | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/include/linux/bpfptr.h b/include/linux/bpfptr.h
index 46e1757d06a3..79b2f78eec1a 100644
--- a/include/linux/bpfptr.h
+++ b/include/linux/bpfptr.h
@@ -49,7 +49,9 @@ static inline void bpfptr_add(bpfptr_t *bpfptr, size_t val)
static inline int copy_from_bpfptr_offset(void *dst, bpfptr_t src,
size_t offset, size_t size)
{
- return copy_from_sockptr_offset(dst, (sockptr_t) src, offset, size);
+ if (!bpfptr_is_kernel(src))
+ return copy_from_user(dst, src.user + offset, size);
+ return copy_from_kernel_nofault(dst, src.kernel + offset, size);
}
static inline int copy_from_bpfptr(void *dst, bpfptr_t src, size_t size)
@@ -78,7 +80,9 @@ static inline void *kvmemdup_bpfptr(bpfptr_t src, size_t len)
static inline long strncpy_from_bpfptr(char *dst, bpfptr_t src, size_t count)
{
- return strncpy_from_sockptr(dst, (sockptr_t) src, count);
+ if (bpfptr_is_kernel(src))
+ return strncpy_from_kernel_nofault(dst, src.kernel, count);
+ return strncpy_from_user(dst, src.user, count);
}
#endif /* _LINUX_BPFPTR_H */
base-commit: 64893e83f916145fd23182209009e9d2ce36d2df
--
2.35.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] BPF: Fix potential bad pointer dereference in bpf_sys_bpf()
2022-07-29 20:17 [PATCH v2] BPF: Fix potential bad pointer dereference in bpf_sys_bpf() Jinghao Jia
@ 2022-08-02 23:50 ` Yonghong Song
2022-08-05 0:10 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Yonghong Song @ 2022-08-02 23:50 UTC (permalink / raw)
To: Jinghao Jia, ast, daniel, andrii
Cc: martin.lau, song, john.fastabend, kpsingh, sdf, haoluo, jolsa,
bpf, mvle, jinghao7
On 7/29/22 1:17 PM, Jinghao Jia wrote:
> The bpf_sys_bpf() helper function allows an eBPF program to load another
> eBPF program from within the kernel. In this case the argument union
> bpf_attr pointer (as well as the insns and license pointers inside) is a
> kernel address instead of a userspace address (which is the case of a
> usual bpf() syscall). To make the memory copying process in the syscall
> work in both cases, bpfptr_t was introduced to wrap around the pointer
> and distinguish its origin. Specifically, when copying memory contents
> from a bpfptr_t, a copy_from_user() is performed in case of a userspace
> address and a memcpy() is performed for a kernel address.
>
> This can lead to problems because the in-kernel pointer is never checked
> for validity. The problem happens when an eBPF syscall program tries to
> call bpf_sys_bpf() to load a program but provides a bad insns pointer --
> say 0xdeadbeef -- in the bpf_attr union. The helper calls __sys_bpf()
> which would then call bpf_prog_load() to load the program.
> bpf_prog_load() is responsible for copying the eBPF instructions to the
> newly allocated memory for the program; it creates a kernel bpfptr_t for
> insns and invokes copy_from_bpfptr(). Internally, all bpfptr_t
> operations are backed by the corresponding sockptr_t operations, which
> performs direct memcpy() on kernel pointers for copy_from/strncpy_from
> operations. Therefore, the code is always happy to dereference the bad
> pointer to trigger a un-handle-able page fault and in turn an oops.
> However, this is not supposed to happen because at that point the eBPF
> program is already verified and should not cause a memory error.
>
> Sample KASAN trace:
>
> [ 25.685056][ T228] ==================================================================
> [ 25.685680][ T228] BUG: KASAN: user-memory-access in copy_from_bpfptr+0x21/0x30
> [ 25.686210][ T228] Read of size 80 at addr 00000000deadbeef by task poc/228
> [ 25.686732][ T228]
> [ 25.686893][ T228] CPU: 3 PID: 228 Comm: poc Not tainted 5.19.0-rc7 #7
> [ 25.687375][ T228] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS d55cb5a 04/01/2014
> [ 25.687991][ T228] Call Trace:
> [ 25.688223][ T228] <TASK>
> [ 25.688429][ T228] dump_stack_lvl+0x73/0x9e
> [ 25.688747][ T228] print_report+0xea/0x200
> [ 25.689061][ T228] ? copy_from_bpfptr+0x21/0x30
> [ 25.689401][ T228] ? _printk+0x54/0x6e
> [ 25.689693][ T228] ? _raw_spin_lock_irqsave+0x70/0xd0
> [ 25.690071][ T228] ? copy_from_bpfptr+0x21/0x30
> [ 25.690412][ T228] kasan_report+0xb5/0xe0
> [ 25.690716][ T228] ? copy_from_bpfptr+0x21/0x30
> [ 25.691059][ T228] kasan_check_range+0x2bd/0x2e0
> [ 25.691405][ T228] ? copy_from_bpfptr+0x21/0x30
> [ 25.691734][ T228] memcpy+0x25/0x60
> [ 25.692000][ T228] copy_from_bpfptr+0x21/0x30
> [ 25.692328][ T228] bpf_prog_load+0x604/0x9e0
> [ 25.692653][ T228] ? cap_capable+0xb4/0xe0
> [ 25.692956][ T228] ? security_capable+0x4f/0x70
> [ 25.693324][ T228] __sys_bpf+0x3af/0x580
> [ 25.693635][ T228] bpf_sys_bpf+0x45/0x240
> [ 25.693937][ T228] bpf_prog_f0ec79a5a3caca46_bpf_func1+0xa2/0xbd
> [ 25.694394][ T228] bpf_prog_run_pin_on_cpu+0x2f/0xb0
> [ 25.694756][ T228] bpf_prog_test_run_syscall+0x146/0x1c0
> [ 25.695144][ T228] bpf_prog_test_run+0x172/0x190
> [ 25.695487][ T228] __sys_bpf+0x2c5/0x580
> [ 25.695776][ T228] __x64_sys_bpf+0x3a/0x50
> [ 25.696084][ T228] do_syscall_64+0x60/0x90
> [ 25.696393][ T228] ? fpregs_assert_state_consistent+0x50/0x60
> [ 25.696815][ T228] ? exit_to_user_mode_prepare+0x36/0xa0
> [ 25.697202][ T228] ? syscall_exit_to_user_mode+0x20/0x40
> [ 25.697586][ T228] ? do_syscall_64+0x6e/0x90
> [ 25.697899][ T228] entry_SYSCALL_64_after_hwframe+0x63/0xcd
> [ 25.698312][ T228] RIP: 0033:0x7f6d543fb759
> [ 25.698624][ T228] Code: 08 5b 89 e8 5d c3 66 2e 0f 1f 84 00 00 00 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 97 a6 0e 00 f7 d8 64 89 01 48
> [ 25.699946][ T228] RSP: 002b:00007ffc3df78468 EFLAGS: 00000287 ORIG_RAX: 0000000000000141
> [ 25.700526][ T228] RAX: ffffffffffffffda RBX: 00007ffc3df78628 RCX: 00007f6d543fb759
> [ 25.701071][ T228] RDX: 0000000000000090 RSI: 00007ffc3df78478 RDI: 000000000000000a
> [ 25.701636][ T228] RBP: 00007ffc3df78510 R08: 0000000000000000 R09: 0000000000300000
> [ 25.702191][ T228] R10: 0000000000000005 R11: 0000000000000287 R12: 0000000000000000
> [ 25.702736][ T228] R13: 00007ffc3df78638 R14: 000055a1584aca68 R15: 00007f6d5456a000
> [ 25.703282][ T228] </TASK>
> [ 25.703490][ T228] ==================================================================
> [ 25.704050][ T228] Disabling lock debugging due to kernel taint
>
> Update copy_from_bpfptr() and strncpy_from_bpfptr() so that:
> - for a kernel pointer, it uses the safe copy_from_kernel_nofault() and
> strncpy_from_kernel_nofault() functions.
> - for a userspace pointer, it performs copy_from_user() and
> strncpy_from_user().
>
> Fixes: af2ac3e13e45 ("bpf: Prepare bpf syscall to be used from kernel and user space.")
> Link: https://lore.kernel.org/bpf/20220727132905.45166-1-jinghao@linux.ibm.com/
> Signed-off-by: Jinghao Jia <jinghao@linux.ibm.com>
Acked-by: Yonghong Song <yhs@fb.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] BPF: Fix potential bad pointer dereference in bpf_sys_bpf()
2022-07-29 20:17 [PATCH v2] BPF: Fix potential bad pointer dereference in bpf_sys_bpf() Jinghao Jia
2022-08-02 23:50 ` Yonghong Song
@ 2022-08-05 0:10 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-08-05 0:10 UTC (permalink / raw)
To: Jinghao Jia
Cc: ast, daniel, andrii, yhs, martin.lau, song, john.fastabend,
kpsingh, sdf, haoluo, jolsa, bpf, mvle, jinghao7
Hello:
This patch was applied to bpf/bpf.git (master)
by Alexei Starovoitov <ast@kernel.org>:
On Fri, 29 Jul 2022 20:17:13 +0000 you wrote:
> The bpf_sys_bpf() helper function allows an eBPF program to load another
> eBPF program from within the kernel. In this case the argument union
> bpf_attr pointer (as well as the insns and license pointers inside) is a
> kernel address instead of a userspace address (which is the case of a
> usual bpf() syscall). To make the memory copying process in the syscall
> work in both cases, bpfptr_t was introduced to wrap around the pointer
> and distinguish its origin. Specifically, when copying memory contents
> from a bpfptr_t, a copy_from_user() is performed in case of a userspace
> address and a memcpy() is performed for a kernel address.
>
> [...]
Here is the summary with links:
- [v2] BPF: Fix potential bad pointer dereference in bpf_sys_bpf()
https://git.kernel.org/bpf/bpf/c/e2dcac2f58f5
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-05 0:10 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-29 20:17 [PATCH v2] BPF: Fix potential bad pointer dereference in bpf_sys_bpf() Jinghao Jia
2022-08-02 23:50 ` Yonghong Song
2022-08-05 0:10 ` patchwork-bot+netdevbpf
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).