Re: [PATCH] NFSD: prevent integer overflow on 32 bit systems

[Dan Carpenter <dan.carpenter@oracle.com>]

On Mon, Mar 14, 2022 at 05:45:59PM +0300, Chuck Lever III wrote:
> Hi Dan-
> 
> > On Mar 14, 2022, at 10:09 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> > 
> > On a 32 bit system, the "len * sizeof(*p)" operation can have an
> > integer overflow.
> > 
> > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> > ---
> > It's hard to pick a Fixes tag for this...  The temptation is to say:
> > Fixes: 37c88763def8 ("NFSv4; Clean up XDR encoding of type bitmap4")
> > But there were integer overflows in the code before that as well.
> > 
> > include/linux/sunrpc/xdr.h | 2 ++
> > 1 file changed, 2 insertions(+)
> > 
> > diff --git a/include/linux/sunrpc/xdr.h b/include/linux/sunrpc/xdr.h
> > index b519609af1d0..61b92e6b9813 100644
> > --- a/include/linux/sunrpc/xdr.h
> > +++ b/include/linux/sunrpc/xdr.h
> > @@ -731,6 +731,8 @@ xdr_stream_decode_uint32_array(struct xdr_stream *xdr,
> > 
> > 	if (unlikely(xdr_stream_decode_u32(xdr, &len) < 0))
> > 		return -EBADMSG;
> > +	if (len > ULONG_MAX / sizeof(*p))
> > +		return -EBADMSG;
> 
> IIUC xdr_inline_decode() returns NULL if the value of
> "len * sizeof(p)" is larger than the remaining XDR buffer
> size. I don't believe this extra check is necessary.
> 

Yes, but because of the integer overflow then "len * sizeof(*p))" will
be a very reasonable small number.

regards,
dan carpenter

> 
> > 	p = xdr_inline_decode(xdr, len * sizeof(*p));
> > 	if (unlikely(!p))
> > 		return -EBADMSG;
> > -- 
> > 2.20.1
> > 
> 
> --
> Chuck Lever
> 
>