[PATCH bpf-next v2 0/7] bpf: Add probe_read_{kernel,user}_dynptr and copy_from_user_dynptr

[PATCH bpf-next v2 0/7] bpf: Add probe_read_{kernel,user}_dynptr and copy_from_user_dynptr

[Levi Zim via B4 Relay]

This series introduce the dynptr counterpart of the
bpf_probe_read_{kernel,user} helpers and bpf_copy_from_user helper.

These helpers are helpful for reading variable-length data from kernel
memory into dynptr without going through an intermediate buffer.

Link: https://lore.kernel.org/bpf/MEYP282MB2312CFCE5F7712FDE313215AC64D2@MEYP282MB2312.AUSP282.PROD.OUTLOOK.COM/
Suggested-by: Andrii Nakryiko <andrii.nakryiko@gmail.com>
Signed-off-by: Levi Zim <rsworktech@outlook.com>
---
Changes in v2:
- Add missing bpf-next prefix. I forgot it in the initial series. Sorry
  about that.
- Link to v1: https://lore.kernel.org/r/20250125-bpf_dynptr_probe-v1-0-c3cb121f6951@outlook.com

---
Levi Zim (7):
      bpf: Implement bpf_probe_read_kernel_dynptr helper
      bpf: Implement bpf_probe_read_user_dynptr helper
      bpf: Implement bpf_copy_from_user_dynptr helper
      tools headers UAPI: Update tools's copy of bpf.h header
      selftests/bpf: probe_read_kernel_dynptr test
      selftests/bpf: probe_read_user_dynptr test
      selftests/bpf: copy_from_user_dynptr test

 include/linux/bpf.h                                |   3 +
 include/uapi/linux/bpf.h                           |  49 ++++++++++
 kernel/bpf/helpers.c                               |  53 ++++++++++-
 kernel/trace/bpf_trace.c                           |  72 ++++++++++++++
 tools/include/uapi/linux/bpf.h                     |  49 ++++++++++
 tools/testing/selftests/bpf/prog_tests/dynptr.c    |  45 ++++++++-
 tools/testing/selftests/bpf/progs/dynptr_success.c | 106 +++++++++++++++++++++
 7 files changed, 374 insertions(+), 3 deletions(-)
---
base-commit: d0d106a2bd21499901299160744e5fe9f4c83ddb
change-id: 20250124-bpf_dynptr_probe-ab483c554f1a

Best regards,
-- 
Levi Zim <rsworktech@outlook.com>

[PATCH bpf-next v2 1/7] bpf: Implement bpf_probe_read_kernel_dynptr helper

[Levi Zim via B4 Relay]

From: Levi Zim <rsworktech@outlook.com>

This patch add a helper function bpf_probe_read_kernel_dynptr:

long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst,
	u32 offset, u32 size, const void *unsafe_ptr, u64 flags);

It is useful for reading variable-length data from kernel memory into
dynptr.

Link: https://lore.kernel.org/bpf/MEYP282MB2312CFCE5F7712FDE313215AC64D2@MEYP282MB2312.AUSP282.PROD.OUTLOOK.COM/
Signed-off-by: Levi Zim <rsworktech@outlook.com>
---
 include/linux/bpf.h      |  2 ++
 include/uapi/linux/bpf.h | 16 ++++++++++++++++
 kernel/bpf/helpers.c     |  8 ++++++--
 kernel/trace/bpf_trace.c | 42 ++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index f3f50e29d63929acaf12c81f8356173f1f5e154b..9d5ae8b4b7d82c4523bf0ab041d4b76bf134a106 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -1323,6 +1323,8 @@ 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_check_off_len(const struct bpf_dynptr_kern *ptr, u32 offset, u32 len);
