On 6/26/25 02:25, Ihor Solodrai wrote:
On 6/25/25 4:45 AM, Mykyta Yatsenko wrote:
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@xxxxxxxx>
---
.../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 {
[...]
+
+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;
A question not directly to Mykyta.
So noalu32 version of this test was failing to verify with this:
118: (85) call bpf_dynptr_read#201
R2 min value is negative, either use unsigned or 'var &= const'
Where R2 refers to `data_sz - offset`
Full log here:
https://github.com/kernel-patches/bpf/actions/runs/15861036149/job/44718289284
I tried various conditions unsuccessfully. But changing u32 to u64
made it work. If handle_tail part is removed, as Mykyta suggested,
this doesn't matter, so I will probably leave u32 in v3.
However I am curious if u32->u64 change is an appropriate workaround
in general for noalu32 problems? AFAIU verifier might get confused by
all the added shifts, and "noalu32" is a backward compatibility thing.
+ char expected_buf[32];
nit: expected_buf[32] = {DYNPTR_MEMSET_VAL};
My bad, it's actually should be `char expected_buf[32] = {[0 ... 31] =
DYNPTR_MEMSET_VAL}`;
Otherwise it initializes just the first element of the expected_buf and
places that array into the .rodata.cst32 section.
I tried that at the beginning. As it turns out, this doesn't work in
BPF the way you'd expect:
Here is a piece of llvm-objdump with explicit memset:
0000000000000968 <test_dynptr_memset_xdp_chunks>:
301: 18 02 00 00 2a 2a 2a 2a 00 00 00 00 2a 2a 2a 2a r2 =
0x2a2a2a2a2a2a2a2a ll
303: 7b 2a c8 ff 00 00 00 00 *(u64 *)(r10 - 0x38) = r2
304: 7b 2a d0 ff 00 00 00 00 *(u64 *)(r10 - 0x30) = r2
305: 7b 2a d8 ff 00 00 00 00 *(u64 *)(r10 - 0x28) = r2
306: 7b 2a e0 ff 00 00 00 00 *(u64 *)(r10 - 0x20) = r2
307: bf a7 00 00 00 00 00 00 r7 = r10
308: 07 07 00 00 e8 ff ff ff r7 += -0x18
309: b7 02 00 00 00 00 00 00 r2 = 0x0
310: bf 73 00 00 00 00 00 00 r3 = r7
311: 85 10 00 00 ff ff ff ff call -0x1
...
You can clearly see a piece of stack filling up with 0x2a
After applying this diff:
diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c
b/tools/testing/selftests/bpf/progs/dynptr_success.c
index 5120acb8b15a..5b351f6fe07c 100644
--- a/tools/testing/selftests/bpf/progs/dynptr_success.c
+++ b/tools/testing/selftests/bpf/progs/dynptr_success.c
@@ -809,12 +809,10 @@ int test_dynptr_memset_xdp_chunks(struct xdp_md
*xdp)
const int max_chunks = 200;
struct bpf_dynptr ptr_xdp;
u32 data_sz, chunk_sz, offset = 0;
- char expected_buf[32];
+ char 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);
We get the following:
0000000000000968 <test_dynptr_memset_xdp_chunks>:
301: bf a7 00 00 00 00 00 00 r7 = r10
302: 07 07 00 00 e8 ff ff ff r7 += -0x18
303: b7 02 00 00 00 00 00 00 r2 = 0x0
304: bf 73 00 00 00 00 00 00 r3 = r7
305: 85 10 00 00 ff ff ff ff call -0x1
...
The stack allocated array is not initialized.
Could be an LLVM bug/incompleteness? I used LLVM 19 while developing.
+ 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);
I don't see a point of doing it for this test. max_chunks is just a
big enough arbitrary constant that works. We do a similar thing in
other tests.
+ 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);
Yeah, you're right.
It ended up like this because I've been fighting the verifier while
writing the test, and this version worked eventually. The critical
piece to uncofuse it was changing:
offset += sizeof(buf)
to
offset = i * sizeof(buf)
I will have to add min_t macro locally though.
+ 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
