Message-ID: <20200228155244.k4h4hz3dqhl7q7ks@wittgenstein>
Date:   Fri, 28 Feb 2020 16:52:44 +0100
From:   Christian Brauner <christian.brauner@...ntu.com>
To:     James Bottomley <James.Bottomley@...senPartnership.com>
Cc:     Steven Whitehouse <swhiteho@...hat.com>,
        Miklos Szeredi <miklos@...redi.hu>,
        Miklos Szeredi <mszeredi@...hat.com>,
        David Howells <dhowells@...hat.com>,
        viro <viro@...iv.linux.org.uk>, Ian Kent <raven@...maw.net>,
        Christian Brauner <christian@...uner.io>,
        Jann Horn <jannh@...gle.com>,
        "Darrick J. Wong" <darrick.wong@...cle.com>,
        Linux API <linux-api@...r.kernel.org>,
        linux-fsdevel <linux-fsdevel@...r.kernel.org>,
        lkml <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 00/17] VFS: Filesystem information and notifications [ver
 #17]

On Tue, Feb 25, 2020 at 07:28:55AM -0800, James Bottomley wrote:
> On Tue, 2020-02-25 at 12:13 +0000, Steven Whitehouse wrote:
> > Hi,
> > 
> > On 24/02/2020 15:28, Miklos Szeredi wrote:
> > > On Mon, Feb 24, 2020 at 3:55 PM James Bottomley
> > > <James.Bottomley@...senpartnership.com> wrote:
> > > 
> > > > Once it's table driven, certainly a sysfs directory becomes
> > > > possible. The problem with ST_DEV is filesystems like btrfs and
> > > > xfs that may have multiple devices.
> > > 
> > > For XFS there's always  a single sb->s_dev though, that's what
> > > st_dev will be set to on all files.
> > > 
> > > Btrfs subvolume is sort of a lightweight superblock, so basically
> > > all such st_dev's are aliases of the same master superblock.  So
> > > lookup of all subvolume st_dev's could result in referencing the
> > > same underlying struct super_block (just like /proc/$PID will
> > > reference the same underlying task group regardless of which of the
> > > task group member's PID is used).
> > > 
> > > Having this info in sysfs would spare us a number of issues that a
> > > set of new syscalls would bring.  The question is, would that be
> > > enough, or is there a reason that sysfs can't be used to present
> > > the various filesystem related information that fsinfo is supposed
> > > to present?
> > > 
> > > Thanks,
> > > Miklos
> > > 
> > 
> > We need a unique id for superblocks anyway. I had wondered about
> > using s_dev some time back, but for the reasons mentioned earlier in
> > this thread I think it might just land up being confusing and
> > difficult to manage. While fake s_devs are created for sbs that don't
> > have a device, I can't help thinking that something closer to
> > ifindex, but for superblocks, is needed here. That would avoid the
> > issue of which device number to use.
> > 
> > In fact we need that anyway for the notifications, since without
> > that  there is a race that can lead to missing remounts of the same
> > device, in  case a umount/mount pair is missed due to an overrun, and
> > then fsinfo returns the same device as before, with potentially the
> > same mount options too. So I think a unique id for a superblock is a
> > generically useful feature, which would also allow for sensible sysfs
> > directory naming, if required,
> 
> But would this be informative and useful for the user?  I'm sure we can
> find a persistent id for a persistent superblock, but what about tmpfs
> ... that's going to have to change with every reboot.  It's going to be
> remarkably inconvenient if I want to get fsinfo on /run to have to keep
> finding what the id is.
> 
> The other thing a file descriptor does that sysfs doesn't is that it
> solves the information leak: if I'm in a mount namespace that has no
> access to certain mounts, I can't fspick them and thus I can't see the
> information.  By default, with sysfs I can.

Difficult to figure out which part of the thread to reply too. :)

sysfs strikes me as fundamentally misguided for this task.

Init systems or any large-scale daemon will hate parsing things, there's
that and parts of the reason why mountinfo sucks is because of parsing a
possibly a potentially enormous file. Exposing information in sysfs will
require parsing again one way or the other. I've been discussing these
bottlenecks with Lennart quite a bit and reliable and performant mount
notifications without needing to parse stuff is very high on the issue
list. But even if that isn't an issue for some reason the namespace
aspect is definitely something I'd consider a no-go.
James has been poking at this a little already and I agree. More
specifically, sysfs and proc already are a security nightmare for
namespace-aware workloads and require special care. Not leaking
information in any way is a difficult task. I mean, over the last two
years I sent quite a lot of patches to the networking-namespace aware
part of sysfs alone either fixing information leaks, or making other
parts namespace aware that weren't and were causing issues (There's
another large-ish series sitting in Dave's tree right now.). And tbh,
network namespacing in sysfs is imho trivial compared to what we would
need to do to handle mount namespacing and especially mount propagation.
fsinfo() is way cleaner and ultimately simpler approach. We very much
want it file-descriptor based. The mount api opens up the road to secure
and _delegatable_ querying of filesystem information.

Christian

Powered by blists - more mailing lists