linux-kernel - Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for powerpc

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: <20170622180738.e97092d4fa6458b9199a03d7@kernel.org>
Date:   Thu, 22 Jun 2017 18:07:38 +0900
From:   Masami Hiramatsu <mhiramat@...nel.org>
To:     "Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com>
Cc:     Steven Rostedt <rostedt@...dmis.org>,
        Ananth N Mavinakayanahalli <ananth@...ux.vnet.ibm.com>,
        Masami Hiramatsu <mhiramat@...nel.org>,
        Michael Ellerman <mpe@...erman.id.au>,
        linux-kernel@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH 2/2] selftests/ftrace: Update multiple kprobes test for
 powerpc

On Thu, 22 Jun 2017 00:20:28 +0530
"Naveen N. Rao" <naveen.n.rao@...ux.vnet.ibm.com> wrote:

> KPROBES_ON_FTRACE is only available on powerpc64le. Update comment to
> clarify this.
> 
> Also, we should use an offset of 8 to ensure that the probe does not
> fall on ftrace location. The current offset of 4 will fall before the
> function local entry point and won't fire, while an offset of 12 or 16
> will fall on ftrace location. Offset 8 is currently guaranteed to not be
> the ftrace location.

OK, these part seems good to me.

> 
> Finally, do not filter out symbols with a dot. Powerpc Elfv1 uses dot
> prefix for all functions and this prevents us from testing some of those
> symbols. Furthermore, with the patch to derive event names properly in
> the presence of ':' and '.', such names are accepted by kprobe_events
> and constitutes a good test for those symbols.

Hmm, the reason why I added such filter was to avoid symbols including
gcc-generated suffixes like as .constprop or .isra etc.
So if the Powerpc Elfv1 use dot prefix, that is OK, in that case,
could you update the filter as "^.*\\..*" ?

Thank you,

> 
> Signed-off-by: Naveen N. Rao <naveen.n.rao@...ux.vnet.ibm.com>
> ---
>  tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
> index f4d1ff785d67..d209c071b2c0 100644
> --- a/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
> +++ b/tools/testing/selftests/ftrace/test.d/kprobe/multiple_kprobes.tc
> @@ -2,16 +2,16 @@
>  # description: Register/unregister many kprobe events
>  
>  # ftrace fentry skip size depends on the machine architecture.
> -# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc
> +# Currently HAVE_KPROBES_ON_FTRACE defined on x86 and powerpc64le
>  case `uname -m` in
>    x86_64|i[3456]86) OFFS=5;;
> -  ppc*) OFFS=4;;
> +  ppc64le) OFFS=8;;
>    *) OFFS=0;;
>  esac
>  
>  echo "Setup up to 256 kprobes"
> -grep t /proc/kallsyms | cut -f3 -d" " | grep -v .*\\..* | \
> -head -n 256 | while read i; do echo p ${i}+${OFFS} ; done > kprobe_events ||:
> +grep t /proc/kallsyms | cut -f3 -d" " | head -n 256 | \
> +while read i; do echo p ${i}+${OFFS} ; done > kprobe_events ||:
>  
>  echo 1 > events/kprobes/enable
>  echo 0 > events/kprobes/enable
> -- 
> 2.13.1
> 


-- 
Masami Hiramatsu <mhiramat@...nel.org>