On Sat, Nov 13, 2021 at 09:06:03PM +0000, Chuck Lever III wrote:
>
> > On Nov 13, 2021, at 3:58 PM, rtm@xxxxxxxxxxxxx wrote:
> >
> > 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().
>
> Thanks, I'll look into addressing this for v5.16-rc.
>
> By the way, can you tell if this exposure was in the code
> before 2548aa784d76 ("NFSD: Add a separate decoder to handle
> state_protect_ops") ? (ie, do we need a separate fix for
> this for pre-5.11 NFSD -- I'm guessing no).
It may not have been an EXCHANGE_ID problem, but:
> Is the current implementation of nfsd4_decode_bitmap() a
> problem for its other consumers?
Yeah, I don't see that there's anything a caller could do that would
prevent it, so the problem starts with the introduction of
nfsd4_decode_bitmap4.
Not actually tested, but I suppose we want the following.
--b.
commit 8211c4817cc0
Author: J. Bruce Fields <bfields@xxxxxxxxxx>
Date: Sat Nov 13 16:11:58 2021 -0500
nfsd: fix overrun in nfsd4_decode_bitmap4
rtm says: "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()."
Reported-by: <rtm@xxxxxxxxxxxxx>
Cc: stable@xxxxxxxxxxxxxxx
Fixes: d1c263a031e8 "NFSD: Replace READ* macros in nfsd4_decode_fattr()"
Signed-off-by: J. Bruce Fields <bfields@xxxxxxxxxx>
diff --git a/fs/nfsd/nfs4xdr.c b/fs/nfsd/nfs4xdr.c
index 9b609aac47e1..7aa97c09b5a9 100644
--- a/fs/nfsd/nfs4xdr.c
+++ b/fs/nfsd/nfs4xdr.c
@@ -282,8 +282,7 @@ nfsd4_decode_bitmap4(struct nfsd4_compoundargs *argp, u32 *bmval, u32 bmlen)
if (xdr_stream_decode_u32(argp->xdr, &count) < 0)
return nfserr_bad_xdr;
- /* request sanity */
- if (count > 1000)
+ if (count > bmlen)
return nfserr_bad_xdr;
p = xdr_inline_decode(argp->xdr, count << 2);
if (!p)
