[PATCH 0/2] selftests/ftrace: Fix ftracetest test cases

[PATCH 0/2] selftests/ftrace: Fix ftracetest test cases

[Masami Hiramatsu (Google)]

Here is a couple of patches to fix issues. I think mount_options.tc's one
is a real bug(I'm not sure how it worked), but another one is an enhancement
for (my) execution environment.
Anyway, those should go through kselftests tree.

Thank you,
---

Masami Hiramatsu (Google) (2):
      selftests/ftrace: Fix to use remount when testing mount GID option
      selftests/ftrace: Make uprobe test more robust against binary name


 .../ftrace/test.d/00basic/mount_options.tc         |    8 ++++----
 .../ftrace/test.d/dynevent/add_remove_uprobe.tc    |    4 +++-
 2 files changed, 7 insertions(+), 5 deletions(-)

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

[PATCH 1/2] selftests/ftrace: Fix to use remount when testing mount GID option

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Fix mount_options.tc to use remount option to mount the tracefs.
Since the current implementation does not umount the tracefs,
this test always fails because of -EBUSY error.
Using remount option will allow us to change the mount option.

Fixes: 8b55572e5180 ("tracing/selftests: Add tracefs mount options test")
Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
Cc: stable@vger.kernel.org
---
 .../ftrace/test.d/00basic/mount_options.tc         |    8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/ftrace/test.d/00basic/mount_options.tc b/tools/testing/selftests/ftrace/test.d/00basic/mount_options.tc
index 35e8d47d6072..8a7ce647a60d 100644
--- a/tools/testing/selftests/ftrace/test.d/00basic/mount_options.tc
+++ b/tools/testing/selftests/ftrace/test.d/00basic/mount_options.tc
@@ -15,11 +15,11 @@ find_alternate_gid() {
 	tac /etc/group | grep -v ":$original_gid:" | head -1 | cut -d: -f3
 }
 
-mount_tracefs_with_options() {
+remount_tracefs_with_options() {
 	local mount_point="$1"
 	local options="$2"
 
-	mount -t tracefs -o "$options" nodev "$mount_point"
+	mount -t tracefs -o "remount,$options" nodev "$mount_point"
 
 	setup
 }
@@ -81,7 +81,7 @@ test_gid_mount_option() {
 
 	# Unmount existing tracefs instance and mount with new GID
 	unmount_tracefs "$mount_point"
-	mount_tracefs_with_options "$mount_point" "$new_options"
+	remount_tracefs_with_options "$mount_point" "$new_options"
 
 	check_gid "$mount_point" "$other_group"
 
@@ -92,7 +92,7 @@ test_gid_mount_option() {
 
 	# Unmount and remount with the original GID
 	unmount_tracefs "$mount_point"
-	mount_tracefs_with_options "$mount_point" "$mount_options"
+	remount_tracefs_with_options "$mount_point" "$mount_options"
 	check_gid "$mount_point" "$original_group"
 }
 

[PATCH 2/2] selftests/ftrace: Make uprobe test more robust against binary name

[Masami Hiramatsu (Google)]

From: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Make add_remove_uprobe test case more robust against various real
binary name.
Current add_remove_uprobe.tc test expects the real binary of /bin/sh
is '*/bin/*sh', but it does not work on busybox environment.
Instead of using fixed pattern, use readlink to identify real binary
name.

Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
---
 .../ftrace/test.d/dynevent/add_remove_uprobe.tc    |    4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc
index a275decdc880..86c76679c56e 100644
--- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc
+++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc
@@ -6,8 +6,10 @@
 echo 0 > events/enable
 echo > dynamic_events
 
+REALBIN=`readlink -f /bin/sh`
+
 echo 'cat /proc/$$/maps' | /bin/sh | \
-	grep "r-xp .*/bin/.*sh$" | \
+	grep "r-xp .*${REALBIN}$" | \
 	awk '{printf "p:myevent %s:0x%s\n", $6,$3 }' >> uprobe_events
 
 grep -q myevent uprobe_events

Re: [PATCH 0/2] selftests/ftrace: Fix ftracetest test cases

[Shuah Khan]

On 1/7/25 05:10, Masami Hiramatsu (Google) wrote:
> Here is a couple of patches to fix issues. I think mount_options.tc's one
> is a real bug(I'm not sure how it worked), but another one is an enhancement
> for (my) execution environment.
> Anyway, those should go through kselftests tree.
> 
> Thank you,
> ---
> 
> Masami Hiramatsu (Google) (2):
>        selftests/ftrace: Fix to use remount when testing mount GID option
>        selftests/ftrace: Make uprobe test more robust against binary name
> 
> 
>   .../ftrace/test.d/00basic/mount_options.tc         |    8 ++++----
>   .../ftrace/test.d/dynevent/add_remove_uprobe.tc    |    4 +++-
>   2 files changed, 7 insertions(+), 5 deletions(-)
> 
> --
> Masami Hiramatsu (Google) <mhiramat@kernel.org>

I applied these two patches to linux-kselftest next fro Linux 6.14-rc1

thanks,
-- Shuah

Re: [PATCH 2/2] selftests/ftrace: Make uprobe test more robust against binary name

[Heiko Carstens]

On Tue, Jan 07, 2025 at 09:11:16PM +0900, Masami Hiramatsu (Google) wrote:
> From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> 
> Make add_remove_uprobe test case more robust against various real
> binary name.
> Current add_remove_uprobe.tc test expects the real binary of /bin/sh
> is '*/bin/*sh', but it does not work on busybox environment.
> Instead of using fixed pattern, use readlink to identify real binary
> name.
> 
> Signed-off-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>
> ---
>  .../ftrace/test.d/dynevent/add_remove_uprobe.tc    |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc
> index a275decdc880..86c76679c56e 100644
> --- a/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc
> +++ b/tools/testing/selftests/ftrace/test.d/dynevent/add_remove_uprobe.tc
> @@ -6,8 +6,10 @@
>  echo 0 > events/enable
>  echo > dynamic_events
>  
> +REALBIN=`readlink -f /bin/sh`
> +
>  echo 'cat /proc/$$/maps' | /bin/sh | \
> -	grep "r-xp .*/bin/.*sh$" | \
> +	grep "r-xp .*${REALBIN}$" | \
>  	awk '{printf "p:myevent %s:0x%s\n", $6,$3 }' >> uprobe_events

This test fails on s390, not because of this change, but since the
test seems to assume that the entry point of /bin/sh is at offset 0;
or at least there is some code that is executed at least once at
offset 0; but that is not the case (at least) for s390.

Not sure how this can be addressed. Using "readelf -f" to get the
entry point address of the executable and use that for the uprobe
would work, but it would also add a dependency to binutils for the
ftrace selftests, which doesn't exist yet, as far as I can tell.

Would it maybe an option for architectures to opt-in to use readelf
and skip the test (exit_unsupported) if readelf is not available?

Re: [PATCH 2/2] selftests/ftrace: Make uprobe test more robust against binary name

[Steven Rostedt]

On Mon, 17 Feb 2025 14:42:44 +0100
Heiko Carstens <hca@linux.ibm.com> wrote:

> > +REALBIN=`readlink -f /bin/sh`
> > +
> >  echo 'cat /proc/$$/maps' | /bin/sh | \
> > -	grep "r-xp .*/bin/.*sh$" | \
> > +	grep "r-xp .*${REALBIN}$" | \
> >  	awk '{printf "p:myevent %s:0x%s\n", $6,$3 }' >> uprobe_events  
> 
> This test fails on s390, not because of this change, but since the
> test seems to assume that the entry point of /bin/sh is at offset 0;
> or at least there is some code that is executed at least once at
> offset 0; but that is not the case (at least) for s390.
> 
> Not sure how this can be addressed. Using "readelf -f" to get the
> entry point address of the executable and use that for the uprobe
> would work, but it would also add a dependency to binutils for the
> ftrace selftests, which doesn't exist yet, as far as I can tell.
> 
> Would it maybe an option for architectures to opt-in to use readelf
> and skip the test (exit_unsupported) if readelf is not available?

I'm OK for adding a dependency on readelf existing on the test system, as
it is part of binutils.

Masami and Shuah,

Are you OK with that?

-- Steve