From: Masami Hiramatsu (Google) <mhiramat@kernel.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
Linux Trace Kernel <linux-trace-kernel@vger.kernel.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Mark Rutland <mark.rutland@arm.com>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Shuah Khan <shuah@kernel.org>,
Linux selftests <linux-kselftest@vger.kernel.org>
Subject: Re: [PATCH v2] tracing/selftests: Add ownership modification tests for eventfs
Date: Fri, 22 Dec 2023 10:21:48 +0900 [thread overview]
Message-ID: <20231222102148.2aa3863d7c11f3928549335a@kernel.org> (raw)
In-Reply-To: <20231221194516.53e1ee43@gandalf.local.home>
Hi Steve,
On Thu, 21 Dec 2023 19:45:16 -0500
Steven Rostedt <rostedt@goodmis.org> wrote:
> From: "Steven Rostedt (Google)" <rostedt@goodmis.org>
>
> As there were bugs found with the ownership of eventfs dynamic file
> creation. Add a test to test it.
>
> It will remount tracefs with a different gid and check the ownership of
> the eventfs directory, as well as the system and event directories. It
> will also check the event file directories.
>
> It then does a chgrp on each of these as well to see if they all get
> updated as expected.
>
> Then it remounts the tracefs file system back to the original group and
> makes sure that all the updated files and directories were reset back to
> the original ownership.
>
> It does the same for instances that change the ownership of he instance
> directory.
>
> Note, because the uid is not reset by a remount, it is tested for every
> file by switching it to a new owner and then back again.
>
The testcase itself is OK but is there any way to identify the system
supports eventfs or not? I ran this test on v6.5.13 for checking then
it failed. We may need to skip (unsupported) this test for such case.
Thank you,
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> Changes since v1: https://lore.kernel.org/linux-trace-kernel/20231221193551.13a0b7bd@gandalf.local.home
>
> - Fixed a cut and paste error of using $original_group for finding another uid
>
> .../ftrace/test.d/00basic/test_ownership.tc | 113 ++++++++++++++++++
> 1 file changed, 113 insertions(+)
> create mode 100755 tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc
>
> diff --git a/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc b/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc
> new file mode 100755
> index 000000000000..83cbd116d06b
> --- /dev/null
> +++ b/tools/testing/selftests/ftrace/test.d/00basic/test_ownership.tc
> @@ -0,0 +1,113 @@
> +#!/bin/sh
> +# description: Test file and directory owership changes for eventfs
> +
> +original_group=`stat -c "%g" .`
> +original_owner=`stat -c "%u" .`
> +
> +mount_point=`stat -c '%m' .`
> +mount_options=`mount | grep "$mount_point" | sed -e 's/.*(\(.*\)).*/\1/'`
> +
> +# find another owner and group that is not the original
> +other_group=`tac /etc/group | grep -v ":$original_group:" | head -1 | cut -d: -f3`
> +other_owner=`tac /etc/passwd | grep -v ":$original_owner:" | head -1 | cut -d: -f3`
> +
> +# Remove any group ownership already
> +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
> +
> +canary="events/timer events/timer/timer_cancel events/timer/timer_cancel/format"
> +
> +test() {
> + file=$1
> + test_group=$2
> +
> + owner=`stat -c "%u" $file`
> + group=`stat -c "%g" $file`
> +
> + echo "testing $file $owner=$original_owner and $group=$test_group"
> + if [ $owner -ne $original_owner ]; then
> + exit_fail
> + fi
> + if [ $group -ne $test_group ]; then
> + exit_fail
> + fi
> +
> + # Note, the remount does not update ownership so test going to and from owner
> + echo "test owner $file to $other_owner"
> + chown $other_owner $file
> + owner=`stat -c "%u" $file`
> + if [ $owner -ne $other_owner ]; then
> + exit_fail
> + fi
> +
> + chown $original_owner $file
> + owner=`stat -c "%u" $file`
> + if [ $owner -ne $original_owner ]; then
> + exit_fail
> + fi
> +
> +}
> +
> +run_tests() {
> + for d in "." "events" "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do
> + test "$d" $other_group
> + done
> +
> + chgrp $original_group events
> + test "events" $original_group
> + for d in "." "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do
> + test "$d" $other_group
> + done
> +
> + chgrp $original_group events/sched
> + test "events/sched" $original_group
> + for d in "." "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do
> + test "$d" $other_group
> + done
> +
> + chgrp $original_group events/sched/sched_switch
> + test "events/sched/sched_switch" $original_group
> + for d in "." "events/sched/sched_switch/enable" $canary; do
> + test "$d" $other_group
> + done
> +
> + chgrp $original_group events/sched/sched_switch/enable
> + test "events/sched/sched_switch/enable" $original_group
> + for d in "." $canary; do
> + test "$d" $other_group
> + done
> +}
> +
> +mount -o remount,"$new_options" .
> +
> +run_tests
> +
> +mount -o remount,"$mount_options" .
> +
> +for d in "." "events" "events/sched" "events/sched/sched_switch" "events/sched/sched_switch/enable" $canary; do
> + test "$d" $original_group
> +done
> +
> +# check instances as well
> +
> +chgrp $other_group instances
> +
> +instance="foo-$(mktemp -u XXXXX)"
> +
> +mkdir instances/$instance
> +
> +cd instances/$instance
> +
> +run_tests
> +
> +cd ../..
> +
> +rmdir instances/$instance
> +
> +chgrp $original_group instances
> +
> +exit 0
> --
> 2.42.0
>
>
--
Masami Hiramatsu (Google) <mhiramat@kernel.org>
next prev parent reply other threads:[~2023-12-22 1:21 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-12-22 0:45 [PATCH v2] tracing/selftests: Add ownership modification tests for eventfs Steven Rostedt
2023-12-22 1:21 ` Masami Hiramatsu [this message]
2023-12-22 1:28 ` Steven Rostedt
2023-12-22 1:31 ` Masami Hiramatsu
2023-12-22 1:48 ` Masami Hiramatsu
2023-12-22 1:52 ` Masami Hiramatsu
2023-12-22 2:01 ` Steven Rostedt
2023-12-22 2:07 ` Steven Rostedt
2023-12-22 4:11 ` Masami Hiramatsu
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20231222102148.2aa3863d7c11f3928549335a@kernel.org \
--to=mhiramat@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=mathieu.desnoyers@efficios.com \
--cc=rostedt@goodmis.org \
--cc=shuah@kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.