+enum bpf_dynptr_type bpf_dynptr_get_type(const struct bpf_dynptr_kern *ptr);
 
 #ifdef CONFIG_BPF_JIT
 int bpf_trampoline_link_prog(struct bpf_tramp_link *link,
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 2acf9b33637174bd16b1d12ccc6410c5f55a7ea9..2e08a59527ecf56732ea14ac34446b5eb25b5690 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5805,6 +5805,21 @@ union bpf_attr {
  *		0 on success.
  *
  *		**-ENOENT** if the bpf_local_storage cannot be found.
+ *
+ * long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst, u32 offset, u32 size, const void *unsafe_ptr, u64 flags)
+ *	Description
+ *		Safely attempt to read *size* bytes from kernel space address
+ *		*unsafe_ptr* and store the data in *dst* starting from *offset*.
+ *		*flags* is currently unused.
+ *	Return
+ *		0 on success.
+ *
+ *		**-E2BIG** if *offset* + *len* exceeds the length of *src*'s data
+ *
+ *		**-EINVAL** if *src* is an invalid dynptr or doesn't support this
+ *		support this helper, or if *flags* is not 0.
+ *
+ *		Or other negative errors on failure reading kernel memory.
  */
 #define ___BPF_FUNC_MAPPER(FN, ctx...)			\
 	FN(unspec, 0, ##ctx)				\
@@ -6019,6 +6034,7 @@ union bpf_attr {
 	FN(user_ringbuf_drain, 209, ##ctx)		\
 	FN(cgrp_storage_get, 210, ##ctx)		\
 	FN(cgrp_storage_delete, 211, ##ctx)		\
+	FN(probe_read_kernel_dynptr, 212, ##ctx)		\
 	/* */
 
 /* backwards-compatibility macros for users of __BPF_FUNC_MAPPER that don't
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index f27ce162427ab4040d2e2d2eb84a883fe57de59e..a736dc9e7be98571103ba404420be0da4dac4fbe 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1678,7 +1678,7 @@ static void bpf_dynptr_set_type(struct bpf_dynptr_kern *ptr, enum bpf_dynptr_typ
 	ptr->size |= type << DYNPTR_TYPE_SHIFT;
 }
 
-static enum bpf_dynptr_type bpf_dynptr_get_type(const struct bpf_dynptr_kern *ptr)
+enum bpf_dynptr_type bpf_dynptr_get_type(const struct bpf_dynptr_kern *ptr)
 {
 	return (ptr->size & ~(DYNPTR_RDONLY_BIT)) >> DYNPTR_TYPE_SHIFT;
 }
@@ -1714,7 +1714,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);
 
@@ -1900,6 +1900,7 @@ const struct bpf_func_proto bpf_probe_read_user_proto __weak;
 const struct bpf_func_proto bpf_probe_read_user_str_proto __weak;
 const struct bpf_func_proto bpf_probe_read_kernel_proto __weak;
 const struct bpf_func_proto bpf_probe_read_kernel_str_proto __weak;
+const struct bpf_func_proto bpf_probe_read_kernel_dynptr_proto __weak;
 const struct bpf_func_proto bpf_task_pt_regs_proto __weak;
 
 const struct bpf_func_proto *
@@ -2031,6 +2032,9 @@ bpf_base_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 	case BPF_FUNC_probe_read_kernel:
 		return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ?
 		       NULL : &bpf_probe_read_kernel_proto;
+	case BPF_FUNC_probe_read_kernel_dynptr:
+		return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ?
+		       NULL : &bpf_probe_read_kernel_dynptr_proto;
 	case BPF_FUNC_probe_read_user_str:
 		return &bpf_probe_read_user_str_proto;
 	case BPF_FUNC_probe_read_kernel_str:
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index adc947587eb8132ebbd54778d2db937b3b8861de..75c9d1e8d04c3b8930ae81345f5586756ce8b5ec 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -248,6 +248,48 @@ const struct bpf_func_proto bpf_probe_read_kernel_proto = {
 	.arg3_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_5(bpf_probe_read_kernel_dynptr, const struct bpf_dynptr_kern *, dst,
+	u32, offset, u32, size, void *, unsafe_ptr, u64, flags)
+{
+	enum bpf_dynptr_type type;
+	int err;
+
+	if (!dst->data || __bpf_dynptr_is_rdonly(dst))
+		return -EINVAL;
+
+	err = bpf_dynptr_check_off_len(dst, offset, size);
+	if (err)
+		return err;
+
+	type = bpf_dynptr_get_type(dst);
+
+	switch (type) {
+	case BPF_DYNPTR_TYPE_LOCAL:
+	case BPF_DYNPTR_TYPE_RINGBUF:
+		if (flags)
+			return -EINVAL;
+		return bpf_probe_read_kernel_common(dst->data + dst->offset + offset,
+				size, unsafe_ptr);
+	case BPF_DYNPTR_TYPE_SKB:
+	case BPF_DYNPTR_TYPE_XDP:
+		return -EINVAL;
+	default:
+		WARN_ONCE(true, "%s: unknown dynptr type %d\n", __func__, type);
+		return -EFAULT;
+	}
+}
+
+const struct bpf_func_proto bpf_probe_read_kernel_dynptr_proto = {
+	.func		= bpf_probe_read_kernel_dynptr,
+	.gpl_only	= true,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_DYNPTR | MEM_RDONLY,
+	.arg2_type	= ARG_ANYTHING,
+	.arg3_type	= ARG_ANYTHING,
+	.arg4_type	= ARG_ANYTHING,
+	.arg5_type	= ARG_ANYTHING,
+};
+
 static __always_inline int
 bpf_probe_read_kernel_str_common(void *dst, u32 size, const void *unsafe_ptr)
 {

-- 
2.48.1

Re: [PATCH bpf-next v2 1/7] bpf: Implement bpf_probe_read_kernel_dynptr helper

[Alexei Starovoitov]

On Sat, Jan 25, 2025 at 12:30 AM Levi Zim via B4 Relay
<devnull+rsworktech.outlook.com@kernel.org> wrote:
>
> From: Levi Zim <rsworktech@outlook.com>
>
> This patch add a helper function bpf_probe_read_kernel_dynptr:
>
> long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst,
>         u32 offset, u32 size, const void *unsafe_ptr, u64 flags);

We stopped adding helpers years ago.
Only new kfuncs are allowed.

This particular one doesn't look useful as-is.
The same logic can be expressed with
- create dynptr
- dynptr_slice
- copy_from_kernel

pw-bot: cr

Re: [PATCH bpf-next v2 1/7] bpf: Implement bpf_probe_read_kernel_dynptr helper

[Levi Zim]

On 2025/1/26 00:58, Alexei Starovoitov wrote:
 > On Sat, Jan 25, 2025 at 12:30 AM Levi Zim via B4 Relay
 > <devnull+rsworktech.outlook.com@kernel.org> wrote:
 >> From: Levi Zim <rsworktech@outlook.com>
 >>
 >> This patch add a helper function bpf_probe_read_kernel_dynptr:
 >>
 >> long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst,
 >>          u32 offset, u32 size, const void *unsafe_ptr, u64 flags);
 > We stopped adding helpers years ago.
 > Only new kfuncs are allowed.

Sorry, I didn't know that. Just asking, is there any 
documentation/discussion
about stopping adding helpers?

I will switch the implementation to kfuncs in v3.

 > This particular one doesn't look useful as-is.
 > The same logic can be expressed with
 > - create dynptr
 > - dynptr_slice
 > - copy_from_kernel

By copy_from_kernel I assume you mean bpf_probe_read_kernel. The problem
with dynptr_slice_rdwr and probe_read_kernel is that they only support a
compile-time constant size [1].

But in order to best utilize the space on a BPF ringbuf, it is possible 
to reserve a
variable length of space as dynptr on a ringbuf with 
bpf_ringbuf_reserve_dynptr.

Then currently we have no way to read a variable length of kernel memory
into this dynptr, except doing it chunk by chunk[2], which is kinda awkward.
That's the problem the new helpers trying to solve.
And I am not the only one needing this kind of feature [3].

Andrii said it would be a straightforward addition as it is a super thin 
wrapper
around existing functionality (we are just avoiding fixed buffer size 
restrictions of
existing probe/copy_from APIs)

[1]: 
https://elixir.bootlin.com/linux/v6.12.6/source/kernel/bpf/helpers.c#L2600-L2601
[2]: 
https://github.com/libbpf/libbpf-bootstrap/commit/046fad60df3e39540937b5ec6ee86054f33d3f28
[3]: https://github.com/libbpf/libbpf-rs/issues/1041
[4]: 
https://lore.kernel.org/bpf/CAEf4BzZctXJsR+TwMhmXNWnR0_BV802-3KJw226ZZt8St4xNkw@mail.gmail.com/

 > pw-bot: cr

Re: [PATCH bpf-next v2 1/7] bpf: Implement bpf_probe_read_kernel_dynptr helper

[Alexei Starovoitov]

On Sat, Jan 25, 2025 at 5:05 PM Levi Zim <rsworktech@outlook.com> wrote:
>
> On 2025/1/26 00:58, Alexei Starovoitov wrote:
>  > On Sat, Jan 25, 2025 at 12:30 AM Levi Zim via B4 Relay
>  > <devnull+rsworktech.outlook.com@kernel.org> wrote:
>  >> From: Levi Zim <rsworktech@outlook.com>
>  >>
>  >> This patch add a helper function bpf_probe_read_kernel_dynptr:
>  >>
>  >> long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst,
>  >>          u32 offset, u32 size, const void *unsafe_ptr, u64 flags);
>  > We stopped adding helpers years ago.
>  > Only new kfuncs are allowed.
>
> Sorry, I didn't know that. Just asking, is there any
> documentation/discussion
> about stopping adding helpers?
>
> I will switch the implementation to kfuncs in v3.
>
>  > This particular one doesn't look useful as-is.
>  > The same logic can be expressed with
>  > - create dynptr
>  > - dynptr_slice
>  > - copy_from_kernel
>
> By copy_from_kernel I assume you mean bpf_probe_read_kernel. The problem
> with dynptr_slice_rdwr and probe_read_kernel is that they only support a
> compile-time constant size [1].
>
> But in order to best utilize the space on a BPF ringbuf, it is possible
> to reserve a
> variable length of space as dynptr on a ringbuf with
> bpf_ringbuf_reserve_dynptr.

That makes sense. The commit log didn't call it out.
Please spell out the motivation clearly.
Also why bpf_probe_read_kernel_common ?
Do we need to memset() it on failure?

Re: [PATCH bpf-next v2 1/7] bpf: Implement bpf_probe_read_kernel_dynptr helper

[Andrei Matei]

On Mon, Jan 27, 2025 at 5:04 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Sat, Jan 25, 2025 at 5:05 PM Levi Zim <rsworktech@outlook.com> wrote:
> >
> > On 2025/1/26 00:58, Alexei Starovoitov wrote:
> >  > On Sat, Jan 25, 2025 at 12:30 AM Levi Zim via B4 Relay
> >  > <devnull+rsworktech.outlook.com@kernel.org> wrote:
> >  >> From: Levi Zim <rsworktech@outlook.com>
> >  >>
> >  >> This patch add a helper function bpf_probe_read_kernel_dynptr:
> >  >>
> >  >> long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst,
> >  >>          u32 offset, u32 size, const void *unsafe_ptr, u64 flags);
> >  > We stopped adding helpers years ago.
> >  > Only new kfuncs are allowed.
> >
> > Sorry, I didn't know that. Just asking, is there any
> > documentation/discussion
> > about stopping adding helpers?
> >
> > I will switch the implementation to kfuncs in v3.
> >
> >  > This particular one doesn't look useful as-is.
> >  > The same logic can be expressed with
> >  > - create dynptr
> >  > - dynptr_slice
> >  > - copy_from_kernel
> >
> > By copy_from_kernel I assume you mean bpf_probe_read_kernel. The problem
> > with dynptr_slice_rdwr and probe_read_kernel is that they only support a
> > compile-time constant size [1].
> >
> > But in order to best utilize the space on a BPF ringbuf, it is possible
> > to reserve a
> > variable length of space as dynptr on a ringbuf with
> > bpf_ringbuf_reserve_dynptr.

For our uprobes, we've run into similar issues around doing variable-sized
bpf_probe_read_user() into ring buffers for our debugger [1]. Our use case
is that we generate uprobes that recursively read data structures until we
fill up a buffer. The verifier's insistence on knowing statically that a read
fits into the buffer makes for awkward code, and makes it hard to pack the
buffer fully; we have to split our reads into a couple of static size classes.

Any chance there'd be interest in taking the opportunity to support
dynamically-sized reads from userspace too? :)

[1] https://side-eye.io

>
> That makes sense. The commit log didn't call it out.
> Please spell out the motivation clearly.
> Also why bpf_probe_read_kernel_common ?
> Do we need to memset() it on failure?
>

Re: [PATCH bpf-next v2 1/7] bpf: Implement bpf_probe_read_kernel_dynptr helper

[Andrii Nakryiko]

On Mon, Jan 27, 2025 at 2:54 PM Andrei Matei <andreimatei1@gmail.com> wrote:
>
> On Mon, Jan 27, 2025 at 5:04 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Sat, Jan 25, 2025 at 5:05 PM Levi Zim <rsworktech@outlook.com> wrote:
> > >
> > > On 2025/1/26 00:58, Alexei Starovoitov wrote:
> > >  > On Sat, Jan 25, 2025 at 12:30 AM Levi Zim via B4 Relay
> > >  > <devnull+rsworktech.outlook.com@kernel.org> wrote:
> > >  >> From: Levi Zim <rsworktech@outlook.com>
> > >  >>
> > >  >> This patch add a helper function bpf_probe_read_kernel_dynptr:
> > >  >>
> > >  >> long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst,
> > >  >>          u32 offset, u32 size, const void *unsafe_ptr, u64 flags);
> > >  > We stopped adding helpers years ago.
> > >  > Only new kfuncs are allowed.
> > >
> > > Sorry, I didn't know that. Just asking, is there any
> > > documentation/discussion
> > > about stopping adding helpers?
> > >
> > > I will switch the implementation to kfuncs in v3.
> > >
> > >  > This particular one doesn't look useful as-is.
> > >  > The same logic can be expressed with
> > >  > - create dynptr
> > >  > - dynptr_slice
> > >  > - copy_from_kernel
> > >
> > > By copy_from_kernel I assume you mean bpf_probe_read_kernel. The problem
> > > with dynptr_slice_rdwr and probe_read_kernel is that they only support a
> > > compile-time constant size [1].
> > >
> > > But in order to best utilize the space on a BPF ringbuf, it is possible
> > > to reserve a
> > > variable length of space as dynptr on a ringbuf with
> > > bpf_ringbuf_reserve_dynptr.
>
> For our uprobes, we've run into similar issues around doing variable-sized
> bpf_probe_read_user() into ring buffers for our debugger [1]. Our use case
> is that we generate uprobes that recursively read data structures until we
> fill up a buffer. The verifier's insistence on knowing statically that a read
> fits into the buffer makes for awkward code, and makes it hard to pack the
> buffer fully; we have to split our reads into a couple of static size classes.
>
> Any chance there'd be interest in taking the opportunity to support
> dynamically-sized reads from userspace too? :)

That's bpf_probe_read_user_dynptr() from patch #2, no?

But generally speaking, here's a list of new APIs that we'd need to
cover all existing fixed buffer versions:

- non-sleepable probe reads:

  bpf_probe_read_kernel_dynptr()
  bpf_probe_read_user_dynptr()
  bpf_probe_read_kernel_str_dynptr()
  bpf_probe_read_user_str_dynptr()

- sleepable probe reads (copy_from_user):

bpf_copy_from_user_dynptr()
bpf_copy_from_user_str_dynptr()

- and then we have complementary task-based APIs for non-current process:

bpf_probe_read_user_task_dynptr()
bpf_probe_read_user_str_task_dynptr()
bpf_copy_from_user_task_dynptr()
bpf_copy_from_user_str_task_dynptr()

Jordan is working on non-dynptr version of
bpf_copy_from_user_str_task(), once he's done with that, we'll add
dynptr version, probably.

>
> [1] https://side-eye.io
>
> >
> > That makes sense. The commit log didn't call it out.
> > Please spell out the motivation clearly.
> > Also why bpf_probe_read_kernel_common ?
> > Do we need to memset() it on failure?
> >

Re: [PATCH bpf-next v2 1/7] bpf: Implement bpf_probe_read_kernel_dynptr helper

[Levi Zim]

On 2025-01-28 07:09, Andrii Nakryiko wrote:
> On Mon, Jan 27, 2025 at 2:54 PM Andrei Matei <andreimatei1@gmail.com> wrote:
>> On Mon, Jan 27, 2025 at 5:04 PM Alexei Starovoitov
>> <alexei.starovoitov@gmail.com> wrote:
>>> On Sat, Jan 25, 2025 at 5:05 PM Levi Zim <rsworktech@outlook.com> wrote:
>>>> On 2025/1/26 00:58, Alexei Starovoitov wrote:
>>>>   > On Sat, Jan 25, 2025 at 12:30 AM Levi Zim via B4 Relay
>>>>   > <devnull+rsworktech.outlook.com@kernel.org> wrote:
>>>>   >> From: Levi Zim <rsworktech@outlook.com>
>>>>   >>
>>>>   >> This patch add a helper function bpf_probe_read_kernel_dynptr:
>>>>   >>
>>>>   >> long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst,
>>>>   >>          u32 offset, u32 size, const void *unsafe_ptr, u64 flags);
>>>>   > We stopped adding helpers years ago.
>>>>   > Only new kfuncs are allowed.
>>>>
>>>> Sorry, I didn't know that. Just asking, is there any
>>>> documentation/discussion
>>>> about stopping adding helpers?
>>>>
>>>> I will switch the implementation to kfuncs in v3.
>>>>
>>>>   > This particular one doesn't look useful as-is.
>>>>   > The same logic can be expressed with
>>>>   > - create dynptr
>>>>   > - dynptr_slice
>>>>   > - copy_from_kernel
>>>>
>>>> By copy_from_kernel I assume you mean bpf_probe_read_kernel. The problem
>>>> with dynptr_slice_rdwr and probe_read_kernel is that they only support a
>>>> compile-time constant size [1].
>>>>
>>>> But in order to best utilize the space on a BPF ringbuf, it is possible
>>>> to reserve a
>>>> variable length of space as dynptr on a ringbuf with
>>>> bpf_ringbuf_reserve_dynptr.
>> For our uprobes, we've run into similar issues around doing variable-sized
>> bpf_probe_read_user() into ring buffers for our debugger [1]. Our use case
>> is that we generate uprobes that recursively read data structures until we
>> fill up a buffer. The verifier's insistence on knowing statically that a read
>> fits into the buffer makes for awkward code, and makes it hard to pack the
>> buffer fully; we have to split our reads into a couple of static size classes.
>>
>> Any chance there'd be interest in taking the opportunity to support
>> dynamically-sized reads from userspace too? :)
> That's bpf_probe_read_user_dynptr() from patch #2, no?
>
> But generally speaking, here's a list of new APIs that we'd need to
> cover all existing fixed buffer versions:
>
> - non-sleepable probe reads:
>
>    bpf_probe_read_kernel_dynptr()
>    bpf_probe_read_user_dynptr()
>    bpf_probe_read_kernel_str_dynptr()
I think the _str_dynptr versions are probably not worth adding.
For example, when we use probe_read_kernel_str, the length of the str is 
usually
not known and we usually allocate a fixed size buffer for it. If we do 
know the
length of the str beforehand, we can just use probe_read_kernel_dynptr.
>    bpf_probe_read_user_str_dynptr()
>
> - sleepable probe reads (copy_from_user):
>
> bpf_copy_from_user_dynptr()
> bpf_copy_from_user_str_dynptr()
>
> - and then we have complementary task-based APIs for non-current process:
>
> bpf_probe_read_user_task_dynptr()
> bpf_probe_read_user_str_task_dynptr()
> bpf_copy_from_user_task_dynptr()
> bpf_copy_from_user_str_task_dynptr()
>
> Jordan is working on non-dynptr version of
> bpf_copy_from_user_str_task(), once he's done with that, we'll add
> dynptr version, probably.
>
>> [1] https://side-eye.io
>>
>>> That makes sense. The commit log didn't call it out.
>>> Please spell out the motivation clearly.
>>> Also why bpf_probe_read_kernel_common ?
>>> Do we need to memset() it on failure?
>>>

Re: [PATCH bpf-next v2 1/7] bpf: Implement bpf_probe_read_kernel_dynptr helper

[Alexei Starovoitov]

On Mon, Jan 27, 2025 at 3:09 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Mon, Jan 27, 2025 at 2:54 PM Andrei Matei <andreimatei1@gmail.com> wrote:
> >
> > On Mon, Jan 27, 2025 at 5:04 PM Alexei Starovoitov
> > <alexei.starovoitov@gmail.com> wrote:
> > >
> > > On Sat, Jan 25, 2025 at 5:05 PM Levi Zim <rsworktech@outlook.com> wrote:
> > > >
> > > > On 2025/1/26 00:58, Alexei Starovoitov wrote:
> > > >  > On Sat, Jan 25, 2025 at 12:30 AM Levi Zim via B4 Relay
> > > >  > <devnull+rsworktech.outlook.com@kernel.org> wrote:
> > > >  >> From: Levi Zim <rsworktech@outlook.com>
> > > >  >>
> > > >  >> This patch add a helper function bpf_probe_read_kernel_dynptr:
> > > >  >>
> > > >  >> long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst,
> > > >  >>          u32 offset, u32 size, const void *unsafe_ptr, u64 flags);
> > > >  > We stopped adding helpers years ago.
> > > >  > Only new kfuncs are allowed.
> > > >
> > > > Sorry, I didn't know that. Just asking, is there any
> > > > documentation/discussion
> > > > about stopping adding helpers?
> > > >
> > > > I will switch the implementation to kfuncs in v3.
> > > >
> > > >  > This particular one doesn't look useful as-is.
> > > >  > The same logic can be expressed with
> > > >  > - create dynptr
> > > >  > - dynptr_slice
> > > >  > - copy_from_kernel
> > > >
> > > > By copy_from_kernel I assume you mean bpf_probe_read_kernel. The problem
> > > > with dynptr_slice_rdwr and probe_read_kernel is that they only support a
> > > > compile-time constant size [1].
> > > >
> > > > But in order to best utilize the space on a BPF ringbuf, it is possible
> > > > to reserve a
> > > > variable length of space as dynptr on a ringbuf with
> > > > bpf_ringbuf_reserve_dynptr.
> >
> > For our uprobes, we've run into similar issues around doing variable-sized
> > bpf_probe_read_user() into ring buffers for our debugger [1]. Our use case
> > is that we generate uprobes that recursively read data structures until we
> > fill up a buffer. The verifier's insistence on knowing statically that a read
> > fits into the buffer makes for awkward code, and makes it hard to pack the
> > buffer fully; we have to split our reads into a couple of static size classes.
> >
> > Any chance there'd be interest in taking the opportunity to support
> > dynamically-sized reads from userspace too? :)
>
> That's bpf_probe_read_user_dynptr() from patch #2, no?
>
> But generally speaking, here's a list of new APIs that we'd need to
> cover all existing fixed buffer versions:
>
> - non-sleepable probe reads:
>
>   bpf_probe_read_kernel_dynptr()
>   bpf_probe_read_user_dynptr()
>   bpf_probe_read_kernel_str_dynptr()
>   bpf_probe_read_user_str_dynptr()
>
> - sleepable probe reads (copy_from_user):
>
> bpf_copy_from_user_dynptr()
> bpf_copy_from_user_str_dynptr()
>
> - and then we have complementary task-based APIs for non-current process:
>
> bpf_probe_read_user_task_dynptr()
> bpf_probe_read_user_str_task_dynptr()
> bpf_copy_from_user_task_dynptr()
> bpf_copy_from_user_str_task_dynptr()
>
> Jordan is working on non-dynptr version of
> bpf_copy_from_user_str_task(), once he's done with that, we'll add
> dynptr version, probably.

This is quite a bunch of kfuncs.
It doesn't look like adding _dynptr suffix and duplicating
kfuncs approach scales.

Let's make the existing helpers/kfuncs more flexible ?

We can introduce a kfunc bpf_dynptr_buf() that checks that
dynptr is not readonly and type == local or ringbuf and
return dynptr->data as PTR_TO_MEM | dynptr_flag | VERIFIER_ADDS_SIZE_CHECK.

Then allow bpf_probe_read_user/kernel/... all of them to accept
this register type where PTR_TO_MEM is required
while relaxing ARG_CONST_SIZE 2nd argument to ARG_ANYTHING.
Then the verifier will insert an extra check
if (arg1->size < arg2)
before the call.

Not only the bpf_probe_read_kernel/user, _str variants will work
but things like bpf_strtol, bpf_strncmp, bpf_snprintf, bpf_get_stack
will auto-magically work as well.

I think those are quite valuable to make available with non-constant size.
bpf_get_stack_*() directly into the ring buffer sounds very useful.

Re: [PATCH bpf-next v2 1/7] bpf: Implement bpf_probe_read_kernel_dynptr helper

[Levi Zim]

On 2025/1/28 10:57, Alexei Starovoitov wrote:
> On Mon, Jan 27, 2025 at 3:09 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
>> On Mon, Jan 27, 2025 at 2:54 PM Andrei Matei <andreimatei1@gmail.com> wrote:
>>> On Mon, Jan 27, 2025 at 5:04 PM Alexei Starovoitov
>>> <alexei.starovoitov@gmail.com> wrote:
>>>> On Sat, Jan 25, 2025 at 5:05 PM Levi Zim <rsworktech@outlook.com> wrote:
>>>>> On 2025/1/26 00:58, Alexei Starovoitov wrote:
>>>>>   > On Sat, Jan 25, 2025 at 12:30 AM Levi Zim via B4 Relay
>>>>>   > <devnull+rsworktech.outlook.com@kernel.org> wrote:
>>>>>   >> From: Levi Zim <rsworktech@outlook.com>
>>>>>   >>
>>>>>   >> This patch add a helper function bpf_probe_read_kernel_dynptr:
>>>>>   >>
>>>>>   >> long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst,
>>>>>   >>          u32 offset, u32 size, const void *unsafe_ptr, u64 flags);
>>>>>   > We stopped adding helpers years ago.
>>>>>   > Only new kfuncs are allowed.
>>>>>
>>>>> Sorry, I didn't know that. Just asking, is there any
>>>>> documentation/discussion
>>>>> about stopping adding helpers?
>>>>>
>>>>> I will switch the implementation to kfuncs in v3.
>>>>>
>>>>>   > This particular one doesn't look useful as-is.
>>>>>   > The same logic can be expressed with
>>>>>   > - create dynptr
>>>>>   > - dynptr_slice
>>>>>   > - copy_from_kernel
>>>>>
>>>>> By copy_from_kernel I assume you mean bpf_probe_read_kernel. The problem
>>>>> with dynptr_slice_rdwr and probe_read_kernel is that they only support a
>>>>> compile-time constant size [1].
>>>>>
>>>>> But in order to best utilize the space on a BPF ringbuf, it is possible
>>>>> to reserve a
>>>>> variable length of space as dynptr on a ringbuf with
>>>>> bpf_ringbuf_reserve_dynptr.
>>> For our uprobes, we've run into similar issues around doing variable-sized
>>> bpf_probe_read_user() into ring buffers for our debugger [1]. Our use case
>>> is that we generate uprobes that recursively read data structures until we
>>> fill up a buffer. The verifier's insistence on knowing statically that a read
>>> fits into the buffer makes for awkward code, and makes it hard to pack the
>>> buffer fully; we have to split our reads into a couple of static size classes.
>>>
>>> Any chance there'd be interest in taking the opportunity to support
>>> dynamically-sized reads from userspace too? :)
>> That's bpf_probe_read_user_dynptr() from patch #2, no?
>>
>> But generally speaking, here's a list of new APIs that we'd need to
>> cover all existing fixed buffer versions:
>>
>> - non-sleepable probe reads:
>>
>>    bpf_probe_read_kernel_dynptr()
>>    bpf_probe_read_user_dynptr()
>>    bpf_probe_read_kernel_str_dynptr()
>>    bpf_probe_read_user_str_dynptr()
>>
>> - sleepable probe reads (copy_from_user):
>>
>> bpf_copy_from_user_dynptr()
>> bpf_copy_from_user_str_dynptr()
>>
>> - and then we have complementary task-based APIs for non-current process:
>>
>> bpf_probe_read_user_task_dynptr()
>> bpf_probe_read_user_str_task_dynptr()
>> bpf_copy_from_user_task_dynptr()
>> bpf_copy_from_user_str_task_dynptr()
>>
>> Jordan is working on non-dynptr version of
>> bpf_copy_from_user_str_task(), once he's done with that, we'll add
>> dynptr version, probably.
> This is quite a bunch of kfuncs.
> It doesn't look like adding _dynptr suffix and duplicating
> kfuncs approach scales.

The _str_dynptr versions might not worth adding [1].
So only four read_{kernel,user}_dynptr and copy_from_user{,_task}_dynptr 
are needed,
which seems manageable for now.

But taking other helpers like bpf_strtol into account does quickly show 
that this approach
is not scalable.

> Let's make the existing helpers/kfuncs more flexible ?
>
> We can introduce a kfunc bpf_dynptr_buf() that checks that
> dynptr is not readonly and type == local or ringbuf and
> return dynptr->data as PTR_TO_MEM | dynptr_flag | VERIFIER_ADDS_SIZE_CHECK.
>
> Then allow bpf_probe_read_user/kernel/... all of them to accept
> this register type where PTR_TO_MEM is required
> while relaxing ARG_CONST_SIZE 2nd argument to ARG_ANYTHING.
> Then the verifier will insert an extra check
> if (arg1->size < arg2)
> before the call.
Nice idea. I will try this approach first.
>
> Not only the bpf_probe_read_kernel/user, _str variants will work
> but things like bpf_strtol, bpf_strncmp, bpf_snprintf, bpf_get_stack
> will auto-magically work as well.
>
> I think those are quite valuable to make available with non-constant size.
> bpf_get_stack_*() directly into the ring buffer sounds very useful.

[1]: 
https://lore.kernel.org/bpf/20250125-bpf_dynptr_probe-v2-0-c42c87f97afe@outlook.com/T/#m9700146d286a88abc0b25ef47041015ba6c477a3

Re: [PATCH bpf-next v2 1/7] bpf: Implement bpf_probe_read_kernel_dynptr helper

[Levi Zim]

On 2025-01-28 10:57, Alexei Starovoitov wrote:
> On Mon, Jan 27, 2025 at 3:09 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
>> On Mon, Jan 27, 2025 at 2:54 PM Andrei Matei <andreimatei1@gmail.com> wrote:
>>> On Mon, Jan 27, 2025 at 5:04 PM Alexei Starovoitov
>>> <alexei.starovoitov@gmail.com> wrote:
>>>> On Sat, Jan 25, 2025 at 5:05 PM Levi Zim <rsworktech@outlook.com> wrote:
>>>>> On 2025/1/26 00:58, Alexei Starovoitov wrote:
>>>>>   > On Sat, Jan 25, 2025 at 12:30 AM Levi Zim via B4 Relay
>>>>>   > <devnull+rsworktech.outlook.com@kernel.org> wrote:
>>>>>   >> From: Levi Zim <rsworktech@outlook.com>
>>>>>   >>
>>>>>   >> This patch add a helper function bpf_probe_read_kernel_dynptr:
>>>>>   >>
>>>>>   >> long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst,
>>>>>   >>          u32 offset, u32 size, const void *unsafe_ptr, u64 flags);
>>>>>   > We stopped adding helpers years ago.
>>>>>   > Only new kfuncs are allowed.
>>>>>
>>>>> Sorry, I didn't know that. Just asking, is there any
>>>>> documentation/discussion
>>>>> about stopping adding helpers?
>>>>>
>>>>> I will switch the implementation to kfuncs in v3.
>>>>>
>>>>>   > This particular one doesn't look useful as-is.
>>>>>   > The same logic can be expressed with
>>>>>   > - create dynptr
>>>>>   > - dynptr_slice
>>>>>   > - copy_from_kernel
>>>>>
>>>>> By copy_from_kernel I assume you mean bpf_probe_read_kernel. The problem
>>>>> with dynptr_slice_rdwr and probe_read_kernel is that they only support a
>>>>> compile-time constant size [1].
>>>>>
>>>>> But in order to best utilize the space on a BPF ringbuf, it is possible
>>>>> to reserve a
>>>>> variable length of space as dynptr on a ringbuf with
>>>>> bpf_ringbuf_reserve_dynptr.
>>> For our uprobes, we've run into similar issues around doing variable-sized
>>> bpf_probe_read_user() into ring buffers for our debugger [1]. Our use case
>>> is that we generate uprobes that recursively read data structures until we
>>> fill up a buffer. The verifier's insistence on knowing statically that a read
>>> fits into the buffer makes for awkward code, and makes it hard to pack the
>>> buffer fully; we have to split our reads into a couple of static size classes.
>>>
>>> Any chance there'd be interest in taking the opportunity to support
>>> dynamically-sized reads from userspace too? :)
>> That's bpf_probe_read_user_dynptr() from patch #2, no?
>>
>> But generally speaking, here's a list of new APIs that we'd need to
>> cover all existing fixed buffer versions:
>>
>> - non-sleepable probe reads:
>>
>>    bpf_probe_read_kernel_dynptr()
>>    bpf_probe_read_user_dynptr()
>>    bpf_probe_read_kernel_str_dynptr()
>>    bpf_probe_read_user_str_dynptr()
>>
>> - sleepable probe reads (copy_from_user):
>>
>> bpf_copy_from_user_dynptr()
>> bpf_copy_from_user_str_dynptr()
>>
>> - and then we have complementary task-based APIs for non-current process:
>>
>> bpf_probe_read_user_task_dynptr()
>> bpf_probe_read_user_str_task_dynptr()
>> bpf_copy_from_user_task_dynptr()
>> bpf_copy_from_user_str_task_dynptr()
>>
>> Jordan is working on non-dynptr version of
>> bpf_copy_from_user_str_task(), once he's done with that, we'll add
>> dynptr version, probably.
> This is quite a bunch of kfuncs.
> It doesn't look like adding _dynptr suffix and duplicating
> kfuncs approach scales.
>
> Let's make the existing helpers/kfuncs more flexible ?
>
> We can introduce a kfunc bpf_dynptr_buf() that checks that
> dynptr is not readonly and type == local or ringbuf and
> return dynptr->data as PTR_TO_MEM | dynptr_flag | VERIFIER_ADDS_SIZE_CHECK.
>
> Then allow bpf_probe_read_user/kernel/... all of them to accept
> this register type where PTR_TO_MEM is required
> while relaxing ARG_CONST_SIZE 2nd argument to ARG_ANYTHING.
> Then the verifier will insert an extra check
> if (arg1->size < arg2)
> before the call.
The verifier does static analysis of the program and I can't find any 
existing
example that modifies the program in the verifier. I don't think I am 
familiar
enough with the verifier to change it to modify the program to insert a 
bound
check. And I think inserting extra instructions into the program while 
verifying
it would add a lot of complexity.

But I think the verifier could instead verify that the user program contains
such a bound check before calling helpers like bpf_probe_read_user on the
buf and otherwise reject the program. That being said, I don't know how
the verifier could verify if there is a bound check in the user program.

Still, there lacks a mechanism to add another prototype for a helper.
I think we need such a mechanism for adding multiple prototypes for a helper
and the verifier will try them one by one.

So that we can have two prototypes for bpf_probe_read_user:

The old one and the new one where size is ARG_ANYTHING and dst is
PTR_TO_MANUAL_BOUND_CHECKED_MEM.
>
> Not only the bpf_probe_read_kernel/user, _str variants will work
> but things like bpf_strtol, bpf_strncmp, bpf_snprintf, bpf_get_stack
> will auto-magically work as well.
>
> I think those are quite valuable to make available with non-constant size.
> bpf_get_stack_*() directly into the ring buffer sounds very useful.

Re: [PATCH bpf-next v2 1/7] bpf: Implement bpf_probe_read_kernel_dynptr helper

[Levi Zim]

On 2025/1/28 06:04, Alexei Starovoitov wrote:
> On Sat, Jan 25, 2025 at 5:05 PM Levi Zim <rsworktech@outlook.com> wrote:
>> On 2025/1/26 00:58, Alexei Starovoitov wrote:
>>   > On Sat, Jan 25, 2025 at 12:30 AM Levi Zim via B4 Relay
>>   > <devnull+rsworktech.outlook.com@kernel.org> wrote:
>>   >> From: Levi Zim <rsworktech@outlook.com>
>>   >>
>>   >> This patch add a helper function bpf_probe_read_kernel_dynptr:
>>   >>
>>   >> long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst,
>>   >>          u32 offset, u32 size, const void *unsafe_ptr, u64 flags);
>>   > We stopped adding helpers years ago.
>>   > Only new kfuncs are allowed.
>>
>> Sorry, I didn't know that. Just asking, is there any
>> documentation/discussion
>> about stopping adding helpers?
>>
>> I will switch the implementation to kfuncs in v3.
>>
>>   > This particular one doesn't look useful as-is.
>>   > The same logic can be expressed with
>>   > - create dynptr
>>   > - dynptr_slice
>>   > - copy_from_kernel
>>
>> By copy_from_kernel I assume you mean bpf_probe_read_kernel. The problem
>> with dynptr_slice_rdwr and probe_read_kernel is that they only support a
>> compile-time constant size [1].
>>
>> But in order to best utilize the space on a BPF ringbuf, it is possible
>> to reserve a
>> variable length of space as dynptr on a ringbuf with
>> bpf_ringbuf_reserve_dynptr.
> That makes sense. The commit log didn't call it out.
> Please spell out the motivation clearly.
Thanks for the advice! I will include it in v3.
> Also why bpf_probe_read_kernel_common ?
> Do we need to memset() it on failure?
Since the current patch is basically a thin wrapper around 
bpf_probe_read_kernel,
I think we'd better not deviate from the wrapped function.

Re: [PATCH bpf-next v2 1/7] bpf: Implement bpf_probe_read_kernel_dynptr helper

[Andrii Nakryiko]

On Sat, Jan 25, 2025 at 5:05 PM Levi Zim <rsworktech@outlook.com> wrote:
>
> On 2025/1/26 00:58, Alexei Starovoitov wrote:
>  > On Sat, Jan 25, 2025 at 12:30 AM Levi Zim via B4 Relay
>  > <devnull+rsworktech.outlook.com@kernel.org> wrote:
>  >> From: Levi Zim <rsworktech@outlook.com>
>  >>
>  >> This patch add a helper function bpf_probe_read_kernel_dynptr:
>  >>
>  >> long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst,
>  >>          u32 offset, u32 size, const void *unsafe_ptr, u64 flags);
>  > We stopped adding helpers years ago.
>  > Only new kfuncs are allowed.
>
> Sorry, I didn't know that. Just asking, is there any
> documentation/discussion
> about stopping adding helpers?
>
> I will switch the implementation to kfuncs in v3.

you might want to look at Documentation/bpf/kfuncs.rst as well

>
>  > This particular one doesn't look useful as-is.
>  > The same logic can be expressed with
>  > - create dynptr
>  > - dynptr_slice
>  > - copy_from_kernel
>
> By copy_from_kernel I assume you mean bpf_probe_read_kernel. The problem
> with dynptr_slice_rdwr and probe_read_kernel is that they only support a
> compile-time constant size [1].
>
> But in order to best utilize the space on a BPF ringbuf, it is possible
> to reserve a
> variable length of space as dynptr on a ringbuf with
> bpf_ringbuf_reserve_dynptr.
>
> Then currently we have no way to read a variable length of kernel memory
> into this dynptr, except doing it chunk by chunk[2], which is kinda awkward.
> That's the problem the new helpers trying to solve.
> And I am not the only one needing this kind of feature [3].
>
> Andrii said it would be a straightforward addition as it is a super thin
> wrapper
> around existing functionality (we are just avoiding fixed buffer size
> restrictions of
> existing probe/copy_from APIs)

Yep, exactly. All the probe/copy APIs we have require statically known
dst buffer and, worst still, statically known read size, which makes
it very hard and cumbersome to request data which size is known at
runtime (e.g., process argv/environ data).

So, in short, I think these new dynptr-based APIs are a good addition,
but yeah, they'll have to be added as kfuncs.

One thing that we need to figure out is how to deal with SKB/XDP
dynptrs, we can't just reject them as destination buffers for all
possible APIs. I think we'll need to develop a set of internal helpers
that would handle skb/xdp dynptr cases almost transparently:

  - if skb/xdp memory is linear, we just memcpy/probe_read into it directly;
  - if it's not linear, we use on-the-stack small buffer (128 or 256
bytes, something like that), to do desired operation (memcpy or probe)
into that buffer, and then using generic dynptr write functionality
from that buffer into skb/xdp.

We probably can start simple with excluding xdp/skb, but this should
be the second step.

>
> [1]:
> https://elixir.bootlin.com/linux/v6.12.6/source/kernel/bpf/helpers.c#L2600-L2601
> [2]:
> https://github.com/libbpf/libbpf-bootstrap/commit/046fad60df3e39540937b5ec6ee86054f33d3f28
> [3]: https://github.com/libbpf/libbpf-rs/issues/1041
> [4]:
> https://lore.kernel.org/bpf/CAEf4BzZctXJsR+TwMhmXNWnR0_BV802-3KJw226ZZt8St4xNkw@mail.gmail.com/
>
>  > pw-bot: cr

[PATCH bpf-next v2 2/7] bpf: Implement bpf_probe_read_user_dynptr helper

[Levi Zim via B4 Relay]

From: Levi Zim <rsworktech@outlook.com>

This patch add a helper function bpf_probe_read_user_dynptr:

long bpf_probe_read_user_dynptr(const struct bpf_dynptr *dst,
	u32 offset, u32 size, const void *unsafe_ptr, u64 flags);

It is useful for reading variable-length data from user memory into
dynptr.

Signed-off-by: Levi Zim <rsworktech@outlook.com>
---
 include/uapi/linux/bpf.h | 16 ++++++++++
 kernel/bpf/helpers.c     |  3 ++
 kernel/trace/bpf_trace.c | 76 +++++++++++++++++++++++++++++++++---------------
 3 files changed, 71 insertions(+), 24 deletions(-)

diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 2e08a59527ecf56732ea14ac34446b5eb25b5690..d7d7a9ddd5dca07ba89d81ba77101a704af3163b 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5820,6 +5820,21 @@ union bpf_attr {
  *		support this helper, or if *flags* is not 0.
  *
  *		Or other negative errors on failure reading kernel memory.
+ *
+ * long bpf_probe_read_user_dynptr(const struct bpf_dynptr *dst, u32 offset, u32 size, const void *unsafe_ptr, u64 flags)
+ *	Description
+ *		Safely attempt to read *size* bytes from user space address
+ *		*unsafe_ptr* and store the data in *dst* starting from *offset*.
+ *		*flags* is currently unused.
+ *	Return
+ *		0 on success.
+ *
+ *		**-E2BIG** if *offset* + *len* exceeds the length of *src*'s data
+ *
+ *		**-EINVAL** if *src* is an invalid dynptr or doesn't support this
+ *		support this helper, or if *flags* is not 0.
+ *
+ *		Or other negative errors on failure reading user space memory.
  */
 #define ___BPF_FUNC_MAPPER(FN, ctx...)			\
 	FN(unspec, 0, ##ctx)				\
@@ -6035,6 +6050,7 @@ union bpf_attr {
 	FN(cgrp_storage_get, 210, ##ctx)		\
 	FN(cgrp_storage_delete, 211, ##ctx)		\
 	FN(probe_read_kernel_dynptr, 212, ##ctx)		\
+	FN(probe_read_user_dynptr, 213, ##ctx)		\
 	/* */
 
 /* backwards-compatibility macros for users of __BPF_FUNC_MAPPER that don't
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index a736dc9e7be98571103ba404420be0da4dac4fbe..ac563d09082e7c721999d7de035aabc000206a29 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -1898,6 +1898,7 @@ const struct bpf_func_proto bpf_get_current_task_proto __weak;
 const struct bpf_func_proto bpf_get_current_task_btf_proto __weak;
 const struct bpf_func_proto bpf_probe_read_user_proto __weak;
 const struct bpf_func_proto bpf_probe_read_user_str_proto __weak;
+const struct bpf_func_proto bpf_probe_read_user_dynptr_proto __weak;
 const struct bpf_func_proto bpf_probe_read_kernel_proto __weak;
 const struct bpf_func_proto bpf_probe_read_kernel_str_proto __weak;
 const struct bpf_func_proto bpf_probe_read_kernel_dynptr_proto __weak;
@@ -2029,6 +2030,8 @@ bpf_base_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_get_current_task_btf_proto;
 	case BPF_FUNC_probe_read_user:
 		return &bpf_probe_read_user_proto;
+	case BPF_FUNC_probe_read_user_dynptr:
+		return &bpf_probe_read_user_dynptr_proto;
 	case BPF_FUNC_probe_read_kernel:
 		return security_locked_down(LOCKDOWN_BPF_READ_KERNEL) < 0 ?
 		       NULL : &bpf_probe_read_kernel_proto;
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index 75c9d1e8d04c3b8930ae81345f5586756ce8b5ec..d9f704c1342773c74b2414be4adfc8271d6d364d 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -181,6 +181,36 @@ bpf_probe_read_user_common(void *dst, u32 size, const void __user *unsafe_ptr)
 	return ret;
 }
 
+static int bpf_probe_read_check_dynptr(const struct bpf_dynptr_kern *dst,
+	u32 offset, u32 size, u64 flags)
+{
+	enum bpf_dynptr_type type;
+	int err;
+
+	if (!dst->data || __bpf_dynptr_is_rdonly(dst))
+		return -EINVAL;
+
+	err = bpf_dynptr_check_off_len(dst, offset, size);
+	if (err)
+		return err;
+
+	type = bpf_dynptr_get_type(dst);
+
+	switch (type) {
+	case BPF_DYNPTR_TYPE_LOCAL:
+	case BPF_DYNPTR_TYPE_RINGBUF:
+		if (flags)
+			return -EINVAL;
+		return 0;
+	case BPF_DYNPTR_TYPE_SKB:
+	case BPF_DYNPTR_TYPE_XDP:
+		return -EINVAL;
+	default:
+		WARN_ONCE(true, "%s: unknown dynptr type %d\n", __func__, type);
+		return -EFAULT;
+	}
+}
+
 BPF_CALL_3(bpf_probe_read_user, void *, dst, u32, size,
 	   const void __user *, unsafe_ptr)
 {
@@ -196,6 +226,26 @@ const struct bpf_func_proto bpf_probe_read_user_proto = {
 	.arg3_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_5(bpf_probe_read_user_dynptr, const struct bpf_dynptr_kern *, dst,
+	u32, offset, u32, size, void *, unsafe_ptr, u64, flags)
+{
+	int ret = bpf_probe_read_check_dynptr(dst, offset, size, flags);
+
+	return ret ?: bpf_probe_read_user_common(dst->data + dst->offset + offset,
+				size, unsafe_ptr);
+}
+
+const struct bpf_func_proto bpf_probe_read_user_dynptr_proto = {
+	.func		= bpf_probe_read_user_dynptr,
+	.gpl_only	= true,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_DYNPTR | MEM_RDONLY,
+	.arg2_type	= ARG_ANYTHING,
+	.arg3_type	= ARG_ANYTHING,
+	.arg4_type	= ARG_ANYTHING,
+	.arg5_type	= ARG_ANYTHING,
+};
+
 static __always_inline int
 bpf_probe_read_user_str_common(void *dst, u32 size,
 			       const void __user *unsafe_ptr)
@@ -251,32 +301,10 @@ const struct bpf_func_proto bpf_probe_read_kernel_proto = {
 BPF_CALL_5(bpf_probe_read_kernel_dynptr, const struct bpf_dynptr_kern *, dst,
 	u32, offset, u32, size, void *, unsafe_ptr, u64, flags)
 {
-	enum bpf_dynptr_type type;
-	int err;
-
-	if (!dst->data || __bpf_dynptr_is_rdonly(dst))
-		return -EINVAL;
+	int ret = bpf_probe_read_check_dynptr(dst, offset, size, flags);
 
-	err = bpf_dynptr_check_off_len(dst, offset, size);
-	if (err)
-		return err;
-
-	type = bpf_dynptr_get_type(dst);
-
-	switch (type) {
-	case BPF_DYNPTR_TYPE_LOCAL:
-	case BPF_DYNPTR_TYPE_RINGBUF:
-		if (flags)
-			return -EINVAL;
-		return bpf_probe_read_kernel_common(dst->data + dst->offset + offset,
+	return ret ?: bpf_probe_read_kernel_common(dst->data + dst->offset + offset,
 				size, unsafe_ptr);
-	case BPF_DYNPTR_TYPE_SKB:
-	case BPF_DYNPTR_TYPE_XDP:
-		return -EINVAL;
-	default:
-		WARN_ONCE(true, "%s: unknown dynptr type %d\n", __func__, type);
-		return -EFAULT;
-	}
 }
 
 const struct bpf_func_proto bpf_probe_read_kernel_dynptr_proto = {

-- 
2.48.1

Re: [PATCH bpf-next v2 2/7] bpf: Implement bpf_probe_read_user_dynptr helper

[kernel test robot]

Hi Levi,

kernel test robot noticed the following build warnings:

[auto build test WARNING on d0d106a2bd21499901299160744e5fe9f4c83ddb]

url:    https://github.com/intel-lab-lkp/linux/commits/Levi-Zim-via-B4-Relay/bpf-Implement-bpf_probe_read_kernel_dynptr-helper/20250125-163114
base:   d0d106a2bd21499901299160744e5fe9f4c83ddb
patch link:    https://lore.kernel.org/r/20250125-bpf_dynptr_probe-v2-2-c42c87f97afe%40outlook.com
patch subject: [PATCH bpf-next v2 2/7] bpf: Implement bpf_probe_read_user_dynptr helper
config: x86_64-randconfig-122-20250127 (https://download.01.org/0day-ci/archive/20250127/202501272059.wikSvChi-lkp@intel.com/config)
compiler: clang version 19.1.3 (https://github.com/llvm/llvm-project ab51eccf88f5321e7c60591c5546b254b6afab99)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250127/202501272059.wikSvChi-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501272059.wikSvChi-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
   kernel/trace/bpf_trace.c:899:41: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected void [noderef] __user *[addressable] [assigned] [usertype] sival_ptr @@     got void * @@
   kernel/trace/bpf_trace.c:899:41: sparse:     expected void [noderef] __user *[addressable] [assigned] [usertype] sival_ptr
   kernel/trace/bpf_trace.c:899:41: sparse:     got void *
>> kernel/trace/bpf_trace.c:235:39: sparse: sparse: incorrect type in argument 3 (different address spaces) @@     expected void const [noderef] __user *unsafe_ptr @@     got void *unsafe_ptr @@
   kernel/trace/bpf_trace.c:235:39: sparse:     expected void const [noderef] __user *unsafe_ptr
   kernel/trace/bpf_trace.c:235:39: sparse:     got void *unsafe_ptr
   kernel/trace/bpf_trace.c: note: in included file (through include/linux/smp.h, include/linux/lockdep.h, include/linux/spinlock.h, ...):
   include/linux/list.h:83:21: sparse: sparse: self-comparison always evaluates to true
   kernel/trace/bpf_trace.c: note: in included file (through include/linux/rbtree.h, include/linux/mm_types.h, include/linux/mmzone.h, ...):
   include/linux/rcupdate.h:880:25: sparse: sparse: context imbalance in 'uprobe_prog_run' - unexpected unlock

vim +235 kernel/trace/bpf_trace.c

   228	
   229	BPF_CALL_5(bpf_probe_read_user_dynptr, const struct bpf_dynptr_kern *, dst,
   230		u32, offset, u32, size, void *, unsafe_ptr, u64, flags)
   231	{
   232		int ret = bpf_probe_read_check_dynptr(dst, offset, size, flags);
   233	
   234		return ret ?: bpf_probe_read_user_common(dst->data + dst->offset + offset,
 > 235					size, unsafe_ptr);
   236	}
   237	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

[PATCH bpf-next v2 3/7] bpf: Implement bpf_copy_from_user_dynptr helper

[Levi Zim via B4 Relay]

From: Levi Zim <rsworktech@outlook.com>

This patch add a helper function bpf_copy_from_user_dynptr:

bpf_copy_from_user_dynptr(const struct bpf_dynptr *dst, u32 offset,
	u32 size, const void *user_ptr, u64 flags)

It is useful for reading variable-length data from kernel memory into
dynptr.

Signed-off-by: Levi Zim <rsworktech@outlook.com>
---
 include/linux/bpf.h      |  1 +
 include/uapi/linux/bpf.h | 17 +++++++++++++++++
 kernel/bpf/helpers.c     | 42 ++++++++++++++++++++++++++++++++++++++++++
 kernel/trace/bpf_trace.c |  2 ++
 4 files changed, 62 insertions(+)

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 9d5ae8b4b7d82c4523bf0ab041d4b76bf134a106..d0412eaf63d69c0e437575c77008548edc692335 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -3357,6 +3357,7 @@ extern const struct bpf_func_proto bpf_get_retval_proto;
 extern const struct bpf_func_proto bpf_user_ringbuf_drain_proto;
 extern const struct bpf_func_proto bpf_cgrp_storage_get_proto;
 extern const struct bpf_func_proto bpf_cgrp_storage_delete_proto;
+extern const struct bpf_func_proto bpf_copy_from_user_dynptr_proto;
 
 const struct bpf_func_proto *tracing_prog_func_proto(
   enum bpf_func_id func_id, const struct bpf_prog *prog);
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index d7d7a9ddd5dca07ba89d81ba77101a704af3163b..f92cf809b50bc393d54eb0e8de2e1ce2a39e95d0 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -5835,6 +5835,22 @@ union bpf_attr {
  *		support this helper, or if *flags* is not 0.
  *
  *		Or other negative errors on failure reading user space memory.
+ *
+ * long bpf_copy_from_user_dynptr(const struct bpf_dynptr *dst, u32 offset, u32 size, const void *user_ptr, u64 flags)
+ *	Description
+ *		Read *size* bytes from user space address *user_ptr* and store
+ *		the data in *dst* starting from *offset*.
+ *		This is a wrapper of **copy_from_user**\ ().
+ *		*flags* is currently unused.
+ *	Return
+ *		0 on success.
+ *
+ *		**-E2BIG** if *offset* + *len* exceeds the length of *src*'s data
+ *
+ *		**-EINVAL** if *src* is an invalid dynptr or doesn't support this
+ *		support this helper, or if *flags* is not 0.
+ *
+ *		Or other negative errors on failure reading user space memory.
  */
 #define ___BPF_FUNC_MAPPER(FN, ctx...)			\
 	FN(unspec, 0, ##ctx)				\
@@ -6051,6 +6067,7 @@ union bpf_attr {
 	FN(cgrp_storage_delete, 211, ##ctx)		\
 	FN(probe_read_kernel_dynptr, 212, ##ctx)		\
 	FN(probe_read_user_dynptr, 213, ##ctx)		\
+	FN(copy_from_user_dynptr, 214, ##ctx)		\
 	/* */
 
 /* backwards-compatibility macros for users of __BPF_FUNC_MAPPER that don't
diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index ac563d09082e7c721999d7de035aabc000206a29..d756c80596315bd07fe6e71885b61efc8cb2ef4f 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -676,6 +676,48 @@ const struct bpf_func_proto bpf_copy_from_user_proto = {
 	.arg3_type	= ARG_ANYTHING,
 };
 
+BPF_CALL_5(bpf_copy_from_user_dynptr, const struct bpf_dynptr_kern *, dst,
+	u32, offset, u32, size, const void __user *, user_ptr, u32, flags)
+{
+	enum bpf_dynptr_type type;
+	int err;
+
+	if (!dst->data || __bpf_dynptr_is_rdonly(dst))
+		return -EINVAL;
+
+	err = bpf_dynptr_check_off_len(dst, offset, size);
+	if (err)
+		return err;
+
+	type = bpf_dynptr_get_type(dst);
+
+	switch (type) {
+	case BPF_DYNPTR_TYPE_LOCAL:
+	case BPF_DYNPTR_TYPE_RINGBUF:
+		if (flags)
+			return -EINVAL;
+		return ____bpf_copy_from_user(dst->data + dst->offset + offset, size, user_ptr);
+	case BPF_DYNPTR_TYPE_SKB:
+	case BPF_DYNPTR_TYPE_XDP:
+		return -EINVAL;
+	default:
+		WARN_ONCE(true, "%s: unknown dynptr type %d\n", __func__, type);
+		return -EFAULT;
+	}
+}
+
+const struct bpf_func_proto bpf_copy_from_user_dynptr_proto = {
+	.func		= bpf_copy_from_user_dynptr,
+	.gpl_only	= false,
+	.might_sleep	= true,
+	.ret_type	= RET_INTEGER,
+	.arg1_type	= ARG_PTR_TO_DYNPTR | MEM_RDONLY,
+	.arg2_type	= ARG_ANYTHING,
+	.arg3_type	= ARG_ANYTHING,
+	.arg4_type	= ARG_ANYTHING,
+	.arg5_type	= ARG_ANYTHING,
+};
+
 BPF_CALL_5(bpf_copy_from_user_task, void *, dst, u32, size,
 	   const void __user *, user_ptr, struct task_struct *, tsk, u64, flags)
 {
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index d9f704c1342773c74b2414be4adfc8271d6d364d..424931925fe3b02db083bc19cc64e19918b40c5a 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -1598,6 +1598,8 @@ bpf_tracing_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog)
 		return &bpf_copy_from_user_proto;
 	case BPF_FUNC_copy_from_user_task:
 		return &bpf_copy_from_user_task_proto;
+	case BPF_FUNC_copy_from_user_dynptr:
+		return &bpf_copy_from_user_dynptr_proto;
 	case BPF_FUNC_snprintf_btf:
 		return &bpf_snprintf_btf_proto;
 	case BPF_FUNC_per_cpu_ptr:

-- 
2.48.1

[PATCH bpf-next v2 4/7] tools headers UAPI: Update tools's copy of bpf.h header

[Levi Zim via B4 Relay]

From: Levi Zim <rsworktech@outlook.com>

This update brings the new bpf_probe_read_{kernel,user} helpers

Signed-off-by: Levi Zim <rsworktech@outlook.com>
---
 tools/include/uapi/linux/bpf.h | 49 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 2acf9b33637174bd16b1d12ccc6410c5f55a7ea9..f92cf809b50bc393d54eb0e8de2e1ce2a39e95d0 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -5805,6 +5805,52 @@ union bpf_attr {
  *		0 on success.
  *
  *		**-ENOENT** if the bpf_local_storage cannot be found.
+ *
+ * long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst, u32 offset, u32 size, const void *unsafe_ptr, u64 flags)
+ *	Description
+ *		Safely attempt to read *size* bytes from kernel space address
+ *		*unsafe_ptr* and store the data in *dst* starting from *offset*.
+ *		*flags* is currently unused.
+ *	Return
+ *		0 on success.
+ *
+ *		**-E2BIG** if *offset* + *len* exceeds the length of *src*'s data
+ *
+ *		**-EINVAL** if *src* is an invalid dynptr or doesn't support this
+ *		support this helper, or if *flags* is not 0.
+ *
+ *		Or other negative errors on failure reading kernel memory.
+ *
+ * long bpf_probe_read_user_dynptr(const struct bpf_dynptr *dst, u32 offset, u32 size, const void *unsafe_ptr, u64 flags)
+ *	Description
+ *		Safely attempt to read *size* bytes from user space address
+ *		*unsafe_ptr* and store the data in *dst* starting from *offset*.
+ *		*flags* is currently unused.
+ *	Return
+ *		0 on success.
+ *
+ *		**-E2BIG** if *offset* + *len* exceeds the length of *src*'s data
+ *
+ *		**-EINVAL** if *src* is an invalid dynptr or doesn't support this
+ *		support this helper, or if *flags* is not 0.
+ *
+ *		Or other negative errors on failure reading user space memory.
+ *
+ * long bpf_copy_from_user_dynptr(const struct bpf_dynptr *dst, u32 offset, u32 size, const void *user_ptr, u64 flags)
+ *	Description
+ *		Read *size* bytes from user space address *user_ptr* and store
+ *		the data in *dst* starting from *offset*.
+ *		This is a wrapper of **copy_from_user**\ ().
+ *		*flags* is currently unused.
+ *	Return
+ *		0 on success.
+ *
+ *		**-E2BIG** if *offset* + *len* exceeds the length of *src*'s data
+ *
+ *		**-EINVAL** if *src* is an invalid dynptr or doesn't support this
+ *		support this helper, or if *flags* is not 0.
+ *
+ *		Or other negative errors on failure reading user space memory.
  */
 #define ___BPF_FUNC_MAPPER(FN, ctx...)			\
 	FN(unspec, 0, ##ctx)				\
@@ -6019,6 +6065,9 @@ union bpf_attr {
 	FN(user_ringbuf_drain, 209, ##ctx)		\
 	FN(cgrp_storage_get, 210, ##ctx)		\
 	FN(cgrp_storage_delete, 211, ##ctx)		\
+	FN(probe_read_kernel_dynptr, 212, ##ctx)		\
+	FN(probe_read_user_dynptr, 213, ##ctx)		\
+	FN(copy_from_user_dynptr, 214, ##ctx)		\
 	/* */
 
 /* backwards-compatibility macros for users of __BPF_FUNC_MAPPER that don't

-- 
2.48.1

Re: [PATCH bpf-next v2 4/7] tools headers UAPI: Update tools's copy of bpf.h header

[Andrii Nakryiko]

On Sat, Jan 25, 2025 at 12:30 AM Levi Zim via B4 Relay
<devnull+rsworktech.outlook.com@kernel.org> wrote:
>
> From: Levi Zim <rsworktech@outlook.com>
>
> This update brings the new bpf_probe_read_{kernel,user} helpers
>
> Signed-off-by: Levi Zim <rsworktech@outlook.com>
> ---
>  tools/include/uapi/linux/bpf.h | 49 ++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 49 insertions(+)
>
> diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
> index 2acf9b33637174bd16b1d12ccc6410c5f55a7ea9..f92cf809b50bc393d54eb0e8de2e1ce2a39e95d0 100644
> --- a/tools/include/uapi/linux/bpf.h
> +++ b/tools/include/uapi/linux/bpf.h
> @@ -5805,6 +5805,52 @@ union bpf_attr {
>   *             0 on success.
>   *
>   *             **-ENOENT** if the bpf_local_storage cannot be found.
> + *
> + * long bpf_probe_read_kernel_dynptr(const struct bpf_dynptr *dst, u32 offset, u32 size, const void *unsafe_ptr, u64 flags)
> + *     Description
> + *             Safely attempt to read *size* bytes from kernel space address
> + *             *unsafe_ptr* and store the data in *dst* starting from *offset*.
> + *             *flags* is currently unused.
> + *     Return
> + *             0 on success.
> + *
> + *             **-E2BIG** if *offset* + *len* exceeds the length of *src*'s data
> + *
> + *             **-EINVAL** if *src* is an invalid dynptr or doesn't support this
> + *             support this helper, or if *flags* is not 0.
> + *
> + *             Or other negative errors on failure reading kernel memory.
> + *
> + * long bpf_probe_read_user_dynptr(const struct bpf_dynptr *dst, u32 offset, u32 size, const void *unsafe_ptr, u64 flags)
> + *     Description
> + *             Safely attempt to read *size* bytes from user space address
> + *             *unsafe_ptr* and store the data in *dst* starting from *offset*.
> + *             *flags* is currently unused.
> + *     Return
> + *             0 on success.
> + *
> + *             **-E2BIG** if *offset* + *len* exceeds the length of *src*'s data
> + *
> + *             **-EINVAL** if *src* is an invalid dynptr or doesn't support this
> + *             support this helper, or if *flags* is not 0.
> + *
> + *             Or other negative errors on failure reading user space memory.
> + *
> + * long bpf_copy_from_user_dynptr(const struct bpf_dynptr *dst, u32 offset, u32 size, const void *user_ptr, u64 flags)
> + *     Description
> + *             Read *size* bytes from user space address *user_ptr* and store
> + *             the data in *dst* starting from *offset*.
> + *             This is a wrapper of **copy_from_user**\ ().
> + *             *flags* is currently unused.
> + *     Return
> + *             0 on success.
> + *
> + *             **-E2BIG** if *offset* + *len* exceeds the length of *src*'s data
> + *
> + *             **-EINVAL** if *src* is an invalid dynptr or doesn't support this
> + *             support this helper, or if *flags* is not 0.
> + *
> + *             Or other negative errors on failure reading user space memory.
>   */
>  #define ___BPF_FUNC_MAPPER(FN, ctx...)                 \
>         FN(unspec, 0, ##ctx)                            \
> @@ -6019,6 +6065,9 @@ union bpf_attr {
>         FN(user_ringbuf_drain, 209, ##ctx)              \
>         FN(cgrp_storage_get, 210, ##ctx)                \
>         FN(cgrp_storage_delete, 211, ##ctx)             \

we probably want to add some big and very noticeable comment here
suggesting kfunc route, to save people time next time. Please consider
adding it as a separate patch for v2.

> +       FN(probe_read_kernel_dynptr, 212, ##ctx)                \
> +       FN(probe_read_user_dynptr, 213, ##ctx)          \
> +       FN(copy_from_user_dynptr, 214, ##ctx)           \
>         /* */
>
>  /* backwards-compatibility macros for users of __BPF_FUNC_MAPPER that don't
>
> --
> 2.48.1
>
>

[PATCH bpf-next v2 5/7] selftests/bpf: probe_read_kernel_dynptr test

[Levi Zim via B4 Relay]

From: Levi Zim <rsworktech@outlook.com>

This patch adds a test for probe_read_kernel_dynptr helper,
which tests reading variable length of kernel memory into a ringbuf
backed dynptr.

Signed-off-by: Levi Zim <rsworktech@outlook.com>
---
 tools/testing/selftests/bpf/prog_tests/dynptr.c    | 40 +++++++++++++++++++++-
 tools/testing/selftests/bpf/progs/dynptr_success.c | 38 ++++++++++++++++++++
 2 files changed, 77 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c
index b614a5272dfd6486e287181270a0bcf63f638344..d9a25c2873b6ed4219e063845b1caf978a7016fd 100644
--- a/tools/testing/selftests/bpf/prog_tests/dynptr.c
+++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c
@@ -10,6 +10,7 @@ enum test_setup_type {
 	SETUP_SYSCALL_SLEEP,
 	SETUP_SKB_PROG,
 	SETUP_SKB_PROG_TP,
+	SETUP_RINGBUF,
 };
 
 static struct {
@@ -30,20 +31,37 @@ 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_kernel_dynptr", SETUP_RINGBUF},
 };
 
+static int ringbuf_cb(void *ctx, void *data, size_t len)
+{
+	struct dynptr_success *skel = ctx;
+	const char *buf = data;
+
+	if (!ASSERT_EQ(len, skel->data->test_buf.length, "length"))
+		return -E2BIG;
+
+	if (!ASSERT_MEMEQ(buf, skel->data->test_buf.buf, len, "ringbuf_cb"))
+		return -EINVAL;
+
+	return 0;
+}
+
 static void verify_success(const char *prog_name, enum test_setup_type setup_type)
 {
+	struct ring_buffer *rb = NULL;
 	struct dynptr_success *skel;
 	struct bpf_program *prog;
 	struct bpf_link *link;
-	int err;
+	int err, ret;
 
 	skel = dynptr_success__open();
 	if (!ASSERT_OK_PTR(skel, "dynptr_success__open"))
 		return;
 
 	skel->bss->pid = getpid();
+	skel->data->test_buf.length = 8;
 
 	prog = bpf_object__find_program_by_name(skel->obj, prog_name);
 	if (!ASSERT_OK_PTR(prog, "bpf_object__find_program_by_name"))
@@ -63,6 +81,24 @@ static void verify_success(const char *prog_name, enum test_setup_type setup_typ
 
 		usleep(1);
 
+		bpf_link__destroy(link);
+		break;
+	case SETUP_RINGBUF:
+		link = bpf_program__attach(prog);
+		if (!ASSERT_OK_PTR(link, "bpf_program__attach"))
+			goto cleanup;
+
+		rb = ring_buffer__new(bpf_map__fd(skel->maps.ringbuf), ringbuf_cb, skel, NULL);
+		if (!ASSERT_OK_PTR(rb, "ring_buffer__new"))
+			goto cleanup;
+
+		usleep(1);
+
+		ret = ring_buffer__poll(rb, 5000);
+
+		if (!ASSERT_EQ(ret, 1, "ring_buffer__poll"))
+			goto cleanup;
+
 		bpf_link__destroy(link);
 		break;
 	case SETUP_SKB_PROG:
@@ -125,6 +161,8 @@ static void verify_success(const char *prog_name, enum test_setup_type setup_typ
 	ASSERT_EQ(skel->bss->err, 0, "err");
 
 cleanup:
+	if (rb != NULL)
+		ring_buffer__free(rb);
 	dynptr_success__destroy(skel);
 }
 
diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c b/tools/testing/selftests/bpf/progs/dynptr_success.c
index bfcc85686cf046361b451f97f4cd310a6ccdb1ed..64c698f83a37bfe924db93d36982a0a1c8defe62 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_success.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_success.c
@@ -567,3 +567,41 @@ int BPF_PROG(test_dynptr_skb_tp_btf, void *skb, void *location)
 
 	return 1;
 }
+
+#define MAX_BUFFER_LEN 20
+
+struct {
+	__u32 length;
+	char buf[MAX_BUFFER_LEN];
+} test_buf = {
+	.length = 0,
+	.buf = "0123456789abcdef",
+};
+
+SEC("?tp/syscalls/sys_enter_nanosleep")
+int test_probe_read_kernel_dynptr(void *ctx)
+{
+	int copy_len = test_buf.length;
+	struct bpf_dynptr ptr;
+
+	if (bpf_get_current_pid_tgid() >> 32 != pid)
+		return 0;
+
+	if (test_buf.length > MAX_BUFFER_LEN)
+		copy_len = MAX_BUFFER_LEN;
+
+	bpf_ringbuf_reserve_dynptr(&ringbuf, copy_len, 0, &ptr);
+
+	if (-E2BIG != bpf_probe_read_kernel_dynptr(&ptr, 0, MAX_BUFFER_LEN + 1,
+		test_buf.buf, 0)) {
+		err = 1;
+		goto cleanup;
+	}
+
+	err = bpf_probe_read_kernel_dynptr(&ptr, 0, copy_len,
+		test_buf.buf, 0);
+
+cleanup:
+	bpf_ringbuf_submit_dynptr(&ptr, 0);
+	return 0;
+}

-- 
2.48.1

[PATCH bpf-next v2 6/7] selftests/bpf: probe_read_user_dynptr test

[Levi Zim via B4 Relay]

From: Levi Zim <rsworktech@outlook.com>

This patch adds a test for probe_read_user_dynptr helper,
which tests reading variable length of user memory into a ringbuf
backed dynptr.

Signed-off-by: Levi Zim <rsworktech@outlook.com>
---
 tools/testing/selftests/bpf/prog_tests/dynptr.c    |  2 ++
 tools/testing/selftests/bpf/progs/dynptr_success.c | 36 +++++++++++++++++++++-
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c
index d9a25c2873b6ed4219e063845b1caf978a7016fd..a5a61a4c570d803bfca2af3f1a3b7d5eb0b8197f 100644
--- a/tools/testing/selftests/bpf/prog_tests/dynptr.c
+++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c
@@ -32,6 +32,7 @@ static struct {
 	{"test_dynptr_skb_strcmp", SETUP_SKB_PROG},
 	{"test_dynptr_skb_tp_btf", SETUP_SKB_PROG_TP},
 	{"test_probe_read_kernel_dynptr", SETUP_RINGBUF},
+	{"test_probe_read_user_dynptr", SETUP_RINGBUF},
 };
 
 static int ringbuf_cb(void *ctx, void *data, size_t len)
@@ -61,6 +62,7 @@ static void verify_success(const char *prog_name, enum test_setup_type setup_typ
 		return;
 
 	skel->bss->pid = getpid();
+	skel->bss->user_ptr = (void *)&skel->data->test_buf;
 	skel->data->test_buf.length = 8;
 
 	prog = bpf_object__find_program_by_name(skel->obj, prog_name);
diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c b/tools/testing/selftests/bpf/progs/dynptr_success.c
index 64c698f83a37bfe924db93d36982a0a1c8defe62..5317860290dccb0862b6a0b94bb6f738c8d92835 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_success.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_success.c
@@ -576,7 +576,7 @@ struct {
 } test_buf = {
 	.length = 0,
 	.buf = "0123456789abcdef",
-};
+}, *user_ptr;
 
 SEC("?tp/syscalls/sys_enter_nanosleep")
 int test_probe_read_kernel_dynptr(void *ctx)
@@ -605,3 +605,37 @@ int test_probe_read_kernel_dynptr(void *ctx)
 	bpf_ringbuf_submit_dynptr(&ptr, 0);
 	return 0;
 }
+
+SEC("?tp/syscalls/sys_enter_nanosleep")
+int test_probe_read_user_dynptr(void *ctx)
+{
+	struct bpf_dynptr ptr;
+	int copy_len;
+
+	if (bpf_get_current_pid_tgid() >> 32 != pid)
+		return 0;
+
+	err = bpf_probe_read_user(&copy_len, sizeof(copy_len), &user_ptr->length);
+	if (err < 0 || copy_len < 0) {
+		err = 1;
+		return 1;
+	}
+
+	if (copy_len > MAX_BUFFER_LEN)
+		copy_len = MAX_BUFFER_LEN;
+
+	bpf_ringbuf_reserve_dynptr(&ringbuf, copy_len, 0, &ptr);
+
+	if (-E2BIG != bpf_probe_read_user_dynptr(&ptr, 0, MAX_BUFFER_LEN + 1,
+		&user_ptr->buf, 0)) {
+		err = 2;
+		goto cleanup;
+	}
+
+	err = bpf_probe_read_user_dynptr(&ptr, 0, copy_len,
+		&user_ptr->buf, 0);
+
+cleanup:
+	bpf_ringbuf_submit_dynptr(&ptr, 0);
+	return 0;
+}

-- 
2.48.1

[PATCH bpf-next v2 7/7] selftests/bpf: copy_from_user_dynptr test

[Levi Zim via B4 Relay]

From: Levi Zim <rsworktech@outlook.com>

This patch adds a test for copy_from_user_dynptr helper,
which tests reading variable length of user memory into a ringbuf
backed dynptr.

Signed-off-by: Levi Zim <rsworktech@outlook.com>
---
 tools/testing/selftests/bpf/prog_tests/dynptr.c    |  3 ++
 tools/testing/selftests/bpf/progs/dynptr_success.c | 34 ++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c
index a5a61a4c570d803bfca2af3f1a3b7d5eb0b8197f..a0095204609c710d49a6105ebf8be44723fb53d2 100644
--- a/tools/testing/selftests/bpf/prog_tests/dynptr.c
+++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c
@@ -33,6 +33,7 @@ static struct {
 	{"test_dynptr_skb_tp_btf", SETUP_SKB_PROG_TP},
 	{"test_probe_read_kernel_dynptr", SETUP_RINGBUF},
 	{"test_probe_read_user_dynptr", SETUP_RINGBUF},
+	{"test_copy_from_user_dynptr", SETUP_RINGBUF},
 };
 
 static int ringbuf_cb(void *ctx, void *data, size_t len)
@@ -43,6 +44,8 @@ static int ringbuf_cb(void *ctx, void *data, size_t len)
 	if (!ASSERT_EQ(len, skel->data->test_buf.length, "length"))
 		return -E2BIG;
 
+	ASSERT_EQ(skel->bss->err, 0, "err");
+
 	if (!ASSERT_MEMEQ(buf, skel->data->test_buf.buf, len, "ringbuf_cb"))
 		return -EINVAL;
 
diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c b/tools/testing/selftests/bpf/progs/dynptr_success.c
index 5317860290dccb0862b6a0b94bb6f738c8d92835..10dda58fed66a05ad1b08c8684784fddabe3f4ad 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_success.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_success.c
@@ -639,3 +639,37 @@ int test_probe_read_user_dynptr(void *ctx)
 	bpf_ringbuf_submit_dynptr(&ptr, 0);
 	return 0;
 }
+
+SEC("?fentry.s/" SYS_PREFIX "sys_nanosleep")
+int test_copy_from_user_dynptr(void *ctx)
+{
+	struct bpf_dynptr ptr;
+	int copy_len;
+
+	if (bpf_get_current_pid_tgid() >> 32 != pid)
+		return 0;
+
+	err = bpf_probe_read_user(&copy_len, sizeof(copy_len), &user_ptr->length);
+	if (err < 0 || copy_len < 0) {
+		err = 1;
+		return 0;
+	}
+
+	if (copy_len > MAX_BUFFER_LEN)
+		copy_len = MAX_BUFFER_LEN;
+
+	bpf_ringbuf_reserve_dynptr(&ringbuf, copy_len, 0, &ptr);
+
+	if (-E2BIG != bpf_copy_from_user_dynptr(&ptr, 0, MAX_BUFFER_LEN + 1,
+		&user_ptr->buf, 0)) {
+		err = 2;
+		goto cleanup;
+	}
+
+	err = bpf_copy_from_user_dynptr(&ptr, 0, copy_len,
+		&user_ptr->buf, 0);
+
+cleanup:
+	bpf_ringbuf_submit_dynptr(&ptr, 0);
+	return 0;
+}

-- 
2.48.1