Re: [PATCH] NFS: change sign of nfs_fh length

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux