[PATCH bpf-next 0/4] Introduce kfuncs for memory reads into dynptrs

[PATCH bpf-next 0/4] Introduce kfuncs for memory reads into dynptrs

[Mykyta Yatsenko]

From: Mykyta Yatsenko <yatsenko@meta.com>

This patch adds new kfuncs that enable reading variable-length
user or kernel data directly into dynptrs.
These kfuncs provide a way to perform dynamically-sized reads
while maintaining memory safety. Unlike existing
`bpf_probe_read_{user|kernel}` APIs, which are limited to constant-sized
reads, these new kfuncs allow for more flexible data access.

Mykyta Yatsenko (4):
  helpers: make few bpf helpers public
  bpf: implement dynptr copy kfuncs
  selftests/bpf: introduce tests for dynptr copy kfuncs
  selftests/bpf: disable test_probe_read_user_str_dynptr

 include/linux/bpf.h                           |   7 +
 kernel/bpf/helpers.c                          |  14 +-
 kernel/trace/bpf_trace.c                      | 199 +++++++++++++++++
 tools/testing/selftests/bpf/DENYLIST          |   1 +
 .../testing/selftests/bpf/prog_tests/dynptr.c |  13 ++
 .../selftests/bpf/progs/dynptr_success.c      | 201 ++++++++++++++++++
 6 files changed, 432 insertions(+), 3 deletions(-)

-- 
2.49.0

[PATCH bpf-next 1/4] helpers: make few bpf helpers public

[Mykyta Yatsenko]

From: Mykyta Yatsenko <yatsenko@meta.com>

Make bpf_dynptr_slice_rdwr, bpf_dynptr_check_off_len and
__bpf_dynptr_write available outside of the helpers.c by
adding their prototypes into linux/include/bpf.h.
These functions are going to be used from bpf_trace.c in the next
patch of this series.

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 include/linux/bpf.h  | 7 +++++++
 kernel/bpf/helpers.c | 6 +++---
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 3f0cc89c0622..14f219921b4c 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1349,6 +1349,13 @@ u32 __bpf_dynptr_size(const struct bpf_dynptr_kern *ptr);
 const void *__bpf_dynptr_data(const struct bpf_dynptr_kern *ptr, u32 len);
 void *__bpf_dynptr_data_rw(const struct bpf_dynptr_kern *ptr, u32 len);
 bool __bpf_dynptr_is_rdonly(const struct bpf_dynptr_kern *ptr);
+int __bpf_dynptr_write(const struct bpf_dynptr_kern *dst, u32 offset,
+		       void *src, u32 len, u64 flags);
+int bpf_dynptr_check_off_len(const struct bpf_dynptr_kern *ptr, u32 offset, u32 len);
+void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr *p, u32 offset,
+			    void *buffer__opt, u32 buffer__szk);
+int bpf_dynptr_check_off_len(const struct bpf_dynptr_kern *ptr,
+			     u32 offset, u32 len);
 
 #ifdef CONFIG_BPF_JIT
 int bpf_trampoline_link_prog(struct bpf_tramp_link *link,
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index e3a2662f4e33..2aad7c57425b 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1713,7 +1713,7 @@ void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr)
 	memset(ptr, 0, sizeof(*ptr));
 }
 
