On Mon, Oct 28, 2024 at 4:23 PM Kalesh Singh <kaleshsingh@xxxxxxxxxx> wrote: > > On Mon, Oct 28, 2024 at 4:14 PM Steven Rostedt <rostedt@xxxxxxxxxxx> wrote: > > > > On Mon, 28 Oct 2024 14:43:58 -0700 > > Kalesh Singh <kaleshsingh@xxxxxxxxxx> wrote: > > > > > Add test to check the tracefs gid mount option is applied correctly > > > > > > ./ftracetest test.d/00basic/mount_options.tc > > > > > > Use the new readme string "[gid=<gid>] as a requirement and also update > > > test_ownership.tc requirements to use this. > > > > > > mount_options.tc will fail currently, this is fixed by the subsequent > > > patch in this series. > > > > Test cases should never be added when they can fail. They should always > > come after the fix is applied. But it appears that you check the README > > to make sure that it does work and not fail. > > > > I'll take a look at these patches in more detail later. > > Hi Steve, > > Thank you for the quick reviews, please feel free to ignore until you are back. > > I'll address these comments and resend a v2. I've posted v2 at: https://lore.kernel.org/r/20241030171928.4168869-1-kaleshsingh@xxxxxxxxxx/ Thanks, Kalesh > > Thanks, > Kalesh > > > > > Thanks, > > > > -- Steve > > > > > > > > > > Cc: David Howells <dhowells@xxxxxxxxxx> > > > Cc: Steven Rostedt <rostedt@xxxxxxxxxxx> > > > Cc: Masami Hiramatsu <mhiramat@xxxxxxxxxx> > > > Signed-off-by: Kalesh Singh <kaleshsingh@xxxxxxxxxx> > > > --- > > > .../ftrace/test.d/00basic/mount_options.tc | 101 ++++++++++++++++++ > > > .../ftrace/test.d/00basic/test_ownership.tc | 16 +-- > > > .../testing/selftests/ftrace/test.d/functions | 25 +++++ > > > 3 files changed, 129 insertions(+), 13 deletions(-) > > > create mode 100644 tools/testing/selftests/ftrace/test.d/00basic/mount_options.tc > > > > > > diff --git a/tools/testing/selftests/ftrace/test.d/00basic/mount_options.tc b/tools/testing/selftests/ftrace/test.d/00basic/mount_options.tc > > > new file mode 100644 > > > index 000000000000..b8aff85ec259 > > > --- /dev/null > > > +++ b/tools/testing/selftests/ftrace/test.d/00basic/mount_options.tc > > > @@ -0,0 +1,101 @@ > > > +#!/bin/sh > > > +# SPDX-License-Identifier: GPL-2.0 > > > +# description: Test tracefs GID mount option > > > +# requires: "[gid=<gid>]":README > > > + > > > +fail() { > > > + local msg="$1" > > > + > > > + echo "FAILED: $msg" > > > + exit_fail > > > +} > > > + > > > +find_alternate_gid() { > > > + local original_gid="$1" > > > + tac /etc/group | grep -v ":$original_gid:" | head -1 | cut -d: -f3 > > > +} > > > + > > > +mount_tracefs_with_options() { > > > + local mount_point="$1" > > > + local options="$2" > > > + > > > + mount -t tracefs -o "$options" nodev "$mount_point" > > > + > > > + setup > > > +} > > > + > > > +unmount_tracefs() { > > > + local mount_point="$1" > > > + > > > + # Need to make sure the mount isn't busy so that we can umount it > > > + (cd $mount_point; finish_ftrace;) > > > + > > > + cleanup > > > +} > > > + > > > +create_instance() { > > > + local mount_point="$1" > > > + local instance="$mount_point/instances/$(mktemp -u test-XXXXXX)" > > > + > > > + mkdir "$instance" > > > + echo "$instance" > > > +} > > > + > > > +remove_instance() { > > > + local instance="$1" > > > + > > > + rmdir "$instance" > > > +} > > > + > > > +check_gid() { > > > + local mount_point="$1" > > > + local expected_gid="$2" > > > + > > > + echo "Checking permission group ..." > > > + > > > + cd "$mount_point" > > > + > > > + for file in "." "events" "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable"; do > > > + local gid=`stat -c "%g" $file` > > > + if [ "$gid" -ne "$expected_gid" ]; then > > > + cd - # Return to the previous working direcotry (tracefs root) > > > + fail "$(realpath $file): Expected group $expected_gid; Got group $gid" > > > + fi > > > + done > > > + > > > + cd - # Return to the previous working direcotry (tracefs root) > > > +} > > > + > > > +test_gid_mount_option() { > > > + local mount_point=$(get_mount_point) > > > + local mount_options=$(get_mnt_options "$mount_point") > > > + local original_group=$(stat -c "%g" .) > > > + local other_group=$(find_alternate_gid "$original_group") > > > + > > > + # Set up mount options with new GID for testing > > > + local new_options=`echo "$mount_options" | sed -e "s/gid=[0-9]*/gid=$other_group/"` > > > + if [ "$new_options" = "$mount_options" ]; then > > > + new_options="$mount_options,gid=$other_group" > > > + mount_options="$mount_options,gid=$original_group" > > > + fi > > > + > > > + # Unmount existing tracefs instance and mount with new GID > > > + unmount_tracefs "$mount_point" > > > + mount_tracefs_with_options "$mount_point" "$new_options" > > > + > > > + check_gid "$mount_point" "$other_group" > > > + > > > + # Check that files created after the mount inherit the GID > > > + local instance=$(create_instance "$mount_point") > > > + check_gid "$instance" "$other_group" > > > + remove_instance "$instance" > > > + > > > + # Unmount and remount with the original GID > > > + unmount_tracefs "$mount_point" > > > + mount_tracefs_with_options "$mount_point" "$mount_options" > > > + check_gid "$mount_point" "$original_group" > > > +} > > > + > > > +test_gid_mount_option > > > + > > > +exit 0 > > > diff --git a/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc b/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc > > > index 094419e190c2..e71cc3ad0bdf 100644 > > > --- a/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc > > > +++ b/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc > > > @@ -1,24 +1,14 @@ > > > #!/bin/sh > > > # SPDX-License-Identifier: GPL-2.0 > > > # description: Test file and directory ownership changes for eventfs > > > +# requires: "[gid=<gid>]":README > > > > > > original_group=`stat -c "%g" .` > > > original_owner=`stat -c "%u" .` > > > > > > -mount_point=`stat -c '%m' .` > > > +local mount_point=$(get_mount_point) > > > > > > -# If stat -c '%m' does not work (e.g. busybox) or failed, try to use the > > > -# current working directory (which should be a tracefs) as the mount point. > > > -if [ ! -d "$mount_point" ]; then > > > - if mount | grep -qw $PWD ; then > > > - mount_point=$PWD > > > - else > > > - # If PWD doesn't work, that is an environmental problem. > > > - exit_unresolved > > > - fi > > > -fi > > > - > > > -mount_options=`mount | grep "$mount_point" | sed -e 's/.*(\(.*\)).*/\1/'` > > > +mount_options=$(get_mnt_options "$mount_point") > > > > > > # find another owner and group that is not the original > > > other_group=`tac /etc/group | grep -v ":$original_group:" | head -1 | cut -d: -f3` > > > diff --git a/tools/testing/selftests/ftrace/test.d/functions b/tools/testing/selftests/ftrace/test.d/functions > > > index 779f3e62ec90..84d6a9c7ad67 100644 > > > --- a/tools/testing/selftests/ftrace/test.d/functions > > > +++ b/tools/testing/selftests/ftrace/test.d/functions > > > @@ -193,3 +193,28 @@ ftrace_errlog_check() { # err-prefix command-with-error-pos-by-^ command-file > > > # " Command: " and "^\n" => 13 > > > test $(expr 13 + $pos) -eq $N > > > } > > > + > > > +# Helper to get the tracefs mount point > > > +get_mount_point() { > > > + local mount_point=`stat -c '%m' .` > > > + > > > + # If stat -c '%m' does not work (e.g. busybox) or failed, try to use the > > > + # current working directory (which should be a tracefs) as the mount point. > > > + if [ ! -d "$mount_point" ]; then > > > + if mount | grep -qw "$PWD"; then > > > + mount_point=$PWD > > > + else > > > + # If PWD doesn't work, that is an environmental problem. > > > + exit_unresolved > > > + fi > > > + fi > > > + echo "$mount_point" > > > +} > > > + > > > +# Helper function to retrieve mount options for a given mount point > > > +get_mnt_options() { > > > + local mnt_point="$1" > > > + local opts=$(mount | grep -m1 "$mnt_point" | sed -e 's/.*(\(.*\)).*/\1/') > > > + > > > + echo "$opts" > > > +} > > > \ No newline at end of file > >