lkml.org 
[lkml]   [2022]   [Sep]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 2/2] tracefs: Only clobber mode/uid/gid on remount if asked
Hi Steven,

On Wed, Sep 07, 2022 at 07:44:43AM -0400, Steven Rostedt wrote:
> On Fri, 26 Aug 2022 17:44:17 -0700
> Brian Norris <briannorris@chromium.org> wrote:
>
> > Users may have explicitly configured their tracefs permissions; we
> > shouldn't overwrite those just because a second mount appeared.
> >
> > Only clobber if the options were provided at mount time.
> >
> > Note: the previous behavior was especially surprising in the presence of
> > automounted /sys/kernel/debug/tracing/.
> >
> > # Don't change /sys/kernel/tracing/ permissions on automount.
> > umount /sys/kernel/debug/tracing/
> > stat /sys/kernel/debug/tracing/.
> >
> > # Don't change /sys/kernel/tracing/ permissions.
> > mount -t tracefs none /mnt/foo
> >
> > # Change /sys/kernel/tracing/ mode and uid, but not gid.
> > mount -t tracefs -o uid=bar,mode=0750 none /mnt/baz
> >
>
> The above text doesn't make sense. Is the comments what you are doing or
> what the system is doing? If it is what the system is doing, please show
> the output of the stat command and how it is doing something unexpected.

Sorry, I do see how the text as-is is unclear. The intention is to
describe the new, intended behavior, and I only left the existing
behavior as implied.

> Can you show the example of what is wrong, and what you expected to happen.
> The above is just a bunch of commands, but does not display anything that
> is incorrect.

Sure, here's a narrower description of old (unexpected) and new
(expected) behavior, in case you were wanting to update the changelog on
your own:

---

Existing behavior:

## Pre-existing status: tracefs is 0755.
# stat -c '%A' /sys/kernel/tracing/
drwxr-xr-x

## (Re)trigger the automount.
# umount /sys/kernel/debug/tracing
# stat -c '%A' /sys/kernel/debug/tracing/.
drwx------

## Unexpected: the automount changed mode for other mount instances.
# stat -c '%A' /sys/kernel/tracing/
drwx------

New behavior:

## Pre-existing status: tracefs is 0755.
# stat -c '%A' /sys/kernel/tracing/
drwxr-xr-x

## (Re)trigger the automount.
# umount /sys/kernel/debug/tracing
# stat -c '%A' /sys/kernel/debug/tracing/.
drwxr-xr-x

## Expected: the automount does not change other mount instances.
# stat -c '%A' /sys/kernel/tracing/
drwxr-xr-x

---

There are other variations of old/new behavior (e.g., if using various
mount options), but those are not the main reason for this patch.

> > Signed-off-by: Brian Norris <briannorris@chromium.org>
> > ---
> > I'm open to writing an LTP test case for this, if that seems like a good
> > idea.
>
> Yes, please add a test :-)

Sure, I'm dusting off my LTP VM now. But in case you'd like the patch
as-is, feel free to splice the above into the commit description.
Otherwise, I may resend as Greg requested for patch 1, after I've got a
test patch going.

Brian

\
 
 \ /
  Last update: 2022-09-07 22:53    [W:0.124 / U:0.736 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site