Messages in this thread | | | Date | Wed, 7 Sep 2022 13:52:15 -0700 | From | Brian Norris <> | Subject | Re: [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
| |