* Re: [PATCH] nfsd: prevent SAVEFH from bypassing export security
[not found] ` <IA1PR12MB662781C8C09627A5CDEBF39DF4602@IA1PR12MB6627.namprd12.prod.outlook.com>
@ 2026-10-27 15:41 ` Chuck Lever
[not found] ` <IA1PR12MB662788BBD70D4064432FD117F4602@IA1PR12MB6627.namprd12.prod.outlook.com>
2026-10-28 13:57 ` Jeff Layton
1 sibling, 1 reply; 8+ messages in thread
From: Chuck Lever @ 2026-10-27 15:41 UTC (permalink / raw)
To: Adolph Zeng, Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo,
Tom Talpey
Cc: linux-nfs@vger.kernel.org
On Tue, Oct 27, 2026, at 8:44 AM, Adolph Zeng wrote:
> An AUTH_SYS NFSv4 client can read or write a file on a sec=krb5p
> export using another client's valid OPEN stateid by inserting
> SAVEFH between PUTFH and READ or WRITE.
>
> PUTFH resolves the handle with NFSD_MAY_BYPASS_GSS, expecting
> a later security check. SAVEFH carries OP_HANDLES_WRONGSEC even though
> it only saves the handle, so PUTFH skips NFS4ERR_WRONGSEC. Stateid I/O
> can then reuse a cached nfsd_file.
>
> The original report exported /srv/nfs-sec to * with these options:
>
> rw,sync,no_subtree_check,fsid=0,sec=krb5p,secure,root_squash
>
> The 0600, nobody:nogroup file /srv/nfs-sec/secret.txt held 32 random
> bytes. A Kerberos privacy NFSv4.0 client ran SETCLIENTID, OPEN and
> OPEN_CONFIRM, then kept its read/write stateid live. A separate
> AUTH_SYS client used UID/GID 65534 and a reserved source port.
>
> The AUTH_SYS client enumerated a bounded range of nearby clientid
> counters, stateid numbers and seqids to find the live OPEN stateid.
> With it, PUTFH, READ returned NFS4ERR_WRONGSEC, while PUTFH, SAVEFH,
> READ returned the file data and PUTFH, SAVEFH, WRITE changed it. The
> Kerberos client subsequently read the changed contents.
>
> SAVEFH leaves CURRENT_FH in place. When deciding whether PUTFH must
> report NFS4ERR_WRONGSEC, look past SAVEFH to the first operation that
> uses or replaces CURRENT_FH. If another PUTFH replaces it, leave the
> earlier PUTFH unchecked, as required by RFC 8881 Section 2.6.3.1.1.
>
> Local COPY and CLONE consume SAVED_FH as the source. Check the
> saved export with check_nfsd_access() before using its stateid:
> a cached open stateid can otherwise bypass the sec= policy. Map
> NFS4ERR_WRONGSEC to NFS4ERR_ACCESS at COPY or CLONE, since neither
> operation permits NFS4ERR_WRONGSEC. The check also applies when
> SECINFO_NO_NAME or RESTOREFH intervenes. LINK and RENAME already
> check their saved handle in their own paths.
>
> An inter-server COPY carries a foreign SAVED_FH. The source server
> validates it; the destination must not apply its own export policy
> to that handle. The destination PUTFH still checks its local export.
>
> KASAN guests reproduced the NFSv4.0 and local NFSv4.2 bypasses on
> d135ab453a74. With this patch, direct READ and SAVEFH READ/WRITE
> returned NFS4ERR_WRONGSEC. Local COPY/CLONE with a protected saved
> source returned NFS4ERR_ACCESS; foreign-handle inter-server COPY
> completed.
>
> Fixes: 68d93184352f ("nfsd4: fix wrongsec handling for PUTFH + op cases")
> Assisted-by: LLM
> Signed-off-by: Zeng Adolph <coderzeng@hotmail.com>
> ---
> Testing: two x86_64 guests covered the original NFSv4.0 report,
> six SAVEFH security boundaries, NFSv4.2 local COPY/CLONE, and a
> foreign-handle inter-server COPY. W=1 nfs4proc.o built with
> CONFIG_NFSD_V4_2_INTER_SSC enabled and disabled. The ASan/UBSan
> decision harness passed 262 cases. The KASAN guest reported no
> KASAN, Oops, or NFSD warnings.
>
> fs/nfsd/nfs4proc.c | 36 +++++++++++++++++++-----------------
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index a50f9dcb3..25fb7fb61 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1491,6 +1491,11 @@ nfsd4_verify_copy(struct svc_rqst *rqstp, struct
> nfsd4_compound_state *cstate,
> if (!cstate->save_fh.fh_dentry)
> return nfserr_nofilehandle;
>
> + /* A cached open stateid does not authorize the saved source export. */
> + status = check_nfsd_access(cstate->save_fh.fh_export, rqstp);
> + if (status)
> + return nfserr_acces;
> +
> status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->save_fh,
> src_stateid, RD_STATE, src, NULL);
> if (status)
> @@ -3237,32 +3242,29 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
> struct nfsd4_compoundres *resp = rqstp->rq_resp;
> struct nfsd4_compoundargs *argp = rqstp->rq_argp;
> struct nfsd4_op *this = &argp->ops[resp->opcnt - 1];
> - struct nfsd4_op *next = &argp->ops[resp->opcnt];
> + struct nfsd4_op *next;
> const struct nfsd4_operation *thisd = OPDESC(this);
> const struct nfsd4_operation *nextd;
> + unsigned int i;
>
> - /*
> - * Most ops check wronsec on our own; only the putfh-like ops
> - * have special rules.
> - */
> if (!(thisd->op_flags & OP_IS_PUTFH_LIKE))
> return false;
> - /*
> - * rfc 5661 2.6.3.1.1.6: don't bother erroring out a
> - * put-filehandle operation if we're not going to use the
> - * result:
> - */
> - if (argp->opcnt == resp->opcnt)
> +
> + /* SAVEFH does not consume the current filehandle. */
> + for (i = resp->opcnt; i < argp->opcnt; i++) {
> + next = &argp->ops[i];
> + if (next->opnum != OP_SAVEFH)
> + break;
> + }
> + if (i == argp->opcnt)
> return false;
> +
> + next = &argp->ops[i];
> if (next->opnum == OP_ILLEGAL)
> return false;
> nextd = OPDESC(next);
> - /*
> - * Rest of 2.6.3.1.1: certain operations will return WRONGSEC
> - * errors themselves as necessary; others should check for them
> - * now:
> - */
> - return !(nextd->op_flags & OP_HANDLES_WRONGSEC);
> + return !(nextd->op_flags &
> + (OP_HANDLES_WRONGSEC | OP_IS_PUTFH_LIKE));
> }
>
> #ifdef CONFIG_NFSD_V4_2_INTER_SSC
> --
> 2.43.0
My concern with this fix is that it addresses the issue for READ
and WRITE, but not for COPY/CLONE, which can involve a foreign
filehandle which fails the server's local security policy checks.
I'm interested to hear especially from Neil, who was working in
this area a while ago.
--
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] nfsd: prevent SAVEFH from bypassing export security
[not found] ` <IA1PR12MB662781C8C09627A5CDEBF39DF4602@IA1PR12MB6627.namprd12.prod.outlook.com>
2026-10-27 15:41 ` [PATCH] nfsd: prevent SAVEFH from bypassing export security Chuck Lever
@ 2026-10-28 13:57 ` Jeff Layton
1 sibling, 0 replies; 8+ messages in thread
From: Jeff Layton @ 2026-10-28 13:57 UTC (permalink / raw)
To: Adolph Zeng, cel@kernel.org, neil@brown.name, okorniev@redhat.com,
Dai.Ngo@oracle.com, tom@talpey.com
Cc: linux-nfs@vger.kernel.org
On Tue, 2026-10-27 at 12:44 +0000, Adolph Zeng wrote:
>
> Hi,
> I was informed that Hotmail may reject incoming messages from kernel.org, so I am switching to this Gmail address to avoid missing any replies.
> My original patch was sent from coderzeng@hotmail.com. Please use coderzeng@gmail.com for any further correspondence.
> Apologies for the inconvenience.
> Best regards,
> Adolph Zeng
> From: Zeng Adolph <coderzeng@hotmail.com>
> Sent: Tuesday, October 27, 2026 8:40 PM
> To: cel@kernel.org <cel@kernel.org>; jlayton@kernel.org <jlayton@kernel.org>; neil@brown.name <neil@brown.name>; okorniev@redhat.com <okorniev@redhat.com>; Dai.Ngo@oracle.com <Dai.Ngo@oracle.com>; tom@talpey.com <tom@talpey.com>
> Cc: linux-nfs@vger.kernel.org <linux-nfs@vger.kernel.org>
> Subject: [PATCH] nfsd: prevent SAVEFH from bypassing export security
>
>
>
>
> An AUTH_SYS NFSv4 client can read or write a file on a sec=krb5p
> export using another client's valid OPEN stateid by inserting
> SAVEFH between PUTFH and READ or WRITE.
>
>
> PUTFH resolves the handle with NFSD_MAY_BYPASS_GSS, expecting
> a later security check. SAVEFH carries OP_HANDLES_WRONGSEC even though
> it only saves the handle, so PUTFH skips NFS4ERR_WRONGSEC. Stateid I/O
> can then reuse a cached nfsd_file.
>
>
> The original report exported /srv/nfs-sec to * with these options:
>
>
> rw,sync,no_subtree_check,fsid=0,sec=krb5p,secure,root_squash
>
>
> The 0600, nobody:nogroup file /srv/nfs-sec/secret.txt held 32 random
> bytes. A Kerberos privacy NFSv4.0 client ran SETCLIENTID, OPEN and
> OPEN_CONFIRM, then kept its read/write stateid live. A separate
> AUTH_SYS client used UID/GID 65534 and a reserved source port.
>
>
> The AUTH_SYS client enumerated a bounded range of nearby clientid
> counters, stateid numbers and seqids to find the live OPEN stateid.
> With it, PUTFH, READ returned NFS4ERR_WRONGSEC, while PUTFH, SAVEFH,
> READ returned the file data and PUTFH, SAVEFH, WRITE changed it. The
> Kerberos client subsequently read the changed contents.
>
>
> SAVEFH leaves CURRENT_FH in place. When deciding whether PUTFH must
> report NFS4ERR_WRONGSEC, look past SAVEFH to the first operation that
> uses or replaces CURRENT_FH. If another PUTFH replaces it, leave the
> earlier PUTFH unchecked, as required by RFC 8881 Section 2.6.3.1.1.
>
>
> Local COPY and CLONE consume SAVED_FH as the source. Check the
> saved export with check_nfsd_access() before using its stateid:
> a cached open stateid can otherwise bypass the sec= policy. Map
> NFS4ERR_WRONGSEC to NFS4ERR_ACCESS at COPY or CLONE, since neither
> operation permits NFS4ERR_WRONGSEC. The check also applies when
> SECINFO_NO_NAME or RESTOREFH intervenes. LINK and RENAME already
> check their saved handle in their own paths.
>
>
> An inter-server COPY carries a foreign SAVED_FH. The source server
> validates it; the destination must not apply its own export policy
> to that handle. The destination PUTFH still checks its local export.
>
>
> KASAN guests reproduced the NFSv4.0 and local NFSv4.2 bypasses on
> d135ab453a74. With this patch, direct READ and SAVEFH READ/WRITE
> returned NFS4ERR_WRONGSEC. Local COPY/CLONE with a protected saved
> source returned NFS4ERR_ACCESS; foreign-handle inter-server COPY
> completed.
>
> Fixes: 68d93184352f ("nfsd4: fix wrongsec handling for PUTFH + op cases")
> Assisted-by: LLM
> Signed-off-by: Zeng Adolph <coderzeng@hotmail.com>
> ---
> Testing: two x86_64 guests covered the original NFSv4.0 report,
> six SAVEFH security boundaries, NFSv4.2 local COPY/CLONE, and a
> foreign-handle inter-server COPY. W=1 nfs4proc.o built with
> CONFIG_NFSD_V4_2_INTER_SSC enabled and disabled. The ASan/UBSan
> decision harness passed 262 cases. The KASAN guest reported no
> KASAN, Oops, or NFSD warnings.
>
This needs to be sent as a proper inline patch. We can't apply this as
it's whitespace-damaged.
Also, some suggestions from Claude below that look valid:
>
> fs/nfsd/nfs4proc.c | 36 +++++++++++++++++++-----------------
> 1 file changed, 19 insertions(+), 17 deletions(-)
>
>
> diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
> index a50f9dcb3..25fb7fb61 100644
> --- a/fs/nfsd/nfs4proc.c
> +++ b/fs/nfsd/nfs4proc.c
> @@ -1491,6 +1491,11 @@ nfsd4_verify_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
> if (!cstate->save_fh.fh_dentry)
> return nfserr_nofilehandle;
>
> + /* A cached open stateid does not authorize the saved source export. */
> + status = check_nfsd_access(cstate->save_fh.fh_export, rqstp);
> + if (status)
> + return nfserr_acces;
> +
This is a point fix for the one remaining saved-fh consumer. The common
factor in every variant of this bug is that nfs4_check_file() only
calls nfsd_permission() when it finds a cached nfsd_file. It never
checks the security flavor:
fs/nfsd/nfs4state.c:nfs4_check_file() {
nf = nfs4_find_file(s, flags);
if (nf) {
status = nfsd_permission(&rqstp->rq_cred,
fhp->fh_export, fhp->fh_dentry,
acc | NFSD_MAY_OWNER_OVERRIDE);
Only the nfsd_file_acquire() path goes through fh_verify(). Would it be
worth also adding a check_nfsd_access() there (mapped to nfserr_acces),
as a backstop in case there are other op orderings that skip the PUTFH
check?
> status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->save_fh,
> src_stateid, RD_STATE, src, NULL);
> if (status)
> @@ -3237,32 +3242,29 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
> struct nfsd4_compoundres *resp = rqstp->rq_resp;
> struct nfsd4_compoundargs *argp = rqstp->rq_argp;
> struct nfsd4_op *this = &argp->ops[resp->opcnt - 1];
> - struct nfsd4_op *next = &argp->ops[resp->opcnt];
> + struct nfsd4_op *next;
> const struct nfsd4_operation *thisd = OPDESC(this);
> const struct nfsd4_operation *nextd;
> + unsigned int i;
>
> - /*
> - * Most ops check wronsec on our own; only the putfh-like ops
> - * have special rules.
> - */
> if (!(thisd->op_flags & OP_IS_PUTFH_LIKE))
> return false;
> - /*
> - * rfc 5661 2.6.3.1.1.6: don't bother erroring out a
> - * put-filehandle operation if we're not going to use the
> - * result:
> - */
> - if (argp->opcnt == resp->opcnt)
> +
> + /* SAVEFH does not consume the current filehandle. */
> + for (i = resp->opcnt; i < argp->opcnt; i++) {
> + next = &argp->ops[i];
> + if (next->opnum != OP_SAVEFH)
> + break;
> + }
> + if (i == argp->opcnt)
> return false;
> +
> + next = &argp->ops[i];
This isn't a bug, but next already points to argp->ops[i] when the loop
breaks early, so isn't this assignment redundant?
> if (next->opnum == OP_ILLEGAL)
> return false;
> nextd = OPDESC(next);
> - /*
> - * Rest of 2.6.3.1.1: certain operations will return WRONGSEC
> - * errors themselves as necessary; others should check for them
> - * now:
> - */
> - return !(nextd->op_flags & OP_HANDLES_WRONGSEC);
> + return !(nextd->op_flags &
> + (OP_HANDLES_WRONGSEC | OP_IS_PUTFH_LIKE));
This isn't a bug, but the existing comments explaining which RFC rule
each test implements were dropped. The function now encodes three
separate rules (2.6.3.1.1.1 for SAVEFH, 2.6.3.1.1.2 for consecutive
put-filehandle ops, 2.6.3.1.1.6 for end of compound). Could the
comments be kept and updated to cite the RFC 8881 sections instead?
> }
>
> #ifdef CONFIG_NFSD_V4_2_INTER_SSC
> --
> 2.43.0
>
--
Jeff Layton <jlayton@kernel.org>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] nfsd: prevent SAVEFH from bypassing export security
[not found] ` <IA1PR12MB662788BBD70D4064432FD117F4602@IA1PR12MB6627.namprd12.prod.outlook.com>
@ 2026-10-28 14:09 ` Chuck Lever
0 siblings, 0 replies; 8+ messages in thread
From: Chuck Lever @ 2026-10-28 14:09 UTC (permalink / raw)
To: Adolph Zeng, Jeff Layton, NeilBrown, Olga Kornievskaia, Dai Ngo,
Tom Talpey
Cc: linux-nfs@vger.kernel.org
On Tue, Oct 27, 2026, at 6:19 PM, Adolph Zeng wrote:
> Hi Chuck,
>
> Thanks for the clarification.
>
> Before preparing v2, I'd like to make sure I understand what changes
> are needed. As far as I can tell, v1 already addresses the COPY/CLONE
> concern you raised.
I think that assessment is correct.
Your v2 will be a response to any subsequent concerns that arise from
v1 review.
--
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v2] nfsd: enforce export security for NFSv4 filehandles and stateids
[not found] <SY9P300MB153090355AAD017CD7216634A6602@SY9P300MB1530.AUSP300.PROD.OUTLOOK.COM>
[not found] ` <IA1PR12MB662781C8C09627A5CDEBF39DF4602@IA1PR12MB6627.namprd12.prod.outlook.com>
@ 2026-10-29 1:36 ` VeroFess
2026-10-29 5:14 ` [PATCH v3] " VeroFess
1 sibling, 1 reply; 8+ messages in thread
From: VeroFess @ 2026-10-29 1:36 UTC (permalink / raw)
To: cel, jlayton, neil, okorniev, Dai.Ngo, tom; +Cc: linux-nfs
An AUTH_SYS NFSv4.0 client can read or write a file on a sec=krb5p
export using another client's valid OPEN stateid by inserting
SAVEFH between PUTFH and READ or WRITE.
PUTFH resolves the handle with NFSD_MAY_BYPASS_GSS, expecting
a later security check. SAVEFH carries OP_HANDLES_WRONGSEC even though
it only saves the handle, so PUTFH skips NFS4ERR_WRONGSEC. Stateid I/O
can then reuse a cached nfsd_file.
The original report exported /srv/nfs-sec to * with these options:
rw,sync,no_subtree_check,fsid=0,sec=krb5p,secure,root_squash
The 0600, nobody:nogroup file /srv/nfs-sec/secret.txt held 32 random
bytes. A Kerberos privacy NFSv4.0 client ran SETCLIENTID, OPEN and
OPEN_CONFIRM, then kept its read/write stateid live. A separate
AUTH_SYS client used UID/GID 65534 and a reserved source port.
The AUTH_SYS client enumerated a bounded range of nearby clientid counters,
stateid numbers and seqids to find the live OPEN stateid. With it,
PUTFH, READ returned NFS4ERR_WRONGSEC, while PUTFH, SAVEFH, READ
returned the file data and PUTFH, SAVEFH, WRITE changed it. The
Kerberos client subsequently read the changed contents.
SAVEFH leaves CURRENT_FH in place. When deciding whether PUTFH must
report NFS4ERR_WRONGSEC, look past SAVEFH to the first operation that
uses or replaces CURRENT_FH. If another PUTFH replaces it, leave the
earlier PUTFH unchecked, as required by RFC 8881 Section 2.6.3.1.1.
Local COPY and CLONE consume SAVED_FH as the source. Check the
saved export with check_nfsd_access() before using its stateid:
a cached open stateid can otherwise bypass the sec= policy. Map
NFS4ERR_WRONGSEC to NFS4ERR_ACCESS at COPY or CLONE, since neither
operation permits NFS4ERR_WRONGSEC.
An inter-server COPY carries a foreign SAVED_FH. The source server
validates it; the destination must not apply its own export policy
to that handle. The destination PUTFH still checks its local export.
A bind-mounted regular file can have a stricter export than its parent.
LOOKUP crosses into that export, but nfsd_cross_mnt() checks the
parent export. Check the selected export in nfsd_lookup() before
composing a filehandle, and in READDIR before encoding child attributes.
When rdattr_error is not requested, return NFS4ERR_ACCESS for a flavor
mismatch; NFS4ERR_WRONGSEC is not a READDIR status. Keep the child-export
check out of nfsd_cross_mnt(): SECINFO uses it to discover the child's
supported security flavors.
The LOOKUP check controls whether a client obtains a new child handle.
It does not cover a handle and OPEN stateid retained while the export's
sec= policy changes. The cached nfsd_file branch of nfs4_check_file()
calls nfsd_permission() without checking the current export's flavor.
Check that export before reusing the file for stateid I/O.
COPY_NOTIFY validates a stateid without acquiring an nfsd_file, so it
misses that check. After an export was changed to sec=krb5p, an
AUTH_SYS COPY_NOTIFY using an existing stateid still issued a copy
stateid although AUTH_SYS READ was denied. In the test, a separate
user-space GSS client used that stateid to read the file. Check the
current export before issuing it and return NFS4ERR_ACCESS on a
flavor mismatch.
Fixes: 68d93184352f ("nfsd4: fix wrongsec handling for PUTFH + op cases")
Fixes: 624322f1adc5 ("NFSD add COPY_NOTIFY operation")
Fixes: f0d6ad5a2f67 ("nfsd: move check_nfsd_access() call into nfsd_cross_mnt()")
Assisted-by: LLM
Signed-off-by: VeroFess <coderzeng@gmail.com>
---
Changes since v1:
Check the selected export after LOOKUP and READDIR mount crossing.
Map READDIR's operation-level WRONGSEC to ACCESS when rdattr_error
was not requested. Add guards for cached stateid I/O and COPY_NOTIFY.
Restore the RFC comments, drop a redundant assignment, and add the
missing Fixes tags. Regenerate the diff after v1's whitespace damage.
Tests and review notes:
W=1 object builds and an x86_64 KASAN bzImage completed. Runtime
tests used bcd97658ca9b; the diff also applied without fuzz to
cel/nfsd-testing at 944812cf2b77.
PUTFH, SAVEFH, READ/WRITE returned NFS4ERR_WRONGSEC at PUTFH.
SAVEFH alone, a replacement PUTFH, and SECINFO_NO_NAME remained
allowed; RESTOREFH, READ checked at RESTOREFH.
An ordinary file export kept the parent handle on LOOKUP; a bind
mount selected the stricter child export. With parent sec=sys:krb5p
and child sec=krb5p, AUTH_SYS LOOKUP, READ/WRITE succeeded before
this fix using a GSS-opened stateid. The v4.0 test used another
client; v4.1 and v4.2 changed credentials within one session. The
final kernel returned NFS4ERR_WRONGSEC at LOOKUP on all three
versions. Direct PUTFH already returned NFS4ERR_WRONGSEC. GSS
LOOKUP, AUTH_SYS SECINFO, and access to a public sibling succeeded.
SECINFO returned krb5p for the strict child. Source review found
that the Linux client calls SECINFO(parent, name) after LOOKUP
WRONGSEC and retries LOOKUP with the selected flavor.
Cross-client stateids were not tested on v4.1 or v4.2.
On v4.0, v4.1, and v4.2, READDIR reported the strict child's
NFS4ERR_WRONGSEC in rdattr_error and kept public entries intact.
With size but no rdattr_error, READDIR returned NFS4ERR_ACCESS;
name-only READDIR succeeded. COPY into the strict child copied
4096 bytes before this fix; the final LOOKUP check now prevents
that destination from being selected under AUTH_SYS. CLONE reached
ext4 but returned NFS4ERR_NOTSUPP before the fix, so no clone
completed. Protected saved-source COPY/CLONE still returned
NFS4ERR_ACCESS.
Before restoring the LOOKUP check, the cached-file guard returned
NFS4ERR_ACCESS for READ/WRITE, READ_PLUS, SEEK, ALLOCATE, DEALLOCATE,
and COPY with the strict child handle. GETATTR, ACCESS, and OPEN
already returned NFS4ERR_WRONGSEC. Source review found fh_verify()
in SETATTR, READLINK, VERIFY/NVERIFY, and xattrs; COMMIT acquires
an nfsd_file. LINK/RENAME saved-FH probes returned NFS4ERR_WRONGSEC.
After a live AUTH_SYS OPEN, changing the export to sec=krb5p and
mode 000 denied AUTH_SYS READ but still allowed COPY_NOTIFY to
issue a copy stateid before this fix. A separate user-space GSS
client could not OPEN the file, yet read 4096 bytes with that
stateid and wrote them to another export. An intermediate build
rejected COPY_NOTIFY with NFS4ERR_ACCESS before issuing a stateid.
With the final kernel, both READ and COPY_NOTIFY compounds stop at
LOOKUP with NFS4ERR_WRONGSEC. Source review and repeated denial
found no notification allocation or retained stateid reference
on COPY_NOTIFY's own error path.
Stock NFSD-to-NFSD COPY from the strict export ended with
NFS4ERR_OFFLOAD_DENIED and zero bytes copied; the permissive-source
control copied the file. With the final kernel, an authorized foreign-FH
inter-server COPY completed and the destination matched the source.
A local-only COPY with a foreign FH returned NFS4ERR_STALE at PUTFH.
The targeted KASAN guests logged no KASAN fault or NFSD warning.
pNFS, other architectures, and the full pynfs suite were not tested.
base-commit: bcd97658ca9be5a691eff14065a8f2dcfef6bbaf
fs/nfsd/nfs4proc.c | 43 ++++++++++++++++++++++++++++++-------------
fs/nfsd/nfs4state.c | 6 ++++++
fs/nfsd/nfs4xdr.c | 8 +++++++-
fs/nfsd/vfs.c | 4 ++++
4 files changed, 47 insertions(+), 14 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index a50f9dcb3..3a7f5e96a 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1491,6 +1491,11 @@ nfsd4_verify_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (!cstate->save_fh.fh_dentry)
return nfserr_nofilehandle;
+ /* A cached open stateid does not authorize the saved source export. */
+ status = check_nfsd_access(cstate->save_fh.fh_export, rqstp);
+ if (status)
+ return nfserr_acces;
+
status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->save_fh,
src_stateid, RD_STATE, src, NULL);
if (status)
@@ -2532,6 +2537,12 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (!stid)
return nfserr_bad_stateid;
+ status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
+ if (status) {
+ status = nfserr_acces;
+ goto out;
+ }
+
cn->cpn_lease_time.tv_sec = nn->nfsd4_lease;
cn->cpn_lease_time.tv_nsec = 0;
@@ -3237,32 +3248,38 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
struct nfsd4_compoundres *resp = rqstp->rq_resp;
struct nfsd4_compoundargs *argp = rqstp->rq_argp;
struct nfsd4_op *this = &argp->ops[resp->opcnt - 1];
- struct nfsd4_op *next = &argp->ops[resp->opcnt];
+ struct nfsd4_op *next;
const struct nfsd4_operation *thisd = OPDESC(this);
const struct nfsd4_operation *nextd;
+ unsigned int i;
- /*
- * Most ops check wronsec on our own; only the putfh-like ops
- * have special rules.
- */
+ /* Only put-filehandle operations need a deferred WRONGSEC check. */
if (!(thisd->op_flags & OP_IS_PUTFH_LIKE))
return false;
+
/*
- * rfc 5661 2.6.3.1.1.6: don't bother erroring out a
- * put-filehandle operation if we're not going to use the
- * result:
+ * RFC 8881 2.6.3.1.1.1: ignore SAVEFH when choosing the operation
+ * that determines whether the put-filehandle operation checks WRONGSEC.
*/
- if (argp->opcnt == resp->opcnt)
+ for (i = resp->opcnt; i < argp->opcnt; i++) {
+ next = &argp->ops[i];
+ if (next->opnum != OP_SAVEFH)
+ break;
+ }
+
+ /* RFC 8881 2.6.3.1.1.6: the filehandle is not used. */
+ if (i == argp->opcnt)
return false;
+
if (next->opnum == OP_ILLEGAL)
return false;
nextd = OPDESC(next);
/*
- * Rest of 2.6.3.1.1: certain operations will return WRONGSEC
- * errors themselves as necessary; others should check for them
- * now:
+ * RFC 8881 2.6.3.1.1.2: a later put-filehandle operation replaces
+ * this one. Other marked operations check WRONGSEC themselves.
*/
- return !(nextd->op_flags & OP_HANDLES_WRONGSEC);
+ return !(nextd->op_flags &
+ (OP_HANDLES_WRONGSEC | OP_IS_PUTFH_LIKE));
}
#ifdef CONFIG_NFSD_V4_2_INTER_SSC
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f2b52ba90..27f5b1a58 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -8486,6 +8486,12 @@ nfs4_check_file(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfs4_stid *s,
nf = nfs4_find_file(s, flags);
if (nf) {
+ /* A cached file still needs the current export security check. */
+ status = check_nfsd_access(fhp->fh_export, rqstp);
+ if (status) {
+ nfsd_file_put(nf);
+ return nfserr_acces;
+ }
status = nfsd_permission(&rqstp->rq_cred,
fhp->fh_export, fhp->fh_dentry,
acc | NFSD_MAY_OWNER_OVERRIDE);
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 4d9834446..daf2ad77d 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4677,6 +4677,9 @@ nfsd4_encode_entry4_fattr(struct nfsd4_readdir *cd, const char *name,
* At least, add a comment here to explain....
*/
nfserr = nfsd_cross_mnt(cd->rd_rqstp, &dentry, &exp);
+ if (nfserr)
+ goto out_put;
+ nfserr = check_nfsd_access(exp, cd->rd_rqstp);
if (nfserr)
goto out_put;
crossed = true;
@@ -4775,8 +4778,11 @@ nfsd4_encode_entry4(void *ccdv, const char *name, int namlen,
* then in accordance with the spec, we fail the
* entire READDIR operation(!)
*/
- if (!(cd->rd_bmval[0] & FATTR4_WORD0_RDATTR_ERROR))
+ if (!(cd->rd_bmval[0] & FATTR4_WORD0_RDATTR_ERROR)) {
+ if (nfserr == nfserr_wrongsec)
+ nfserr = nfserr_acces;
goto fail;
+ }
if (nfsd4_encode_entry4_rdattr_error(xdr, nfserr)) {
nfserr = nfserr_toosmall;
goto fail;
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f59a55f89..8935a6fcc 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -340,6 +340,9 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
if (err)
return err;
+ err = check_nfsd_access(exp, rqstp);
+ if (err)
+ goto out;
/*
* Note: we compose the file handle now, but as the
* dentry may be negative, it may need to be updated.
@@ -348,6 +351,7 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
if (!err && d_really_is_negative(dentry))
err = nfserr_noent;
+out:
dput(dentry);
exp_put(exp);
return err;
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v3] nfsd: enforce export security for NFSv4 filehandles and stateids
2026-10-29 1:36 ` [PATCH v2] nfsd: enforce export security for NFSv4 filehandles and stateids VeroFess
@ 2026-10-29 5:14 ` VeroFess
2026-10-29 6:39 ` [PATCH v4] " VeroFess
0 siblings, 1 reply; 8+ messages in thread
From: VeroFess @ 2026-10-29 5:14 UTC (permalink / raw)
To: cel, jlayton, neil, okorniev, Dai.Ngo, tom; +Cc: linux-nfs
An AUTH_SYS NFSv4.0 client can read or write a file on a sec=krb5p
export using another client's valid OPEN stateid by inserting
SAVEFH between PUTFH and READ or WRITE.
PUTFH resolves the handle with NFSD_MAY_BYPASS_GSS, expecting
a later security check. SAVEFH carries OP_HANDLES_WRONGSEC even though
it only saves the handle, so PUTFH skips NFS4ERR_WRONGSEC. Stateid I/O
can then reuse a cached nfsd_file.
The original report used these export options:
rw,sync,no_subtree_check,fsid=0,sec=krb5p,secure,root_squash
A Kerberos privacy NFSv4.0 client held an OPEN stateid on that export.
With the same stateid, an AUTH_SYS request was rejected by PUTFH, READ,
but SAVEFH between those operations allowed READ and WRITE. The Kerberos
client subsequently read the changed contents.
SAVEFH leaves CURRENT_FH in place. When deciding whether PUTFH must
report NFS4ERR_WRONGSEC, look past SAVEFH to the first operation that
uses or replaces CURRENT_FH. If another PUTFH replaces it, leave the
earlier PUTFH unchecked, as required by RFC 8881 Section 2.6.3.1.1.
Local COPY and CLONE consume SAVED_FH as the source. Check the
saved export with check_nfsd_access() before using its stateid:
a cached open stateid can otherwise bypass the sec= policy. Map
NFS4ERR_WRONGSEC to NFS4ERR_ACCESS at COPY or CLONE, since neither
operation permits NFS4ERR_WRONGSEC.
An inter-server COPY carries a foreign SAVED_FH. The source server
validates it; the destination must not apply its own export policy
to that handle. The destination PUTFH still checks its local export.
A bind-mounted regular file can have a stricter export than its parent.
LOOKUP crosses into that export, but nfsd_cross_mnt() checks the
parent export. Check the selected export in nfsd_lookup() before
composing a filehandle, and in READDIR before encoding child attributes.
When rdattr_error is not requested, return NFS4ERR_ACCESS for a flavor
mismatch; NFS4ERR_WRONGSEC is not a READDIR status. Keep the child-export
check out of nfsd_cross_mnt(): SECINFO uses it to discover the child's
supported security flavors.
SECINFO and SECINFO_NO_NAME report export security policies. RFC 8881
Section 2.6.3.1.1.5 forbids NFS4ERR_WRONGSEC from either operation.
SECINFO_NO_NAME(PARENT) needs to look up the parent without enforcing
the parent's policy; ordinary LOOKUPP still checks the selected export.
The starting directory's fh_verify(..., NFSD_MAY_EXEC) call also
rejects SECINFO and SECINFO_NO_NAME(PARENT) when that export does not
allow the request's flavor. When SECINFO crosses a mount, it can hit
another check on the source export.
The fsid=0 lookup uses the request's auth domains to find an export;
it does not itself enforce the export's sec= policy.
Pass NFSD_MAY_SECINFO to fh_verify() so it skips the starting export's
flavor and xprtsec policy checks. A SECINFO mount crossing also skips
the source export's policy check. The directory type, secure-port, and
execute-permission checks still apply. Ordinary LOOKUP and LOOKUPP
continue to enforce export security.
The LOOKUP check controls whether a client obtains a new child handle.
It does not cover a handle and OPEN stateid retained while the export's
sec= policy changes. The cached nfsd_file branch of nfs4_check_file()
calls nfsd_permission() without checking the current export's flavor.
Check that export before reusing the file for stateid I/O.
COPY_NOTIFY validates a stateid without acquiring an nfsd_file, so it
misses that check. After an export was changed to sec=krb5p, an
AUTH_SYS COPY_NOTIFY using an existing stateid still issued a copy
stateid although AUTH_SYS READ was denied. In the test, a separate
user-space GSS client used that stateid to read the file. Check the
current export before issuing it and return NFS4ERR_ACCESS on a
flavor mismatch.
Fixes: 68d93184352f ("nfsd4: fix wrongsec handling for PUTFH + op cases")
Fixes: 624322f1adc5 ("NFSD add COPY_NOTIFY operation")
Fixes: f0d6ad5a2f67 ("nfsd: move check_nfsd_access() call into nfsd_cross_mnt()")
Fixes: 29a78a3ed7fc ("nfsd4: make fh_verify responsibility of nfsd_lookup_dentry caller")
Fixes: 04f4ad16b231 ("nfsd4: implement secinfo_no_name")
Assisted-by: LLM
Signed-off-by: VeroFess <coderzeng@gmail.com>
---
Changes since v2:
SECINFO_NO_NAME(PARENT) uses the parent lookup path. The selected-export
check added in v2 made it return NFS4ERR_WRONGSEC when the parent export
rejected the RPC flavor. Use nfsd_lookup_dentry() for this query;
leave nfsd_lookup() and its selected-export check in place for LOOKUPP.
Also allow SECINFO and SECINFO_NO_NAME(PARENT) to query a strict
starting export. SECINFO mount crossing skips the source export's
policy check. Directory traversal permission remains enforced.
Changes since v1:
Check cached stateid I/O and COPY_NOTIFY against the current export.
Check the selected export after LOOKUP and READDIR mount crossing, and
map READDIR's operation-level NFS4ERR_WRONGSEC to NFS4ERR_ACCESS when
rdattr_error was not requested. Restore the RFC comments, drop a
redundant assignment, and add the missing Fixes tags. Regenerate the
diff after v1's whitespace damage.
Tests and review notes:
SECINFO_NO_NAME(PARENT) was tested on v4.1 and v4.2 with a retained
child-directory handle after the parent export's policy changed. It
returned the parent's AUTH_SYS flavor before v2, NFS4ERR_WRONGSEC in
v2, and the AUTH_SYS flavor in v3. With a strict starting export,
AUTH_SYS SECINFO_NO_NAME(PARENT) also returned the parent's flavors.
SECINFO returned the named file's or crossed-mount export's flavors
on v4.0, v4.1 and v4.2. LOOKUP and LOOKUPP still returned
NFS4ERR_WRONGSEC. Without directory execute permission, the queries
returned NFS4ERR_ACCESS.
SECINFO can distinguish existing from nonexistent names while
negotiating security flavors. This is consistent with the protocol's
security negotiation semantics.
The original SAVEFH READ/WRITE tests returned NFS4ERR_WRONGSEC at
PUTFH. SAVEFH alone, a replacement PUTFH and
SECINFO_NO_NAME(CURRENT_FH) remained allowed; RESTOREFH checked the
restored handle.
A regular-file export with stricter security than its parent was
also tested. The v4.0 test used another client's stateid; v4.1 and
v4.2 changed credentials within one session. The fixed LOOKUP
returned NFS4ERR_WRONGSEC on all three versions. GSS LOOKUP,
AUTH_SYS SECINFO, and access to a public sibling still succeeded.
The Linux client uses SECINFO to retry LOOKUP with an allowed flavor
after NFS4ERR_WRONGSEC. Cross-client stateids were not tested on
v4.1 or v4.2.
In earlier v3 tests, READDIR with rdattr_error reported the strict
child's NFS4ERR_WRONGSEC per entry and kept the public entry; with
size but no rdattr_error it returned NFS4ERR_ACCESS. Name-only
READDIR succeeded on v4.0, v4.1 and v4.2. Before the fix, COPY into
the strict child transferred 4096 bytes; the LOOKUP check now stops
that destination from being selected under AUTH_SYS. CLONE reached
ext4 but returned NFS4ERR_NOTSUPP, so no clone completed in that test.
In an intermediate build without the LOOKUP check, nfs4_check_file()
denied stateid READ/WRITE, READ_PLUS, SEEK, ALLOCATE,
DEALLOCATE and COPY. GETATTR, ACCESS and OPEN already returned
NFS4ERR_WRONGSEC. Source review found fh_verify() in SETATTR,
READLINK, VERIFY/NVERIFY and xattrs; COMMIT acquires an nfsd_file.
LINK/RENAME saved-handle probes returned NFS4ERR_WRONGSEC.
Protected saved-source COPY/CLONE returned NFS4ERR_ACCESS; authorized
local COPY/CLONE and a foreign-filehandle inter-server COPY completed.
Local-only COPY with a foreign handle returned NFS4ERR_STALE.
Before the COPY_NOTIFY fix, a policy change left AUTH_SYS READ denied
while COPY_NOTIFY still issued a copy stateid usable by a GSS client.
Stock NFSD-to-NFSD COPY from the strict source returned
NFS4ERR_OFFLOAD_DENIED with zero bytes copied; the permissive-source
control completed. The v2 guard denied a retained-handle COPY_NOTIFY
before issuing a stateid. Source review and repeated denials found no
notification allocation or retained stateid reference on that path.
Earlier W=1 object builds and the revised x86_64 KASAN bzImage
completed. The targeted guest logged no KASAN fault or NFSD warning.
pNFS, other architectures and the full pynfs suite were not tested.
base-commit: bcd97658ca9be5a691eff14065a8f2dcfef6bbaf
fs/nfsd/nfs4proc.c | 80 +++++++++++++++++++++++++++++++++++----------
fs/nfsd/nfs4state.c | 8 ++++-
fs/nfsd/nfs4xdr.c | 8 ++++-
fs/nfsd/nfsfh.c | 15 +++++----
fs/nfsd/trace.h | 3 +-
fs/nfsd/vfs.c | 32 +++++++++++++-----
fs/nfsd/vfs.h | 4 ++-
7 files changed, 114 insertions(+), 36 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index a50f9dcb3..d0eebd3cd 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1079,7 +1079,31 @@ nfsd4_link(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
return status;
}
-static __be32 nfsd4_do_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh)
+/* SECINFO_NO_NAME(PARENT) must be able to report the parent's flavors. */
+static __be32
+nfsd4_secinfo_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh)
+{
+ struct svc_export *exp;
+ struct dentry *dentry;
+ __be32 status;
+
+ status = fh_verify(rqstp, fh, S_IFDIR,
+ NFSD_MAY_EXEC | NFSD_MAY_SECINFO);
+ if (status)
+ return status;
+ status = nfsd_lookup_dentry(rqstp, fh, "..", 2, &exp, &dentry, true);
+ if (status)
+ return status;
+ status = fh_compose(fh, exp, dentry, fh);
+ if (!status && d_really_is_negative(dentry))
+ status = nfserr_noent;
+ dput(dentry);
+ exp_put(exp);
+ return status;
+}
+
+static __be32 nfsd4_do_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh,
+ bool for_secinfo)
{
struct svc_fh tmp_fh;
__be32 ret;
@@ -1093,6 +1117,8 @@ static __be32 nfsd4_do_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh)
return nfserr_noent;
}
fh_put(&tmp_fh);
+ if (for_secinfo)
+ return nfsd4_secinfo_lookupp(rqstp, fh);
return nfsd_lookup(rqstp, fh, "..", 2, fh);
}
@@ -1100,7 +1126,7 @@ static __be32
nfsd4_lookupp(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
union nfsd4_op_u *u)
{
- return nfsd4_do_lookupp(rqstp, &cstate->current_fh);
+ return nfsd4_do_lookupp(rqstp, &cstate->current_fh, false);
}
static __be32
@@ -1252,12 +1278,13 @@ nfsd4_secinfo(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct dentry *dentry;
__be32 err;
- err = fh_verify(rqstp, &cstate->current_fh, S_IFDIR, NFSD_MAY_EXEC);
+ err = fh_verify(rqstp, &cstate->current_fh, S_IFDIR,
+ NFSD_MAY_EXEC | NFSD_MAY_SECINFO);
if (err)
return err;
err = nfsd_lookup_dentry(rqstp, &cstate->current_fh,
secinfo->si_name, secinfo->si_namelen,
- &exp, &dentry);
+ &exp, &dentry, true);
if (err)
return err;
if (d_really_is_negative(dentry)) {
@@ -1282,7 +1309,7 @@ nfsd4_secinfo_no_name(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstat
case NFS4_SECINFO_STYLE4_CURRENT_FH:
break;
case NFS4_SECINFO_STYLE4_PARENT:
- err = nfsd4_do_lookupp(rqstp, &cstate->current_fh);
+ err = nfsd4_do_lookupp(rqstp, &cstate->current_fh, true);
if (err)
return err;
break;
@@ -1491,6 +1518,11 @@ nfsd4_verify_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (!cstate->save_fh.fh_dentry)
return nfserr_nofilehandle;
+ /* A cached open stateid does not authorize the saved source export. */
+ status = check_nfsd_access(cstate->save_fh.fh_export, rqstp);
+ if (status)
+ return nfserr_acces;
+
status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->save_fh,
src_stateid, RD_STATE, src, NULL);
if (status)
@@ -2532,6 +2564,12 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (!stid)
return nfserr_bad_stateid;
+ status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
+ if (status) {
+ status = nfserr_acces;
+ goto out;
+ }
+
cn->cpn_lease_time.tv_sec = nn->nfsd4_lease;
cn->cpn_lease_time.tv_nsec = 0;
@@ -3237,32 +3275,38 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
struct nfsd4_compoundres *resp = rqstp->rq_resp;
struct nfsd4_compoundargs *argp = rqstp->rq_argp;
struct nfsd4_op *this = &argp->ops[resp->opcnt - 1];
- struct nfsd4_op *next = &argp->ops[resp->opcnt];
+ struct nfsd4_op *next;
const struct nfsd4_operation *thisd = OPDESC(this);
const struct nfsd4_operation *nextd;
+ unsigned int i;
- /*
- * Most ops check wronsec on our own; only the putfh-like ops
- * have special rules.
- */
+ /* Only put-filehandle operations need a deferred WRONGSEC check. */
if (!(thisd->op_flags & OP_IS_PUTFH_LIKE))
return false;
+
/*
- * rfc 5661 2.6.3.1.1.6: don't bother erroring out a
- * put-filehandle operation if we're not going to use the
- * result:
+ * RFC 8881 2.6.3.1.1.1: ignore SAVEFH when choosing the operation
+ * that determines whether the put-filehandle operation checks WRONGSEC.
*/
- if (argp->opcnt == resp->opcnt)
+ for (i = resp->opcnt; i < argp->opcnt; i++) {
+ next = &argp->ops[i];
+ if (next->opnum != OP_SAVEFH)
+ break;
+ }
+
+ /* RFC 8881 2.6.3.1.1.6: the filehandle is not used. */
+ if (i == argp->opcnt)
return false;
+
if (next->opnum == OP_ILLEGAL)
return false;
nextd = OPDESC(next);
/*
- * Rest of 2.6.3.1.1: certain operations will return WRONGSEC
- * errors themselves as necessary; others should check for them
- * now:
+ * RFC 8881 2.6.3.1.1.2: a later put-filehandle operation replaces
+ * this one. Other marked operations check WRONGSEC themselves.
*/
- return !(nextd->op_flags & OP_HANDLES_WRONGSEC);
+ return !(nextd->op_flags &
+ (OP_HANDLES_WRONGSEC | OP_IS_PUTFH_LIKE));
}
#ifdef CONFIG_NFSD_V4_2_INTER_SSC
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f2b52ba90..3a12aeb56 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6967,7 +6967,7 @@ nfsd4_verify_deleg_dentry(struct nfsd4_open *open, struct nfs4_file *fp,
err = nfsd_lookup_dentry(open->op_rqstp, parent,
open->op_fname, open->op_fnamelen,
- &exp, &child);
+ &exp, &child, false);
if (err)
return -ESTALE;
@@ -8486,6 +8486,12 @@ nfs4_check_file(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfs4_stid *s,
nf = nfs4_find_file(s, flags);
if (nf) {
+ /* A cached file still needs the current export security check. */
+ status = check_nfsd_access(fhp->fh_export, rqstp);
+ if (status) {
+ nfsd_file_put(nf);
+ return nfserr_acces;
+ }
status = nfsd_permission(&rqstp->rq_cred,
fhp->fh_export, fhp->fh_dentry,
acc | NFSD_MAY_OWNER_OVERRIDE);
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 4d9834446..daf2ad77d 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4677,6 +4677,9 @@ nfsd4_encode_entry4_fattr(struct nfsd4_readdir *cd, const char *name,
* At least, add a comment here to explain....
*/
nfserr = nfsd_cross_mnt(cd->rd_rqstp, &dentry, &exp);
+ if (nfserr)
+ goto out_put;
+ nfserr = check_nfsd_access(exp, cd->rd_rqstp);
if (nfserr)
goto out_put;
crossed = true;
@@ -4775,8 +4778,11 @@ nfsd4_encode_entry4(void *ccdv, const char *name, int namlen,
* then in accordance with the spec, we fail the
* entire READDIR operation(!)
*/
- if (!(cd->rd_bmval[0] & FATTR4_WORD0_RDATTR_ERROR))
+ if (!(cd->rd_bmval[0] & FATTR4_WORD0_RDATTR_ERROR)) {
+ if (nfserr == nfserr_wrongsec)
+ nfserr = nfserr_acces;
goto fail;
+ }
if (nfsd4_encode_entry4_rdattr_error(xdr, nfserr)) {
nfserr = nfserr_toosmall;
goto fail;
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index f1ba5a14b..2627c344a 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -451,10 +451,11 @@ __fh_verify(struct svc_rqst *rqstp,
goto out;
/*
- * NLM is allowed to bypass the xprtsec policy check because lockd
- * doesn't support xprtsec.
+ * SECINFO queries the export's policy, so neither the RPC flavor nor
+ * the transport need match it. Keep the inode permission check below.
+ * NLM also bypasses xprtsec because lockd does not support it.
*/
- if (!(access & NFSD_MAY_NLM)) {
+ if (!(access & (NFSD_MAY_NLM | NFSD_MAY_SECINFO))) {
error = check_xprtsec_policy(exp, rqstp);
if (error)
goto out;
@@ -471,9 +472,11 @@ __fh_verify(struct svc_rqst *rqstp,
&& exp->ex_path.dentry == dentry)
may_bypass_gss = true;
- error = check_security_flavor(exp, rqstp, may_bypass_gss);
- if (error)
- goto out;
+ if (!(access & NFSD_MAY_SECINFO)) {
+ error = check_security_flavor(exp, rqstp, may_bypass_gss);
+ if (error)
+ goto out;
+ }
svc_xprt_set_valid(rqstp->rq_xprt);
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index 951af645d..a7d9f9d7b 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -142,7 +142,8 @@ DEFINE_NFSD_DYNTHREAD_EVENT(trylock_fail);
{ NFSD_MAY_BYPASS_GSS, "BYPASS_GSS" }, \
{ NFSD_MAY_READ_IF_EXEC, "READ_IF_EXEC" }, \
{ NFSD_MAY_64BIT_COOKIE, "64BIT_COOKIE" }, \
- { NFSD_MAY_LOCALIO, "LOCALIO" })
+ { NFSD_MAY_LOCALIO, "LOCALIO" }, \
+ { NFSD_MAY_SECINFO, "SECINFO" })
TRACE_EVENT(nfsd_compound,
TP_PROTO(
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f59a55f89..63ebde0fd 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -124,14 +124,14 @@ nfserrno(int errno)
}
/*
- * Called from nfsd_lookup and encode_dirent. Check if we have crossed
+ * Called from lookup, SECINFO, and encode_dirent. Check if we have crossed
* a mount point.
* Returns an nfs error leaving *dpp and *expp unchanged,
* or nfs_ok having possibly changed *dpp and *expp
*/
-__be32
-nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
- struct svc_export **expp)
+static __be32
+nfsd_cross_mnt_common(struct svc_rqst *rqstp, struct dentry **dpp,
+ struct svc_export **expp, bool for_secinfo)
{
struct svc_export *exp = *expp, *exp2 = NULL;
struct dentry *dentry = *dpp;
@@ -169,7 +169,9 @@ nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
err = 0;
} else if (nfsd_v4client(rqstp) ||
(exp->ex_flags & NFSEXP_CROSSMOUNT) || EX_NOHIDE(exp2)) {
- nfserr = check_nfsd_access(exp, rqstp);
+ /* SECINFO needs the target's flavors even if the source differs. */
+ if (!for_secinfo)
+ nfserr = check_nfsd_access(exp, rqstp);
if (nfserr == nfs_ok) {
/* successfully crossed mount point */
/*
@@ -193,6 +195,13 @@ nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
return nfserrno(err);
}
+__be32
+nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
+ struct svc_export **expp)
+{
+ return nfsd_cross_mnt_common(rqstp, dpp, expp, false);
+}
+
static void follow_to_parent(struct path *path)
{
struct dentry *dp;
@@ -255,7 +264,8 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
__be32
nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
const char *name, unsigned int len,
- struct svc_export **exp_ret, struct dentry **dentry_ret)
+ struct svc_export **exp_ret, struct dentry **dentry_ret,
+ bool for_secinfo)
{
struct svc_export *exp;
struct dentry *dparent;
@@ -288,7 +298,8 @@ nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
if (IS_ERR(dentry))
goto out_nfserr;
if (nfsd_mountpoint(dentry, exp)) {
- __be32 nfserr = nfsd_cross_mnt(rqstp, &dentry, &exp);
+ __be32 nfserr = nfsd_cross_mnt_common(rqstp,
+ &dentry, &exp, for_secinfo);
if (nfserr) {
dput(dentry);
@@ -337,9 +348,13 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_EXEC);
if (err)
return err;
- err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
+ err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry,
+ false);
if (err)
return err;
+ err = check_nfsd_access(exp, rqstp);
+ if (err)
+ goto out;
/*
* Note: we compose the file handle now, but as the
* dentry may be negative, it may need to be updated.
@@ -348,6 +363,7 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
if (!err && d_really_is_negative(dentry))
err = nfserr_noent;
+out:
dput(dentry);
exp_put(exp);
return err;
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 1f05c83a3..e7724628f 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -33,6 +33,7 @@
#define NFSD_MAY_64BIT_COOKIE 0x1000 /* 64 bit readdir cookies for >= NFSv3 */
#define NFSD_MAY_LOCALIO 0x2000 /* for tracing, reflects when localio used */
+#define NFSD_MAY_SECINFO 0x4000 /* query export security policy */
#define NFSD_MAY_CREATE (NFSD_MAY_EXEC|NFSD_MAY_WRITE)
#define NFSD_MAY_REMOVE (NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC)
@@ -133,7 +134,8 @@ __be32 nfsd_lookup(struct svc_rqst *, struct svc_fh *,
const char *, unsigned int, struct svc_fh *);
__be32 nfsd_lookup_dentry(struct svc_rqst *, struct svc_fh *,
const char *, unsigned int,
- struct svc_export **, struct dentry **);
+ struct svc_export **, struct dentry **,
+ bool for_secinfo);
__be32 nfsd_setattr(struct svc_rqst *, struct svc_fh *,
struct nfsd_attrs *, const struct timespec64 *);
int nfsd_mountpoint(struct dentry *, struct svc_export *);
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH v4] nfsd: enforce export security for NFSv4 filehandles and stateids
2026-10-29 5:14 ` [PATCH v3] " VeroFess
@ 2026-10-29 6:39 ` VeroFess
2026-10-29 15:21 ` Chuck Lever
0 siblings, 1 reply; 8+ messages in thread
From: VeroFess @ 2026-10-29 6:39 UTC (permalink / raw)
To: cel, jlayton, neil, okorniev, Dai.Ngo, tom; +Cc: linux-nfs
An AUTH_SYS NFSv4.0 client can read or write a file on a sec=krb5p
export using another client's valid OPEN stateid by inserting
SAVEFH between PUTFH and READ or WRITE.
PUTFH resolves the handle with NFSD_MAY_BYPASS_GSS, expecting
a later security check. SAVEFH carries OP_HANDLES_WRONGSEC even though
it only saves the handle, so PUTFH skips NFS4ERR_WRONGSEC. Stateid I/O
can then reuse a cached nfsd_file.
The original report used these export options:
rw,sync,no_subtree_check,fsid=0,sec=krb5p,secure,root_squash
A Kerberos privacy NFSv4.0 client held an OPEN stateid on that export.
With the same stateid, an AUTH_SYS request was rejected by PUTFH, READ,
but SAVEFH between those operations allowed READ and WRITE. The Kerberos
client subsequently read the changed contents.
SAVEFH leaves CURRENT_FH in place. When deciding whether PUTFH must
report NFS4ERR_WRONGSEC, look past SAVEFH to the first operation that
uses or replaces CURRENT_FH. If another PUTFH replaces it, leave the
earlier PUTFH unchecked, as required by RFC 8881 Section 2.6.3.1.1.
Local COPY and CLONE consume SAVED_FH as the source. Check the
saved export with check_nfsd_access() before using its stateid:
a cached open stateid can otherwise bypass the sec= policy. Map
NFS4ERR_WRONGSEC to NFS4ERR_ACCESS at COPY or CLONE, since neither
operation permits NFS4ERR_WRONGSEC.
An inter-server COPY carries a foreign SAVED_FH. The source server
validates it; the destination must not apply its own export policy
to that handle. The destination PUTFH still checks its local export.
A bind-mounted regular file can have a stricter export than its parent.
LOOKUP crosses into that export, but nfsd_cross_mnt() checks the
parent export. Check the selected export in nfsd_lookup() before
composing a filehandle, and in READDIR before encoding child attributes.
When rdattr_error is not requested, return NFS4ERR_ACCESS for a flavor
mismatch; NFS4ERR_WRONGSEC is not a READDIR status. Keep the child-export
check out of nfsd_cross_mnt(): SECINFO uses it to discover the child's
supported security flavors.
SECINFO and SECINFO_NO_NAME report export security policies. RFC 8881
Section 2.6.3.1.1.5 forbids NFS4ERR_WRONGSEC from either operation.
SECINFO_NO_NAME(PARENT) needs to look up the parent without enforcing
the parent's policy; ordinary LOOKUPP still checks the selected export.
The starting directory's fh_verify(..., NFSD_MAY_EXEC) call also
rejects SECINFO and SECINFO_NO_NAME(PARENT) when that export does not
allow the request's flavor. When SECINFO crosses a mount, it can hit
another check on the source export.
The fsid=0 lookup uses the request's auth domains to find an export;
it does not itself enforce the export's sec= policy.
Pass NFSD_MAY_SECINFO to fh_verify() so it skips the starting export's
security flavor check. RFC 8881 section 2.6.3.1.1.5 forbids SECINFO
from returning NFS4ERR_WRONGSEC, so it must report the flavors even
when the caller's flavor is not one the export accepts. The export's
transport policy (xprtsec=) is a transport-layer requirement, not part
of the flavor negotiation SECINFO performs, so SECINFO does not bypass
it: fh_verify() still applies check_xprtsec_policy(). A SECINFO mount
crossing likewise skips only the crossed-into export's flavor check and
keeps its transport policy. The directory type, secure-port, transport,
and execute-permission checks still apply. Ordinary LOOKUP and LOOKUPP
continue to enforce export security.
The LOOKUP check controls whether a client obtains a new child handle.
It does not cover a handle and OPEN stateid retained while the export's
sec= policy changes. The cached nfsd_file branch of nfs4_check_file()
calls nfsd_permission() without checking the current export's flavor.
Check that export before reusing the file for stateid I/O.
COPY_NOTIFY validates a stateid without acquiring an nfsd_file, so it
misses that check. After an export was changed to sec=krb5p, an
AUTH_SYS COPY_NOTIFY using an existing stateid still issued a copy
stateid although AUTH_SYS READ was denied. In the test, a separate
user-space GSS client used that stateid to read the file. Check the
current export before issuing it and return NFS4ERR_ACCESS on a
flavor mismatch.
Fixes: 68d93184352f ("nfsd4: fix wrongsec handling for PUTFH + op cases")
Fixes: 624322f1adc5 ("NFSD add COPY_NOTIFY operation")
Fixes: f0d6ad5a2f67 ("nfsd: move check_nfsd_access() call into nfsd_cross_mnt()")
Fixes: 29a78a3ed7fc ("nfsd4: make fh_verify responsibility of nfsd_lookup_dentry caller")
Fixes: 04f4ad16b231 ("nfsd4: implement secinfo_no_name")
Assisted-by: LLM
Signed-off-by: VeroFess <coderzeng@gmail.com>
---
Changes since v3:
Narrow the SECINFO exemption to the RPC security flavor check only.
v3 also skipped the export's xprtsec= transport policy for SECINFO,
which let a plaintext client run SECINFO against a TLS-only export.
RFC 8881 only requires SECINFO to avoid NFS4ERR_WRONGSEC for the
security flavor; the transport requirement is separate and remains
enforced. __fh_verify() now skips only check_security_flavor() for
SECINFO. Because a SECINFO lookup can cross a mount or step up to a
parent, nfsd_lookup_dentry() re-checks check_xprtsec_policy() whenever
it selects a different export object, so a SECINFO that crosses into a
stricter transport is still denied while the already-verified starting
export is not checked twice.
Changes since v2:
SECINFO_NO_NAME(PARENT) uses the parent lookup path. The selected-export
check added in v2 made it return NFS4ERR_WRONGSEC when the parent export
rejected the RPC flavor. Use nfsd_lookup_dentry() for this query;
leave nfsd_lookup() and its selected-export check in place for LOOKUPP.
Also allow SECINFO and SECINFO_NO_NAME(PARENT) to query a strict
starting export. SECINFO mount crossing skips the source export's
policy check. Directory traversal permission remains enforced.
Changes since v1:
Check cached stateid I/O and COPY_NOTIFY against the current export.
Check the selected export after LOOKUP and READDIR mount crossing, and
map READDIR's operation-level NFS4ERR_WRONGSEC to NFS4ERR_ACCESS when
rdattr_error was not requested. Restore the RFC comments, drop a
redundant assignment, and add the missing Fixes tags. Regenerate the
diff after v1's whitespace damage.
Tests and review notes:
SECINFO_NO_NAME(PARENT) was tested on v4.1 and v4.2 with a retained
child-directory handle after the parent export's policy changed. It
returned the parent's AUTH_SYS flavor before v2, NFS4ERR_WRONGSEC in
v2, and the AUTH_SYS flavor in v3. With a strict starting export,
AUTH_SYS SECINFO_NO_NAME(PARENT) also returned the parent's flavors.
SECINFO returned the named file's or crossed-mount export's flavors
on v4.0, v4.1 and v4.2. LOOKUP and LOOKUPP still returned
NFS4ERR_WRONGSEC. Without directory execute permission, the queries
returned NFS4ERR_ACCESS.
SECINFO can distinguish existing from nonexistent names while
negotiating security flavors. This is consistent with the protocol's
security negotiation semantics.
On v4 of this patch, a plaintext client querying an xprtsec=tls,
sec=sys export had SECINFO rejected with NFS4ERR_WRONGSEC, matching
GETATTR and LOOKUP; v3 had allowed that SECINFO. A plaintext SECINFO
that crossed a mount into an xprtsec=tls child export was likewise
rejected, while SECINFO of a non-TLS sibling and of a sec=krb5p
export without xprtsec= still succeeded, so the flavor exemption is
intact. An xprtsec=mtls export behaved the same for a plaintext
client, both as the starting export and across a mount crossing. The
TLS-session-without-peer-authentication case that distinguishes mtls
from tls was not exercised at runtime; it uses the same
check_xprtsec_policy() path. Tested on v4.0, v4.1 and v4.2.
nfsd_lookup_dentry() has three callers; only nfsd4_secinfo() and the
SECINFO_NO_NAME(PARENT) helper pass for_secinfo=true, and both run
fh_verify() on the starting export first. svc_export objects are
immutable once published (a policy change installs a new object and
expires the old, which held references keep unchanged), so a filehandle
reference gives a stable policy view for the request. The transport
re-check keys on export-object identity (exp != fhp->fh_export): any
switch re-authorizes the new object, and the unchanged starting object
is not re-verified. Call counts observed with a kprobe on
check_xprtsec_policy() were one for a non-crossing SECINFO and two for
a crossing one (starting export plus crossed-into export), consistent
with that design; the counts are evidence, not the correctness
argument, which rests on the immutable-reference model above.
The original SAVEFH READ/WRITE tests returned NFS4ERR_WRONGSEC at
PUTFH. SAVEFH alone, a replacement PUTFH and
SECINFO_NO_NAME(CURRENT_FH) remained allowed; RESTOREFH checked the
restored handle.
A regular-file export with stricter security than its parent was
also tested. The v4.0 test used another client's stateid; v4.1 and
v4.2 changed credentials within one session. The fixed LOOKUP
returned NFS4ERR_WRONGSEC on all three versions. GSS LOOKUP,
AUTH_SYS SECINFO, and access to a public sibling still succeeded.
The Linux client uses SECINFO to retry LOOKUP with an allowed flavor
after NFS4ERR_WRONGSEC. Cross-client stateids were not tested on
v4.1 or v4.2.
In earlier v3 tests, READDIR with rdattr_error reported the strict
child's NFS4ERR_WRONGSEC per entry and kept the public entry; with
size but no rdattr_error it returned NFS4ERR_ACCESS. Name-only
READDIR succeeded on v4.0, v4.1 and v4.2. Before the fix, COPY into
the strict child transferred 4096 bytes; the LOOKUP check now stops
that destination from being selected under AUTH_SYS. CLONE reached
ext4 but returned NFS4ERR_NOTSUPP, so no clone completed in that test.
In an intermediate build without the LOOKUP check, nfs4_check_file()
denied stateid READ/WRITE, READ_PLUS, SEEK, ALLOCATE,
DEALLOCATE and COPY. GETATTR, ACCESS and OPEN already returned
NFS4ERR_WRONGSEC. Source review found fh_verify() in SETATTR,
READLINK, VERIFY/NVERIFY and xattrs; COMMIT acquires an nfsd_file.
LINK/RENAME saved-handle probes returned NFS4ERR_WRONGSEC.
Protected saved-source COPY/CLONE returned NFS4ERR_ACCESS; authorized
local COPY/CLONE and a foreign-filehandle inter-server COPY completed.
Local-only COPY with a foreign handle returned NFS4ERR_STALE.
Before the COPY_NOTIFY fix, a policy change left AUTH_SYS READ denied
while COPY_NOTIFY still issued a copy stateid usable by a GSS client.
Stock NFSD-to-NFSD COPY from the strict source returned
NFS4ERR_OFFLOAD_DENIED with zero bytes copied; the permissive-source
control completed. The v2 guard denied a retained-handle COPY_NOTIFY
before issuing a stateid. Source review and repeated denials found no
notification allocation or retained stateid reference on that path.
Earlier W=1 object builds and the revised x86_64 KASAN bzImage
completed. The targeted guest logged no KASAN fault or NFSD warning.
pNFS, other architectures and the full pynfs suite were not tested.
base-commit: bcd97658ca9be5a691eff14065a8f2dcfef6bbaf
fs/nfsd/nfs4proc.c | 80 +++++++++++++++++++++++++++++++++++----------
fs/nfsd/nfs4state.c | 8 ++++-
fs/nfsd/nfs4xdr.c | 8 ++++-
fs/nfsd/nfsfh.c | 16 +++++++--
fs/nfsd/trace.h | 3 +-
fs/nfsd/vfs.c | 49 ++++++++++++++++++++++-----
fs/nfsd/vfs.h | 4 ++-
7 files changed, 135 insertions(+), 33 deletions(-)
diff --git a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c
index a50f9dcb3..d0eebd3cd 100644
--- a/fs/nfsd/nfs4proc.c
+++ b/fs/nfsd/nfs4proc.c
@@ -1079,7 +1079,31 @@ nfsd4_link(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
return status;
}
-static __be32 nfsd4_do_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh)
+/* SECINFO_NO_NAME(PARENT) must be able to report the parent's flavors. */
+static __be32
+nfsd4_secinfo_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh)
+{
+ struct svc_export *exp;
+ struct dentry *dentry;
+ __be32 status;
+
+ status = fh_verify(rqstp, fh, S_IFDIR,
+ NFSD_MAY_EXEC | NFSD_MAY_SECINFO);
+ if (status)
+ return status;
+ status = nfsd_lookup_dentry(rqstp, fh, "..", 2, &exp, &dentry, true);
+ if (status)
+ return status;
+ status = fh_compose(fh, exp, dentry, fh);
+ if (!status && d_really_is_negative(dentry))
+ status = nfserr_noent;
+ dput(dentry);
+ exp_put(exp);
+ return status;
+}
+
+static __be32 nfsd4_do_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh,
+ bool for_secinfo)
{
struct svc_fh tmp_fh;
__be32 ret;
@@ -1093,6 +1117,8 @@ static __be32 nfsd4_do_lookupp(struct svc_rqst *rqstp, struct svc_fh *fh)
return nfserr_noent;
}
fh_put(&tmp_fh);
+ if (for_secinfo)
+ return nfsd4_secinfo_lookupp(rqstp, fh);
return nfsd_lookup(rqstp, fh, "..", 2, fh);
}
@@ -1100,7 +1126,7 @@ static __be32
nfsd4_lookupp(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
union nfsd4_op_u *u)
{
- return nfsd4_do_lookupp(rqstp, &cstate->current_fh);
+ return nfsd4_do_lookupp(rqstp, &cstate->current_fh, false);
}
static __be32
@@ -1252,12 +1278,13 @@ nfsd4_secinfo(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
struct dentry *dentry;
__be32 err;
- err = fh_verify(rqstp, &cstate->current_fh, S_IFDIR, NFSD_MAY_EXEC);
+ err = fh_verify(rqstp, &cstate->current_fh, S_IFDIR,
+ NFSD_MAY_EXEC | NFSD_MAY_SECINFO);
if (err)
return err;
err = nfsd_lookup_dentry(rqstp, &cstate->current_fh,
secinfo->si_name, secinfo->si_namelen,
- &exp, &dentry);
+ &exp, &dentry, true);
if (err)
return err;
if (d_really_is_negative(dentry)) {
@@ -1282,7 +1309,7 @@ nfsd4_secinfo_no_name(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstat
case NFS4_SECINFO_STYLE4_CURRENT_FH:
break;
case NFS4_SECINFO_STYLE4_PARENT:
- err = nfsd4_do_lookupp(rqstp, &cstate->current_fh);
+ err = nfsd4_do_lookupp(rqstp, &cstate->current_fh, true);
if (err)
return err;
break;
@@ -1491,6 +1518,11 @@ nfsd4_verify_copy(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (!cstate->save_fh.fh_dentry)
return nfserr_nofilehandle;
+ /* A cached open stateid does not authorize the saved source export. */
+ status = check_nfsd_access(cstate->save_fh.fh_export, rqstp);
+ if (status)
+ return nfserr_acces;
+
status = nfs4_preprocess_stateid_op(rqstp, cstate, &cstate->save_fh,
src_stateid, RD_STATE, src, NULL);
if (status)
@@ -2532,6 +2564,12 @@ nfsd4_copy_notify(struct svc_rqst *rqstp, struct nfsd4_compound_state *cstate,
if (!stid)
return nfserr_bad_stateid;
+ status = check_nfsd_access(cstate->current_fh.fh_export, rqstp);
+ if (status) {
+ status = nfserr_acces;
+ goto out;
+ }
+
cn->cpn_lease_time.tv_sec = nn->nfsd4_lease;
cn->cpn_lease_time.tv_nsec = 0;
@@ -3237,32 +3275,38 @@ static bool need_wrongsec_check(struct svc_rqst *rqstp)
struct nfsd4_compoundres *resp = rqstp->rq_resp;
struct nfsd4_compoundargs *argp = rqstp->rq_argp;
struct nfsd4_op *this = &argp->ops[resp->opcnt - 1];
- struct nfsd4_op *next = &argp->ops[resp->opcnt];
+ struct nfsd4_op *next;
const struct nfsd4_operation *thisd = OPDESC(this);
const struct nfsd4_operation *nextd;
+ unsigned int i;
- /*
- * Most ops check wronsec on our own; only the putfh-like ops
- * have special rules.
- */
+ /* Only put-filehandle operations need a deferred WRONGSEC check. */
if (!(thisd->op_flags & OP_IS_PUTFH_LIKE))
return false;
+
/*
- * rfc 5661 2.6.3.1.1.6: don't bother erroring out a
- * put-filehandle operation if we're not going to use the
- * result:
+ * RFC 8881 2.6.3.1.1.1: ignore SAVEFH when choosing the operation
+ * that determines whether the put-filehandle operation checks WRONGSEC.
*/
- if (argp->opcnt == resp->opcnt)
+ for (i = resp->opcnt; i < argp->opcnt; i++) {
+ next = &argp->ops[i];
+ if (next->opnum != OP_SAVEFH)
+ break;
+ }
+
+ /* RFC 8881 2.6.3.1.1.6: the filehandle is not used. */
+ if (i == argp->opcnt)
return false;
+
if (next->opnum == OP_ILLEGAL)
return false;
nextd = OPDESC(next);
/*
- * Rest of 2.6.3.1.1: certain operations will return WRONGSEC
- * errors themselves as necessary; others should check for them
- * now:
+ * RFC 8881 2.6.3.1.1.2: a later put-filehandle operation replaces
+ * this one. Other marked operations check WRONGSEC themselves.
*/
- return !(nextd->op_flags & OP_HANDLES_WRONGSEC);
+ return !(nextd->op_flags &
+ (OP_HANDLES_WRONGSEC | OP_IS_PUTFH_LIKE));
}
#ifdef CONFIG_NFSD_V4_2_INTER_SSC
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index f2b52ba90..3a12aeb56 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -6967,7 +6967,7 @@ nfsd4_verify_deleg_dentry(struct nfsd4_open *open, struct nfs4_file *fp,
err = nfsd_lookup_dentry(open->op_rqstp, parent,
open->op_fname, open->op_fnamelen,
- &exp, &child);
+ &exp, &child, false);
if (err)
return -ESTALE;
@@ -8486,6 +8486,12 @@ nfs4_check_file(struct svc_rqst *rqstp, struct svc_fh *fhp, struct nfs4_stid *s,
nf = nfs4_find_file(s, flags);
if (nf) {
+ /* A cached file still needs the current export security check. */
+ status = check_nfsd_access(fhp->fh_export, rqstp);
+ if (status) {
+ nfsd_file_put(nf);
+ return nfserr_acces;
+ }
status = nfsd_permission(&rqstp->rq_cred,
fhp->fh_export, fhp->fh_dentry,
acc | NFSD_MAY_OWNER_OVERRIDE);
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 4d9834446..daf2ad77d 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -4677,6 +4677,9 @@ nfsd4_encode_entry4_fattr(struct nfsd4_readdir *cd, const char *name,
* At least, add a comment here to explain....
*/
nfserr = nfsd_cross_mnt(cd->rd_rqstp, &dentry, &exp);
+ if (nfserr)
+ goto out_put;
+ nfserr = check_nfsd_access(exp, cd->rd_rqstp);
if (nfserr)
goto out_put;
crossed = true;
@@ -4775,8 +4778,11 @@ nfsd4_encode_entry4(void *ccdv, const char *name, int namlen,
* then in accordance with the spec, we fail the
* entire READDIR operation(!)
*/
- if (!(cd->rd_bmval[0] & FATTR4_WORD0_RDATTR_ERROR))
+ if (!(cd->rd_bmval[0] & FATTR4_WORD0_RDATTR_ERROR)) {
+ if (nfserr == nfserr_wrongsec)
+ nfserr = nfserr_acces;
goto fail;
+ }
if (nfsd4_encode_entry4_rdattr_error(xdr, nfserr)) {
nfserr = nfserr_toosmall;
goto fail;
diff --git a/fs/nfsd/nfsfh.c b/fs/nfsd/nfsfh.c
index f1ba5a14b..901948f0b 100644
--- a/fs/nfsd/nfsfh.c
+++ b/fs/nfsd/nfsfh.c
@@ -471,9 +471,19 @@ __fh_verify(struct svc_rqst *rqstp,
&& exp->ex_path.dentry == dentry)
may_bypass_gss = true;
- error = check_security_flavor(exp, rqstp, may_bypass_gss);
- if (error)
- goto out;
+ /*
+ * SECINFO reports an export's RPC security flavors, and RFC 8881
+ * section 2.6.3.1.1.5 forbids it from returning NFS4ERR_WRONGSEC, so
+ * it must succeed even when the caller's flavor is not one the export
+ * accepts. Skip only the flavor check for SECINFO. The export's
+ * xprtsec= policy constrains the transport security mode and stays
+ * enforced above, independently of the RPC flavor negotiation.
+ */
+ if (!(access & NFSD_MAY_SECINFO)) {
+ error = check_security_flavor(exp, rqstp, may_bypass_gss);
+ if (error)
+ goto out;
+ }
svc_xprt_set_valid(rqstp->rq_xprt);
diff --git a/fs/nfsd/trace.h b/fs/nfsd/trace.h
index 951af645d..a7d9f9d7b 100644
--- a/fs/nfsd/trace.h
+++ b/fs/nfsd/trace.h
@@ -142,7 +142,8 @@ DEFINE_NFSD_DYNTHREAD_EVENT(trylock_fail);
{ NFSD_MAY_BYPASS_GSS, "BYPASS_GSS" }, \
{ NFSD_MAY_READ_IF_EXEC, "READ_IF_EXEC" }, \
{ NFSD_MAY_64BIT_COOKIE, "64BIT_COOKIE" }, \
- { NFSD_MAY_LOCALIO, "LOCALIO" })
+ { NFSD_MAY_LOCALIO, "LOCALIO" }, \
+ { NFSD_MAY_SECINFO, "SECINFO" })
TRACE_EVENT(nfsd_compound,
TP_PROTO(
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index f59a55f89..0012c68d6 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -124,14 +124,14 @@ nfserrno(int errno)
}
/*
- * Called from nfsd_lookup and encode_dirent. Check if we have crossed
+ * Called from lookup, SECINFO, and encode_dirent. Check if we have crossed
* a mount point.
* Returns an nfs error leaving *dpp and *expp unchanged,
* or nfs_ok having possibly changed *dpp and *expp
*/
-__be32
-nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
- struct svc_export **expp)
+static __be32
+nfsd_cross_mnt_common(struct svc_rqst *rqstp, struct dentry **dpp,
+ struct svc_export **expp, bool for_secinfo)
{
struct svc_export *exp = *expp, *exp2 = NULL;
struct dentry *dentry = *dpp;
@@ -169,7 +169,9 @@ nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
err = 0;
} else if (nfsd_v4client(rqstp) ||
(exp->ex_flags & NFSEXP_CROSSMOUNT) || EX_NOHIDE(exp2)) {
- nfserr = check_nfsd_access(exp, rqstp);
+ /* SECINFO needs the target's flavors even if the source differs. */
+ if (!for_secinfo)
+ nfserr = check_nfsd_access(exp, rqstp);
if (nfserr == nfs_ok) {
/* successfully crossed mount point */
/*
@@ -193,6 +195,13 @@ nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
return nfserrno(err);
}
+__be32
+nfsd_cross_mnt(struct svc_rqst *rqstp, struct dentry **dpp,
+ struct svc_export **expp)
+{
+ return nfsd_cross_mnt_common(rqstp, dpp, expp, false);
+}
+
static void follow_to_parent(struct path *path)
{
struct dentry *dp;
@@ -255,7 +264,8 @@ int nfsd_mountpoint(struct dentry *dentry, struct svc_export *exp)
__be32
nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
const char *name, unsigned int len,
- struct svc_export **exp_ret, struct dentry **dentry_ret)
+ struct svc_export **exp_ret, struct dentry **dentry_ret,
+ bool for_secinfo)
{
struct svc_export *exp;
struct dentry *dparent;
@@ -288,7 +298,8 @@ nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
if (IS_ERR(dentry))
goto out_nfserr;
if (nfsd_mountpoint(dentry, exp)) {
- __be32 nfserr = nfsd_cross_mnt(rqstp, &dentry, &exp);
+ __be32 nfserr = nfsd_cross_mnt_common(rqstp,
+ &dentry, &exp, for_secinfo);
if (nfserr) {
dput(dentry);
@@ -297,6 +308,23 @@ nfsd_lookup_dentry(struct svc_rqst *rqstp, struct svc_fh *fhp,
}
}
}
+ /*
+ * For SECINFO the caller's fh_verify() has already checked the
+ * starting export's xprtsec= policy. A mount crossing or parent
+ * traversal above can switch to a different export; since
+ * svc_export objects are immutable once published, re-check the
+ * transport policy only for that new export. The flavor is left
+ * for SECINFO to report (RFC 8881 section 2.6.3.1.1.5).
+ */
+ if (for_secinfo && exp != fhp->fh_export) {
+ __be32 nfserr = check_xprtsec_policy(exp, rqstp);
+
+ if (nfserr) {
+ dput(dentry);
+ exp_put(exp);
+ return nfserr;
+ }
+ }
*dentry_ret = dentry;
*exp_ret = exp;
return 0;
@@ -337,9 +365,13 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
err = fh_verify(rqstp, fhp, S_IFDIR, NFSD_MAY_EXEC);
if (err)
return err;
- err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry);
+ err = nfsd_lookup_dentry(rqstp, fhp, name, len, &exp, &dentry,
+ false);
if (err)
return err;
+ err = check_nfsd_access(exp, rqstp);
+ if (err)
+ goto out;
/*
* Note: we compose the file handle now, but as the
* dentry may be negative, it may need to be updated.
@@ -348,6 +380,7 @@ nfsd_lookup(struct svc_rqst *rqstp, struct svc_fh *fhp, const char *name,
if (!err && d_really_is_negative(dentry))
err = nfserr_noent;
+out:
dput(dentry);
exp_put(exp);
return err;
diff --git a/fs/nfsd/vfs.h b/fs/nfsd/vfs.h
index 1f05c83a3..e7724628f 100644
--- a/fs/nfsd/vfs.h
+++ b/fs/nfsd/vfs.h
@@ -33,6 +33,7 @@
#define NFSD_MAY_64BIT_COOKIE 0x1000 /* 64 bit readdir cookies for >= NFSv3 */
#define NFSD_MAY_LOCALIO 0x2000 /* for tracing, reflects when localio used */
+#define NFSD_MAY_SECINFO 0x4000 /* query export security policy */
#define NFSD_MAY_CREATE (NFSD_MAY_EXEC|NFSD_MAY_WRITE)
#define NFSD_MAY_REMOVE (NFSD_MAY_EXEC|NFSD_MAY_WRITE|NFSD_MAY_TRUNC)
@@ -133,7 +134,8 @@ __be32 nfsd_lookup(struct svc_rqst *, struct svc_fh *,
const char *, unsigned int, struct svc_fh *);
__be32 nfsd_lookup_dentry(struct svc_rqst *, struct svc_fh *,
const char *, unsigned int,
- struct svc_export **, struct dentry **);
+ struct svc_export **, struct dentry **,
+ bool for_secinfo);
__be32 nfsd_setattr(struct svc_rqst *, struct svc_fh *,
struct nfsd_attrs *, const struct timespec64 *);
int nfsd_mountpoint(struct dentry *, struct svc_export *);
--
2.43.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v4] nfsd: enforce export security for NFSv4 filehandles and stateids
2026-10-29 6:39 ` [PATCH v4] " VeroFess
@ 2026-10-29 15:21 ` Chuck Lever
2026-10-29 21:33 ` VeroFess
0 siblings, 1 reply; 8+ messages in thread
From: Chuck Lever @ 2026-10-29 15:21 UTC (permalink / raw)
To: VeroFess; +Cc: jlayton, neil, okorniev, Dai.Ngo, tom, linux-nfs
On 10/29/26 2:39 AM, VeroFess wrote:
> An AUTH_SYS NFSv4.0 client can read or write a file on a sec=krb5p
> export using another client's valid OPEN stateid by inserting
> SAVEFH between PUTFH and READ or WRITE.
The v4 revision of your patch appears to combine the original fix with
several unrelated changes to SECINFO, READDIR, LOOKUP, and stateid
processing. That makes it unlikely to apply to the stable kernels
that need the original fix. Let's split the single patch into a
series.
1. The need_wrongsec_check() change by itself, with
"Fixes: 68d93184352f". need_wrongsec_check() has not changed since
v5.10, so this patch should apply cleanly to every LTS kernel. As
far as I can tell it is enough to stop the PUTFH; SAVEFH; READ/WRITE
sequence in your report. Please confirm that with your reproducer
on a kernel that has only this change.
2. The check_nfsd_access() call in nfsd4_verify_copy(), which covers
both COPY and CLONE. RFC 8881 Section 2.6.3.1.1 forbids PUTFH from
returning NFS4ERR_WRONGSEC in PUTFH; SAVEFH; PUTFH, so the source
handle can reach COPY or CLONE unchecked even after patch 1.
3. Anything else, one change per patch, each with its own rationale and
only the Fixes: tags that match its code. These do not need to go to
stable.
Or, if you'd like to move on, I can take things from here and simply
ask you to confirm that any new series addresses your issue and no
longer reproduces.
> COPY_NOTIFY validates a stateid without acquiring an nfsd_file, so it
> misses that check. After an export was changed to sec=krb5p, an
> AUTH_SYS COPY_NOTIFY using an existing stateid still issued a copy
> stateid although AUTH_SYS READ was denied.
COPY_NOTIFY does not carry OP_HANDLES_WRONGSEC, so in PUTFH;
COPY_NOTIFY the PUTFH already gets the deferred check_nfsd_access()
call in nfsd4_proc_compound(). That check uses the same fh_export that
READ uses, and the PUTFH holds that export for the rest of the
compound. How can READ be denied while COPY_NOTIFY succeeds? If the
test compound included SAVEFH, patch 1 covers it. Unless there is
another path, I would drop the nfs4_check_file() and COPY_NOTIFY
hunks.
> Pass NFSD_MAY_SECINFO to fh_verify() so it skips the starting export's
> security flavor check.
With this change, a client using any flavor can learn whether a name
exists in a directory on a sec=krb5p export. RFC 8881 Section
2.6.3.1.1.5 allows that, and your patch description should clarify
that the code behavior is changed to conform with the spec.
Also, the new check_xprtsec_policy() call in nfsd_lookup_dentry()
returns nfserr_wrongsec from SECINFO when it crosses into an export
with a different xprtsec= policy. Is that not the NFS4ERR_WRONGSEC
return from SECINFO that the description says is forbidden?
> When rdattr_error is not requested, return NFS4ERR_ACCESS for a flavor
> mismatch; NFS4ERR_WRONGSEC is not a READDIR status.
On a pseudo-root export, nfsd4_encode_entry4_fattr() always crosses
into child exports. A client that does not request rdattr_error now
gets NFS4ERR_ACCESS for the whole READDIR of the pseudo-root whenever
any child export rejects its flavor. Could the encoder fall back to
returning the mountpoint's own attributes instead?
The new check_nfsd_access() call in nfsd_lookup() fixes a mount
crossing that never checks the child export. The CREATE paths do the
same crossing: NFSv3 CREATE UNCHECKED, NFSv2 CREATE, and NFSv4 OPEN
UNCHECKED all call nfsd_cross_mnt() and then fh_compose() on the child.
If this check goes in, it probably belongs in nfsd_cross_mnt() so that
every caller gets it.
Longer term, I would rather not keep adding check_nfsd_access() calls
to individual operations. A filehandle whose security check PUTFH
deferred could instead carry a "check pending" state that the COMPOUND
loop enforces before any operation consumes it. That can come later,
it does not need to hold up these fixes.
--
Chuck Lever (Come to NFS bake-a-thon! https://nfsv4bat.org)
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v4] nfsd: enforce export security for NFSv4 filehandles and stateids
2026-10-29 15:21 ` Chuck Lever
@ 2026-10-29 21:33 ` VeroFess
0 siblings, 0 replies; 8+ messages in thread
From: VeroFess @ 2026-10-29 21:33 UTC (permalink / raw)
To: cel; +Cc: jlayton, neil, okorniev, Dai.Ngo, tom, linux-nfs
Hi Chuck,
Thank you very much for offering to help develop the patches. I would
like to take you up on that: I am happy to run follow-up tests and
confirm the resulting series, but I am not in a position to develop
the split patches myself. I ran isolated tests to answer your
questions; results are inline below.
> The v4 revision of your patch appears to combine the original fix
> with several unrelated changes to SECINFO, READDIR, LOOKUP, and
> stateid processing. [...] Let's split the single patch into a series.
Agreed that it should be split.
> 1. The need_wrongsec_check() change by itself, with
> "Fixes: 68d93184352f". [...] Please confirm that with your
> reproducer on a kernel that has only this change.
Confirmed. I built the v4 parent plus only the need_wrongsec_check()
change, with no other NFSD change. Reusing a live krb5p OPEN stateid
over an AUTH_SYS connection, PUTFH; SAVEFH; READ and PUTFH; SAVEFH;
WRITE both returned NFS4ERR_WRONGSEC at PUTFH, the file was unchanged,
and the krb5p owner could still read it. That change alone is enough
for the reported sequence.
> 2. The check_nfsd_access() call in nfsd4_verify_copy(), which covers
> both COPY and CLONE. [...] the source handle can reach COPY or
> CLONE unchecked even after patch 1.
Reproduced on the patch-1-only kernel: AUTH_SYS direct READ of the
krb5p source returned WRONGSEC, but PUTFH; SAVEFH; PUTFH; COPY
completed and wrote the source bytes to the destination. Adding only
the nfsd4_verify_copy() check made COPY return NFS4ERR_ACCESS and left
the destination unchanged. CLONE is NFS4ERR_NOTSUPP on this ext4
fixture with or without the check, so I could not demonstrate a
reflink; its justification is the shared source-check path with COPY.
So this is a separate concern from patch 1.
> 3. Anything else, one change per patch, each with its own rationale
> and only the Fixes: tags that match its code. These do not need
> to go to stable.
Understood; I have kept each finding below as its own separate
concern.
> COPY_NOTIFY does not carry OP_HANDLES_WRONGSEC, so in PUTFH;
> COPY_NOTIFY the PUTFH already gets the deferred check_nfsd_access()
> call in nfsd4_proc_compound(). [...] How can READ be denied while
> COPY_NOTIFY succeeds? [...] Unless there is another path, I would
> drop the nfs4_check_file() and COPY_NOTIFY hunks.
You are right. That result came from an intermediate tree, before the
selected-export check on LOOKUP; both compounds used PUTROOTFH; LOOKUP,
and only READ took the nfs4_check_file() path. I rebuilt v4 with just
those two guards removed: with a retained stateid after the export
changed to krb5p, READ and COPY_NOTIFY were both rejected with
NFS4ERR_WRONGSEC before the operation, in the direct, SAVEFH, LOOKUP,
and RESTOREFH paths. I found no PUTFH; COPY_NOTIFY path that patch 1
misses, so those two hunks and the COPY_NOTIFY claim do not look
needed.
> With this change, a client using any flavor can learn whether a name
> exists in a directory on a sec=krb5p export. RFC 8881 Section
> 2.6.3.1.1.5 allows that, and your patch description should clarify
> that the code behavior is changed to conform with the spec.
Confirmed. With directory traversal permission, AUTH_SYS SECINFO on a
krb5p export returned OK for an existing name and NOENT for a missing
one; without traversal permission both returned NFS4ERR_ACCESS. I
agree the description should state this is an intentional,
spec-conforming part of security negotiation.
> Also, the new check_xprtsec_policy() call in nfsd_lookup_dentry()
> returns nfserr_wrongsec from SECINFO when it crosses into an export
> with a different xprtsec= policy. Is that not the NFS4ERR_WRONGSEC
> return from SECINFO that the description says is forbidden?
It is, and that is a real inconsistency in v4. Plaintext SECINFO
crossing into an xprtsec=tls child returned NFS4ERR_WRONGSEC on
NFSv4.0/4.1/4.2. I also tested a variant that maps a transport-policy
mismatch to NFS4ERR_ACCESS (RFC 8881 Section 18.29.3) at the starting
and selected-export checks: SECINFO then returned ACCESS instead of
WRONGSEC, while ordinary LOOKUP still returned WRONGSEC. That test
used a plaintext connection, not TLS. The unqualified "forbidden"
wording in my description is wrong either way.
> On a pseudo-root export, nfsd4_encode_entry4_fattr() always crosses
> into child exports. [...] Could the encoder fall back to returning
> the mountpoint's own attributes instead?
I tried that. The unrestricted fallback made size-only READDIR
succeed, but when the client requested FILEHANDLE it received the
covered mountpoint's handle and could then list the hidden underlay
(confirmed with a sentinel placed under the mount). So the
unrestricted fallback is not safe. A restricted variant that falls
back only when FILEHANDLE is not requested did behave: size-only
READDIR succeeded; a FILEHANDLE request still returned ACCESS; an
rdattr_error request reported WRONGSEC in the child entry; direct
LOOKUP stayed WRONGSEC.
> The new check_nfsd_access() call in nfsd_lookup() fixes a mount
> crossing that never checks the child export. The CREATE paths do the
> same crossing [...] If this check goes in, it probably belongs in
> nfsd_cross_mnt() so that every caller gets it.
Agreed, and the CREATE gap is real. On v4, AUTH_SYS NFSv2 CREATE and
NFSv3 CREATE UNCHECKED returned a filehandle from the stricter child
export (a bind-mounted single-file export); NFSv4.0/4.1/4.2 OPEN
UNCHECKED already returned WRONGSEC at OPEN. Moving the selected-export
check into nfsd_cross_mnt_common() made NFSv2/NFSv3 CREATE return
ACCESS while ordinary CREATE in the same parent still succeeded, so
the check does look like it belongs on the common mount-crossing path,
with the SECINFO exception preserved.
> Longer term, I would rather not keep adding check_nfsd_access()
> calls to individual operations. A filehandle whose security check
> PUTFH deferred could instead carry a "check pending" state [...]
> That can come later, it does not need to hold up these fixes.
That design makes sense to me, and I agree it need not block the
split.
Please go ahead and take things from here. I will confirm that the
resulting series addresses the issue and no longer reproduces, and run
any other tests you would like.
Thanks,
VeroFess
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-10-29 21:34 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <SY9P300MB153090355AAD017CD7216634A6602@SY9P300MB1530.AUSP300.PROD.OUTLOOK.COM>
[not found] ` <IA1PR12MB662781C8C09627A5CDEBF39DF4602@IA1PR12MB6627.namprd12.prod.outlook.com>
2026-10-27 15:41 ` [PATCH] nfsd: prevent SAVEFH from bypassing export security Chuck Lever
[not found] ` <IA1PR12MB662788BBD70D4064432FD117F4602@IA1PR12MB6627.namprd12.prod.outlook.com>
2026-10-28 14:09 ` Chuck Lever
2026-10-28 13:57 ` Jeff Layton
2026-10-29 1:36 ` [PATCH v2] nfsd: enforce export security for NFSv4 filehandles and stateids VeroFess
2026-10-29 5:14 ` [PATCH v3] " VeroFess
2026-10-29 6:39 ` [PATCH v4] " VeroFess
2026-10-29 15:21 ` Chuck Lever
2026-10-29 21:33 ` VeroFess
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox