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

Paul Moore
2024-10-16T23:05:25Z

On Wed, Oct 16, 2024 at 10:23 AM Christian Brauner <brauner@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@kernel.org>
> > Cc: Anna Schumaker <anna@kernel.org>
> > Cc: Alexander Viro <viro@zeniv.linux.org.uk>
> > Cc: Christian Brauner <brauner@kernel.org>
> > Cc: Jan Kara <jack@suse.cz>
> > Signed-off-by: Mickaël Salaün <mic@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?

--
paul-moore.com