[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20131203034029.GA8807@kroah.com>
Date: Mon, 2 Dec 2013 19:40:29 -0800
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: "Hans J. Koch" <hjk@...sjkoch.de>,
Nico Golde <oss-security+ml@...lde.de>,
oss-security@...ts.openwall.com, security@...nel.org,
Dan Carpenter <dan.carpenter@...cle.com>
Subject: Re: kernel: uio: CVE-2013-6763 [was: Re: some
unstracked linux kernel security fixes]
On Tue, Nov 26, 2013 at 01:18:39PM +0100, Petr Matousek wrote:
> Adding Greg as he's also UIO maintainer (at least according to
> MAINTAINERS).
>
> On Thu, Nov 14, 2013 at 05:52:12PM +0100, Petr Matousek wrote:
> > 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?
This should now be fixed in Linus's tree, right?
thanks,
greg k-h
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.
