On 11/06/15 07:21, Laurent Pinchart wrote:
> Hello,
>
> (CC'ing Tomi Valkeinen)
>
> On Wednesday 10 June 2015 06:20:45 Mauro Carvalho Chehab wrote:
>> From: Jan Kara <jack@xxxxxxx>
>>
>> Convert omap_vout_uservirt_to_phys() to use get_vaddr_pfns() instead of
>> hand made mapping of virtual address to physical address. Also the
>> function leaked page reference from get_user_pages() so fix that by
>> properly release the reference when omap_vout_buffer_release() is
>> called.
>>
>> Signed-off-by: Jan Kara <jack@xxxxxxx>
>> Signed-off-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>> [hans.verkuil@xxxxxxxxx: remove unused struct omap_vout_device *vout
>> variable]
>>
>> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxxxxxxx>
>>
>> diff --git a/drivers/media/platform/omap/omap_vout.c
>> b/drivers/media/platform/omap/omap_vout.c index f09c5f17a42f..7feb6394f111
>> 100644
>> --- a/drivers/media/platform/omap/omap_vout.c
>> +++ b/drivers/media/platform/omap/omap_vout.c
>> @@ -195,46 +195,34 @@ static int omap_vout_try_format(struct v4l2_pix_format
>> *pix) }
>>
>> /*
>> - * omap_vout_uservirt_to_phys: This inline function is used to convert user
>> - * space virtual address to physical address.
>> + * omap_vout_get_userptr: Convert user space virtual address to physical
>> + * address.
>> */
>> -static unsigned long omap_vout_uservirt_to_phys(unsigned long virtp)
>> +static int omap_vout_get_userptr(struct videobuf_buffer *vb, u32 virtp,
>> + u32 *physp)
>> {
>> - unsigned long physp = 0;
>> - struct vm_area_struct *vma;
>> - struct mm_struct *mm = current->mm;
>> + struct frame_vector *vec;
>> + int ret;
>>
>> /* For kernel direct-mapped memory, take the easy way */
>> - if (virtp >= PAGE_OFFSET)
>> - return virt_to_phys((void *) virtp);
>> -
>> - down_read(¤t->mm->mmap_sem);
>> - vma = find_vma(mm, virtp);
>> - if (vma && (vma->vm_flags & VM_IO) && vma->vm_pgoff) {
>> - /* this will catch, kernel-allocated, mmaped-to-usermode
>> - addresses */
>> - physp = (vma->vm_pgoff << PAGE_SHIFT) + (virtp - vma->vm_start);
>> - up_read(¤t->mm->mmap_sem);
>> - } else {
>> - /* otherwise, use get_user_pages() for general userland pages */
>> - int res, nr_pages = 1;
>> - struct page *pages;
>> + if (virtp >= PAGE_OFFSET) {
>> + *physp = virt_to_phys((void *)virtp);
>
> Lovely. virtp comes from userspace and as far as I know it arrives here
> completely unchecked. The problem isn't introduced by this patch, but
> omap_vout buffer management seems completely broken to me, and nobody seems to
> care about the driver. Given that omapdrm should now provide the video output
> capabilities that are missing from omapfb and resulted in the development of
> omap_vout, shouldn't we drop the omap_vout driver ?
>
> Tomi, any opinion on this ? Do you see any omap_vout capability missing from
> omapdrm ?
I've never used omap_vout, so I don't even know what it offers. I do
know it supports VRFB rotation (omap3), which we don't have on omapdrm,
though. Whether anyone uses that (or omap_vout), I have no idea...
I'd personally love to get rid of omap_vout, as it causes complications
on omapfb/omapdss side.
Tomi
Attachment:
signature.asc
Description: OpenPGP digital signature
