All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chuck Lever <chuck.lever@oracle.com>
To: bfields@redhat.com
Cc: linux-nfs@vger.kernel.org
Subject: [PATCH v1] NFSD: Fix exposure in nfsd4_decode_bitmap()
Date: Sun, 14 Nov 2021 15:16:04 -0500	[thread overview]
Message-ID: <163692036074.16710.5678362976688977923.stgit@klimt.1015granger.net> (raw)

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;
 }


             reply	other threads:[~2021-11-14 20:16 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-14 20:16 Chuck Lever [this message]
2021-11-14 21:57 ` [PATCH v1] NFSD: Fix exposure in nfsd4_decode_bitmap() J. Bruce Fields
2021-11-14 21:58 ` Trond Myklebust
2021-11-15 14:00   ` Chuck Lever III

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=163692036074.16710.5678362976688977923.stgit@klimt.1015granger.net \
    --to=chuck.lever@oracle.com \
    --cc=bfields@redhat.com \
    --cc=linux-nfs@vger.kernel.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.