| 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
| ||
|
Message-ID: <CAC_TJvf8aU07JRiBK9tgWynixXBX9MpqFJh6GVaUcEzqX6aLhQ@mail.gmail.com>
Date: Mon, 28 Oct 2024 16:23:33 -0700
From: Kalesh Singh <kaleshsingh@...gle.com>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: dhowells@...hat.com, mhiramat@...nel.org, surenb@...gle.com,
jyescas@...gle.com, kernel-team@...roid.com, android-mm@...gle.com,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>, Shuah Khan <shuah@...nel.org>,
Ali Zahraee <ahzahraee@...il.com>, Eric Sandeen <sandeen@...hat.com>,
Christian Brauner <brauner@...nel.org>, linux-kernel@...r.kernel.org,
linux-trace-kernel@...r.kernel.org, linux-kselftest@...r.kernel.org
Subject: Re: [PATCH 2/3] tracing/selftests: Add tracefs mount options test
On Mon, Oct 28, 2024 at 4:14 PM Steven Rostedt <rostedt@...dmis.org> wrote:
>
> On Mon, 28 Oct 2024 14:43:58 -0700
> Kalesh Singh <kaleshsingh@...gle.com> 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.
Thanks,
Kalesh
>
> Thanks,
>
> -- Steve
>
>
> >
> > Cc: David Howells <dhowells@...hat.com>
> > Cc: Steven Rostedt <rostedt@...dmis.org>
> > Cc: Masami Hiramatsu <mhiramat@...nel.org>
> > Signed-off-by: Kalesh Singh <kaleshsingh@...gle.com>
> > ---
> > .../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
>
Powered by blists - more mailing lists