> On Mar 14, 2022, at 1:03 PM, Dan Carpenter <dan.carpenter@xxxxxxxxxx> wrote:
>
> 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.
Got it.
> regards,
> dan carpenter
>
>>
>>> p = xdr_inline_decode(xdr, len * sizeof(*p));
>>> if (unlikely(!p))
>>> return -EBADMSG;
>>> --
>>> 2.20.1
>>>
>>
>> --
>> Chuck Lever
--
Chuck Lever
