All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next] selftests/bpf: Fix dynptr/test_probe_read_user_str_dynptr test failure
@ 2025-05-15 19:51 Yonghong Song
  2025-05-16 16:03 ` Mykyta Yatsenko
  0 siblings, 1 reply; 3+ messages in thread
From: Yonghong Song @ 2025-05-15 19:51 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
	Martin KaFai Lau, Mykyta Yatsenko

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;
-- 
2.47.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next] selftests/bpf: Fix dynptr/test_probe_read_user_str_dynptr test failure
  2025-05-15 19:51 [PATCH bpf-next] selftests/bpf: Fix dynptr/test_probe_read_user_str_dynptr test failure Yonghong Song
@ 2025-05-16 16:03 ` Mykyta Yatsenko
  2025-05-16 16:23   ` Yonghong Song
  0 siblings, 1 reply; 3+ messages in thread
From: Mykyta Yatsenko @ 2025-05-16 16:03 UTC (permalink / raw)
  To: Yonghong Song, bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
	Martin KaFai Lau, Mykyta Yatsenko

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.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH bpf-next] selftests/bpf: Fix dynptr/test_probe_read_user_str_dynptr test failure
  2025-05-16 16:03 ` Mykyta Yatsenko
@ 2025-05-16 16:23   ` Yonghong Song
  0 siblings, 0 replies; 3+ messages in thread
From: Yonghong Song @ 2025-05-16 16:23 UTC (permalink / raw)
  To: Mykyta Yatsenko, bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann, kernel-team,
	Martin KaFai Lau, Mykyta Yatsenko



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...



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2025-05-16 16:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-05-15 19:51 [PATCH bpf-next] selftests/bpf: Fix dynptr/test_probe_read_user_str_dynptr test failure Yonghong Song
2025-05-16 16:03 ` Mykyta Yatsenko
2025-05-16 16:23   ` Yonghong Song

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.