Linux NFS development
 help / color / mirror / Atom feed
From: NeilBrown <neil@brown.name>
To: Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	David Howells <dhowells@redhat.com>,
	Chuck Lever <chuck.lever@oracle.com>,
	Jeff Layton <jlayton@kernel.org>
Cc: linux-nfs@vger.kernel.org, netfs@lists.linux.dev,
	linux-fsdevel@vger.kernel.org
Subject: [PATCH 0/8 RFC] tidy up various VFS lookup functions
Date: Fri, 14 Mar 2025 11:34:06 +1100	[thread overview]
Message-ID: <20250314045655.603377-1-neil@brown.name> (raw)

VFS has some functions with names containing "lookup_one_len" and others
without the "_len".  This difference has nothing to do with "len".

The functions without "_len" take a "mnt_idmap" pointer.  This is found
in the "vfsmount" and that is an important question when choosing which
to use: do you have a vfsmount, or are you "inside" the filesystem.  A
related question is "is permission checking relevant here?".

nfsd and cachefiles *do* have a vfsmount but *don't* use the non-_len
functions.  They pass nop_mnt_idmap which is not correct if the vfsmount
is actually idmaped.  For cachefiles it probably (?) doesn't matter as
the accesses to the backing filesystem are always does with elevated privileged (?).

For nfsd it would matter if anyone exported an idmapped filesystem.  I
wonder if anyone has tried...

These patches change the "lookup_one" functions to take a vfsmount
instead of a mnt_idmap because I think that makes the intention clearer.

It also renames the "_one" functions to be "_noperm" and removes the
permission checking completely.  In all cases where they are (correctly)
used permission checking is irrelevant.

I haven't included changes to afs because there are patches in vfs.all
which make a lot of changes to lookup in afs.  I think (if they are seen
as a good idea) these patches should aim to land after the afs patches
and any further fixup in afs can happen then.

The nfsd and cachefiles patches probably should be separate.  Maybe I
should submit those to relevant maintainers first, and one afs,
cachefiles, and nfsd changes have landed I can submit this series with
appropriate modifications.

May main question for review is : have I understood mnt_idmap correctly?

These patches are based on vfs-6.15.async.dir as they touch mkdir
related code.  There is a small conflict with the recently posted patch
to remove locking from try_lookup_one_len() calls.

Thanks,
NeilBrown


 [PATCH 1/8] VFS: improve interface for lookup_one functions
 [PATCH 2/8] nfsd: Use lookup_one() rather than lookup_one_len()
 [PATCH 3/8] nfsd: use correct idmap for all accesses.
 [PATCH 4/8] cachefiles: Use lookup_one() rather than lookup_one_len()
 [PATCH 5/8] cachefiles: use correct mnt_idmap
 [PATCH 6/8] VFS: rename lookup_one_len() family to lookup_noperm()
 [PATCH 7/8] Use try_lookup_noperm() instead of d_hash_and_lookup()
 [PATCH 8/8] VFS: change lookup_one_common and lookup_noperm_common to

             reply	other threads:[~2025-03-14  4:57 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-14  0:34 NeilBrown [this message]
2025-03-14  0:34 ` [PATCH 1/8] VFS: improve interface for lookup_one functions NeilBrown
2025-03-14  0:34 ` [PATCH 2/8] nfsd: Use lookup_one() rather than lookup_one_len() NeilBrown
2025-03-14  0:34 ` [PATCH 3/8] nfsd: use correct idmap for all accesses NeilBrown
2025-03-14  0:34 ` [PATCH 4/8] cachefiles: Use lookup_one() rather than lookup_one_len() NeilBrown
2025-03-14  0:34 ` [PATCH 5/8] cachefiles: use correct mnt_idmap NeilBrown
2025-03-14  0:34 ` [PATCH 6/8] VFS: rename lookup_one_len() family to lookup_noperm() and remove permission check NeilBrown
2025-03-14  0:34 ` [PATCH 7/8] Use try_lookup_noperm() instead of d_hash_and_lookup() outside of VFS NeilBrown
2025-03-14  0:34 ` [PATCH 8/8] VFS: change lookup_one_common and lookup_noperm_common to take a qstr NeilBrown
2025-03-14 10:38 ` [PATCH 0/8 RFC] tidy up various VFS lookup functions Christian Brauner
2025-03-17  2:06   ` NeilBrown
2025-03-18 13:57     ` Christian Brauner

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250314045655.603377-1-neil@brown.name \
    --to=neil@brown.name \
    --cc=brauner@kernel.org \
    --cc=chuck.lever@oracle.com \
    --cc=dhowells@redhat.com \
    --cc=jack@suse.cz \
    --cc=jlayton@kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=netfs@lists.linux.dev \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox