Thread (78 messages) 78 messages, 14 authors, 2024-10-21

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

From: Trond Myklebust <hidden>
Date: 2024-10-17 14:30:44
Also in: linux-fsdevel, linux-nfs

On Wed, 2024-10-16 at 19:05 -0400, Paul Moore wrote:
On Wed, Oct 16, 2024 at 10:23 AM Christian Brauner
[off-list ref] wrote:
quoted
On Thu, Oct 10, 2024 at 05:26:41PM +0200, Mickaël Salaün wrote:
quoted
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.
quoted
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.
quoted
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".
quoted
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.

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help