From: Jeff Layton <jlayton@kernel.org>
To: Chuck Lever <cel@kernel.org>, NeilBrown <neil@brown.name>,
Olga Kornievskaia <okorniev@redhat.com>,
Dai Ngo <dai.ngo@oracle.com>, Tom Talpey <tom@talpey.com>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [PATCH v1 2/2] NFSD: Move version-specific ACCESS maps into per-version code
Date: Mon, 10 Aug 2026 13:23:49 -0400 [thread overview]
Message-ID: <00e0823c793e0783244237e9d3b1bddab74788ab.camel@kernel.org> (raw)
In-Reply-To: <20260810141900.33846-2-cel@kernel.org>
On Mon, 2026-08-10 at 10:19 -0400, Chuck Lever wrote:
> nfsd_access() owns three static tables that map on-the-wire ACCESS
> bits to NFSD_MAY flags, and every NFS version shares them. That puts
> protocol-version specifics in the version-agnostic VFS layer. The
> NFSv4.2 extended-attribute bits are wedged into the NFSv3 tables
> under CONFIG_NFSD_V4.
>
> Give each version its own tables in its proc code and pass the
> matching set into nfsd_access() as a new argument.
>
> Splitting the tables also stops the NFSv2-ACL and NFSv3 ACCESS paths
> from answering for the NFSv4.2 extended-attribute bits. Both use the
> NFSv4-augmented table whenever CONFIG_NFSD_V4 is set, so an NFSv3
> request that sets an xattr bit has it echoed back in the reply even
> though those bits are undefined for v3.
>
> Signed-off-by: Chuck Lever <cel@kernel.org>
> ---
> fs/nfsd/nfs2acl.c | 46 ++++++++++++++++++++++++++-
> fs/nfsd/nfs3proc.c | 41 +++++++++++++++++++++++-
> fs/nfsd/nfs4proc.c | 42 +++++++++++++++++++++++--
> fs/nfsd/vfs.c | 78 ++++++++++------------------------------------
> fs/nfsd/vfs.h | 15 ++++++++-
> 5 files changed, 155 insertions(+), 67 deletions(-)
>
> diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c
> index aba69dd278a1..33610deda3b0 100644
> --- a/fs/nfsd/nfs2acl.c
> +++ b/fs/nfsd/nfs2acl.c
> @@ -16,6 +16,48 @@
>
> #define NFSDDBG_FACILITY NFSDDBG_PROC
>
> +/*
> + * These maps are identical to the NFSv3 maps (nfs3proc.c). This enables
> + * the behavior of the two versions to diverge if needed.
> + */
> +static const struct nfsd_access_map nfsd2_regaccess[] = {
> + { NFS3_ACCESS_READ, NFSD_MAY_READ },
> + { NFS3_ACCESS_EXECUTE, NFSD_MAY_EXEC },
> + { NFS3_ACCESS_MODIFY, NFSD_MAY_WRITE|NFSD_MAY_TRUNC },
> + { NFS3_ACCESS_EXTEND, NFSD_MAY_WRITE },
> + { 0, 0 }
> +};
> +
> +static const struct nfsd_access_map nfsd2_diraccess[] = {
> + { NFS3_ACCESS_READ, NFSD_MAY_READ },
> + { NFS3_ACCESS_LOOKUP, NFSD_MAY_EXEC },
> + { NFS3_ACCESS_MODIFY, NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC },
> + { NFS3_ACCESS_EXTEND, NFSD_MAY_EXEC|NFSD_MAY_WRITE },
> + { NFS3_ACCESS_DELETE, NFSD_MAY_REMOVE },
> + { 0, 0 }
> +};
> +
> +/*
> + * Some clients - Solaris 2.6 at least, make an access call to the NFS
> + * server to check for access for things like /dev/null (which really,
> + * NFSD doesn't care about). So NFSD provides simple access checking
> + * for those objects, looking mainly at mode bits, ignoring read-only
> + * filesystem checks.
> + */
> +static const struct nfsd_access_map nfsd2_otheraccess[] = {
> + { NFS3_ACCESS_READ, NFSD_MAY_READ },
> + { NFS3_ACCESS_EXECUTE, NFSD_MAY_EXEC },
> + { NFS3_ACCESS_MODIFY, NFSD_MAY_WRITE|NFSD_MAY_LOCAL_ACCESS },
> + { NFS3_ACCESS_EXTEND, NFSD_MAY_WRITE|NFSD_MAY_LOCAL_ACCESS },
> + { 0, 0 }
> +};
> +
> +static const struct nfsd_access_maps nfsd2_access_maps = {
> + .regular = nfsd2_regaccess,
> + .directory = nfsd2_diraccess,
> + .other = nfsd2_otheraccess,
> +};
> +
> /*
> * NULL call.
> */
> @@ -181,7 +223,9 @@ static __be32 nfsacld_proc_access(struct svc_rqst *rqstp)
>
> fh_copy(&resp->fh, &argp->fh);
> resp->access = argp->access;
> - resp->status = nfsd_access(rqstp, &resp->fh, &resp->access, NULL);
> +
> + resp->status = nfsd_access(rqstp, &resp->fh, &nfsd2_access_maps,
> + &resp->access, NULL);
> if (resp->status != nfs_ok)
> goto out;
> resp->status = fh_getattr(&resp->fh, &resp->stat);
> diff --git a/fs/nfsd/nfs3proc.c b/fs/nfsd/nfs3proc.c
> index 19ab0a713d82..17bbe5d13f18 100644
> --- a/fs/nfsd/nfs3proc.c
> +++ b/fs/nfsd/nfs3proc.c
> @@ -49,6 +49,44 @@ static bool nfsd3_time_in_range(const struct iattr *iap)
> return true;
> }
>
> +static const struct nfsd_access_map nfsd3_regaccess[] = {
> + { NFS3_ACCESS_READ, NFSD_MAY_READ },
> + { NFS3_ACCESS_EXECUTE, NFSD_MAY_EXEC },
> + { NFS3_ACCESS_MODIFY, NFSD_MAY_WRITE|NFSD_MAY_TRUNC },
> + { NFS3_ACCESS_EXTEND, NFSD_MAY_WRITE },
> + { 0, 0 }
> +};
> +
> +static const struct nfsd_access_map nfsd3_diraccess[] = {
> + { NFS3_ACCESS_READ, NFSD_MAY_READ },
> + { NFS3_ACCESS_LOOKUP, NFSD_MAY_EXEC },
> + { NFS3_ACCESS_MODIFY, NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC },
> + { NFS3_ACCESS_EXTEND, NFSD_MAY_EXEC|NFSD_MAY_WRITE },
> + { NFS3_ACCESS_DELETE, NFSD_MAY_REMOVE },
> + { 0, 0 }
> +};
> +
> +/*
> + * Some clients - Solaris 2.6 at least, make an access call to the NFS
> + * server to check for access for things like /dev/null (which really,
> + * NFSD doesn't care about). So NFSD provides simple access checking
> + * for those objects, looking mainly at mode bits, ignoring read-only
> + * filesystem checks.
> + */
> +static const struct nfsd_access_map nfsd3_otheraccess[] = {
> + { NFS3_ACCESS_READ, NFSD_MAY_READ },
> + { NFS3_ACCESS_EXECUTE, NFSD_MAY_EXEC },
> + { NFS3_ACCESS_MODIFY, NFSD_MAY_WRITE|NFSD_MAY_LOCAL_ACCESS },
> + { NFS3_ACCESS_EXTEND, NFSD_MAY_WRITE|NFSD_MAY_LOCAL_ACCESS },
> + { 0, 0 }
> +};
> +
> +static const struct nfsd_access_maps nfsd3_access_maps = {
> + .regular = nfsd3_regaccess,
> + .directory = nfsd3_diraccess,
> + .other = nfsd3_otheraccess,
> +};
> +
> static int nfsd3_iocb_flags(enum nfs3_stable_how how)
> {
> switch (how) {
> @@ -186,7 +224,8 @@ nfsd3_proc_access(struct svc_rqst *rqstp)
>
> fh_copy(&resp->fh, &argp->fh);
> resp->access = argp->access;
> - resp->status = nfsd_access(rqstp, &resp->fh, &resp->access, NULL);
> + resp->status = nfsd_access(rqstp, &resp->fh, &nfsd3_access_maps,
> + &resp->access, NULL);
> resp->status = nfsd3_map_status(resp->status);
> return rpc_success;
> }
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index a2bea8947ef5..54593f4667f8 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -72,6 +72,43 @@ MODULE_PARM_DESC(nfsd4_ssc_umount_timeout,
>
> #define NFSDDBG_FACILITY NFSDDBG_PROC
>
> +static const struct nfsd_access_map nfsd4_regaccess[] = {
> + { NFS4_ACCESS_READ, NFSD_MAY_READ },
> + { NFS4_ACCESS_EXECUTE, NFSD_MAY_EXEC },
> + { NFS4_ACCESS_MODIFY, NFSD_MAY_WRITE|NFSD_MAY_TRUNC },
> + { NFS4_ACCESS_EXTEND, NFSD_MAY_WRITE },
> + { NFS4_ACCESS_XAREAD, NFSD_MAY_READ },
> + { NFS4_ACCESS_XAWRITE, NFSD_MAY_WRITE },
> + { NFS4_ACCESS_XALIST, NFSD_MAY_READ },
> + { 0, 0 }
> +};
> +
> +static const struct nfsd_access_map nfsd4_diraccess[] = {
> + { NFS4_ACCESS_READ, NFSD_MAY_READ },
> + { NFS4_ACCESS_LOOKUP, NFSD_MAY_EXEC },
> + { NFS4_ACCESS_MODIFY, NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC },
> + { NFS4_ACCESS_EXTEND, NFSD_MAY_EXEC|NFSD_MAY_WRITE },
> + { NFS4_ACCESS_DELETE, NFSD_MAY_REMOVE },
> + { NFS4_ACCESS_XAREAD, NFSD_MAY_READ },
> + { NFS4_ACCESS_XAWRITE, NFSD_MAY_WRITE },
> + { NFS4_ACCESS_XALIST, NFSD_MAY_READ },
> + { 0, 0 }
> +};
> +
> +static const struct nfsd_access_map nfsd4_otheraccess[] = {
> + { NFS4_ACCESS_READ, NFSD_MAY_READ },
> + { NFS4_ACCESS_EXECUTE, NFSD_MAY_EXEC },
> + { NFS4_ACCESS_MODIFY, NFSD_MAY_WRITE|NFSD_MAY_LOCAL_ACCESS },
> + { NFS4_ACCESS_EXTEND, NFSD_MAY_WRITE|NFSD_MAY_LOCAL_ACCESS },
> + { 0, 0 }
> +};
> +
> +static const struct nfsd_access_maps nfsd4_access_maps = {
> + .regular = nfsd4_regaccess,
> + .directory = nfsd4_diraccess,
> + .other = nfsd4_otheraccess,
> +};
> +
> static int nfsd4_iocb_flags(enum stable_how4 how)
> {
> switch (how) {
> @@ -851,10 +888,9 @@ nfsd4_access(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
>
> if (access->ac_req_access & ~access_full)
> return nfserr_inval;
> -
> access->ac_resp_access = access->ac_req_access;
> - return nfsd_access(rqstp, &cstate->current_fh, &access->ac_resp_access,
> - &access->ac_supported);
> + return nfsd_access(rqstp, &cstate->current_fh, &nfsd4_access_maps,
> + &access->ac_resp_access, &access->ac_supported);
> }
>
> static __be32
> diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> index 807e09521e0c..6d865f4f9ba3 100644
> --- a/fs/nfsd/vfs.c
> +++ b/fs/nfsd/vfs.c
> @@ -782,64 +782,21 @@ __be32 nfsd4_vfs_fallocate(struct svc_rqst *rqstp, struct svc_fh *fhp,
> }
> #endif /* defined(CONFIG_NFSD_V4) */
>
> -/*
> - * Check server access rights to a file system object
> +/**
> + * nfsd_access - Check caller's access rights to a file system object
> + * @rqstp: RPC transaction context
> + * @fhp: target NFS filehandle
> + * @maps: tables mapping on-the-wire access bits to NFSD_MAY flags
> + * @access: requested access bits on entry, permitted bits on return
> + * @supported: optional output of the access bits the server supports
> + *
> + * Return: nfs_ok on success, otherwise an nfserr status code
> */
> -struct accessmap {
> - u32 access;
> - int how;
> -};
> -static struct accessmap nfs3_regaccess[] = {
> - { NFS3_ACCESS_READ, NFSD_MAY_READ },
> - { NFS3_ACCESS_EXECUTE, NFSD_MAY_EXEC },
> - { NFS3_ACCESS_MODIFY, NFSD_MAY_WRITE|NFSD_MAY_TRUNC },
> - { NFS3_ACCESS_EXTEND, NFSD_MAY_WRITE },
> -
> -#ifdef CONFIG_NFSD_V4
> - { NFS4_ACCESS_XAREAD, NFSD_MAY_READ },
> - { NFS4_ACCESS_XAWRITE, NFSD_MAY_WRITE },
> - { NFS4_ACCESS_XALIST, NFSD_MAY_READ },
> -#endif
> -
> - { 0, 0 }
> -};
> -
> -static struct accessmap nfs3_diraccess[] = {
> - { NFS3_ACCESS_READ, NFSD_MAY_READ },
> - { NFS3_ACCESS_LOOKUP, NFSD_MAY_EXEC },
> - { NFS3_ACCESS_MODIFY, NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC},
> - { NFS3_ACCESS_EXTEND, NFSD_MAY_EXEC|NFSD_MAY_WRITE },
> - { NFS3_ACCESS_DELETE, NFSD_MAY_REMOVE },
> -
> -#ifdef CONFIG_NFSD_V4
> - { NFS4_ACCESS_XAREAD, NFSD_MAY_READ },
> - { NFS4_ACCESS_XAWRITE, NFSD_MAY_WRITE },
> - { NFS4_ACCESS_XALIST, NFSD_MAY_READ },
> -#endif
> -
> - { 0, 0 }
> -};
> -
> -static struct accessmap nfs3_anyaccess[] = {
> - /* Some clients - Solaris 2.6 at least, make an access call
> - * to the server to check for access for things like /dev/null
> - * (which really, the server doesn't care about). So
> - * We provide simple access checking for them, looking
> - * mainly at mode bits, and we make sure to ignore read-only
> - * filesystem checks
> - */
> - { NFS3_ACCESS_READ, NFSD_MAY_READ },
> - { NFS3_ACCESS_EXECUTE, NFSD_MAY_EXEC },
> - { NFS3_ACCESS_MODIFY, NFSD_MAY_WRITE|NFSD_MAY_LOCAL_ACCESS },
> - { NFS3_ACCESS_EXTEND, NFSD_MAY_WRITE|NFSD_MAY_LOCAL_ACCESS },
> -
> - { 0, 0 }
> -};
> -
> -__be32
> -nfsd_access(struct svc_rqst *rqstp, struct svc_fh *fhp, u32 *access, u32 *supported)
> +__be32 nfsd_access(struct svc_rqst *rqstp, struct svc_fh *fhp,
> + const struct nfsd_access_maps *maps,
> + u32 *access, u32 *supported)
> {
> - struct accessmap *map;
> + const struct nfsd_access_map *map;
> struct svc_export *export;
> struct dentry *dentry;
> u32 query, result = 0, sresult = 0;
> @@ -853,12 +810,11 @@ nfsd_access(struct svc_rqst *rqstp, struct svc_fh *fhp, u32 *access, u32 *suppor
> dentry = fhp->fh_dentry;
>
> if (d_is_reg(dentry))
> - map = nfs3_regaccess;
> + map = maps->regular;
> else if (d_is_dir(dentry))
> - map = nfs3_diraccess;
> + map = maps->directory;
> else
> - map = nfs3_anyaccess;
> -
> + map = maps->other;
>
> query = *access;
> for (; map->access; map++) {
> @@ -868,7 +824,7 @@ nfsd_access(struct svc_rqst *rqstp, struct svc_fh *fhp, u32 *access, u32 *suppor
> sresult |= map->access;
>
> err2 = nfsd_permission(&rqstp->rq_cred, export,
> - dentry, map->how);
> + dentry, map->may);
> switch (err2) {
> case nfs_ok:
> result |= map->access;
> diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
> index aa7679d4c54a..3aa4522ca0a4 100644
> --- a/fs/nfsd/vfs.h
> +++ b/fs/nfsd/vfs.h
> @@ -37,6 +37,17 @@
> #define NFSD_MAY_CREATE (NFSD_MAY_EXEC|NFSD_MAY_WRITE)
> #define NFSD_MAY_REMOVE (NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC)
>
> +struct nfsd_access_map {
> + u32 access;
> + int may;
> +};
> +
> +struct nfsd_access_maps {
> + const struct nfsd_access_map *regular;
> + const struct nfsd_access_map *directory;
> + const struct nfsd_access_map *other;
> +};
> +
> struct nfsd_file;
>
> /*
> @@ -100,7 +111,9 @@ __be32 nfsd_create_locked(struct svc_rqst *, struct svc_fh *,
> __be32 nfsd_create(struct svc_rqst *, struct svc_fh *,
> char *name, int len, struct nfsd_attrs *attrs,
> int type, dev_t rdev, struct svc_fh *res);
> -__be32 nfsd_access(struct svc_rqst *, struct svc_fh *, u32 *, u32 *);
> +__be32 nfsd_access(struct svc_rqst *rqstp, struct svc_fh *fhp,
> + const struct nfsd_access_maps *maps,
> + u32 *access, u32 *supported);
> __be32 nfsd_create_setattr(struct svc_rqst *rqstp, struct svc_fh *fhp,
> struct svc_fh *resfhp, struct nfsd_attrs *iap);
> __be32 nfsd_commit(struct svc_rqst *rqst, struct svc_fh *fhp,
Reviewed-by: Jeff Layton <jlayton@kernel.org>
next prev parent reply other threads:[~2026-08-10 17:23 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-08-10 14:18 [PATCH v1 1/2] NFSD: Replace NFS3_ACCESS_FULL in nfsd4_access() Chuck Lever
2026-08-10 14:19 ` [PATCH v1 2/2] NFSD: Move version-specific ACCESS maps into per-version code Chuck Lever
2026-08-10 17:23 ` Jeff Layton [this message]
2026-08-10 17:23 ` [PATCH v1 1/2] NFSD: Replace NFS3_ACCESS_FULL in nfsd4_access() Jeff Layton
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=00e0823c793e0783244237e9d3b1bddab74788ab.camel@kernel.org \
--to=jlayton@kernel.org \
--cc=cel@kernel.org \
--cc=dai.ngo@oracle.com \
--cc=linux-nfs@vger.kernel.org \
--cc=neil@brown.name \
--cc=okorniev@redhat.com \
--cc=tom@talpey.com \
/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