[PATCH bpf-next 1/2] bpf: add bpf_dynptr_memset() kfunc

[PATCH bpf-next 1/2] bpf: add bpf_dynptr_memset() kfunc

[Ihor Solodrai]

Currently there is no straightforward way to fill dynptr memory with a
value (most commonly zero). One can do it with bpf_dynptr_write(), but
a temporary buffer is necessary for that.

Implement bpf_dynptr_memset() - an analogue of memset() from libc.

Signed-off-by: Ihor Solodrai <isolodrai@meta.com>
---
 kernel/bpf/helpers.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index b71e428ad936..dfd04628a522 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2906,6 +2906,33 @@ __bpf_kfunc int bpf_dynptr_copy(struct bpf_dynptr *dst_ptr, u32 dst_off,
 	return 0;
 }
 
+/**
+ * bpf_dynptr_memset() - Fill dynptr memory with a constant byte.
+ * @ptr: Destination dynptr - where data will be filled
+ * @val: Constant byte to fill the memory with
+ * @n: Number of bytes to fill
+ *
+ * Fills the first n bytes of the memory area pointed to by ptr
+ * with the constant byte val.
+ * Returns 0 on success; negative error, otherwise.
+ */
+ __bpf_kfunc int bpf_dynptr_memset(struct bpf_dynptr *ptr, u8 val, u32 n)
+ {
+	struct bpf_dynptr_kern *p = (struct bpf_dynptr_kern *)ptr;
+	int err;
+
+	if (__bpf_dynptr_is_rdonly(p))
+		return -EINVAL;
+
+	err = bpf_dynptr_check_off_len(p, 0, n);
+	if (err)
+		return err;
+
+	memset(p->data + p->offset, val, n);
+
+	return 0;
+}
+
 __bpf_kfunc void *bpf_cast_to_kern_ctx(void *obj)
 {
 	return obj;
@@ -3364,6 +3391,7 @@ BTF_ID_FLAGS(func, bpf_dynptr_is_rdonly)
 BTF_ID_FLAGS(func, bpf_dynptr_size)
 BTF_ID_FLAGS(func, bpf_dynptr_clone)
 BTF_ID_FLAGS(func, bpf_dynptr_copy)
+BTF_ID_FLAGS(func, bpf_dynptr_memset)
 #ifdef CONFIG_NET
 BTF_ID_FLAGS(func, bpf_modify_return_test_tp)
 #endif
-- 
2.47.1

[PATCH bpf-next 2/2] selftests/bpf: add test cases for bpf_dynptr_memset()

[Ihor Solodrai]

Add tests to verify the behavior of bpf_dynptr_memset():
  * normal memset 0
  * memset in dynptr that was adjusted
  * error: size overflow
  * error: readonly dynptr

Signed-off-by: Ihor Solodrai <isolodrai@meta.com>
---
 .../testing/selftests/bpf/prog_tests/dynptr.c |  4 ++
 .../selftests/bpf/progs/dynptr_success.c      | 66 +++++++++++++++++++
 2 files changed, 70 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c
index 62e7ec775f24..423e95755aa0 100644
--- a/tools/testing/selftests/bpf/prog_tests/dynptr.c
+++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c
@@ -21,6 +21,10 @@ static struct {
 	{"test_dynptr_data", SETUP_SYSCALL_SLEEP},
 	{"test_dynptr_copy", SETUP_SYSCALL_SLEEP},
 	{"test_dynptr_copy_xdp", SETUP_XDP_PROG},
+	{"test_dynptr_memset_zero", SETUP_SYSCALL_SLEEP},
+	{"test_dynptr_memset_zero_offset", SETUP_SYSCALL_SLEEP},
+	{"test_dynptr_memset_overflow", SETUP_SYSCALL_SLEEP},
+	{"test_dynptr_memset_readonly", SETUP_SKB_PROG},
 	{"test_ringbuf", SETUP_SYSCALL_SLEEP},
 	{"test_skb_readonly", SETUP_SKB_PROG},
 	{"test_dynptr_skb_data", SETUP_SKB_PROG},
diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c b/tools/testing/selftests/bpf/progs/dynptr_success.c
index a0391f9da2d4..204e02816947 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_success.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_success.c
@@ -681,6 +681,72 @@ int test_dynptr_copy_xdp(struct xdp_md *xdp)
 	return XDP_DROP;
 }
 
+char memset_zero_data[] = "data to be zeroed";
+
+SEC("?tp/syscalls/sys_enter_nanosleep")
+int test_dynptr_memset_zero(void *ctx)
+{
+	__u32 data_sz = sizeof(memset_zero_data);
+	char zeroes[32] = {'\0'};
+	struct bpf_dynptr ptr;
+
+	err = bpf_dynptr_from_mem(memset_zero_data, data_sz, 0, &ptr);
+	err = err ?: bpf_dynptr_memset(&ptr, 0, data_sz);
+	err = err ?: bpf_memcmp(zeroes, memset_zero_data, data_sz);
+
+	return 0;
+}
+
+char memset_zero_offset_data[] = "data to be zeroed partially";
+
+SEC("?tp/syscalls/sys_enter_nanosleep")
+int test_dynptr_memset_zero_offset(void *ctx)
+{
+	char expected[] = "data\0\0\0\0be zeroed partially";
+	__u32 data_sz = sizeof(memset_zero_offset_data);
+	struct bpf_dynptr ptr;
+
+	err = bpf_dynptr_from_mem(memset_zero_offset_data, data_sz, 0, &ptr);
+	err = err ?: bpf_dynptr_adjust(&ptr, 4, 8);
+	err = err ?: bpf_dynptr_memset(&ptr, 0, bpf_dynptr_size(&ptr));
+	err = err ?: bpf_memcmp(expected, memset_zero_offset_data, data_sz);
+
+	return 0;
+}
+
+char memset_overflow_data[] = "memset overflow data";
+
+SEC("?tp/syscalls/sys_enter_nanosleep")
+int test_dynptr_memset_overflow(void *ctx)
+{
+	__u32 data_sz = sizeof(memset_overflow_data);
+	struct bpf_dynptr ptr;
+	int ret;
+
+	err = bpf_dynptr_from_mem(memset_overflow_data, data_sz, 0, &ptr);
+	ret = bpf_dynptr_memset(&ptr, 0, data_sz + 1);
+	if (ret != -E2BIG)
+		err = 1;
+
+	return 0;
+}
+
+SEC("?cgroup_skb/egress")
+int test_dynptr_memset_readonly(struct __sk_buff *skb)
+{
+	struct bpf_dynptr ptr;
+	int ret;
+
+	err = bpf_dynptr_from_skb(skb, 0, &ptr);
+
+	/* cgroup skbs are read only, memset should fail */
+	ret = bpf_dynptr_memset(&ptr, 0, bpf_dynptr_size(&ptr));
+	if (ret != -EINVAL)
+		err = 1;
+
+	return 0;
+}
+
 void *user_ptr;
 /* Contains the copy of the data pointed by user_ptr.
  * Size 384 to make it not fit into a single kernel chunk when copying
-- 
2.47.1

Re: [PATCH bpf-next 1/2] bpf: add bpf_dynptr_memset() kfunc

[Mykyta Yatsenko]

On 6/18/25 23:33, Ihor Solodrai wrote:
> Currently there is no straightforward way to fill dynptr memory with a
> value (most commonly zero). One can do it with bpf_dynptr_write(), but
> a temporary buffer is necessary for that.
>
> Implement bpf_dynptr_memset() - an analogue of memset() from libc.
>
> Signed-off-by: Ihor Solodrai <isolodrai@meta.com>
> ---
>   kernel/bpf/helpers.c | 28 ++++++++++++++++++++++++++++
>   1 file changed, 28 insertions(+)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index b71e428ad936..dfd04628a522 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2906,6 +2906,33 @@ __bpf_kfunc int bpf_dynptr_copy(struct bpf_dynptr *dst_ptr, u32 dst_off,
>   	return 0;
>   }
>   
> +/**
> + * bpf_dynptr_memset() - Fill dynptr memory with a constant byte.
> + * @ptr: Destination dynptr - where data will be filled
> + * @val: Constant byte to fill the memory with
> + * @n: Number of bytes to fill
> + *
> + * Fills the first n bytes of the memory area pointed to by ptr
> + * with the constant byte val.
> + * Returns 0 on success; negative error, otherwise.
> + */
> + __bpf_kfunc int bpf_dynptr_memset(struct bpf_dynptr *ptr, u8 val, u32 n)
> + {
> +	struct bpf_dynptr_kern *p = (struct bpf_dynptr_kern *)ptr;
> +	int err;
> +
> +	if (__bpf_dynptr_is_rdonly(p))
> +		return -EINVAL;
> +
> +	err = bpf_dynptr_check_off_len(p, 0, n);
> +	if (err)
> +		return err;
> +
> +	memset(p->data + p->offset, val, n);
Do we need to handle non-contiguous buffers, similarly to 
bpf_dynptr_write (BPF_DYNPTR_TYPE_XDP case)?
> +
> +	return 0;
> +}
> +
>   __bpf_kfunc void *bpf_cast_to_kern_ctx(void *obj)
>   {
>   	return obj;
> @@ -3364,6 +3391,7 @@ BTF_ID_FLAGS(func, bpf_dynptr_is_rdonly)
>   BTF_ID_FLAGS(func, bpf_dynptr_size)
>   BTF_ID_FLAGS(func, bpf_dynptr_clone)
>   BTF_ID_FLAGS(func, bpf_dynptr_copy)
> +BTF_ID_FLAGS(func, bpf_dynptr_memset)
>   #ifdef CONFIG_NET
>   BTF_ID_FLAGS(func, bpf_modify_return_test_tp)
>   #endif

Re: [PATCH bpf-next 1/2] bpf: add bpf_dynptr_memset() kfunc

[Ihor Solodrai]

On 6/18/25 4:44 PM, Mykyta Yatsenko wrote:
> On 6/18/25 23:33, Ihor Solodrai wrote:
>> Currently there is no straightforward way to fill dynptr memory with a
>> value (most commonly zero). One can do it with bpf_dynptr_write(), but
>> a temporary buffer is necessary for that.
>>
>> Implement bpf_dynptr_memset() - an analogue of memset() from libc.
>>
>> Signed-off-by: Ihor Solodrai <isolodrai@meta.com>
>> ---
>>   kernel/bpf/helpers.c | 28 ++++++++++++++++++++++++++++
>>   1 file changed, 28 insertions(+)
>>
>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>> index b71e428ad936..dfd04628a522 100644
>> --- a/kernel/bpf/helpers.c
>> +++ b/kernel/bpf/helpers.c
>> @@ -2906,6 +2906,33 @@ __bpf_kfunc int bpf_dynptr_copy(struct 
>> bpf_dynptr *dst_ptr, u32 dst_off,
>>       return 0;
>>   }
>> +/**
>> + * bpf_dynptr_memset() - Fill dynptr memory with a constant byte.
>> + * @ptr: Destination dynptr - where data will be filled
>> + * @val: Constant byte to fill the memory with
>> + * @n: Number of bytes to fill
>> + *
>> + * Fills the first n bytes of the memory area pointed to by ptr
>> + * with the constant byte val.
>> + * Returns 0 on success; negative error, otherwise.
>> + */
>> + __bpf_kfunc int bpf_dynptr_memset(struct bpf_dynptr *ptr, u8 val, 
>> u32 n)
>> + {
>> +    struct bpf_dynptr_kern *p = (struct bpf_dynptr_kern *)ptr;
>> +    int err;
>> +
>> +    if (__bpf_dynptr_is_rdonly(p))
>> +        return -EINVAL;
>> +
>> +    err = bpf_dynptr_check_off_len(p, 0, n);
>> +    if (err)
>> +        return err;
>> +
>> +    memset(p->data + p->offset, val, n);
> Do we need to handle non-contiguous buffers, similarly to 
> bpf_dynptr_write (BPF_DYNPTR_TYPE_XDP case)?

It appears so, yes. I am a little lost on how exactly to do it though.

Looking at bpf_xdp_store_bytes [1]:

   BPF_CALL_4(bpf_xdp_store_bytes, struct xdp_buff *, xdp, u32, offset,
   	   void *, buf, u32, len)
   {
   	void *ptr;

   	ptr = bpf_xdp_pointer(xdp, offset, len);
   	if (IS_ERR(ptr))
   		return PTR_ERR(ptr);

   	if (!ptr)
   		bpf_xdp_copy_buf(xdp, offset, buf, len, true);
   	else
   		memcpy(ptr, buf, len);

   	return 0;
   }

   ...

   int __bpf_xdp_store_bytes(struct xdp_buff *xdp, u32 offset, void 
*buf, u32 len)
   {
   	return ____bpf_xdp_store_bytes(xdp, offset, buf, len);
   }

It looks like I'd need to implement similar logic for memset.

But then, does memset even make sense for xdp/skb buffers?
Maybe -ENOTSUPP is more appropriate?

I'd appreciate any hints.

[1] 
https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git/tree/net/core/filter.c#n4084

>> +
>> +    return 0;
>> +}
>> +
>>   __bpf_kfunc void *bpf_cast_to_kern_ctx(void *obj)
>>   {
>>       return obj;
>> @@ -3364,6 +3391,7 @@ BTF_ID_FLAGS(func, bpf_dynptr_is_rdonly)
>>   BTF_ID_FLAGS(func, bpf_dynptr_size)
>>   BTF_ID_FLAGS(func, bpf_dynptr_clone)
>>   BTF_ID_FLAGS(func, bpf_dynptr_copy)
>> +BTF_ID_FLAGS(func, bpf_dynptr_memset)
>>   #ifdef CONFIG_NET
>>   BTF_ID_FLAGS(func, bpf_modify_return_test_tp)
>>   #endif
> 

Re: [PATCH bpf-next 1/2] bpf: add bpf_dynptr_memset() kfunc

[Eduard Zingerman]

On Thu, 2025-06-19 at 10:09 -0700, Ihor Solodrai wrote:

[...]

> But then, does memset even make sense for xdp/skb buffers?

Why not?

> Maybe -ENOTSUPP is more appropriate?
> 
> I'd appreciate any hints.

I think Mykyta has kernel/trace/bpf_trace.c:__bpf_dynptr_copy_str() in mind.

Re: [PATCH bpf-next 1/2] bpf: add bpf_dynptr_memset() kfunc

[Ihor Solodrai]

On 6/19/25 10:19 AM, Eduard Zingerman wrote:
> On Thu, 2025-06-19 at 10:09 -0700, Ihor Solodrai wrote:
> 
> [...]
> 
>> But then, does memset even make sense for xdp/skb buffers?
> 
> Why not?

I was thinking that in a BPF program you'd usually be reading xdp/skb
and not writing to it, especially not via memset. But never mind, I am
likely wrong.

> 
>> Maybe -ENOTSUPP is more appropriate?
>>
>> I'd appreciate any hints.
> 
> I think Mykyta has kernel/trace/bpf_trace.c:__bpf_dynptr_copy_str() in mind.

Thanks for the pointer. So it looks like memset can use
bpf_dynptr_slice_rdwr() to handle xdp/skb cases. Something like:

     void *ptr = bpf_dynptr_slice_rdwr(&dynptr, 0, buffer, sizeof(buffer));
     if (!ptr)
         return -EINVAL;

     memset(ptr, val, n);

     if (ptr == buffer)
         bpf_dynptr_write(&dynptr, 0, buffer, sizeof(buffer), 0);


> 

Re: [PATCH bpf-next 1/2] bpf: add bpf_dynptr_memset() kfunc

[Eduard Zingerman]

On Thu, 2025-06-19 at 10:55 -0700, Ihor Solodrai wrote:

[...]

> > I think Mykyta has kernel/trace/bpf_trace.c:__bpf_dynptr_copy_str() in mind.
> 
> Thanks for the pointer. So it looks like memset can use
> bpf_dynptr_slice_rdwr() to handle xdp/skb cases. Something like:
> 
>      void *ptr = bpf_dynptr_slice_rdwr(&dynptr, 0, buffer, sizeof(buffer));
>      if (!ptr)
>          return -EINVAL;
> 
>      memset(ptr, val, n);
> 
>      if (ptr == buffer)
>          bpf_dynptr_write(&dynptr, 0, buffer, sizeof(buffer), 0);
> 
> 

I think so. In a loop.

Re: [PATCH bpf-next 1/2] bpf: add bpf_dynptr_memset() kfunc

[Ihor Solodrai]

On 6/19/25 10:57 AM, Eduard Zingerman wrote:
> On Thu, 2025-06-19 at 10:55 -0700, Ihor Solodrai wrote:
> 
> [...]
> 
>>> I think Mykyta has kernel/trace/bpf_trace.c:__bpf_dynptr_copy_str() in mind.
>>
>> Thanks for the pointer. So it looks like memset can use
>> bpf_dynptr_slice_rdwr() to handle xdp/skb cases. Something like:
>>
>>       void *ptr = bpf_dynptr_slice_rdwr(&dynptr, 0, buffer, sizeof(buffer));
>>       if (!ptr)
>>           return -EINVAL;
>>
>>       memset(ptr, val, n);
>>
>>       if (ptr == buffer)
>>           bpf_dynptr_write(&dynptr, 0, buffer, sizeof(buffer), 0);
>>
>>
> 
> I think so. In a loop.

But allocating and copying buffers for memset...

It should be possible to walk through fragments like
net/core/filter.c:bpf_xdp_copy_buf does.

Any reasons it's a bad idea?

> 

Re: [PATCH bpf-next 1/2] bpf: add bpf_dynptr_memset() kfunc

[Eduard Zingerman]

On Thu, 2025-06-19 at 11:04 -0700, Ihor Solodrai wrote:

[...]

> It should be possible to walk through fragments like
> net/core/filter.c:bpf_xdp_copy_buf does.
> 
> Any reasons it's a bad idea?
> 

You can do that as well.

Also, what's the plan if you'd like to memset only a fragment of the
memory pointed-to by dynptr?

Re: [PATCH bpf-next 1/2] bpf: add bpf_dynptr_memset() kfunc

[Eduard Zingerman]

On Thu, 2025-06-19 at 11:13 -0700, Eduard Zingerman wrote:

[...]

> Also, what's the plan if you'd like to memset only a fragment of the
> memory pointed-to by dynptr?

Oh, I see, there is bpf_dynptr_adjust(), sorry for the noise.

Re: [PATCH bpf-next 1/2] bpf: add bpf_dynptr_memset() kfunc

[Mykyta Yatsenko]

On 6/19/25 18:19, Eduard Zingerman wrote:
> On Thu, 2025-06-19 at 10:09 -0700, Ihor Solodrai wrote:
>
> [...]
>
>> But then, does memset even make sense for xdp/skb buffers?
> Why not?
>
>> Maybe -ENOTSUPP is more appropriate?
>>
>> I'd appreciate any hints.
> I think Mykyta has kernel/trace/bpf_trace.c:__bpf_dynptr_copy_str() in mind.
>
Right, similarly to `__bpf_dynptr_copy` from bpf_trace.c: fast path if 
destination has enough contiguous memory,
just call memset.
Slow path - use temp buffer, that you can memset once and then 
__bpf_dynptr_write it in a loop. __bpf_dynptr_write
handles the trickiness of the non-contiguous buffers.

Re: [PATCH bpf-next 1/2] bpf: add bpf_dynptr_memset() kfunc

[Andrii Nakryiko]

On Thu, Jun 19, 2025 at 11:17 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Thu, 2025-06-19 at 11:13 -0700, Eduard Zingerman wrote:
>
> [...]
>
> > Also, what's the plan if you'd like to memset only a fragment of the
> > memory pointed-to by dynptr?
>
> Oh, I see, there is bpf_dynptr_adjust(), sorry for the noise.
>

Even though we do have bpf_dynptr_adjust() for maximum generality and
flexibility, for most dynptr-based APIs we try to pass also additional
offset into dynptr to avoid unnecessary overhead. So it's not a bad
idea to add this to bpf_memset(), IMO.

bpf_memset(struct bpf_dynptr *dptr, u32 off, u8 val, u32 n) ?

a bit unfortunate that we have 3 integers that you need to be careful
to not swap accidentally, but even with just val and n you'd have to
be careful. For other APIs we normally have offset to follow dynptr
pointer, so hopefully this arrangement won't that surprising.

Thoughts?

Re: [PATCH bpf-next 1/2] bpf: add bpf_dynptr_memset() kfunc

[Eduard Zingerman]

On Mon, 2025-06-23 at 14:38 -0700, Andrii Nakryiko wrote:
> On Thu, Jun 19, 2025 at 11:17 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > 
> > On Thu, 2025-06-19 at 11:13 -0700, Eduard Zingerman wrote:
> > 
> > [...]
> > 
> > > Also, what's the plan if you'd like to memset only a fragment of the
> > > memory pointed-to by dynptr?
> > 
> > Oh, I see, there is bpf_dynptr_adjust(), sorry for the noise.
> > 
> 
> Even though we do have bpf_dynptr_adjust() for maximum generality and
> flexibility, for most dynptr-based APIs we try to pass also additional
> offset into dynptr to avoid unnecessary overhead. So it's not a bad
> idea to add this to bpf_memset(), IMO.
> 
> bpf_memset(struct bpf_dynptr *dptr, u32 off, u8 val, u32 n) ?
> 
> a bit unfortunate that we have 3 integers that you need to be careful
> to not swap accidentally, but even with just val and n you'd have to
> be careful. For other APIs we normally have offset to follow dynptr
> pointer, so hopefully this arrangement won't that surprising.
> 
> Thoughts?

Unfortunate indeed.
The off and n being separated looks weird, tbh.
For dynptr funcs we actually have "dptr, off, size" everywhere, maybe
do the same here?

Re: [PATCH bpf-next 1/2] bpf: add bpf_dynptr_memset() kfunc

[Andrii Nakryiko]

On Mon, Jun 23, 2025 at 2:45 PM Eduard Zingerman <eddyz87@gmail.com> wrote:
>
> On Mon, 2025-06-23 at 14:38 -0700, Andrii Nakryiko wrote:
> > On Thu, Jun 19, 2025 at 11:17 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> > >
> > > On Thu, 2025-06-19 at 11:13 -0700, Eduard Zingerman wrote:
> > >
> > > [...]
> > >
> > > > Also, what's the plan if you'd like to memset only a fragment of the
> > > > memory pointed-to by dynptr?
> > >
> > > Oh, I see, there is bpf_dynptr_adjust(), sorry for the noise.
> > >
> >
> > Even though we do have bpf_dynptr_adjust() for maximum generality and
> > flexibility, for most dynptr-based APIs we try to pass also additional
> > offset into dynptr to avoid unnecessary overhead. So it's not a bad
> > idea to add this to bpf_memset(), IMO.
> >
> > bpf_memset(struct bpf_dynptr *dptr, u32 off, u8 val, u32 n) ?
> >
> > a bit unfortunate that we have 3 integers that you need to be careful
> > to not swap accidentally, but even with just val and n you'd have to
> > be careful. For other APIs we normally have offset to follow dynptr
> > pointer, so hopefully this arrangement won't that surprising.
> >
> > Thoughts?
>
> Unfortunate indeed.
> The off and n being separated looks weird, tbh.
> For dynptr funcs we actually have "dptr, off, size" everywhere, maybe
> do the same here?

yep, fine by me. I never liked libc's memset() order anyways :)