Fix BPF probe memory helpers
Subsystems (13)
Daniel Borkmann
maintainerv2 cover Fix BPF probe memory helpers
This set adds probe_read_{user,kernel}(), probe_read_str_{user,kernel}() helpers, fixes probe_write_user() helper and selftests. For details please see individual patches.
Thanks!
v1 -> v2:
- standardize unsafe_ptr terminology in uapi header comment (Andrii)
- probe_read_{user,kernel}[_str] naming scheme (Andrii)
- use global data in last test case, remove relaxed_maps (Andrii)
- add strict non-pagefault kernel read funcs to avoid warning in kernel probe read helpers (Alexei)
Daniel Borkmann (8):
uaccess: Add non-pagefault user-space write function
uaccess: Add strict non-pagefault kernel-space read function
bpf: Make use of probe_user_write in probe write helper
bpf: Add probe_read_{user,kernel} and probe_read_{user,kernel}_str helpers
bpf: Switch BPF probe insns to bpf_probe_read_kernel
bpf, samples: Use bpf_probe_read_user where appropriate
bpf, testing: Convert prog tests to probe_read_{user,kernel}{,_str} helper bpf, testing: Add selftest to read/write sockaddr from user space
arch/x86/mm/Makefile | 2 +- arch/x86/mm/maccess.c | 38 ++++ include/linux/uaccess.h | 16 ++ include/uapi/linux/bpf.h | 120 +++++++---- kernel/bpf/core.c | 9 +- kernel/trace/bpf_trace.c | 187 +++++++++++++----- mm/maccess.c | 70 ++++++- samples/bpf/map_perf_test_kern.c | 4 +- samples/bpf/test_map_in_map_kern.c | 4 +- samples/bpf/test_probe_write_user_kern.c | 2 +- tools/include/uapi/linux/bpf.h | 120 +++++++---- .../selftests/bpf/prog_tests/probe_user.c | 78 ++++++++ tools/testing/selftests/bpf/progs/kfree_skb.c | 4 +- tools/testing/selftests/bpf/progs/pyperf.h | 67 ++++--- .../testing/selftests/bpf/progs/strobemeta.h | 36 ++-- .../selftests/bpf/progs/test_probe_user.c | 26 +++ .../selftests/bpf/progs/test_tcp_estats.c | 2 +- 17 files changed, 590 insertions(+), 195 deletions(-) create mode 100644 arch/x86/mm/maccess.c create mode 100644 tools/testing/selftests/bpf/prog_tests/probe_user.c create mode 100644 tools/testing/selftests/bpf/progs/test_probe_user.c
Signature · 1 line
2.21.0
Daniel Borkmann
maintainerPATCH v2 1/8 uaccess: Add non-pagefault user-space write function
Commit 3d7081822f7f ("uaccess: Add non-pagefault user-space read functions") missed to add probe write function, therefore factor out a probe_write_common() helper with most logic of probe_kernel_write() except setting KERNEL_DS, and add a new probe_user_write() helper so it can be used from BPF side.
Again, on some archs, the user address space and kernel address space can co-exist and be overlapping, so in such case, setting KERNEL_DS would mean that the given address is treated as being in kernel address space.
--- include/linux/uaccess.h | 12 +++++++++++ mm/maccess.c | 45 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 53 insertions(+), 4 deletions(-)
include/linux/uaccess.h +12 -0
@@ -337,6 +337,18 @@ extern long __probe_user_read(void *dst, const void __user *src, size_t size);extern long notrace probe_kernel_write (void *dst,const void *src,size_t size);extern long notrace __probe_kernel_write (void *dst,const void *src,size_t size); +/* +* probe_user_write(): safely attempt to write to a location in user space +* @dst: address to write to +* @src: pointer to the data that shall be written +* @size: size of the data chunk +* +* Safely write to address @dst from the buffer at @src. If a kernel fault +* happens, handle that and return -EFAULT. +*/ +extern long notrace probe_user_write (void __user *dst,const void *src,size_t size); +extern long notrace __probe_user_write (void __user *dst,const void *src,size_t size); +extern long strncpy_from_unsafe (char *dst,const void *unsafe_addr,long count);extern long strncpy_from_unsafe_user (char *dst,const void __user *unsafe_addr,long count);
mm/maccess.c +41 -4
68 lines not on this page · raw message
Signature · 1 line
2.21.0
Daniel Borkmann
maintainerPATCH v2 2/8 uaccess: Add strict non-pagefault kernel-space read function
In reply to Daniel BorkmannAdd two new probe_kernel_read_strict() and strncpy_from_unsafe_strict() helpers which by default alias to the __probe_kernel_read() and the __strncpy_from_unsafe(), respectively, but can be overridden by archs which have non-overlapping address ranges for kernel space and user space in order to bail out with -EFAULT when attempting to probe user memory including non-canonical user access addresses [0].
The idea is that these helpers are complementary to the probe_user_read() and strncpy_from_unsafe_user() which probe user-only memory. Both added helpers here do the same, but for kernel-only addresses.
Both set of helpers are going to be used for BPF tracing. They also explicitly avoid throwing the splat for non-canonical user addresses from 00c42373d397 ("x86-64: add warning for non-canonical user access address dereferences").
For compat, the current probe_kernel_read() and strncpy_from_unsafe() are left as-is.
[0] Documentation/x86/x86_64/mm.txt
4-level page tables: 0x0000800000000000 - 0xffff7fffffffffff
5-level page tables: 0x0100000000000000 - 0xfeffffffffffffff--- arch/x86/mm/Makefile | 2 +- arch/x86/mm/maccess.c | 38 ++++++++++++++++++++++++++++++++++++++ include/linux/uaccess.h | 4 ++++ mm/maccess.c | 25 ++++++++++++++++++++++++- 4 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 arch/x86/mm/maccess.c
arch/x86/mm/Makefile +1 -1
@@ -13,7 +13,7 @@ CFLAGS_REMOVE_mem_encrypt_identity.o = -pg endif obj-y := init.o init_$(BITS).o fault.o ioremap.o extable.o pageattr.o mmap.o \ - pat.o pgtable.o physaddr.o setup_nx.o tlb.o cpu_entry_area.o + pat.o pgtable.o physaddr.o setup_nx.o tlb.o cpu_entry_area.o maccess.o # Make sure __phys_addr has no stackprotector nostackp := $(call cc-option, -fno-stack-protector)
arch/x86/mm/maccess.c new file +38 -0
@@ -0,0 +1,38 @@ +// SPDX-License-Identifier: GPL-2.0-only + +#include <linux/uaccess.h> +#include <linux/kernel.h> + +static __always_inline u64 canonical_address(u64 vaddr, u8 vaddr_bits) +{ +return ((s64 )vaddr << (64 - vaddr_bits)) >> (64 - vaddr_bits); +} + +static __always_inline bool non_canonical_address(u64 vaddr) +{ +#ifdef CONFIG_X86_64 +return canonical_address (vaddr, boot_cpu_data.x86_virt_bits) != vaddr; +#else +return false; +#endif +} + +long probe_kernel_read_strict (void *dst,const void *src,size_t size) +{ +u64 addr = (unsigned long )src; + +if (unlikely (addr <TASK_SIZE_MAX ||non_canonical_address (addr))) +return -EFAULT ; + +return __probe_kernel_read (dst, src, size); +} + +long strncpy_from_unsafe_strict (char *dst,const void *unsafe_addr,long count) +{ +u64 addr = (unsigned long )unsafe_addr; + +if (unlikely (addr <TASK_SIZE_MAX ||non_canonical_address (addr))) +return -EFAULT ; + +return __strncpy_from_unsafe (dst, unsafe_addr, count); +}
include/linux/uaccess.h +4 -0
@@ -311,6 +311,7 @@ copy_struct_from_user(void *dst, size_t ksize, const void __user *src,* happens, handle that and return -EFAULT. */ extern long probe_kernel_read (void *dst,const void *src,size_t size); +extern long probe_kernel_read_strict (void *dst,const void *src,size_t size);extern long __probe_kernel_read (void *dst,const void *src,size_t size);/*
@@ -350,6 +351,9 @@ extern long notrace probe_user_write(void __user *dst, const void *src, size_t sextern long notrace __probe_user_write (void __user *dst,const void *src,size_t size);extern long strncpy_from_unsafe (char *dst,const void *unsafe_addr,long count); +extern long strncpy_from_unsafe_strict (char *dst,const void *unsafe_addr, +long count); +extern long __strncpy_from_unsafe (char *dst,const void *unsafe_addr,long count);extern long strncpy_from_unsafe_user (char *dst,const void __user *unsafe_addr,long count);extern long strnlen_unsafe_user (const void __user *unsafe_addr,long count);
mm/maccess.c +24 -1
@@ -43,11 +43,20 @@ probe_write_common(void __user *dst, const void *src, size_t size)* do_page_fault() doesn't attempt to take mmap_sem. This makes * probe_kernel_read() suitable for use within regions where the caller * already holds mmap_sem, or other locks which nest inside mmap_sem. +* +* probe_kernel_read_strict() is the same as probe_kernel_read() except for +* the case where architectures have non-overlapping user and kernel address +* ranges: probe_kernel_read_strict() will additionally return -EFAULT for +* probing memory on a user address range where probe_user_read() is supposed +* to be used instead. */ long __weak probe_kernel_read (void *dst,const void *src,size_t size) __attribute__((alias ("__probe_kernel_read" ))); +long __weak probe_kernel_read_strict (void *dst,const void *src,size_t size) + __attribute__((alias ("__probe_kernel_read" ))); +long __probe_kernel_read (void *dst,const void *src,size_t size) {long ret;
@@ -157,8 +166,22 @@ EXPORT_SYMBOL_GPL(probe_user_write);* * If @count is smaller than the length of the string, copies @count-1 bytes, * sets the last byte of @dst buffer to NUL and returns @count. +* +* strncpy_from_unsafe_strict() is the same as strncpy_from_unsafe() except +* for the case where architectures have non-overlapping user and kernel address +* ranges: strncpy_from_unsafe_strict() will additionally return -EFAULT for +* probing memory on a user address range where strncpy_from_unsafe_user() is +* supposed to be used instead. */ -long strncpy_from_unsafe (char *dst,const void *unsafe_addr,long count) + +long __weak strncpy_from_unsafe (char *dst,const void *unsafe_addr,long count) + __attribute__((alias ("__strncpy_from_unsafe" ))); + +long __weak strncpy_from_unsafe_strict (char *dst,const void *unsafe_addr, +long count) + __attribute__((alias ("__strncpy_from_unsafe" ))); + +long __strncpy_from_unsafe (char *dst,const void *unsafe_addr,long count) {mm_segment_t old_fs =get_fs ();const void *src = unsafe_addr;
Signature · 1 line
2.21.0
Re: [PATCH bpf-next v2 2/8] uaccess: Add strict non-pagefault kernel-space read function
Quoted 22 lines · Daniel Borkmann
Add two new
probe_kernel_read_strict()andstrncpy_from_unsafe_strict()helpers which by default alias to the__probe_kernel_read()and the__strncpy_from_unsafe(), respectively, but can be overridden by archs which have non-overlapping address ranges for kernel space and user space in order to bail out with-EFAULTwhen attempting to probe user memory including non-canonical user access addresses [0].The idea is that these helpers are complementary to the
probe_user_read()andstrncpy_from_unsafe_user()which probe user-only memory. Both added helpers here do the same, but for kernel-only addresses.Both set of helpers are going to be used for BPF tracing. They also explicitly avoid throwing the splat for non-canonical user addresses from
00c42373d397("x86-64: add warning for non-canonical user access address dereferences").For compat, the current
probe_kernel_read()andstrncpy_from_unsafe()are left as-is.[0]
Documentation/x86/x86_64/mm.txt
Quoted 8 lines
4-level page tables: 0x0000800000000000 - 0xffff7fffffffffff 5-level page tables: 0x0100000000000000 - 0xfeffffffffffffffSigned-off-by Daniel Borkmann <[email protected]>Cc Linus Torvalds <[email protected]>Cc Masami Hiramatsu <[email protected]>---
Quoted 7 lines
arch/x86/mm/Makefile | 2 +- arch/x86/mm/maccess.c | 38 ++++++++++++++++++++++++++++++++++++++ include/linux/uaccess.h | 4 ++++ mm/maccess.c | 25 ++++++++++++++++++++++++- 4 files changed, 67 insertions(+), 2 deletions(-) create mode 100644 arch/x86/mm/maccess.c
[...]
Daniel Borkmann
maintainerPATCH v2 3/8 bpf: Make use of probe_user_write in probe write helper
In reply to Daniel BorkmannConvert the bpf_probe_write_user() helper to probe_user_write() such that writes are not attempted under KERNEL_DS anymore which is buggy as kernel and user space pointers can have overlapping addresses. Also, given we have the access_ok() check inside probe_user_write(), the helper doesn't need to do it twice.
--- kernel/trace/bpf_trace.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)
kernel/trace/bpf_trace.c +2 -4
@@ -163,7 +163,7 @@ static const struct bpf_func_proto bpf_probe_read_proto = { .arg3_type = ARG_ANYTHING, }; -BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src, +BPF_CALL_3(bpf_probe_write_user, void __user *, unsafe_ptr, const void *, src, u32, size) {/*
@@ -186,10 +186,8 @@ BPF_CALL_3(bpf_probe_write_user, void *, unsafe_ptr, const void *, src,return -EPERM ;if (unlikely (!nmi_uaccess_okay ()))return -EPERM ; -if (!access_ok (unsafe_ptr, size)) -return -EPERM ; -return probe_kernel_write (unsafe_ptr, src, size); +return probe_user_write (unsafe_ptr, src, size); }static const struct bpf_func_proto bpf_probe_write_user_proto = {
Signature · 1 line
2.21.0
Daniel Borkmann
maintainerPATCH v2 4/8 bpf: Add probe_read_{user,kernel} and probe_read_{user,kernel}_str helpers
In reply to Daniel BorkmannThe current bpf_probe_read() and bpf_probe_read_str() helpers are broken in that they assume they can be used for probing memory access for kernel space addresses /as well as/ user space addresses.
However, plain use of probe_kernel_read() for both cases will attempt to always access kernel space address space given access is performed under KERNEL_DS and some archs in-fact have overlapping address spaces where a kernel pointer and user pointer would have the /same/ address value and therefore accessing application memory via bpf_probe_read{,_str}() would read garbage values.
Lets fix BPF side by making use of recently added 3d7081822f7f ("uaccess:
Add non-pagefault user-space read functions"). Unfortunately, the only way to fix this status quo is to add dedicated bpf_probe_read_{user,kernel}() and bpf_probe_read_{user,kernel}_str() helpers. The bpf_probe_read{,_str}() helpers are kept as-is to retain their current behavior.
The two *_user() variants attempt the access always under USER_DS set, the two *_kernel() variants will -EFAULT when accessing user memory if the underlying architecture has non-overlapping address ranges, also avoiding throwing the kernel warning via 00c42373d397 ("x86-64: add warning for non-canonical user access address dereferences").
--- include/uapi/linux/bpf.h | 120 +++++++++++++++------- kernel/trace/bpf_trace.c | 181 ++++++++++++++++++++++++--------- tools/include/uapi/linux/bpf.h | 120 +++++++++++++++------- 3 files changed, 297 insertions(+), 124 deletions(-)
include/uapi/linux/bpf.h +81 -39
@@ -561,10 +561,13 @@ union bpf_attr {* Return * 0 on success, or a negative error in case of failure. * -* int bpf_probe_read(void *dst, u32 size, const void *src) +* int bpf_probe_read(void *dst, u32 size, const void *unsafe_ptr) * Description * For tracing programs, safely attempt to read *size* bytes from -* address *src* and store the data in *dst*. +* kernel space address *unsafe_ptr* and store the data in *dst*. +* +* Generally, use bpf_probe_read_user() or bpf_probe_read_kernel() +* instead. * Return * 0 on success, or a negative error in case of failure. *
@@ -1428,43 +1431,12 @@ union bpf_attr {* * int bpf_probe_read_str(void *dst, int size, const void *unsafe_ptr) * Description -* Copy a NUL terminated string from an unsafe address -* *unsafe_ptr* to *dst*. The *size* should include the -* terminating NUL byte. In case the string length is smaller than -* *size*, the target is not padded with further NUL bytes. If the -* string length is larger than *size*, just *size*-1 bytes are -* copied and the last byte is set to NUL. -* -* On success, the length of the copied string is returned. This -* makes this helper useful in tracing programs for reading -* strings, and more importantly to get its length at runtime. See -* the following snippet: -* -* :: -* -* SEC("kprobe/sys_open") -* void bpf_sys_open(struct pt_regs *ctx) -* { -* char buf[PATHLEN]; // PATHLEN is defined to 256 -* int res = bpf_probe_read_str(buf, sizeof(buf), -* ctx->di); -* -* // Consume buf, for example push it to -* // userspace via bpf_perf_event_output(); we -* // can use res (the string length) as event -* // size, after checking its boundaries. -* } -* -* In comparison, using **bpf_probe_read()** helper here instead -* to read the string would require to estimate the length at -* compile time, and would often result in copying more memory -* than necessary. +* Copy a NUL terminated string from an unsafe kernel address +* *unsafe_ptr* to *dst*. See bpf_probe_read_kernel_str() for +* more details. * -* Another useful use case is when parsing individual process -* arguments or individual environment variables navigating -* *current*\ **->mm->arg_start** and *current*\ -* **->mm->env_start**: using this helper and the return value, -* one can quickly iterate at the right offset of the memory area. +* Generally, use bpf_probe_read_user_str() or bpf_probe_read_kernel_str() +* instead. * Return * On success, the strictly positive length of the string, * including the trailing NUL character. On error, a negative
@@ -2775,6 +2747,72 @@ union bpf_attr {* restricted to raw_tracepoint bpf programs. * Return * 0 on success, or a negative error in case of failure. +* +* int bpf_probe_read_user(void *dst, u32 size, const void *unsafe_ptr) +* Description +* Safely attempt to read *size* bytes from user space address +* *unsafe_ptr* and store the data in *dst*. +* Return +* 0 on success, or a negative error in case of failure. +* +* int bpf_probe_read_kernel(void *dst, u32 size, const void *unsafe_ptr) +* Description +* Safely attempt to read *size* bytes from kernel space address +* *unsafe_ptr* and store the data in *dst*. +* Return +* 0 on success, or a negative error in case of failure. +* +* int bpf_probe_read_user_str(void *dst, int size, const void *unsafe_ptr) +* Description +* Copy a NUL terminated string from an unsafe user address +* *unsafe_ptr* to *dst*. The *size* should include the +* terminating NUL byte. In case the string length is smaller than +* *size*, the target is not padded with further NUL bytes. If the +* string length is larger than *size*, just *size*-1 bytes are +* copied and the last byte is set to NUL. +* +* On success, the length of the copied string is returned. This +* makes this helper useful in tracing programs for reading +* strings, and more importantly to get its length at runtime. See +* the following snippet: +* +* :: +* +* SEC("kprobe/sys_open") +* void bpf_sys_open(struct pt_regs *ctx) +* { +* char buf[PATHLEN]; // PATHLEN is defined to 256 +* int res = bpf_probe_read_user_str(buf, sizeof(buf), +* ctx->di); +* +* // Consume buf, for example push it to +* // userspace via bpf_perf_event_output(); we +* // can use res (the string length) as event +* // size, after checking its boundaries. +* } +* +* In comparison, using **bpf_probe_read_user()** helper here +* instead to read the string would require to estimate the length +* at compile time, and would often result in copying more memory +* than necessary. +* +* Another useful use case is when parsing individual process +* arguments or individual environment variables navigating +* *current*\ **->mm->arg_start** and *current*\ +* **->mm->env_start**: using this helper and the return value, +* one can quickly iterate at the right offset of the memory area. +* Return +* On success, the strictly positive length of the string, +* including the trailing NUL character. On error, a negative +* value. +* +* int bpf_probe_read_kernel_str(void *dst, int size, const void *unsafe_ptr) +* Description +* Copy a NUL terminated string from an unsafe kernel address *unsafe_ptr* +* to *dst*. Same semantics as with bpf_probe_read_user_str() apply. +* Return +* On success, the strictly positive length of the string, including +* the trailing NUL character. On error, a negative value. */ #define __BPF_FUNC_MAPPER (FN ) \ FN(unspec), \
@@ -2888,7 +2926,11 @@ union bpf_attr {FN (sk_storage_delete), \FN (send_signal), \FN (tcp_gen_syncookie), \ -FN (skb_output), +FN (skb_output), \ +FN (probe_read_user), \ +FN (probe_read_kernel), \ +FN (probe_read_user_str), \ +FN (probe_read_kernel_str),/* integer value in 'imm' field of BPF_CALL instruction selects which helper * function eBPF program intends to call
kernel/trace/bpf_trace.c +135 -46
216 lines not on this page · raw message
tools/include/uapi/linux/bpf.h +81 -39
147 lines not on this page · raw message
Signature · 1 line
2.21.0
Re: [PATCH bpf-next v2 4/8] bpf: Add probe_read_{user,kernel} and probe_read_{user,kernel}_str helpers
Quoted 24 lines · Daniel Borkmann
The current
bpf_probe_read()andbpf_probe_read_str()helpers are broken in that they assume they can be used for probing memory access for kernel space addresses /as well as/ user space addresses.However, plain use of
probe_kernel_read()for both cases will attempt to always access kernel space address space given access is performed underKERNEL_DSand some archs in-fact have overlapping address spaces where a kernel pointer and user pointer would have the /same/ address value and therefore accessing application memory via bpf_probe_read{,_str}() would read garbage values.Lets fix BPF side by making use of recently added
3d7081822f7f("uaccess:
Add non-pagefault user-space read functions"). Unfortunately, the only way to fix this status quo is to add dedicated bpf_probe_read_{user,kernel}() and bpf_probe_read_{user,kernel}_str() helpers. The bpf_probe_read{,_str}() helpers are kept as-is to retain their current behavior.The two
*_user()variants attempt the access always underUSER_DSset, the two*_kernel()variants will-EFAULTwhen accessing user memory if the underlying architecture has non-overlapping address ranges, also avoiding throwing the kernel warning via00c42373d397("x86-64: add warning for non-canonical user access address dereferences").
Quoted 4 lines
Fixes a5e8c07059d0 ("bpf: add bpf_probe_read_str helper")Fixes 2541517c32be ("tracing, perf: Implement BPF programs attached to kprobes")Signed-off-by Daniel Borkmann <[email protected]>---
LGTM!
Quoted 5 lines
include/uapi/linux/bpf.h | 120 +++++++++++++++------- kernel/trace/bpf_trace.c | 181 ++++++++++++++++++++++++--------- tools/include/uapi/linux/bpf.h | 120 +++++++++++++++------- 3 files changed, 297 insertions(+), 124 deletions(-)
Daniel Borkmann
maintainerPATCH v2 5/8 bpf: Switch BPF probe insns to bpf_probe_read_kernel
In reply to Daniel BorkmannCommit 2a02759ef5f8 ("bpf: Add support for BTF pointers to interpreter") explicitly states that the pointer to BTF object is a pointer to a kernel object or NULL. Therefore we should also switch to using the strict kernel probe helper which is restricted to kernel addresses only when architectures have non-overlapping address spaces.
--- kernel/bpf/core.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
kernel/bpf/core.c +5 -4
@@ -1309,11 +1309,12 @@ bool bpf_opcode_in_insntable(u8 code) }#ifndef CONFIG_BPF_JIT_ALWAYS_ON -u64 __weak bpf_probe_read (void * dst,u32 size,const void * unsafe_ptr) +u64 __weak bpf_probe_read_kernel (void *dst,u32 size,const void *unsafe_ptr) {memset (dst,0 , size);return -EFAULT ; } +/** * __bpf_prog_run - run eBPF program on a given context * @regs: is the array of MAX_BPF_EXT_REG eBPF pseudo-registers
@@ -1569,9 +1570,9 @@ static u64 __no_fgcse ___bpf_prog_run(u64 *regs, const struct bpf_insn *insn, u6 LDST(W, u32) LDST(DW, u64)#undef LDST -#define LDX_PROBE (SIZEOP ,SIZE ) \ - LDX_PROBE_MEM_##SIZEOP: \ - bpf_probe_read(&DST, SIZE, (const void *)(long) SRC); \ +#define LDX_PROBE (SIZEOP ,SIZE ) \ + LDX_PROBE_MEM_##SIZEOP: \ + bpf_probe_read_kernel(&DST, SIZE, (const void *)(long) SRC); \ CONT; LDX_PROBE(B, 1)LDX_PROBE (H ,2 )
Signature · 1 line
2.21.0
Re: [PATCH bpf-next v2 5/8] bpf: Switch BPF probe insns to bpf_probe_read_kernel
Quoted 9 lines · Daniel Borkmann
Commit
2a02759ef5f8("bpf: Add support for BTF pointers to interpreter") explicitly states that the pointer to BTF object is a pointer to a kernel object or NULL. Therefore we should also switch to using the strict kernel probe helper which is restricted to kernel addresses only when architectures have non-overlapping address spaces.Signed-off-by Daniel Borkmann <[email protected]>---
Quoted 3 lines
kernel/bpf/core.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-)
[...]
Daniel Borkmann
maintainerPATCH v2 6/8 bpf, samples: Use bpf_probe_read_user where appropriate
In reply to Daniel BorkmannUse bpf_probe_read_user() helper instead of bpf_probe_read() for samples that attach to kprobes probing on user addresses.
--- samples/bpf/map_perf_test_kern.c | 4 ++-- samples/bpf/test_map_in_map_kern.c | 4 ++-- samples/bpf/test_probe_write_user_kern.c | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-)
samples/bpf/map_perf_test_kern.c +2 -2
@@ -181,8 +181,8 @@ int stress_lru_hmap_alloc(struct pt_regs *ctx)if (addrlen !=sizeof (*in6))return 0 ; - ret =bpf_probe_read (test_params.dst6,sizeof (test_params.dst6), - &in6->sin6_addr); + ret =bpf_probe_read_user (test_params.dst6,sizeof (test_params.dst6), + &in6->sin6_addr);if (ret) goto done;
samples/bpf/test_map_in_map_kern.c +2 -2
@@ -118,7 +118,7 @@ int trace_sys_connect(struct pt_regs *ctx)if (addrlen !=sizeof (*in6))return 0 ; - ret =bpf_probe_read (dst6,sizeof (dst6), &in6->sin6_addr); + ret =bpf_probe_read_user (dst6,sizeof (dst6), &in6->sin6_addr);if (ret) { inline_ret = ret; goto done;
@@ -129,7 +129,7 @@ int trace_sys_connect(struct pt_regs *ctx) test_case = dst6[7 ]; - ret =bpf_probe_read (&port,sizeof (port), &in6->sin6_port); + ret =bpf_probe_read_user (&port,sizeof (port), &in6->sin6_port);if (ret) { inline_ret = ret; goto done;
samples/bpf/test_probe_write_user_kern.c +1 -1
@@ -37,7 +37,7 @@ int bpf_prog1(struct pt_regs *ctx)if (sockaddr_len >sizeof (orig_addr))return 0 ; -if (bpf_probe_read (&orig_addr,sizeof (orig_addr), sockaddr_arg) !=0 ) +if (bpf_probe_read_user (&orig_addr,sizeof (orig_addr), sockaddr_arg) !=0 )return 0 ; mapped_addr =bpf_map_lookup_elem (&dnat_map, &orig_addr);
Signature · 1 line
2.21.0
Daniel Borkmann
maintainerPATCH v2 7/8 bpf, testing: Convert prog tests to probe_read_{user,kernel}{,_str} helper
In reply to Daniel BorkmannUse probe read *_{kernel,user}{,_str}() helpers instead of bpf_probe_read() or bpf_probe_read_user_str() for program tests where appropriate.
--- tools/testing/selftests/bpf/progs/kfree_skb.c | 4 +- tools/testing/selftests/bpf/progs/pyperf.h | 67 ++++++++++--------- .../testing/selftests/bpf/progs/strobemeta.h | 36 +++++----- .../selftests/bpf/progs/test_tcp_estats.c | 2 +- 4 files changed, 57 insertions(+), 52 deletions(-)
tools/testing/selftests/bpf/progs/kfree_skb.c +2 -2
@@ -79,11 +79,11 @@ int trace_kfree_skb(struct trace_kfree_skb *ctx) func = ptr->func; })); -bpf_probe_read (&pkt_type,sizeof (pkt_type),_ (&skb->__pkt_type_offset)); +bpf_probe_read_kernel (&pkt_type,sizeof (pkt_type),_ (&skb->__pkt_type_offset)); pkt_type &=7 ;/* read eth proto */ -bpf_probe_read (&pkt_data,sizeof (pkt_data), data +12 ); +bpf_probe_read_kernel (&pkt_data,sizeof (pkt_data), data +12 );bpf_printk ("rcuhead.next %llx func %llx\n" , ptr, func); bpf_printk("skb->len %d users %d pkt_type %x\n",
tools/testing/selftests/bpf/progs/pyperf.h +36 -31
105 lines not on this page · raw message
tools/testing/selftests/bpf/progs/strobemeta.h +18 -18
@@ -98,7 +98,7 @@ struct strobe_map_raw {/* * having volatile doesn't change anything on BPF side, but clang * emits warnings for passing `volatile const char *` into -* bpf_probe_read_str that expects just `const char *` +* bpf_probe_read_user_str that expects just `const char *` */ const char * tag;/*
@@ -309,18 +309,18 @@ static __always_inline void *calc_location(struct strobe_value_loc *loc,dtv_t *dtv;void *tls_ptr; -bpf_probe_read (&tls_index,sizeof (struct tls_index ), - (void *)loc->offset); +bpf_probe_read_user (&tls_index,sizeof (struct tls_index ), + (void *)loc->offset);/* valid module index is always positive */ if (tls_index.module >0 ) {/* dtv = ((struct tcbhead *)tls_base)->dtv[tls_index.module] */ -bpf_probe_read (&dtv,sizeof (dtv), - &((struct tcbhead *)tls_base)->dtv); +bpf_probe_read_user (&dtv,sizeof (dtv), + &((struct tcbhead *)tls_base)->dtv); dtv += tls_index.module; }else { dtv =NULL ; } -bpf_probe_read (&tls_ptr,sizeof (void *), dtv); +bpf_probe_read_user (&tls_ptr,sizeof (void *), dtv);/* if pointer has (void *)-1 value, then TLS wasn't initialized yet */ return tls_ptr && tls_ptr != (void *)-1 ? tls_ptr + tls_index.offset
@@ -336,7 +336,7 @@ static __always_inline void read_int_var(struct strobemeta_cfg *cfg,if (!location)return ; -bpf_probe_read (value,sizeof (struct strobe_value_generic ), location); +bpf_probe_read_user (value,sizeof (struct strobe_value_generic ), location); data->int_vals[idx] = value->val;if (value->header.len) data->int_vals_set_mask |= (1 << idx);
@@ -356,13 +356,13 @@ static __always_inline uint64_t read_str_var(struct strobemeta_cfg *cfg,if (!location)return 0 ; -bpf_probe_read (value,sizeof (struct strobe_value_generic ), location); - len =bpf_probe_read_str (payload,STROBE_MAX_STR_LEN , value->ptr); +bpf_probe_read_user (value,sizeof (struct strobe_value_generic ), location); + len =bpf_probe_read_user_str (payload,STROBE_MAX_STR_LEN , value->ptr);/* -* if bpf_probe_read_str returns error (<0), due to casting to +* if bpf_probe_read_user_str returns error (<0), due to casting to * unsinged int, it will become big number, so next check is * sufficient to check for errors AND prove to BPF verifier, that -* bpf_probe_read_str won't return anything bigger than +* bpf_probe_read_user_str won't return anything bigger than * STROBE_MAX_STR_LEN */ if (len >STROBE_MAX_STR_LEN )
@@ -391,8 +391,8 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,if (!location)return payload; -bpf_probe_read (value,sizeof (struct strobe_value_generic ), location); -if (bpf_probe_read (&map,sizeof (struct strobe_map_raw ), value->ptr)) +bpf_probe_read_user (value,sizeof (struct strobe_value_generic ), location); +if (bpf_probe_read_user (&map,sizeof (struct strobe_map_raw ), value->ptr))return payload; descr->id = map.id;
@@ -402,7 +402,7 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg, data->req_meta_valid =1 ; } - len =bpf_probe_read_str (payload,STROBE_MAX_STR_LEN , map.tag); + len =bpf_probe_read_user_str (payload,STROBE_MAX_STR_LEN , map.tag);if (len <=STROBE_MAX_STR_LEN ) { descr->tag_len = len; payload += len;
@@ -418,15 +418,15 @@ static __always_inline void *read_map_var(struct strobemeta_cfg *cfg,break ; descr->key_lens[i] =0 ; - len =bpf_probe_read_str (payload,STROBE_MAX_STR_LEN , - map.entries[i].key); + len =bpf_probe_read_user_str (payload,STROBE_MAX_STR_LEN , + map.entries[i].key);if (len <=STROBE_MAX_STR_LEN ) { descr->key_lens[i] = len; payload += len; } descr->val_lens[i] =0 ; - len =bpf_probe_read_str (payload,STROBE_MAX_STR_LEN , - map.entries[i].val); + len =bpf_probe_read_user_str (payload,STROBE_MAX_STR_LEN , + map.entries[i].val);if (len <=STROBE_MAX_STR_LEN ) { descr->val_lens[i] = len; payload += len;
tools/testing/selftests/bpf/progs/test_tcp_estats.c +1 -1
@@ -38,7 +38,7 @@#include <sys/socket.h> #include "bpf_helpers.h" -#define _ (P ) ({typeof(P) val = 0; bpf_probe_read(&val, sizeof(val), &P); val;}) +#define _ (P ) ({typeof(P) val = 0; bpf_probe_read_kernel(&val, sizeof(val), &P); val;})#define TCP_ESTATS_MAGIC 0xBAADBEEF/* This test case needs "sock" and "pt_regs" data structure.
Signature · 1 line
2.21.0
Re: [PATCH bpf-next v2 7/8] bpf, testing: Convert prog tests to probe_read_{user,kernel}{,_str} helper
Quoted 6 lines · Daniel Borkmann
Use probe read *_{kernel,user}{,_str}() helpers instead of
bpf_probe_read()orbpf_probe_read_user_str()for program tests where appropriate.Signed-off-by Daniel Borkmann <[email protected]>---
Thanks!
Quoted 6 lines
tools/testing/selftests/bpf/progs/kfree_skb.c | 4 +- tools/testing/selftests/bpf/progs/pyperf.h | 67 ++++++++++--------- .../testing/selftests/bpf/progs/strobemeta.h | 36 +++++----- .../selftests/bpf/progs/test_tcp_estats.c | 2 +- 4 files changed, 57 insertions(+), 52 deletions(-)
[...]
Daniel Borkmann
maintainerPATCH v2 8/8 bpf, testing: Add selftest to read/write sockaddr from user space
In reply to Daniel BorkmannTested on x86-64 and Ilya was also kind enough to give it a spin on s390x, both passing with probe_user:OK there. The test is using the newly added bpf_probe_read_user() to dump sockaddr from connect call into .bss BPF map and overrides the user buffer via bpf_probe_write_user():
# ./test_progs [...] #17 pkt_md_access:OK #18 probe_user:OK #19 prog_run_xattr:OK [...]
--- .../selftests/bpf/prog_tests/probe_user.c | 78 +++++++++++++++++++ .../selftests/bpf/progs/test_probe_user.c | 26 +++++++ 2 files changed, 104 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/probe_user.c create mode 100644 tools/testing/selftests/bpf/progs/test_probe_user.c
tools/testing/selftests/bpf/prog_tests/probe_user.c new file +78 -0
@@ -0,0 +1,78 @@ +// SPDX-License-Identifier: GPL-2.0 +#include <test_progs.h> + +void test_probe_user (void ) +{ +#define kprobe_name "__sys_connect" +const char *prog_name ="kprobe/" kprobe_name; +const char *obj_file ="./test_probe_user.o" ; + DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts, ); +int err, results_map_fd, sock_fd, duration; +struct sockaddr curr, orig, tmp; +struct sockaddr_in *in = (struct sockaddr_in *)&curr; +struct bpf_link *kprobe_link =NULL ; +struct bpf_program *kprobe_prog; +struct bpf_object *obj; +static const int zero =0 ; + + obj =bpf_object__open_file (obj_file, &opts); +if (CHECK (IS_ERR (obj),"obj_open_file" ,"err %ld\n" ,PTR_ERR (obj))) +return ; + + kprobe_prog =bpf_object__find_program_by_title (obj, prog_name); +if (CHECK (!kprobe_prog,"find_probe" , +"prog '%s' not found\n" , prog_name)) + goto cleanup; + + err =bpf_object__load (obj); +if (CHECK (err,"obj_load" ,"err %d\n" , err)) + goto cleanup; + + results_map_fd =bpf_find_map (__func__, obj,"test_pro.bss" ); +if (CHECK (results_map_fd <0 ,"find_bss_map" , +"err %d\n" , results_map_fd)) + goto cleanup; + + kprobe_link =bpf_program__attach_kprobe (kprobe_prog, false, + kprobe_name); +if (CHECK (IS_ERR (kprobe_link),"attach_kprobe" , +"err %ld\n" ,PTR_ERR (kprobe_link))) { + kprobe_link =NULL ; + goto cleanup; + } + +memset (&curr,0 ,sizeof (curr)); + in->sin_family =AF_INET ; + in->sin_port =htons (5555 ); + in->sin_addr.s_addr =inet_addr ("255.255.255.255" ); +memcpy (&orig, &curr,sizeof (curr)); + + sock_fd =socket (AF_INET ,SOCK_STREAM ,0 ); +if (CHECK (sock_fd <0 ,"create_sock_fd" ,"err %d\n" , sock_fd)) + goto cleanup; + +connect (sock_fd, &curr,sizeof (curr)); +close (sock_fd); + + err =bpf_map_lookup_elem (results_map_fd, &zero, &tmp); +if (CHECK (err,"get_kprobe_res" , +"failed to get kprobe res: %d\n" , err)) + goto cleanup; + + in = (struct sockaddr_in *)&tmp; +if (CHECK (memcmp (&tmp, &orig,sizeof (orig)),"check_kprobe_res" , +"wrong kprobe res from probe read: %s:%u\n" , +inet_ntoa (in->sin_addr),ntohs (in->sin_port))) + goto cleanup; + +memset (&tmp,0xab ,sizeof (tmp)); + + in = (struct sockaddr_in *)&curr; +if (CHECK (memcmp (&curr, &tmp,sizeof (tmp)),"check_kprobe_res" , +"wrong kprobe res from probe write: %s:%u\n" , +inet_ntoa (in->sin_addr),ntohs (in->sin_port))) + goto cleanup; +cleanup: +bpf_link__destroy (kprobe_link); +bpf_object__close (obj); +}
tools/testing/selftests/bpf/progs/test_probe_user.c new file +26 -0
@@ -0,0 +1,26 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/ptrace.h> +#include <linux/bpf.h> + +#include <netinet/in.h> + +#include "bpf_helpers.h" +#include "bpf_tracing.h" + +static struct sockaddr_in old; + +SEC("kprobe/__sys_connect") +int handle_sys_connect (struct pt_regs *ctx) +{ +void *ptr = (void *)PT_REGS_PARM2 (ctx); +struct sockaddr_in new; + +bpf_probe_read_user (&old,sizeof (old), ptr); +__builtin_memset (&new,0xab ,sizeof (new)); +bpf_probe_write_user (ptr, &new,sizeof (new)); + +return 0 ; +} + +char _license[] SEC("license") = "GPL";
Signature · 1 line
2.21.0