-static int bpf_dynptr_check_off_len(const struct bpf_dynptr_kern *ptr, u32 offset, u32 len)
+int bpf_dynptr_check_off_len(const struct bpf_dynptr_kern *ptr, u32 offset, u32 len)
 {
 	u32 size = __bpf_dynptr_size(ptr);
 
@@ -1809,8 +1809,8 @@ static const struct bpf_func_proto bpf_dynptr_read_proto = {
 	.arg5_type	= ARG_ANYTHING,
 };
 
-static int __bpf_dynptr_write(const struct bpf_dynptr_kern *dst, u32 offset, void *src,
-			      u32 len, u64 flags)
+int __bpf_dynptr_write(const struct bpf_dynptr_kern *dst, u32 offset, void *src,
+		       u32 len, u64 flags)
 {
 	enum bpf_dynptr_type type;
 	int err;
-- 
2.49.0

[PATCH bpf-next 2/4] bpf: implement dynptr copy kfuncs

[Mykyta Yatsenko]

From: Mykyta Yatsenko <yatsenko@meta.com>

This patch introduces a new set of kfuncs for working with dynptrs in
BPF programs, enabling reading variable-length user or kernel data
into dynptr directly. To enable memory-safety, verifier allows only
constant-sized reads via existing bpf_probe_read_{user|kernel} etc.
kfuncs, dynptr-based kfuncs allow dynamically-sized reads without memory
safety shortcomings.

The following kfuncs are introduced:
* `bpf_probe_read_kernel_dynptr()`: probes kernel-space data into a dynptr
* `bpf_probe_read_user_dynptr()`: probes user-space data into a dynptr
* `bpf_probe_read_kernel_str_dynptr()`: probes kernel-space string into
a dynptr
* `bpf_probe_read_user_str_dynptr()`: probes user-space string into a
dynptr
* `bpf_copy_from_user_dynptr()`: sleepable, copies user-space data into
a dynptr for the current task
* `bpf_copy_from_user_str_dynptr()`: sleepable, copies user-space string
into a dynptr for the current task
* `bpf_copy_from_user_task_dynptr()`: sleepable, copies user-space data
of the task into a dynptr
* `bpf_copy_from_user_task_str_dynptr()`: sleepable, copies user-space
string of the task into a dynptr

The implementation is built on two generic functions:
 * __bpf_dynptr_copy
 * __bpf_dynptr_copy_str
These functions take function pointers as arguments, enabling the
copying of data from various sources, including both kernel and user
space. Notably, these indirect calls are typically inlined.

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 kernel/bpf/helpers.c     |   8 ++
 kernel/trace/bpf_trace.c | 199 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 207 insertions(+)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index 2aad7c57425b..7d72d3e87324 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -3294,6 +3294,14 @@ BTF_ID_FLAGS(func, bpf_iter_kmem_cache_next, KF_ITER_NEXT | KF_RET_NULL | KF_SLE
 BTF_ID_FLAGS(func, bpf_iter_kmem_cache_destroy, KF_ITER_DESTROY | KF_SLEEPABLE)
 BTF_ID_FLAGS(func, bpf_local_irq_save)
 BTF_ID_FLAGS(func, bpf_local_irq_restore)
+BTF_ID_FLAGS(func, bpf_probe_read_user_dynptr)
+BTF_ID_FLAGS(func, bpf_probe_read_kernel_dynptr)
+BTF_ID_FLAGS(func, bpf_probe_read_user_str_dynptr)
+BTF_ID_FLAGS(func, bpf_probe_read_kernel_str_dynptr)
+BTF_ID_FLAGS(func, bpf_copy_from_user_dynptr, KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_copy_from_user_str_dynptr, KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_copy_from_user_task_dynptr, KF_SLEEPABLE)
+BTF_ID_FLAGS(func, bpf_copy_from_user_task_str_dynptr, KF_SLEEPABLE)
 BTF_KFUNCS_END(common_btf_ids)
 
 static const struct btf_kfunc_id_set common_kfunc_set = {
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 52c432a44aeb..c455137c108c 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -3499,6 +3499,147 @@ static int __init bpf_kprobe_multi_kfuncs_init(void)
 
 late_initcall(bpf_kprobe_multi_kfuncs_init);
 
+typedef int (*copy_fn_t)(void *dst, const void *src, u32 size, struct task_struct *tsk);
+
+static __always_inline int __bpf_dynptr_copy_str(struct bpf_dynptr *dptr, u32 doff, u32 size,
+						 const void __user *unsafe_src,
+						 copy_fn_t str_copy_fn,
+						 struct task_struct *tsk)
+{
+	struct bpf_dynptr_kern *dst;
+	u32 chunk_sz, off;
+	void *dst_slice;
+	int cnt, err;
+	char buf[256];
+
+	dst_slice = bpf_dynptr_slice_rdwr(dptr, doff, NULL, size);
+	if (likely(dst_slice))
+		return str_copy_fn(dst_slice, unsafe_src, size, tsk);
+
+	dst = (struct bpf_dynptr_kern *)dptr;
+	if (bpf_dynptr_check_off_len(dst, doff, size))
+		return -E2BIG;
+
+	for (off = 0; off < size; off += chunk_sz - 1) {
+		chunk_sz = min_t(u32, sizeof(buf), size - off);
+		/* Expect str_copy_fn to return count of copied bytes, including
+		 * zero terminator. Next iteration increment off by chunk_sz - 1 to
+		 * overwrite NUL.
+		 */
+		cnt = str_copy_fn(buf, unsafe_src + off, chunk_sz, tsk);
+		if (cnt < 0)
+			return cnt;
+		err = __bpf_dynptr_write(dst, doff + off, buf, cnt, 0);
+		if (err)
+			return err;
+		if (cnt < chunk_sz || chunk_sz == 1) /* we are done */
+			return off + cnt;
+	}
+	return off;
+}
+
+static __always_inline int __bpf_dynptr_copy(const struct bpf_dynptr *dptr, u32 doff,
+					     u32 size, const void __user *unsafe_src,
+					     copy_fn_t copy_fn, struct task_struct *tsk)
+{
+	struct bpf_dynptr_kern *dst;
+	void *dst_slice;
+	char buf[256];
+	u32 off, chunk_sz;
+	int err;
+
+	dst_slice = bpf_dynptr_slice_rdwr(dptr, doff, NULL, size);
+	if (likely(dst_slice))
+		return copy_fn(dst_slice, unsafe_src, size, tsk);
+
+	dst = (struct bpf_dynptr_kern *)dptr;
+	if (bpf_dynptr_check_off_len(dst, doff, size))
+		return -E2BIG;
+
+	for (off = 0; off < size; off += chunk_sz) {
+		chunk_sz = min_t(u32, sizeof(buf), size - off);
+		err = copy_fn(buf, unsafe_src + off, chunk_sz, tsk);
+		if (err)
+			return err;
+		err = __bpf_dynptr_write(dst, doff + off, buf, chunk_sz, 0);
+		if (err)
+			return err;
+	}
+	return 0;
+}
+
+static __always_inline int copy_user_data_nofault(void *dst, const void __user *unsafe_src,
+						  u32 size, struct task_struct *tsk)
+{
+	if (WARN_ON_ONCE(tsk))
+		return -EFAULT;
+
+	return copy_from_user_nofault(dst, unsafe_src, size);
+}
+
+static __always_inline int copy_user_data_sleepable(void *dst, const void __user *unsafe_src,
+						    u32 size, struct task_struct *tsk)
+{
+	int ret;
+
+	if (!tsk) /* Read from the current task */
+		return copy_from_user(dst, unsafe_src, size);
+
+	ret = access_process_vm(tsk, (unsigned long)unsafe_src, dst, size, 0);
+	if (ret != size)
+		return -EFAULT;
+	return 0;
+}
+
+static __always_inline int copy_kernel_data_nofault(void *dst, const void *unsafe_src,
+						    u32 size, struct task_struct *tsk)
+{
+	if (WARN_ON_ONCE(tsk))
+		return -EFAULT;
+
+	return copy_from_kernel_nofault(dst, unsafe_src, size);
+}
+
+static __always_inline int copy_user_data_str_nofault(void *dst, const void __user *unsafe_src,
+						      u32 size, struct task_struct *tsk)
+{
+	if (WARN_ON_ONCE(tsk))
+		return -EFAULT;
+
+	return strncpy_from_user_nofault(dst, unsafe_src, size);
+}
+
+static __always_inline int copy_user_data_str_sleepable(void *dst, const void __user *unsafe_src,
+							u32 size, struct task_struct *tsk)
+{
+	int ret;
+
+	if (unlikely(size == 0))
+		return 0;
+
+	if (tsk) {
+		ret = copy_remote_vm_str(tsk, (unsigned long)unsafe_src, dst, size, 0);
+	} else {
+		ret = strncpy_from_user(dst, unsafe_src, size - 1);
+		/* strncpy_from_user does not guarantee NUL termination */
+		if (ret >= 0)
+			((char *)dst)[ret] = '\0';
+	}
+
+	if (ret < 0)
+		return ret;
+	return ret + 1;
+}
+
+static __always_inline int copy_kernel_data_str_nofault(void *dst, const void *unsafe_src,
+							u32 size, struct task_struct *tsk)
+{
+	if (WARN_ON_ONCE(tsk))
+		return -EFAULT;
+
+	return strncpy_from_kernel_nofault(dst, unsafe_src, size);
+}
+
 __bpf_kfunc_start_defs();
 
 __bpf_kfunc int bpf_send_signal_task(struct task_struct *task, int sig, enum pid_type type,
@@ -3510,4 +3651,62 @@ __bpf_kfunc int bpf_send_signal_task(struct task_struct *task, int sig, enum pid
 	return bpf_send_signal_common(sig, type, task, value);
 }
 
+__bpf_kfunc int bpf_probe_read_user_dynptr(struct bpf_dynptr *dptr, u32 off,
+					   u32 size, const void __user *unsafe_ptr__ign)
+{
+	return __bpf_dynptr_copy(dptr, off, size, unsafe_ptr__ign,
+				 copy_user_data_nofault, NULL);
+}
+
+__bpf_kfunc int bpf_probe_read_kernel_dynptr(struct bpf_dynptr *dptr, u32 off,
+					     u32 size, const void *unsafe_ptr__ign)
+{
+	return __bpf_dynptr_copy(dptr, off, size, unsafe_ptr__ign,
+				 copy_kernel_data_nofault, NULL);
+}
+
+__bpf_kfunc int bpf_probe_read_user_str_dynptr(struct bpf_dynptr *dptr, u32 off,
+					       u32 size, const void  *unsafe_ptr__ign)
+{
+	return __bpf_dynptr_copy_str(dptr, off, size, unsafe_ptr__ign,
+				     copy_user_data_str_nofault, NULL);
+}
+
+__bpf_kfunc int bpf_probe_read_kernel_str_dynptr(struct bpf_dynptr *dptr, u32 off,
+						 u32 size, const void  *unsafe_ptr__ign)
+{
+	return __bpf_dynptr_copy_str(dptr, off, size, unsafe_ptr__ign,
+				     copy_kernel_data_str_nofault, NULL);
+}
+
+__bpf_kfunc int bpf_copy_from_user_dynptr(struct bpf_dynptr *dptr, u32 off,
+					  u32 size, const void __user *unsafe_ptr__ign)
+{
+	return __bpf_dynptr_copy(dptr, off, size, unsafe_ptr__ign,
+				 copy_user_data_sleepable, NULL);
+}
+
+__bpf_kfunc int bpf_copy_from_user_str_dynptr(struct bpf_dynptr *dptr, u32 off,
+					      u32 size, const void __user *unsafe_ptr__ign)
+{
+	return __bpf_dynptr_copy_str(dptr, off, size, unsafe_ptr__ign,
+				     copy_user_data_str_sleepable, NULL);
+}
+
+__bpf_kfunc int bpf_copy_from_user_task_dynptr(struct bpf_dynptr *dptr, u32 off,
+					       u32 size, const void __user *unsafe_ptr__ign,
+					       struct task_struct *tsk)
+{
+	return __bpf_dynptr_copy(dptr, off, size, unsafe_ptr__ign,
+				 copy_user_data_sleepable, tsk);
+}
+
+__bpf_kfunc int bpf_copy_from_user_task_str_dynptr(struct bpf_dynptr *dptr, u32 off,
+						   u32 size, const void *unsafe_ptr__ign,
+						   struct task_struct *tsk)
+{
+	return __bpf_dynptr_copy_str(dptr, off, size, unsafe_ptr__ign,
+				     copy_user_data_str_sleepable, tsk);
+}
+
 __bpf_kfunc_end_defs();
-- 
2.49.0

[PATCH bpf-next 3/4] selftests/bpf: introduce tests for dynptr copy kfuncs

[Mykyta Yatsenko]

From: Mykyta Yatsenko <yatsenko@meta.com>

Introduce selftests verifying newly-added dynptr copy kfuncs.
Covering contiguous and non-contiguous memory backed dynptrs.

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 .../testing/selftests/bpf/prog_tests/dynptr.c |  13 ++
 .../selftests/bpf/progs/dynptr_success.c      | 201 ++++++++++++++++++
 2 files changed, 214 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c
index e29cc16124c2..62e7ec775f24 100644
--- a/tools/testing/selftests/bpf/prog_tests/dynptr.c
+++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c
@@ -33,10 +33,19 @@ static struct {
 	{"test_dynptr_skb_no_buff", SETUP_SKB_PROG},
 	{"test_dynptr_skb_strcmp", SETUP_SKB_PROG},
 	{"test_dynptr_skb_tp_btf", SETUP_SKB_PROG_TP},
+	{"test_probe_read_user_dynptr", SETUP_XDP_PROG},
+	{"test_probe_read_kernel_dynptr", SETUP_XDP_PROG},
+	{"test_probe_read_user_str_dynptr", SETUP_XDP_PROG},
+	{"test_probe_read_kernel_str_dynptr", SETUP_XDP_PROG},
+	{"test_copy_from_user_dynptr", SETUP_SYSCALL_SLEEP},
+	{"test_copy_from_user_str_dynptr", SETUP_SYSCALL_SLEEP},
+	{"test_copy_from_user_task_dynptr", SETUP_SYSCALL_SLEEP},
+	{"test_copy_from_user_task_str_dynptr", SETUP_SYSCALL_SLEEP},
 };
 
 static void verify_success(const char *prog_name, enum test_setup_type setup_type)
 {
+	char user_data[384] = {[0 ... 382] = 'a', '\0'};
 	struct dynptr_success *skel;
 	struct bpf_program *prog;
 	struct bpf_link *link;
@@ -58,6 +67,10 @@ static void verify_success(const char *prog_name, enum test_setup_type setup_typ
 	if (!ASSERT_OK(err, "dynptr_success__load"))
 		goto cleanup;
 
+	skel->bss->user_ptr = user_data;
+	skel->data->test_len[0] = sizeof(user_data);
+	memcpy(skel->bss->expected_str, user_data, sizeof(user_data));
+
 	switch (setup_type) {
 	case SETUP_SYSCALL_SLEEP:
 		link = bpf_program__attach(prog);
diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c b/tools/testing/selftests/bpf/progs/dynptr_success.c
index e1fba28e4a86..753d35eb47d9 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_success.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_success.c
@@ -680,3 +680,204 @@ int test_dynptr_copy_xdp(struct xdp_md *xdp)
 	bpf_ringbuf_discard_dynptr(&ptr_buf, 0);
 	return XDP_DROP;
 }
+
+void *user_ptr;
+char expected_str[384]; /* Contains the copy of the data pointed by user_ptr */
+__u32 test_len[7] = {0/* placeholder */, 0, 1, 2, 255, 256, 257};
+
+typedef int (*bpf_read_dynptr_fn_t)(struct bpf_dynptr *dptr, u32 off,
+				    u32 size, const void *unsafe_ptr);
+
+static __always_inline void test_dynptr_probe(void *ptr, bpf_read_dynptr_fn_t bpf_read_dynptr_fn)
+{
+	char buf[sizeof(expected_str)];
+	struct bpf_dynptr ptr_buf;
+	int i;
+
+	err = bpf_ringbuf_reserve_dynptr(&ringbuf, sizeof(buf), 0, &ptr_buf);
+
+	bpf_for(i, 0, ARRAY_SIZE(test_len)) {
+		__u32 len = test_len[i];
+
+		err = err ?: bpf_read_dynptr_fn(&ptr_buf, 0, test_len[i], ptr);
+		if (len > sizeof(buf))
+			break;
+		err = err ?: bpf_dynptr_read(&buf, len, &ptr_buf, 0, 0);
+
+		if (err || bpf_memcmp(expected_str, buf, len))
+			err = 1;
+
+		/* Reset buffer and dynptr */
+		__builtin_memset(buf, 0, sizeof(buf));
+		err = err ?: bpf_dynptr_write(&ptr_buf, 0, buf, len, 0);
+	}
+	bpf_ringbuf_discard_dynptr(&ptr_buf, 0);
+}
+
+static __always_inline void test_dynptr_probe_str(void *ptr,
+						  bpf_read_dynptr_fn_t bpf_read_dynptr_fn)
+{
+	char buf[sizeof(expected_str)];
+	struct bpf_dynptr ptr_buf;
+	__u32 cnt, i;
+
+	bpf_ringbuf_reserve_dynptr(&ringbuf, sizeof(buf), 0, &ptr_buf);
+
+	bpf_for(i, 0, ARRAY_SIZE(test_len)) {
+		__u32 len = test_len[i];
+
+		cnt = bpf_read_dynptr_fn(&ptr_buf, 0, len, ptr);
+		if (cnt != len)
+			err = 1;
+
+		if (len > sizeof(buf))
+			continue;
+		err = err ?: bpf_dynptr_read(&buf, len, &ptr_buf, 0, 0);
+		if (!len)
+			continue;
+		if (err || bpf_memcmp(expected_str, buf, len - 1) || buf[len - 1] != '\0')
+			err = 1;
+	}
+	bpf_ringbuf_discard_dynptr(&ptr_buf, 0);
+}
+
+static __always_inline void test_dynptr_probe_xdp(struct xdp_md *xdp, void *ptr,
+						  bpf_read_dynptr_fn_t bpf_read_dynptr_fn)
+{
+	struct bpf_dynptr ptr_xdp;
+	char buf[sizeof(expected_str)];
+	__u32 off, i;
+
+	/* Set offset near the seam between buffers */
+	off = 3500;
+
+	err = bpf_dynptr_from_xdp(xdp, 0, &ptr_xdp);
+
+	bpf_for(i, 0, ARRAY_SIZE(test_len)) {
+		__u32 len = test_len[i];
+
+		err = err ?: bpf_read_dynptr_fn(&ptr_xdp, off, len, ptr);
+		if (len > sizeof(buf))
+			continue;
+		err = err ?: bpf_dynptr_read(&buf, len, &ptr_xdp, off, 0);
+		if (err || bpf_memcmp(expected_str, buf, len))
+			err = 1;
+		/* Reset buffer and dynptr */
+		__builtin_memset(buf, 0, sizeof(buf));
+		err = err ?: bpf_dynptr_write(&ptr_xdp, off, buf, len, 0);
+	}
+}
+
+static __always_inline void test_dynptr_probe_str_xdp(struct xdp_md *xdp, void *ptr,
+						      bpf_read_dynptr_fn_t bpf_read_dynptr_fn)
+{
+	struct bpf_dynptr ptr_xdp;
+	char buf[sizeof(expected_str)];
+	__u32 cnt, off, i;
+
+	/* Set offset near the seam between buffers */
+	off = 3500;
+	err = bpf_dynptr_from_xdp(xdp, 0, &ptr_xdp);
+	if (err)
+		return;
+
+	bpf_for(i, 0, ARRAY_SIZE(test_len)) {
+		__u32 len = test_len[i];
+
+		cnt = bpf_read_dynptr_fn(&ptr_xdp, off, len, ptr);
+		if (cnt != len)
+			err = 1;
+
+		if (len > sizeof(buf))
+			continue;
+		err = err ?: bpf_dynptr_read(&buf, len, &ptr_xdp, off, 0);
+
+		if (!len)
+			continue;
+		if (err || bpf_memcmp(expected_str, buf, len - 1) || buf[len - 1] != '\0')
+			err = 1;
+
+		__builtin_memset(buf, 0, sizeof(buf));
+		err = err ?: bpf_dynptr_write(&ptr_xdp, off, buf, len, 0);
+	}
+}
+
+SEC("xdp")
+int test_probe_read_user_dynptr(struct xdp_md *xdp)
+{
+	test_dynptr_probe(user_ptr, bpf_probe_read_user_dynptr);
+	if (!err)
+		test_dynptr_probe_xdp(xdp, user_ptr, bpf_probe_read_user_dynptr);
+	return XDP_PASS;
+}
+
+SEC("xdp")
+int test_probe_read_kernel_dynptr(struct xdp_md *xdp)
+{
+	test_dynptr_probe(expected_str, bpf_probe_read_kernel_dynptr);
+	if (!err)
+		test_dynptr_probe_xdp(xdp, expected_str, bpf_probe_read_kernel_dynptr);
+	return XDP_PASS;
+}
+
+SEC("xdp")
+int test_probe_read_user_str_dynptr(struct xdp_md *xdp)
+{
+	test_dynptr_probe_str(user_ptr, bpf_probe_read_user_str_dynptr);
+	if (!err)
+		test_dynptr_probe_str_xdp(xdp, user_ptr, bpf_probe_read_user_str_dynptr);
+	return XDP_PASS;
+}
+
+SEC("xdp")
+int test_probe_read_kernel_str_dynptr(struct xdp_md *xdp)
+{
+	test_dynptr_probe_str(expected_str, bpf_probe_read_kernel_str_dynptr);
+	if (!err)
+		test_dynptr_probe_str_xdp(xdp, expected_str, bpf_probe_read_kernel_str_dynptr);
+	return XDP_PASS;
+}
+
+SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
+int test_copy_from_user_dynptr(void *ctx)
+{
+	test_dynptr_probe(user_ptr, bpf_copy_from_user_dynptr);
+	return 0;
+}
+
+SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
+int test_copy_from_user_str_dynptr(void *ctx)
+{
+	test_dynptr_probe_str(user_ptr, bpf_copy_from_user_str_dynptr);
+	return 0;
+}
+
+static int bpf_copy_data_from_user_task(struct bpf_dynptr *dptr, u32 off,
+					u32 size, const void *unsafe_ptr)
+{
+	struct task_struct *task = bpf_get_current_task_btf();
+
+	return bpf_copy_from_user_task_dynptr(dptr, off, size, unsafe_ptr, task);
+}
+
+static int bpf_copy_data_from_user_task_str(struct bpf_dynptr *dptr, u32 off,
+					    u32 size, const void *unsafe_ptr)
+{
+	struct task_struct *task = bpf_get_current_task_btf();
+
+	return bpf_copy_from_user_task_str_dynptr(dptr, off, size, unsafe_ptr, task);
+}
+
+SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
+int test_copy_from_user_task_dynptr(void *ctx)
+{
+	test_dynptr_probe(user_ptr, bpf_copy_data_from_user_task);
+	return 0;
+}
+
+SEC("fentry.s/" SYS_PREFIX "sys_nanosleep")
+int test_copy_from_user_task_str_dynptr(void *ctx)
+{
+	test_dynptr_probe_str(user_ptr, bpf_copy_data_from_user_task_str);
+	return 0;
+}
-- 
2.49.0

[PATCH bpf-next 4/4] selftests/bpf: disable test_probe_read_user_str_dynptr

[Mykyta Yatsenko]

From: Mykyta Yatsenko <yatsenko@meta.com>

Disable test that triggers bug in strncpy_from_user_nofault.
Patch to fix the issue [1].

[1] https://patchwork.kernel.org/project/linux-mm/patch/20250422131449.57177-1-mykyta.yatsenko5@gmail.com/

Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
---
 tools/testing/selftests/bpf/DENYLIST | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tools/testing/selftests/bpf/DENYLIST b/tools/testing/selftests/bpf/DENYLIST
index f748f2c33b22..839eb892adc7 100644
--- a/tools/testing/selftests/bpf/DENYLIST
+++ b/tools/testing/selftests/bpf/DENYLIST
@@ -1,5 +1,6 @@
 # TEMPORARY
 # Alphabetical order
+dynptr/test_probe_read_user_str_dynptr # disabled until https://patchwork.kernel.org/project/linux-mm/patch/20250422131449.57177-1-mykyta.yatsenko5@gmail.com/ is landed
 get_stack_raw_tp    # spams with kernel warnings until next bpf -> bpf-next merge
 stacktrace_build_id
 stacktrace_build_id_nmi
-- 
2.49.0

Re: [PATCH bpf-next 1/4] helpers: make few bpf helpers public

[Andrii Nakryiko]

On Fri, Apr 25, 2025 at 5:59 AM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> From: Mykyta Yatsenko <yatsenko@meta.com>
>
> Make bpf_dynptr_slice_rdwr, bpf_dynptr_check_off_len and
> __bpf_dynptr_write available outside of the helpers.c by
> adding their prototypes into linux/include/bpf.h.
> These functions are going to be used from bpf_trace.c in the next
> patch of this series.
>
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  include/linux/bpf.h  | 7 +++++++
>  kernel/bpf/helpers.c | 6 +++---
>  2 files changed, 10 insertions(+), 3 deletions(-)
>

LGTM

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 3f0cc89c0622..14f219921b4c 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -1349,6 +1349,13 @@ u32 __bpf_dynptr_size(const struct bpf_dynptr_kern *ptr);
>  const void *__bpf_dynptr_data(const struct bpf_dynptr_kern *ptr, u32 len);
>  void *__bpf_dynptr_data_rw(const struct bpf_dynptr_kern *ptr, u32 len);
>  bool __bpf_dynptr_is_rdonly(const struct bpf_dynptr_kern *ptr);
> +int __bpf_dynptr_write(const struct bpf_dynptr_kern *dst, u32 offset,
> +                      void *src, u32 len, u64 flags);
> +int bpf_dynptr_check_off_len(const struct bpf_dynptr_kern *ptr, u32 offset, u32 len);
> +void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr *p, u32 offset,
> +                           void *buffer__opt, u32 buffer__szk);
> +int bpf_dynptr_check_off_len(const struct bpf_dynptr_kern *ptr,
> +                            u32 offset, u32 len);
>
>  #ifdef CONFIG_BPF_JIT
>  int bpf_trampoline_link_prog(struct bpf_tramp_link *link,
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index e3a2662f4e33..2aad7c57425b 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -1713,7 +1713,7 @@ void bpf_dynptr_set_null(struct bpf_dynptr_kern *ptr)
>         memset(ptr, 0, sizeof(*ptr));
>  }
>
> -static int bpf_dynptr_check_off_len(const struct bpf_dynptr_kern *ptr, u32 offset, u32 len)
> +int bpf_dynptr_check_off_len(const struct bpf_dynptr_kern *ptr, u32 offset, u32 len)
>  {
>         u32 size = __bpf_dynptr_size(ptr);
>
> @@ -1809,8 +1809,8 @@ static const struct bpf_func_proto bpf_dynptr_read_proto = {
>         .arg5_type      = ARG_ANYTHING,
>  };
>
> -static int __bpf_dynptr_write(const struct bpf_dynptr_kern *dst, u32 offset, void *src,
> -                             u32 len, u64 flags)
> +int __bpf_dynptr_write(const struct bpf_dynptr_kern *dst, u32 offset, void *src,
> +                      u32 len, u64 flags)
>  {
>         enum bpf_dynptr_type type;
>         int err;
> --
> 2.49.0
>

Re: [PATCH bpf-next 2/4] bpf: implement dynptr copy kfuncs

[Andrii Nakryiko]

On Fri, Apr 25, 2025 at 5:59 AM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> From: Mykyta Yatsenko <yatsenko@meta.com>
>
> This patch introduces a new set of kfuncs for working with dynptrs in
> BPF programs, enabling reading variable-length user or kernel data
> into dynptr directly. To enable memory-safety, verifier allows only
> constant-sized reads via existing bpf_probe_read_{user|kernel} etc.
> kfuncs, dynptr-based kfuncs allow dynamically-sized reads without memory
> safety shortcomings.
>
> The following kfuncs are introduced:
> * `bpf_probe_read_kernel_dynptr()`: probes kernel-space data into a dynptr
> * `bpf_probe_read_user_dynptr()`: probes user-space data into a dynptr
> * `bpf_probe_read_kernel_str_dynptr()`: probes kernel-space string into
> a dynptr
> * `bpf_probe_read_user_str_dynptr()`: probes user-space string into a
> dynptr
> * `bpf_copy_from_user_dynptr()`: sleepable, copies user-space data into
> a dynptr for the current task
> * `bpf_copy_from_user_str_dynptr()`: sleepable, copies user-space string
> into a dynptr for the current task
> * `bpf_copy_from_user_task_dynptr()`: sleepable, copies user-space data
> of the task into a dynptr
> * `bpf_copy_from_user_task_str_dynptr()`: sleepable, copies user-space
> string of the task into a dynptr
>
> The implementation is built on two generic functions:
>  * __bpf_dynptr_copy
>  * __bpf_dynptr_copy_str
> These functions take function pointers as arguments, enabling the
> copying of data from various sources, including both kernel and user
> space. Notably, these indirect calls are typically inlined.

you mean there are no indirect calls due to __bpf_dynptr_copy[_str]
marked as __always_inline, right? We still call
strncpy_from_user_nofault (as one example) as an underlying data
reading step, right?

>
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  kernel/bpf/helpers.c     |   8 ++
>  kernel/trace/bpf_trace.c | 199 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 207 insertions(+)
>

Logic and code structure look great, few nits around naming below, but
LGTM overall.

Reviewed-by: Andrii Nakryiko <andrii@kernel.org>

> +static __always_inline int copy_kernel_data_nofault(void *dst, const void *unsafe_src,
> +                                                   u32 size, struct task_struct *tsk)
> +{
> +       if (WARN_ON_ONCE(tsk))
> +               return -EFAULT;
> +
> +       return copy_from_kernel_nofault(dst, unsafe_src, size);
> +}
> +
> +static __always_inline int copy_user_data_str_nofault(void *dst, const void __user *unsafe_src,

"user_data_str" is a bit mouthful, maybe just "copy_user_str_nofault"?

> +                                                     u32 size, struct task_struct *tsk)
> +{
> +       if (WARN_ON_ONCE(tsk))
> +               return -EFAULT;
> +
> +       return strncpy_from_user_nofault(dst, unsafe_src, size);
> +}
> +
> +static __always_inline int copy_user_data_str_sleepable(void *dst, const void __user *unsafe_src,
> +                                                       u32 size, struct task_struct *tsk)
> +{
> +       int ret;
> +
> +       if (unlikely(size == 0))
> +               return 0;
> +
> +       if (tsk) {
> +               ret = copy_remote_vm_str(tsk, (unsigned long)unsafe_src, dst, size, 0);
> +       } else {
> +               ret = strncpy_from_user(dst, unsafe_src, size - 1);
> +               /* strncpy_from_user does not guarantee NUL termination */
> +               if (ret >= 0)
> +                       ((char *)dst)[ret] = '\0';
> +       }
> +
> +       if (ret < 0)
> +               return ret;
> +       return ret + 1;
> +}
> +
> +static __always_inline int copy_kernel_data_str_nofault(void *dst, const void *unsafe_src,
> +                                                       u32 size, struct task_struct *tsk)

ditto here and above, "data_str" is confusing. let's use "data" for
fixed-size reads and "str" for zero-terminated strings?

> +{
> +       if (WARN_ON_ONCE(tsk))
> +               return -EFAULT;
> +
> +       return strncpy_from_kernel_nofault(dst, unsafe_src, size);
> +}
> +
>  __bpf_kfunc_start_defs();
>
>  __bpf_kfunc int bpf_send_signal_task(struct task_struct *task, int sig, enum pid_type type,

[...]

Re: [PATCH bpf-next 3/4] selftests/bpf: introduce tests for dynptr copy kfuncs

[Andrii Nakryiko]

On Fri, Apr 25, 2025 at 5:59 AM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> From: Mykyta Yatsenko <yatsenko@meta.com>
>
> Introduce selftests verifying newly-added dynptr copy kfuncs.
> Covering contiguous and non-contiguous memory backed dynptrs.
>
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  .../testing/selftests/bpf/prog_tests/dynptr.c |  13 ++
>  .../selftests/bpf/progs/dynptr_success.c      | 201 ++++++++++++++++++
>  2 files changed, 214 insertions(+)
>

Acked-by: Andrii Nakryiko <andrii@kernel.org>

[...]

Re: [PATCH bpf-next 4/4] selftests/bpf: disable test_probe_read_user_str_dynptr

[Andrii Nakryiko]

On Fri, Apr 25, 2025 at 5:59 AM Mykyta Yatsenko
<mykyta.yatsenko5@gmail.com> wrote:
>
> From: Mykyta Yatsenko <yatsenko@meta.com>
>
> Disable test that triggers bug in strncpy_from_user_nofault.
> Patch to fix the issue [1].
>
> [1] https://patchwork.kernel.org/project/linux-mm/patch/20250422131449.57177-1-mykyta.yatsenko5@gmail.com/
>
> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
> ---
>  tools/testing/selftests/bpf/DENYLIST | 1 +
>  1 file changed, 1 insertion(+)
>

might be a good idea to just fold this into previous patch, but
ultimately doesn't matter much

Acked-by: Andrii Nakryiko <andrii@kernel.org>

> diff --git a/tools/testing/selftests/bpf/DENYLIST b/tools/testing/selftests/bpf/DENYLIST
> index f748f2c33b22..839eb892adc7 100644
> --- a/tools/testing/selftests/bpf/DENYLIST
> +++ b/tools/testing/selftests/bpf/DENYLIST
> @@ -1,5 +1,6 @@
>  # TEMPORARY
>  # Alphabetical order
> +dynptr/test_probe_read_user_str_dynptr # disabled until https://patchwork.kernel.org/project/linux-mm/patch/20250422131449.57177-1-mykyta.yatsenko5@gmail.com/ is landed

nit: it's not "is landed", it's more "makes it into bpf-next tree"

>  get_stack_raw_tp    # spams with kernel warnings until next bpf -> bpf-next merge
>  stacktrace_build_id
>  stacktrace_build_id_nmi
> --
> 2.49.0
>

Re: [PATCH bpf-next 2/4] bpf: implement dynptr copy kfuncs

[Mykyta Yatsenko]

On 4/25/25 19:20, Andrii Nakryiko wrote:
> On Fri, Apr 25, 2025 at 5:59 AM Mykyta Yatsenko
> <mykyta.yatsenko5@gmail.com> wrote:
>> From: Mykyta Yatsenko <yatsenko@meta.com>
>>
>> This patch introduces a new set of kfuncs for working with dynptrs in
>> BPF programs, enabling reading variable-length user or kernel data
>> into dynptr directly. To enable memory-safety, verifier allows only
>> constant-sized reads via existing bpf_probe_read_{user|kernel} etc.
>> kfuncs, dynptr-based kfuncs allow dynamically-sized reads without memory
>> safety shortcomings.
>>
>> The following kfuncs are introduced:
>> * `bpf_probe_read_kernel_dynptr()`: probes kernel-space data into a dynptr
>> * `bpf_probe_read_user_dynptr()`: probes user-space data into a dynptr
>> * `bpf_probe_read_kernel_str_dynptr()`: probes kernel-space string into
>> a dynptr
>> * `bpf_probe_read_user_str_dynptr()`: probes user-space string into a
>> dynptr
>> * `bpf_copy_from_user_dynptr()`: sleepable, copies user-space data into
>> a dynptr for the current task
>> * `bpf_copy_from_user_str_dynptr()`: sleepable, copies user-space string
>> into a dynptr for the current task
>> * `bpf_copy_from_user_task_dynptr()`: sleepable, copies user-space data
>> of the task into a dynptr
>> * `bpf_copy_from_user_task_str_dynptr()`: sleepable, copies user-space
>> string of the task into a dynptr
>>
>> The implementation is built on two generic functions:
>>   * __bpf_dynptr_copy
>>   * __bpf_dynptr_copy_str
>> These functions take function pointers as arguments, enabling the
>> copying of data from various sources, including both kernel and user
>> space. Notably, these indirect calls are typically inlined.
> you mean there are no indirect calls due to __bpf_dynptr_copy[_str]
> marked as __always_inline, right? We still call
> strncpy_from_user_nofault (as one example) as an underlying data
> reading step, right?
yes, exactly.
>> Signed-off-by: Mykyta Yatsenko <yatsenko@meta.com>
>> ---
>>   kernel/bpf/helpers.c     |   8 ++
>>   kernel/trace/bpf_trace.c | 199 +++++++++++++++++++++++++++++++++++++++
>>   2 files changed, 207 insertions(+)
>>
> Logic and code structure look great, few nits around naming below, but
> LGTM overall.
>
> Reviewed-by: Andrii Nakryiko <andrii@kernel.org>
>
>> +static __always_inline int copy_kernel_data_nofault(void *dst, const void *unsafe_src,
>> +                                                   u32 size, struct task_struct *tsk)
>> +{
>> +       if (WARN_ON_ONCE(tsk))
>> +               return -EFAULT;
>> +
>> +       return copy_from_kernel_nofault(dst, unsafe_src, size);
>> +}
>> +
>> +static __always_inline int copy_user_data_str_nofault(void *dst, const void __user *unsafe_src,
> "user_data_str" is a bit mouthful, maybe just "copy_user_str_nofault"?
>
>> +                                                     u32 size, struct task_struct *tsk)
>> +{
>> +       if (WARN_ON_ONCE(tsk))
>> +               return -EFAULT;
>> +
>> +       return strncpy_from_user_nofault(dst, unsafe_src, size);
>> +}
>> +
>> +static __always_inline int copy_user_data_str_sleepable(void *dst, const void __user *unsafe_src,
>> +                                                       u32 size, struct task_struct *tsk)
>> +{
>> +       int ret;
>> +
>> +       if (unlikely(size == 0))
>> +               return 0;
>> +
>> +       if (tsk) {
>> +               ret = copy_remote_vm_str(tsk, (unsigned long)unsafe_src, dst, size, 0);
>> +       } else {
>> +               ret = strncpy_from_user(dst, unsafe_src, size - 1);
>> +               /* strncpy_from_user does not guarantee NUL termination */
>> +               if (ret >= 0)
>> +                       ((char *)dst)[ret] = '\0';
>> +       }
>> +
>> +       if (ret < 0)
>> +               return ret;
>> +       return ret + 1;
>> +}
>> +
>> +static __always_inline int copy_kernel_data_str_nofault(void *dst, const void *unsafe_src,
>> +                                                       u32 size, struct task_struct *tsk)
> ditto here and above, "data_str" is confusing. let's use "data" for
> fixed-size reads and "str" for zero-terminated strings?
Yes, makes sense
>
>> +{
>> +       if (WARN_ON_ONCE(tsk))
>> +               return -EFAULT;
>> +
>> +       return strncpy_from_kernel_nofault(dst, unsafe_src, size);
>> +}
>> +
>>   __bpf_kfunc_start_defs();
>>
>>   __bpf_kfunc int bpf_send_signal_task(struct task_struct *task, int sig, enum pid_type type,
> [...]