[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 14 Nov 2013 17:52:13 +0100
From: Petr Matousek <pmatouse@...hat.com>
To: Dan Carpenter <dan.carpenter@...cle.com>
Cc: Nico Golde <oss-security+ml@...lde.de>, oss-security@...ts.openwall.com,
security@...nel.org, "Hans J. Koch" <hjk@...sjkoch.de>
Subject: Re: some unstracked linux kernel security fixes
On Thu, Nov 14, 2013 at 04:25:39PM +0300, Dan Carpenter wrote:
> 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
Is there any rounding down happening? I would expect both vma->vm_start
and vma->vm_end to be page aligned.
> 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.
Imagine addr and size are not page aligned and
((addr & ~PAGE_MASK) + (size & ~PAGE_MASK)) > PAGE_SIZE.
We need to round up two pages instead of one in that case.
> So, yeah, it seems like we do check the size twice now except the first
> time we do it wrong.
With unaligned addr and/or size we can end up with mapping memory
not belonging to the UIO_MEM_PHYS registered region, but that is something
you expect when using this interface from the drivers and/or userspace,
because you want access to the whole region to properly handle the
device, no?
IOW, with the current changes, isn't the functionality broken for
non page-aligned addr and/or size?
--
Petr Matousek / Red Hat Security Response Team
PGP: 0xC44977CA 8107 AF16 A416 F9AF 18F3 D874 3E78 6F42 C449 77CA
Powered by blists - more mailing lists
Please check out the
Open Source Software Security Wiki, which is counterpart to this
mailing list.
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
