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.
Signed-off-by: Steven Rostedt (Google) rostedt@goodmis.org --- Changes since v1: https://lore.kernel.org/linux-trace-kernel/20231221193551.13a0b7bd@gandalf.l...
- 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
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.l...
- 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
On Fri, 22 Dec 2023 10:21:48 +0900 Masami Hiramatsu (Google) mhiramat@kernel.org wrote:
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.
Hmm, honestly, it should technically work on all past versions.
I'll try it out to see what fails for 6.5.13. Perhaps there was another bug that the stable releases need fixing for?
-- Steve
On Thu, 21 Dec 2023 20:28:13 -0500 Steven Rostedt rostedt@goodmis.org wrote:
On Fri, 22 Dec 2023 10:21:48 +0900 Masami Hiramatsu (Google) mhiramat@kernel.org wrote:
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.
Hmm, honestly, it should technically work on all past versions.
Ah, sorry. It was my mistake. I need to make /etc/group,passwd files in my test environment. Let me try it again.
I'll try it out to see what fails for 6.5.13. Perhaps there was another bug that the stable releases need fixing for?
-- Steve
On Thu, 21 Dec 2023 20:28:13 -0500 Steven Rostedt rostedt@goodmis.org wrote:
On Fri, 22 Dec 2023 10:21:48 +0900 Masami Hiramatsu (Google) mhiramat@kernel.org wrote:
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.
Hmm, honestly, it should technically work on all past versions.
I'll try it out to see what fails for 6.5.13. Perhaps there was another bug that the stable releases need fixing for?
I found that the failure was my environmental issue. BTW, for busybox environment,
+instance="foo-$(mktemp -u XXXXX)"
This doesn't work. it needs XXXXXX (6 times X). And this is somewhat wrong usage of mktemp because it can not check there is foo-<random>. What about change it as
instance="$(mktemp -u foo-XXXXXX)"
?
Thanks,
-- Steve
On Fri, 22 Dec 2023 10:48:41 +0900 Masami Hiramatsu (Google) mhiramat@kernel.org wrote:
On Thu, 21 Dec 2023 20:28:13 -0500 Steven Rostedt rostedt@goodmis.org wrote:
On Fri, 22 Dec 2023 10:21:48 +0900 Masami Hiramatsu (Google) mhiramat@kernel.org wrote:
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.
Hmm, honestly, it should technically work on all past versions.
I'll try it out to see what fails for 6.5.13. Perhaps there was another bug that the stable releases need fixing for?
I found that the failure was my environmental issue. BTW, for busybox environment,
+instance="foo-$(mktemp -u XXXXX)"
This doesn't work. it needs XXXXXX (6 times X). And this is somewhat wrong usage of mktemp because it can not check there is foo-<random>. What about change it as
instance="$(mktemp -u foo-XXXXXX)"
?
And I confirmed that this test passed on v6.5.13 with that change.
Thank you,
Thanks,
-- Steve
-- Masami Hiramatsu (Google) mhiramat@kernel.org
On Fri, 22 Dec 2023 10:52:00 +0900 Masami Hiramatsu (Google) mhiramat@kernel.org wrote:
+instance="foo-$(mktemp -u XXXXX)"
This doesn't work. it needs XXXXXX (6 times X). And this is somewhat wrong usage of mktemp because it can not check there is foo-<random>. What about change it as
instance="$(mktemp -u foo-XXXXXX)"
That can work too, although I think I'll change it from "foo" to "test".
?
And I confirmed that this test passed on v6.5.13 with that change.
Thanks, I'll send out a v3 with 6 Xs. ;-)
-- Steve
On Fri, 22 Dec 2023 10:52:00 +0900 Masami Hiramatsu (Google) mhiramat@kernel.org wrote:
On Fri, 22 Dec 2023 10:48:41 +0900 Masami Hiramatsu (Google) mhiramat@kernel.org wrote:
And I confirmed that this test passed on v6.5.13 with that change.
I just ran it on 6.5.13 and it took *forever*!
But I do have a bit of debug, and before 6.6 creating the instance and deleting it required creating and deleting thousands of inodes and dentries.
-- Steve
On Thu, 21 Dec 2023 21:07:57 -0500 Steven Rostedt rostedt@goodmis.org wrote:
On Fri, 22 Dec 2023 10:52:00 +0900 Masami Hiramatsu (Google) mhiramat@kernel.org wrote:
On Fri, 22 Dec 2023 10:48:41 +0900 Masami Hiramatsu (Google) mhiramat@kernel.org wrote:
And I confirmed that this test passed on v6.5.13 with that change.
I just ran it on 6.5.13 and it took *forever*!
But I do have a bit of debug, and before 6.6 creating the instance and deleting it required creating and deleting thousands of inodes and dentries.
Hmm, it may depends on the machine. I could ran it (on 64 vcpu VM)
Thank you,
-- Steve
linux-kselftest-mirror@lists.linaro.org