netdev - Re: [PATCH bpf-next 3/3] selftests/bpf: Fix test dynptr/test_dynptr_memset_xdp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <1e0b23c5-336c-436e-a568-f2eba0cdcbbf@linux.dev>
Date: Fri, 25 Jul 2025 18:41:04 -0700
From: Martin KaFai Lau <martin.lau@...ux.dev>
To: Yonghong Song <yonghong.song@...ux.dev>,
 Lorenzo Bianconi <lorenzo@...nel.org>
Cc: Alexei Starovoitov <ast@...nel.org>, Andrii Nakryiko <andrii@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>, kernel-team@...com,
 Martin KaFai Lau <martin.lau@...nel.org>, bpf@...r.kernel.org,
 Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH bpf-next 3/3] selftests/bpf: Fix test
 dynptr/test_dynptr_memset_xdp_chunks failure

On 7/25/25 5:59 PM, Yonghong Song wrote:
> 
> 
> On 7/25/25 4:29 PM, Martin KaFai Lau wrote:
>> On 7/24/25 9:34 PM, Yonghong Song wrote:
>>> For arm64 64K page size, the xdp data size was set to be more than 64K
>>> in one of previous patches. This will cause failure for bpf_dynptr_memset().
>>> Since the failure of bpf_dynptr_memset() is expected with 64K page size,
>>> return success.
>>>
>>> Signed-off-by: Yonghong Song <yonghong.song@...ux.dev>
>>> ---
>>>   tools/testing/selftests/bpf/progs/dynptr_success.c | 13 ++++++++++++-
>>>   1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tools/testing/selftests/bpf/progs/dynptr_success.c b/tools/ 
>>> testing/selftests/bpf/progs/dynptr_success.c
>>> index 3094a1e4ee91..8315273cb900 100644
>>> --- a/tools/testing/selftests/bpf/progs/dynptr_success.c
>>> +++ b/tools/testing/selftests/bpf/progs/dynptr_success.c
>>> @@ -9,6 +9,8 @@
>>>   #include "bpf_misc.h"
>>>   #include "errno.h"
>>>   +#define PAGE_SIZE_64K 65536
>>> +
>>>   char _license[] SEC("license") = "GPL";
>>>     int pid, err, val;
>>> @@ -821,8 +823,17 @@ int test_dynptr_memset_xdp_chunks(struct xdp_md *xdp)
>>>       data_sz = bpf_dynptr_size(&ptr_xdp);
>>>         err = bpf_dynptr_memset(&ptr_xdp, 0, data_sz, DYNPTR_MEMSET_VAL);
>>> -    if (err)
>>> +    if (err) {
>>> +        /* bpf_dynptr_memset() eventually called bpf_xdp_pointer()
>>
>> I don't think I understand why the test fixed in patch 1 (e.g. 
>> test_probe_read_user_dynptr) can pass the bpf_xdp_pointer test on 0xffff. I 
>> thought the bpf_probe_read_user_str_dynptr will eventually call the 
>> __bpf_xdp_store_bytes which also does a bpf_xdp_pointer?
> 
> For example, for test_probe_read_user_dynptr, for function test_dynptr_probe_xdp(),
> for this one:
>     off = xdp_near_frag_end_offset();
> 
> the off = 64928. Note that xdp_near_frag_end_offset() return value depends page 
> size.
> 
> __u32 xdp_near_frag_end_offset(void)
> {
>          const __u32 headroom = 256;
>          const __u32 max_frag_size =  __PAGE_SIZE - headroom - sizeof(struct 
> skb_shared_info);
>          /* 32 bytes before the approximate end of the fragment */
>          return max_frag_size - 32;
> }
> 
> The 'len' depends on 'test_len[i]' and test_len is
>     __u32 test_len[7] = {0/* placeholder */, 0, 1, 2, 255, 256, 257};
> 
> In bpf_xdp_pointer, we have the following test
> 
>          if (unlikely(offset > 0xffff || len > 0xffff))

Thanks for the explanation. Applied. Thanks.

I wonder if the 0xffff check can be removed from bpf_xdp_pointer() and depend on 
checking the xdp_get_buff_len(). The 0xffff check was also removed from the 
bpf_skb_load_bytes some time ago. [cc: Lorenzo, netdev]

Otherwise, it is not very useful to be able to create such xdp buff from the 
bpf_prog_test_run_xdp.