Thread (4 messages) 4 messages, 3 authors, 2021-11-15
STALE1656d

[PATCH v1] NFSD: Fix exposure in nfsd4_decode_bitmap()

From: Chuck Lever <chuck.lever@oracle.com>
Date: 2021-11-14 20:16:10
Subsystem: filesystems (vfs and infrastructure), kernel nfsd, sunrpc, and lockd servers, the rest · Maintainers: Alexander Viro, Christian Brauner, Chuck Lever, Jeff Layton, Linus Torvalds

rtm@csail.mit.edu reports:
nfsd4_decode_bitmap4() will write beyond bmval[bmlen-1] if the RPC
directs it to do so. This can cause nfsd4_decode_state_protect4_a()
to write client-supplied data beyond the end of
nfsd4_exchange_id.spo_must_allow[] when called by
nfsd4_decode_exchange_id().
Rewrite the loops so nfsd4_decode_bitmap() cannot iterate beyond
@bmlen.

Reported by: rtm@csail.mit.edu
Fixes: d1c263a031e8 ("NFSD: Replace READ* macros in nfsd4_decode_fattr()")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
---
 fs/nfsd/nfs4xdr.c |    7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

Hi Bruce-

This version is cleaned up slightly and has been tested as follows:

- I am not able to crash a patched server using rtm's nfsd_1
- No new FAILUREs with NFSv4.1 pynfs tests
- No new failures with xfstests on NFSv4.1 or NFSv4.2
- No new failures with the git regression suite on NFSv4.1 or NFSv4.2
- No reports of NFS4ERR_BADXDR or GARBAGE_ARGS during xfs or git regr

Hopefully that exercises enough uses of nfsd4_decode_bitmap() to be
confident that it is working properly.

I tested this on top of my XDR tracepoint series, so the line numbers
might be a little off from your current tree. However, it should
otherwise apply cleanly.
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 5ff481e9c85d..479d3452ce6c 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -288,11 +288,8 @@ nfsd4_decode_bitmap4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen)
 	p = xdr_inline_decode(argp->xdr, count << 2);
 	if (!p)
 		return nfserr_bad_xdr;
-	i = 0;
-	while (i < count)
-		bmval[i++] = be32_to_cpup(p++);
-	while (i < bmlen)
-		bmval[i++] = 0;
+	for (i = 0; i < bmlen; i++)
+		bmval[i] = (i < count) ? be32_to_cpup(p++) : 0;
 
 	return nfs_ok;
 }
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help