linux-kernel - Re: [PATCH v2] media: davinci: vpif: align the buffers size to page page size boundary

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 for Android: free password hash cracker in your pocket

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

Date:	Thu, 18 Apr 2013 10:17:14 +0530
From:	Prabhakar Lad <prabhakar.csengg@...il.com>
To:	Marek Szyprowski <m.szyprowski@...sung.com>
Cc:	LMML <linux-media@...r.kernel.org>,
	Mauro Carvalho Chehab <mchehab@...hat.com>,
	DLOS <davinci-linux-open-source@...ux.davincidsp.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Hans Verkuil <hans.verkuil@...co.com>,
	Laurent Pinchart <laurent.pinchart@...asonboard.com>
Subject: Re: [PATCH v2] media: davinci: vpif: align the buffers size to page
 page size boundary

Hi Marek,

On Tue, Apr 16, 2013 at 4:48 PM, Laurent Pinchart
<laurent.pinchart@...asonboard.com> wrote:
> Hi Prabhakar,
>
> (CC'ing Marek)
>
> On Tuesday 16 April 2013 16:24:30 Prabhakar lad wrote:
>> From: Lad, Prabhakar <prabhakar.csengg@...il.com>
>>
>> with recent commit with id 068a0df76023926af958a336a78bef60468d2033
>> which adds add length check for mmap, the application were failing to
>> mmap the buffers.
>>
>> This patch aligns the the buffer size to page size boundary for both
>> capture and display driver so the it pass the check.
>>
>> Signed-off-by: Lad, Prabhakar <prabhakar.csengg@...il.com>
>> Cc: Laurent Pinchart <laurent.pinchart@...asonboard.com>
>> Cc: Hans Verkuil <hans.verkuil@...co.com>
>> Cc: Mauro Carvalho Chehab <mchehab@...hat.com>
>> ---
>>  Changes for v2:
>>  1: Fixed a typo in commit message.
>>
>>  drivers/media/platform/davinci/vpif_capture.c |    1 +
>>  drivers/media/platform/davinci/vpif_display.c |    1 +
>>  2 files changed, 2 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpif_capture.c
>> b/drivers/media/platform/davinci/vpif_capture.c index 5f98df1..25981d6
>> 100644
>> --- a/drivers/media/platform/davinci/vpif_capture.c
>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> @@ -183,6 +183,7 @@ static int vpif_buffer_queue_setup(struct vb2_queue *vq,
>> *nbuffers = config_params.min_numbuffers;
>>
>>       *nplanes = 1;
>> +     size = PAGE_ALIGN(size);
>
> I wonder if that's the best fix.
>
> The queue_setup operation is supposed to return the size required by the
> driver for each plane. Depending on the hardware requirements, that size might
> not be a multiple of the page size.
>
> As we can't mmap() a fraction of a page, the allocated plane size needs to be
> rounded up to the next page boundary to allow mmap() support. The dma-contig
> and dma-sg allocators already do so in their alloc operation, but the vmalloc
> allocator doesn't.
>
> The recent "media: vb2: add length check for mmap" patch verifies that the
> mmap() size requested by userspace doesn't exceed the buffer size. As the
> mmap() size is rounded up to the next page boundary the check will fail for
> buffer sizes that are not multiple of the page size.
>
> Your fix will not result in overallocation (as the allocator already rounds
> the size up), but will prevent the driver from importing a buffer large enough
> for the hardware but not rounded up to the page size.
>
> A better fix might be to round up the buffer size in the buffer size check at
> mmap() time, and fix the vmalloc allocator to round up the size. That the
> allocator, not drivers, is responsible for buffer size alignment should be
> documented in videobuf2-core.h.
>
Do you plan to post a patch fixing it as per Laurent's suggestion ?

Regards,
--Prabhakar

>>       sizes[0] = size;
>>       alloc_ctxs[0] = common->alloc_ctx;
>>
>> diff --git a/drivers/media/platform/davinci/vpif_display.c
>> b/drivers/media/platform/davinci/vpif_display.c index 1b3fb5c..3414715
>> 100644
>> --- a/drivers/media/platform/davinci/vpif_display.c
>> +++ b/drivers/media/platform/davinci/vpif_display.c
>> @@ -162,6 +162,7 @@ static int vpif_buffer_queue_setup(struct vb2_queue *vq,
>> *nbuffers = config_params.min_numbuffers;
>>
>>       *nplanes = 1;
>> +     size = PAGE_ALIGN(size);
>>       sizes[0] = size;
>>       alloc_ctxs[0] = common->alloc_ctx;
>>       return 0;
>
> --
> 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/