Re: [PATCH bpf-next] selftests/bpf: Fix dynptr/test_probe_read_user_str_dynptr test failure

[Yonghong Song <yonghong.song@linux.dev>]

On 5/16/25 12:03 AM, Mykyta Yatsenko wrote:
> On 5/15/25 20:51, Yonghong Song wrote:
>> When running bpf selftests with llvm18 compiler, I hit the following
>> test failure:
>>
>>    verify_success:PASS:dynptr_success__open 0 nsec
>>    verify_success:PASS:bpf_object__find_program_by_name 0 nsec
>>    verify_success:PASS:dynptr_success__load 0 nsec
>>    verify_success:PASS:test_run 0 nsec
>>    verify_success:FAIL:err unexpected err: actual 1 != expected 0
>>    #91/19   dynptr/test_probe_read_user_str_dynptr:FAIL
>>    #91      dynptr:FAIL
>>
>> I did some analysis and found that the test failure is related to
>> lib/strncpy_from_user.c function do_strncpy_from_user():
>>
>>    ...
>>    byte_at_a_time:
>>          while (max) {
>>                  char c;
>>
>>                  unsafe_get_user(c,src+res, efault);
>>                  dst[res] = c;
>>                  if (!c)
>>                          return res;
>>                  res++;
>>                  max--;
>>          }
>>    ...
>>
>> Depending on whether the character 'c' is '\0' or not, the
>> return value 'res' could be different.
>>
>> In prog_tests/dynptr.c, we have
>>    char user_data[384] = {[0 ... 382] = 'a', '\0'};
>> the user_data[383] is '\0'. This will cause the following
>> error in progs/dynptr_success.c:
>>
>>    test_dynptr_probe_str_xdp:
>>    ...
>>          bpf_for(i, 0, ARRAY_SIZE(test_len)) {
>>                  __u32 len = test_len[i];
>>
>>                  cnt = bpf_read_dynptr_fn(&ptr_xdp, off, len, ptr);
>>                  if (cnt != len)
>>                          err = 1; <=== error happens here
>>    ...
>>
>> In the above particular case, len is 384 and cnt is 383.
>>
>> If user_data[384] is changed to
>>    char user_data[384] = {[0 ... 383] = 'a'};
>>
>> The above error will not happen and the test will run successfully.
>>
>> Cc: Mykyta Yatsenko <yatsenko@meta.com>
>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>> ---
>>   tools/testing/selftests/bpf/prog_tests/dynptr.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tools/testing/selftests/bpf/prog_tests/dynptr.c 
>> b/tools/testing/selftests/bpf/prog_tests/dynptr.c
>> index 62e7ec775f24..4cc61afa63b4 100644
>> --- a/tools/testing/selftests/bpf/prog_tests/dynptr.c
>> +++ b/tools/testing/selftests/bpf/prog_tests/dynptr.c
>> @@ -45,7 +45,7 @@ static struct {
>>     static void verify_success(const char *prog_name, enum 
>> test_setup_type setup_type)
>>   {
>> -    char user_data[384] = {[0 ... 382] = 'a', '\0'};
>> +    char user_data[384] = {[0 ... 383] = 'a'};
>>       struct dynptr_success *skel;
>>       struct bpf_program *prog;
>>       struct bpf_link *link;
> This test is disabled in the DENYLIST and the fix was submitted to 
> another tree:
> https://patchwork.kernel.org/project/linux-mm/patch/20250422131449.57177-1-mykyta.yatsenko5@gmail.com/ 
>
> It's a little bit different problem.

Okay. This works too. I thought about this earlier during investigation but decided not to change core kernel code...