[PATCH] selftests/ftrace: check for do_sys_openat2 in user-memory test

[PATCH] selftests/ftrace: check for do_sys_openat2 in user-memory test

[Colin King]

From: Colin Ian King <colin.king@canonical.com>

More recent libc implementations are now using openat/openat2 system
calls so also add do_sys_openat2 to the tracing so that the test
passes on these systems because do_sys_open may not be called.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 .../testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc  | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
index a30a9c07290d..cf1b4c3e9e6b 100644
--- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
+++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
@@ -9,6 +9,8 @@ grep -A10 "fetcharg:" README | grep -q '\[u\]<offset>' || exit_unsupported
 :;: "user-memory access syntax and ustring working on user memory";:
 echo 'p:myevent do_sys_open path=+0($arg2):ustring path2=+u0($arg2):string' \
 	> kprobe_events
+echo 'p:myevent2 do_sys_openat2 path=+0($arg2):ustring path2=+u0($arg2):string' \
+	> kprobe_events
 
 grep myevent kprobe_events | \
 	grep -q 'path=+0($arg2):ustring path2=+u0($arg2):string'
-- 
2.27.0

Re: [PATCH] selftests/ftrace: check for do_sys_openat2 in user-memory test

[Steven Rostedt]

On Thu,  1 Oct 2020 09:56:41 +0100
Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> More recent libc implementations are now using openat/openat2 system
> calls so also add do_sys_openat2 to the tracing so that the test
> passes on these systems because do_sys_open may not be called.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  .../testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc  | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
> index a30a9c07290d..cf1b4c3e9e6b 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
> @@ -9,6 +9,8 @@ grep -A10 "fetcharg:" README | grep -q '\[u\]<offset>' || exit_unsupported
>  :;: "user-memory access syntax and ustring working on user memory";:
>  echo 'p:myevent do_sys_open path=+0($arg2):ustring path2=+u0($arg2):string' \
>  	> kprobe_events  
> +echo 'p:myevent2 do_sys_openat2 path=+0($arg2):ustring path2=+u0($arg2):string' \
> +	> kprobe_events
> 

This still wont work, because the rest of the code only enables the myevent
event, and not the one you just added.

Did you see this broken before, and this patch fixes it?

-- Steve

 
>  grep myevent kprobe_events | \
>  	grep -q 'path=+0($arg2):ustring path2=+u0($arg2):string'

Re: [PATCH] selftests/ftrace: check for do_sys_openat2 in user-memory test

[Colin Ian King]

On 01/10/2020 15:44, Steven Rostedt wrote:
> On Thu,  1 Oct 2020 09:56:41 +0100
> Colin King <colin.king@canonical.com> wrote:
> 
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> More recent libc implementations are now using openat/openat2 system
>> calls so also add do_sys_openat2 to the tracing so that the test
>> passes on these systems because do_sys_open may not be called.
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>  .../testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc  | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
>> index a30a9c07290d..cf1b4c3e9e6b 100644
>> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
>> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
>> @@ -9,6 +9,8 @@ grep -A10 "fetcharg:" README | grep -q '\[u\]<offset>' || exit_unsupported
>>  :;: "user-memory access syntax and ustring working on user memory";:
>>  echo 'p:myevent do_sys_open path=+0($arg2):ustring path2=+u0($arg2):string' \
>>  	> kprobe_events  
>> +echo 'p:myevent2 do_sys_openat2 path=+0($arg2):ustring path2=+u0($arg2):string' \
>> +	> kprobe_events
>>
> 
> This still wont work, because the rest of the code only enables the myevent
> event, and not the one you just added.

Yep, I botched this and tested the wrong fix.

> 
> Did you see this broken before, and this patch fixes it?

So this test breaks with a recent libc and support tools built against
libc.  I believe the do_sys_open is not being detected because
do_sys_openat2 is being called instead.

Not sure now of the correct way to fix this.

> 
> -- Steve
> 
>  
>>  grep myevent kprobe_events | \
>>  	grep -q 'path=+0($arg2):ustring path2=+u0($arg2):string'
> 

Re: [PATCH] selftests/ftrace: check for do_sys_openat2 in user-memory test

[Steven Rostedt]

On Thu, 1 Oct 2020 16:40:33 +0100
Colin Ian King <colin.king@canonical.com> wrote:

> So this test breaks with a recent libc and support tools built against
> libc.  I believe the do_sys_open is not being detected because
> do_sys_openat2 is being called instead.
> 
> Not sure now of the correct way to fix this.

Perhaps by enabling both events?

-- Steve

Re: [PATCH] selftests/ftrace: check for do_sys_openat2 in user-memory test

[Masami Hiramatsu]

On Thu, 1 Oct 2020 16:40:33 +0100
Colin Ian King <colin.king@canonical.com> wrote:

