linux-kernel - Re: [PATCH] Video/UVC: Port mainlined uvc video driver to NOMMU

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <200902051812.54504.laurent.pinchart@skynet.be>
Date:	Thu, 5 Feb 2009 18:12:54 +0100
From:	Laurent Pinchart <laurent.pinchart@...net.be>
To:	Bryan Wu <cooloney@...nel.org>
Cc:	Paul Mundt <lethal@...ux-sh.org>,
	"Hennerich, Michael" <Michael.Hennerich@...log.com>,
	linux-uvc-devel@...ts.berlios.de, video4linux-list@...hat.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] Video/UVC: Port mainlined uvc video driver to NOMMU

Hi Bryan,

On Thursday 05 February 2009, Bryan Wu wrote:
> On Tue, Nov 18, 2008 at 9:39 PM, Paul Mundt <lethal@...ux-sh.org> wrote:
> > On Fri, Nov 14, 2008 at 12:52:10AM +0100, Laurent Pinchart wrote:
> >> On Monday 10 November 2008, Hennerich, Michael wrote:
> >> > >On Thursday 06 November 2008, Bryan Wu wrote:
> >> > > > @@ -1071,7 +1072,20 @@ static int uvc_v4l2_mmap(struct file *file,
> >> > > > struct vm_area_struct *vma) addr += PAGE_SIZE;
> >> > > >                 size -= PAGE_SIZE;
> >> > > >         }
> >> > > > +#else
> >> > > > +       if (i == video->queue.count ||
> >> > > > +               PAGE_ALIGN(size) != video->queue.buf_size) {
> >> > >
> >> > > Just out of curiosity, why do you need to PAGE_ALIGN size for
> >> > > non-MMU platforms ?
> >> >
> >> > Size and video->queue.buf_size is not the 100% same size (off by a few
> >> > bytes < pagesize), I think it's because on NOMMU the kernel calls
> >> > kmalloc() to allocate the buffer, not get_free_page().
> >>
> >> That's right, but only for private mappings. It makes little sense to
> >> create a private mapping on a V4L2 device as the kernel will read()
> >> device data into the buffer when mapping the device (at least on NOMMU
> >> platforms). Only shared mappings make sense in this case.
> >
> > The change itself is crap anyways, since it is just working around the
> > fact that vm_insert_page() presently -EINVAL's out on nommu. The
> > vmalloc_to_page()/vm_insert_page() pair is used extensively across the
> > v4l drivers, and it's unrealistic to expect to patch every call site with
> > this sort of a workaround.
> >
> > While we can't support vm_insert_page() in most cases, these sorts of
> > use cases where it is just iterating over a contiguous block of pages on
> > a VMA that has already been established is something that can at least be
> > handled fairly easily (in this case, even just doing nothing in
> > vm_insert_page() would likely give the desired result, although we ought
> > to fix up vm_insert_page() to do something more useful instead). Working
> > out the bounds of the VMA isn't exactly difficult either, since we have
> > all of that already filled in the VMA by the time vma_insert_page() is
> > called.
> >
> > Regarding the VMA flag, this can be rectified by providing a dummy
> > get_unmapped_area() for the v4l devices which will set some default
> > capabilities. After that, determine_vm_flags() should take care of
> > setting up the proper VMA flags without hacking every driver's mmap()
> > routine. This sort of thing has absolutely no place in the driver,
> > though.
>
> Hi Laurent,
>
> Does this patch make sense? I hope you understand the tricky things in
> nommu mapping world.

I agree with Paul, the problem lies in vm_insert_page() and a fix is needed 
there.

Best regards,

Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/