[RFC PATCH v1 1/7] fs: Add inode_get_ino() and implement get_ino() for NFS

[Paul Moore]

On Thu, Oct 17, 2024 at 10:30 AM Trond Myklebust
<trondmy at hammerspace.com> wrote:
> On Wed, 2024-10-16 at 19:05 -0400, Paul Moore wrote:
> > On Wed, Oct 16, 2024 at 10:23 AM Christian Brauner
> > <brauner at kernel.org> wrote:
> > >
> > > On Thu, Oct 10, 2024 at 05:26:41PM +0200, Mickaël Salaün wrote:
> > > > When a filesystem manages its own inode numbers, like NFS's
> > > > fileid shown
> > > > to user space with getattr(), other part of the kernel may still
> > > > expose
> > > > the private inode->ino through kernel logs and audit.
> > > >
> > > > Another issue is on 32-bit architectures, on which ino_t is 32
> > > > bits,
> > > > whereas the user space's view of an inode number can still be 64
> > > > bits.
> > > >
> > > > Add a new inode_get_ino() helper calling the new struct
> > > > inode_operations' get_ino() when set, to get the user space's
> > > > view of an
> > > > inode number.  inode_get_ino() is called by generic_fillattr().
> > > >
> > > > Implement get_ino() for NFS.
> > > >
> > > > Cc: Trond Myklebust <trondmy at kernel.org>
> > > > Cc: Anna Schumaker <anna at kernel.org>
> > > > Cc: Alexander Viro <viro at zeniv.linux.org.uk>
> > > > Cc: Christian Brauner <brauner at kernel.org>
> > > > Cc: Jan Kara <jack at suse.cz>
> > > > Signed-off-by: Mickaël Salaün <mic at digikod.net>
> > > > ---
> > > >
> > > > I'm not sure about nfs_namespace_getattr(), please review
> > > > carefully.
> > > >
> > > > I guess there are other filesystems exposing inode numbers
> > > > different
> > > > than inode->i_ino, and they should be patched too.
> > >
> > > What are the other filesystems that are presumably affected by this
> > > that
> > > would need an inode accessor?
> >
> > I don't want to speak for Mickaël, but my reading of the patchset was
> > that he was suspecting that other filesystems had the same issue
> > (privately maintained inode numbers) and was posting this as a RFC
> > partly for clarity on this from the VFS developers such as yourself.
> >
> > > If this is just about NFS then just add a helper function that
> > > audit and
> > > whatever can call if they need to know the real inode number
> > > without
> > > forcing a new get_inode() method onto struct inode_operations.
> >
> > If this really is just limited to NFS, or perhaps NFS and a small
> > number of filesystems, then a a helper function is a reasonable
> > solution.  I think Mickaël was worried that private inode numbers
> > would be more common, in which case a get_ino() method makes a bit
> > more sense.
> >
> > > And I don't buy that is suddenly rush hour for this.
> >
> > I don't think Mickaël ever characterized this as a "rush hour" issue
> > and I know I didn't.  It definitely caught us by surprise to learn
> > that inode->i_no wasn't always maintained, and we want to find a
> > solution, but I'm not hearing anyone screaming for a solution
> > "yesterday".
> >
> > > Seemingly no one noticed this in the past idk how many years.
> >
> > Yet the issue has been noticed and we would like to find a solution,
> > one that is acceptable both to the VFS and LSM folks.
> >
> > Can we start with compiling a list of filesystems that maintain their
> > inode numbers outside of inode->i_no?  NFS is obviously first on that
> > list, are there others that the VFS devs can add?
> >
>
> Pretty much any filesystem that uses 64-bit inode numbers has the same
> problem: if the application calls stat(), 32-bit glibc will happily
> convert that into a call to fstatat64() and then cry foul if the kernel
> dares return an inode number that doesn't fit in 32 bits.

Okay, good to know, but I was hoping that there we could come up with
an explicit list of filesystems that maintain their own private inode
numbers outside of inode-i_ino.

-- 
paul-moore.com