Re: some unstracked linux kernel security fixes

[Dan Carpenter <dan.carpenter () oracle com>]

On Thu, Nov 14, 2013 at 11:33:10AM +0100, Petr Matousek wrote:
> On Tue, Nov 12, 2013 at 11:10:32AM +0100, Petr Matousek wrote:
> > Hi,
> >
> > On Sun, Nov 03, 2013 at 05:32:52PM +0100, Nico Golde wrote:
> > > drivers/uio/uio.c: mapping of physical memory to user space without proper size check
> > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=7314e613d5ff
> >
> > there is a size check in uio_mmap() (the only caller of uio_mmap_physical()):
> >
> >         requested_pages = vma_pages(vma);
> >         actual_pages = ((idev->info->mem[mi].addr & ~PAGE_MASK)
> >                         + idev->info->mem[mi].size + PAGE_SIZE -1) >> PAGE_SHIFT;
> >         if (requested_pages > actual_pages)
> >                 return -EINVAL;
> >
> > why it wasn't sufficient?
>
> Apparently there was a CVE split [1] and this is now CVE-2013-6763.
>
>   http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2013-6763
>
> I still think this is a non-issue based on the above mentioned size
> check. Can I please get second opinion from someone more knowledgeable
> on this?

Added Hans to the CC list since he's the maintainer.  Petr is asking if
the size checks in uio_mmap() and uio_mmap_physical() are duplicative.

> Isn't the size check redundant because of 
>
>         requested_pages = vma_pages(vma);
>         actual_pages = ((idev->info->mem[mi].addr & ~PAGE_MASK)
>                         + idev->info->mem[mi].size + PAGE_SIZE -1) >> PAGE_SHIFT;
>         if (requested_pages > actual_pages)
>                 return -EINVAL;

That check is worrying requested_pages is rounded down to the nearest
page but actual_pages is rounded up.  I don't understand why we are
adding "(mem[mi]addr % PAGE_SIZE)" to the pre rounded up actual_pages.

So, yeah, it seems like we do check the size twice now except the first
time we do it wrong.

regards,
dan carpenter