linux-kernel - [PATCH RFC] nfsd: don't hand out write delegations on O

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230731-wdeleg-v1-1-f8fe1ce11b36@kernel.org>
Date:   Mon, 31 Jul 2023 16:27:30 -0400
From:   Jeff Layton <jlayton@...nel.org>
To:     Chuck Lever <chuck.lever@...cle.com>, Neil Brown <neilb@...e.de>,
        Olga Kornievskaia <kolga@...app.com>,
        Dai Ngo <Dai.Ngo@...cle.com>, Tom Talpey <tom@...pey.com>
Cc:     linux-nfs@...r.kernel.org, linux-kernel@...r.kernel.org,
        Jeff Layton <jlayton@...nel.org>
Subject: [PATCH RFC] nfsd: don't hand out write delegations on O_WRONLY
 opens

I noticed that xfstests generic/001 was failing against linux-next nfsd.

The client would request a OPEN4_SHARE_ACCESS_WRITE open, and the server
would hand out a write delegation. The client would then try to use that
write delegation as the source stateid in a COPY or CLONE operation, and
the server would respond with NFS4ERR_STALE.

The problem is that the struct file associated with the delegation does
not necessarily have read permissions. It's handing out a write
delegation on what is effectively an O_WRONLY open. RFC 8881 states:

 "An OPEN_DELEGATE_WRITE delegation allows the client to handle, on its
  own, all opens."

Given that the client didn't request any read permissions, and that nfsd
didn't check for any, it seems wrong to give out a write delegation.

Don't hand out a delegation if the client didn't request
OPEN4_SHARE_ACCESS_BOTH.

This fixes xfstest generic/001.

Closes: https://bugzilla.linux-nfs.org/show_bug.cgi?id=412
Signed-off-by: Jeff Layton <jlayton@...nel.org>
---
 fs/nfsd/nfs4state.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index ef7118ebee00..9f1c90afed72 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -5462,6 +5462,8 @@ nfs4_set_delegation(struct nfsd4_open *open, struct nfs4_ol_stateid *stp,
 		return ERR_PTR(-EAGAIN);

 	if (open->op_share_access & NFS4_SHARE_ACCESS_WRITE) {
+		if (!(open->op_share_access & NFS4_SHARE_ACCESS_READ))
+			return ERR_PTR(-EBADF);
 		nf = find_writeable_file(fp);
 		dl_type = NFS4_OPEN_DELEGATE_WRITE;
 	} else {

---
base-commit: ec89391563792edd11d138a853901bce76d11f44
change-id: 20230731-wdeleg-bbdb6b25a3c6

Best regards,
-- 
Jeff Layton <jlayton@...nel.org>