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

[Steven Rostedt <rostedt@goodmis.org>]

On Thu, 1 Sep 2022 17:58:14 +0200
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:

> On Fri, Aug 26, 2022 at 05:44:16PM -0700, Brian Norris wrote:
> > Users may have explicitly configured their debugfs permissions; we
> > shouldn't overwrite those just because a second mount appeared.  
> 
> What userspace mounts debugfs twice?
> 
> > Only clobber if the options were provided at mount time.
> > 
> >   # Don't change /sys/kernel/debug/ permissions.
> >   mount -t debugfs none /mnt/foo
> > 
> >   # Change /sys/kernel/debug/ mode and uid, but not gid.
> >   mount -t debugfs -o uid=bar,mode=0750 none /mnt/baz  
> 
> So what happens today with this change?  Without it?
> 
> > 
> > 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.  
> 
> If it's really needed, again, why would debugfs be ever mounted more
> than once?
>

The real issue is with tracefs, and I think the debugfs patch is just for
consistency. But I (and others) do have debugfs mounted more that once. ;-)

  # mount |grep debugfs
  debugfs on /sys/kernel/debug type debugfs (rw,nosuid,nodev,noexec,relatime)
  debugfs on /debug type debugfs (rw,relatime)

I added /debug (and I know others that just add /d) for shortcuts to get to
the debugfs directory easier.

This patch series came about because ideally tracefs should be mounted in
/sys/kernel/tracing. But because a lot of scripts use the old path of
/sys/kernel/debug/tracing, I have tracefs mounted there automatically
when debugfs is mounted. This is so that scripts do not break.

I would love to deprecate the /sys/kernel/debug/tracing automatic mounting,
but I do not know what user space will break if that happens. libtracefs
handles finding where tracefs is, so anything that uses that is fine.

But anyway, because tracefs is mounted more than once, if someone has
tracefs mounted in the correct location "/sys/kernel/tracing" and updates
the permissions to the files, but then mounts debugfs, due to the automatic
mounting, all their changes go away.

Perhaps we only need the second patch because of the automatic mounting.
Maybe people do not care if things get reset if they manually mount debugfs
more than once. I (and others) have it in my fstab, so it's done at boot up
and any changes should affect both.

-- Steve