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@xxxxxxxxxx> wrote:
> >
> > On a 32 bit system, the "len * sizeof(*p)" operation can have an
> > integer overflow.
> >
> > Signed-off-by: Dan Carpenter <dan.carpenter@xxxxxxxxxx>
> > ---
> > 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
>
>
