Re: [PATCH 2/2] tracefs: Only clobber mode/uid/gid on remount if asked

[Brian Norris <briannorris@chromium.org>]

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