On 23 Oct 2018, at 12:09, Trond Myklebust wrote:
> On Tue, 2018-10-23 at 08:40 -0700, Matthew Wilcox wrote:
>> On Tue, Oct 23, 2018 at 10:34:57AM -0500, Frank Sorenson wrote:
>>>
>>> The filehandle has a length which is defined as a 32-bit
>>> "unsigned integer". Change sign of the length appropriately.
>>>
>>> Signed-off-by: Frank Sorenson <sorenson@xxxxxxxxxx>
>>
>> Is this a cleanup or does it fix a user-visible bug?
>
> It fixes the following comparison:
>
> if (len > NFS4_FHSIZE)
> return -EIO;
>
> but in practice, the next line should always catch the buffer overflow
> when len is negative:
>
> p = xdr_inline_decode(xdr, len);
> if (unlikely(!p))
> goto out_overflow;
Maybe I am missing something, but if we're depending on:
static __be32 * __xdr_inline_decode(struct xdr_stream *xdr, size_t nbytes)
{
unsigned int nwords = XDR_QUADLEN(nbytes);
__be32 *p = xdr->p;
__be32 *q = p + nwords;
if (unlikely(nwords > xdr->nwords || q > xdr->end || q < p))
return NULL;
and nbytes is 0xffffffff, then nwords ends up being 0.. so this if statement
could easily miss it.
Ben