> On 01/10/2020 15:44, Steven Rostedt wrote:
> > On Thu,  1 Oct 2020 09:56:41 +0100
> > Colin King <colin.king@canonical.com> wrote:
> > 
> >> From: Colin Ian King <colin.king@canonical.com>
> >>
> >> More recent libc implementations are now using openat/openat2 system
> >> calls so also add do_sys_openat2 to the tracing so that the test
> >> passes on these systems because do_sys_open may not be called.
> >>
> >> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> >> ---
> >>  .../testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc  | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
> >> index a30a9c07290d..cf1b4c3e9e6b 100644
> >> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
> >> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
> >> @@ -9,6 +9,8 @@ grep -A10 "fetcharg:" README | grep -q '\[u\]<offset>' || exit_unsupported
> >>  :;: "user-memory access syntax and ustring working on user memory";:
> >>  echo 'p:myevent do_sys_open path=+0($arg2):ustring path2=+u0($arg2):string' \
> >>  	> kprobe_events  
> >> +echo 'p:myevent2 do_sys_openat2 path=+0($arg2):ustring path2=+u0($arg2):string' \
> >> +	> kprobe_events
> >>
> > 
> > This still wont work, because the rest of the code only enables the myevent
> > event, and not the one you just added.
> 
> Yep, I botched this and tested the wrong fix.
> 
> > 
> > Did you see this broken before, and this patch fixes it?
> 
> So this test breaks with a recent libc and support tools built against
> libc.  I believe the do_sys_open is not being detected because
> do_sys_openat2 is being called instead.
> 
> Not sure now of the correct way to fix this.

Hmm, this actually just try to catch the user-string. So the another
function which accepts any user-string, is OK.
Let me try to use another one. 

Thank you for reporting!


> 
> > 
> > -- Steve
> > 
> >  
> >>  grep myevent kprobe_events | \
> >>  	grep -q 'path=+0($arg2):ustring path2=+u0($arg2):string'
> > 
> 


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH] selftests/ftrace: check for do_sys_openat2 in user-memory test

[Masami Hiramatsu]

On Thu,  1 Oct 2020 09:56:41 +0100
Colin King <colin.king@canonical.com> wrote:

> From: Colin Ian King <colin.king@canonical.com>
> 
> More recent libc implementations are now using openat/openat2 system
> calls so also add do_sys_openat2 to the tracing so that the test
> passes on these systems because do_sys_open may not be called.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>  .../testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc  | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
> index a30a9c07290d..cf1b4c3e9e6b 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
> @@ -9,6 +9,8 @@ grep -A10 "fetcharg:" README | grep -q '\[u\]<offset>' || exit_unsupported
>  :;: "user-memory access syntax and ustring working on user memory";:
>  echo 'p:myevent do_sys_open path=+0($arg2):ustring path2=+u0($arg2):string' \
>  	> kprobe_events

OK, at first, you need to check the kernel has do_sys_openat2 from
/proc/kallsyms (grep -qw will help you), because it is new syscall.

> +echo 'p:myevent2 do_sys_openat2 path=+0($arg2):ustring path2=+u0($arg2):string' \
> +	> kprobe_events

Also, you need to append (">>") it instead overwrite (">") to kprobe_events.

>  
>  grep myevent kprobe_events | \
>  	grep -q 'path=+0($arg2):ustring path2=+u0($arg2):string'

And also you have to enable both myevent and myevent2 (if exists) after
this.

Then I think your patch will work correctly.

Thank you!

-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: [PATCH] selftests/ftrace: check for do_sys_openat2 in user-memory test

[Colin Ian King]

On 02/10/2020 14:07, Masami Hiramatsu wrote:
> On Thu,  1 Oct 2020 09:56:41 +0100
> Colin King <colin.king@canonical.com> wrote:
> 
>> From: Colin Ian King <colin.king@canonical.com>
>>
>> More recent libc implementations are now using openat/openat2 system
>> calls so also add do_sys_openat2 to the tracing so that the test
>> passes on these systems because do_sys_open may not be called.
>>
>> Signed-off-by: Colin Ian King <colin.king@canonical.com>
>> ---
>>  .../testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc  | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
>> index a30a9c07290d..cf1b4c3e9e6b 100644
>> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
>> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_user.tc
>> @@ -9,6 +9,8 @@ grep -A10 "fetcharg:" README | grep -q '\[u\]<offset>' || exit_unsupported
>>  :;: "user-memory access syntax and ustring working on user memory";:
>>  echo 'p:myevent do_sys_open path=+0($arg2):ustring path2=+u0($arg2):string' \
>>  	> kprobe_events
> 
> OK, at first, you need to check the kernel has do_sys_openat2 from
> /proc/kallsyms (grep -qw will help you), because it is new syscall.
> 
>> +echo 'p:myevent2 do_sys_openat2 path=+0($arg2):ustring path2=+u0($arg2):string' \
>> +	> kprobe_events
> 
> Also, you need to append (">>") it instead overwrite (">") to kprobe_events.
> 
>>  
>>  grep myevent kprobe_events | \
>>  	grep -q 'path=+0($arg2):ustring path2=+u0($arg2):string'
> 
> And also you have to enable both myevent and myevent2 (if exists) after
> this.
> 
> Then I think your patch will work correctly.

Indeed, thanks. I'll send a V2 shortly.

> 
> Thank you!
>