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

[Mykyta Yatsenko <mykyta.yatsenko5@gmail.com>]

On 6/24/25 21:52, Ihor Solodrai wrote:
> Add tests to verify the behavior of bpf_dynptr_memset():
>    * normal memset 0
>    * normal memset non-0
>    * memset with an offset
>    * memset in dynptr that was adjusted
>    * error: size overflow
>    * error: offset+size overflow
>    * error: readonly dynptr
>    * memset into non-linear xdp dynptr
>
> Signed-off-by: Ihor Solodrai <isolodrai@meta.com>
> ---
>   .../testing/selftests/bpf/prog_tests/dynptr.c |   8 +
>   .../selftests/bpf/progs/dynptr_success.c      | 164 ++++++++++++++++++
>   2 files changed, 172 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c b/tools/testing/selftests/bpf/prog_tests/dynptr.c
> index 62e7ec775f24..f2b65398afce 100644
> --- a/tools/testing/selftests/bpf/prog_tests/dynptr.c
> +++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c
> @@ -21,6 +21,14 @@ 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_notzero", SETUP_SYSCALL_SLEEP},
> +	{"test_dynptr_memset_zero_offset", SETUP_SYSCALL_SLEEP},
> +	{"test_dynptr_memset_zero_adjusted", SETUP_SYSCALL_SLEEP},
> +	{"test_dynptr_memset_overflow", SETUP_SYSCALL_SLEEP},
> +	{"test_dynptr_memset_overflow_offset", SETUP_SYSCALL_SLEEP},
> +	{"test_dynptr_memset_readonly", SETUP_SKB_PROG},
> +	{"test_dynptr_memset_xdp_chunks", SETUP_XDP_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..61d9ae2c6a52 100644
> --- a/tools/testing/selftests/bpf/progs/dynptr_success.c
> +++ b/tools/testing/selftests/bpf/progs/dynptr_success.c
> @@ -681,6 +681,170 @@ 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, 0);
> +	err = err ?: bpf_memcmp(zeroes, memset_zero_data, data_sz);
> +
> +	return 0;
> +}
> +
> +#define DYNPTR_MEMSET_VAL 42
> +
> +char memset_notzero_data[] = "data to be overwritten";
> +
> +SEC("?tp/syscalls/sys_enter_nanosleep")
> +int test_dynptr_memset_notzero(void *ctx)
> +{
> +	u32 data_sz = sizeof(memset_notzero_data);
> +	struct bpf_dynptr ptr;
> +	char expected[32];
> +
> +	__builtin_memset(expected, DYNPTR_MEMSET_VAL, data_sz);
> +
> +	err = bpf_dynptr_from_mem(memset_notzero_data, data_sz, 0, &ptr);
> +	err = err ?: bpf_dynptr_memset(&ptr, 0, data_sz, DYNPTR_MEMSET_VAL);
> +	err = err ?: bpf_memcmp(expected, memset_notzero_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 to \0\0\0\0eroed 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_memset(&ptr, 8, 4, 0);
> +	err = err ?: bpf_memcmp(expected, memset_zero_offset_data, data_sz);
> +
> +	return 0;
> +}
> +
> +char memset_zero_adjusted_data[] = "data to be zeroed partially";
> +
> +SEC("?tp/syscalls/sys_enter_nanosleep")
> +int test_dynptr_memset_zero_adjusted(void *ctx)
> +{
> +	char expected[] = "data\0\0\0\0be zeroed partially";
> +	__u32 data_sz = sizeof(memset_zero_adjusted_data);
> +	struct bpf_dynptr ptr;
> +
> +	err = bpf_dynptr_from_mem(memset_zero_adjusted_data, data_sz, 0, &ptr);
> +	err = err ?: bpf_dynptr_adjust(&ptr, 4, 8);
> +	err = err ?: bpf_dynptr_memset(&ptr, 0, bpf_dynptr_size(&ptr), 0);
> +	err = err ?: bpf_memcmp(expected, memset_zero_adjusted_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, 0);
> +	if (ret != -E2BIG)
> +		err = 1;
> +
> +	return 0;
> +}
> +
> +SEC("?tp/syscalls/sys_enter_nanosleep")
> +int test_dynptr_memset_overflow_offset(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, 1, data_sz, 0);
> +	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), 0);
> +	if (ret != -EINVAL)
> +		err = 1;
> +
> +	return 0;
> +}
> +
> +SEC("xdp")
> +int test_dynptr_memset_xdp_chunks(struct xdp_md *xdp)
> +{
> +	const int max_chunks = 200;
> +	struct bpf_dynptr ptr_xdp;
> +	u32 data_sz, offset = 0;
> +	char expected_buf[32];
nit: expected_buf[32] = {DYNPTR_MEMSET_VAL};
> +	char buf[32];
> +	int i;
> +
> +	__builtin_memset(expected_buf, DYNPTR_MEMSET_VAL, sizeof(expected_buf));
> +
> +	/* ptr_xdp is backed by non-contiguous memory */
> +	bpf_dynptr_from_xdp(xdp, 0, &ptr_xdp);
> +	data_sz = bpf_dynptr_size(&ptr_xdp);
> +
> +	err = bpf_dynptr_memset(&ptr_xdp, 0, data_sz, DYNPTR_MEMSET_VAL);
> +	if (err)
> +		goto out;
> +
Maybe we can calculate max_chunks instead of hardcoding, something like:
max_chunks = data_sz / sizeof(expected_buf) + (data_sz % 
sizeof(expected_buf) ? 1 : 0);
> +	bpf_for(i, 0, max_chunks) {
> +		offset = i * sizeof(buf);
> +		err = bpf_dynptr_read(&buf, sizeof(buf), &ptr_xdp, offset, 0);

handle_tail seems unnecessary, maybe handle tail in the main loop:
__u32 sz = min_t(data_sz - offset : sizeof(buf));
bpf_dynptr_read(&buf, sz, &ptr_xdp, offset, 0);

> +		switch (err) {
> +		case 0:
> +			break;
> +		case -E2BIG:
> +			goto handle_tail;
> +		default:
> +			goto out;
> +		}
> +		err = bpf_memcmp(buf, expected_buf, sizeof(buf));
> +		if (err)
> +			goto out;
> +	}
> +
> +handle_tail:
> +	if (data_sz - offset < sizeof(buf)) {
> +		err = bpf_dynptr_read(&buf, data_sz - offset, &ptr_xdp, offset, 0);
> +		if (err)
> +			goto out;
> +		err = bpf_memcmp(buf, expected_buf, data_sz - offset);
> +	}
> +out:
> +	return XDP_DROP;
> +}
> +
>   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