linux-kernel - Re: [PATCH v4 10/10] test: ftrace: Fix kprobe test for eventfs

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>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230714222705.bc38f83d857473656a45d441@kernel.org>
Date:   Fri, 14 Jul 2023 22:27:05 +0900
From:   Masami Hiramatsu (Google) <mhiramat@...nel.org>
To:     Steven Rostedt <rostedt@...dmis.org>
Cc:     Ajay Kaher <akaher@...are.com>, shuah@...nel.org,
        mhiramat@...nel.org, chinglinyu@...gle.com,
        linux-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org,
        linux-trace-kernel@...r.kernel.org, lkp@...el.com,
        namit@...are.com, oe-lkp@...ts.linux.dev, amakhalov@...are.com,
        er.ajay.kaher@...il.com, srivatsa@...il.mit.edu, tkundu@...are.com,
        vsirnapalli@...are.com
Subject: Re: [PATCH v4 10/10] test: ftrace: Fix kprobe test for eventfs

On Thu, 13 Jul 2023 22:37:58 -0400
Steven Rostedt <rostedt@...dmis.org> wrote:

> On Thu, 13 Jul 2023 17:03:24 +0530
> Ajay Kaher <akaher@...are.com> wrote:
> 
> > kprobe_args_char.tc, kprobe_args_string.tc has validation check
> > for tracefs_create_dir, for eventfs it should be eventfs_create_dir.
> > 
> > Signed-off-by: Ajay Kaher <akaher@...are.com>
> > Co-developed-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
> > Signed-off-by: Steven Rostedt (VMware) <rostedt@...dmis.org>
> > Tested-by: Ching-lin Yu <chinglinyu@...gle.com>
> > Acked-by: Masami Hiramatsu (Google) <mhiramat@...nel.org>
> 
> As this patch as is will break when running on older kernels, I was
> wondering if we should do this instead?

+1 since the latest kselftest is used also for checking the older
stable kernels, the test case has to check the environment and
change the parameter (or make it unsupported for new feature)
So below looks good to me.

Thanks,

> 
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc
> index 285b4770efad..ff7499eb98d6 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc
> @@ -34,14 +34,19 @@ mips*)
>  esac
>  
>  : "Test get argument (1)"
> -echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):char" > kprobe_events
> +if grep -q eventfs_add_dir available_filter_functions; then
> +  DIR_NAME="eventfs_add_dir"
> +else
> +  DIR_NAME="tracefs_create_dir"
> +fi
> +echo "p:testprobe ${DIR_NAME} arg1=+0(${ARG1}):char" > kprobe_events
>  echo 1 > events/kprobes/testprobe/enable
>  echo "p:test $FUNCTION_FORK" >> kprobe_events
>  grep -qe "testprobe.* arg1='t'" trace
>  
>  echo 0 > events/kprobes/testprobe/enable
>  : "Test get argument (2)"
> -echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):char arg2=+0(${ARG1}):char[4]" > kprobe_events
> +echo "p:testprobe ${DIR_NAME} arg1=+0(${ARG1}):char arg2=+0(${ARG1}):char[4]" > kprobe_events
>  echo 1 > events/kprobes/testprobe/enable
>  echo "p:test $FUNCTION_FORK" >> kprobe_events
>  grep -qe "testprobe.* arg1='t' arg2={'t','e','s','t'}" trace
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
> index a4f8e7c53c1f..a202b2ea4baf 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
> @@ -37,14 +37,19 @@ loongarch*)
>  esac
>  
>  : "Test get argument (1)"
> -echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):string" > kprobe_events
> +if grep -q eventfs_add_dir available_filter_functions; then
> +  DIR_NAME="eventfs_add_dir"
> +else
> +  DIR_NAME="tracefs_create_dir"
> +fi
> +echo "p:testprobe ${DIR_NAME} arg1=+0(${ARG1}):string" > kprobe_events
>  echo 1 > events/kprobes/testprobe/enable
>  echo "p:test $FUNCTION_FORK" >> kprobe_events
>  grep -qe "testprobe.* arg1=\"test\"" trace
>  
>  echo 0 > events/kprobes/testprobe/enable
>  : "Test get argument (2)"
> -echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):string arg2=+0(${ARG1}):string" > kprobe_events
> +echo "p:testprobe ${DIR_NAME} arg1=+0(${ARG1}):string arg2=+0(${ARG1}):string" > kprobe_events
>  echo 1 > events/kprobes/testprobe/enable
>  echo "p:test $FUNCTION_FORK" >> kprobe_events
>  grep -qe "testprobe.* arg1=\"test\" arg2=\"test\"" trace
> 
> -- Steve
> 
> > ---
> >  .../selftests/ftrace/test.d/kprobe/kprobe_args_char.tc        | 4 ++--
> >  .../selftests/ftrace/test.d/kprobe/kprobe_args_string.tc      | 4 ++--
> >  2 files changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc
> > index 285b4770efad..523cfb64539f 100644
> > --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_char.tc
> > @@ -34,14 +34,14 @@ mips*)
> >  esac
> >  
> >  : "Test get argument (1)"
> > -echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):char" > kprobe_events
> > +echo "p:testprobe eventfs_add_dir arg1=+0(${ARG1}):char" > kprobe_events
> >  echo 1 > events/kprobes/testprobe/enable
> >  echo "p:test $FUNCTION_FORK" >> kprobe_events
> >  grep -qe "testprobe.* arg1='t'" trace
> >  
> >  echo 0 > events/kprobes/testprobe/enable
> >  : "Test get argument (2)"
> > -echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):char arg2=+0(${ARG1}):char[4]" > kprobe_events
> > +echo "p:testprobe eventfs_add_dir arg1=+0(${ARG1}):char arg2=+0(${ARG1}):char[4]" > kprobe_events
> >  echo 1 > events/kprobes/testprobe/enable
> >  echo "p:test $FUNCTION_FORK" >> kprobe_events
> >  grep -qe "testprobe.* arg1='t' arg2={'t','e','s','t'}" trace
> > diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
> > index a4f8e7c53c1f..b9f8c3f8bae8 100644
> > --- a/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
> > +++ b/tools/testing/selftests/ftrace/test.d/kprobe/kprobe_args_string.tc
> > @@ -37,14 +37,14 @@ loongarch*)
> >  esac
> >  
> >  : "Test get argument (1)"
> > -echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):string" > kprobe_events
> > +echo "p:testprobe eventfs_add_dir arg1=+0(${ARG1}):string" > kprobe_events
> >  echo 1 > events/kprobes/testprobe/enable
> >  echo "p:test $FUNCTION_FORK" >> kprobe_events
> >  grep -qe "testprobe.* arg1=\"test\"" trace
> >  
> >  echo 0 > events/kprobes/testprobe/enable
> >  : "Test get argument (2)"
> > -echo "p:testprobe tracefs_create_dir arg1=+0(${ARG1}):string arg2=+0(${ARG1}):string" > kprobe_events
> > +echo "p:testprobe eventfs_add_dir arg1=+0(${ARG1}):string arg2=+0(${ARG1}):string" > kprobe_events
> >  echo 1 > events/kprobes/testprobe/enable
> >  echo "p:test $FUNCTION_FORK" >> kprobe_events
> >  grep -qe "testprobe.* arg1=\"test\" arg2=\"test\"" trace
> 


-- 
Masami Hiramatsu (Google) <mhiramat@...nel.org>