Hi Mauro,
On Friday 19 April 2013 08:18:01 Mauro Carvalho Chehab wrote:
[snip]
> [media] videobuf2: fix the length check for mmap
>
> Memory maps typically require that the buffer size to be page
s/that the/the/
> aligned. Currently, two memops drivers do such alignment
> internally, but videobuf-vmalloc doesn't.
>
> Also, the buffer overflow check doesn't take it into account.
>
> So, instead of doing it at each memops driver, enforce it at
> VB2 core.
>
> Reported-by: Prabhakar lad <prabhakar.csengg@xxxxxxxxx>
> Signed-off-by: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>
Acked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c index 58c1744..7d833ee 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -54,10 +54,15 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
> void *mem_priv;
> int plane;
>
> - /* Allocate memory for all planes in this buffer */
> + /*
> + * Allocate memory for all planes in this buffer
> + * NOTE: mmapped areas should be page aligned
> + */
> for (plane = 0; plane < vb->num_planes; ++plane) {
> + unsigned long size = PAGE_ALIGN(q->plane_sizes[plane]);
> +
> mem_priv = call_memop(q, alloc, q->alloc_ctx[plane],
> - q->plane_sizes[plane], q->gfp_flags);
> + size, q->gfp_flags);
> if (IS_ERR_OR_NULL(mem_priv))
> goto free;
>
> @@ -1852,6 +1857,7 @@ int vb2_mmap(struct vb2_queue *q, struct
> vm_area_struct *vma) struct vb2_buffer *vb;
> unsigned int buffer, plane;
> int ret;
> + unsigned long length;
>
> if (q->memory != V4L2_MEMORY_MMAP) {
> dprintk(1, "Queue is not currently set up for mmap\n");
> @@ -1886,8 +1892,15 @@ int vb2_mmap(struct vb2_queue *q, struct
> vm_area_struct *vma)
>
> vb = q->bufs[buffer];
>
> - if (vb->v4l2_planes[plane].length < (vma->vm_end - vma->vm_start)) {
> - dprintk(1, "Invalid length\n");
> + /*
> + * MMAP requires page_aligned buffers.
> + * The buffer length was page_aligned at __vb2_buf_mem_alloc(),
> + * so, we need to do the same here.
> + */
> + length = PAGE_ALIGN(vb->v4l2_planes[plane].length);
> + if (length < (vma->vm_end - vma->vm_start)) {
> + dprintk(1,
> + "MMAP invalid, as it would overflow buffer length\n");
> return -EINVAL;
> }
>
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> b/drivers/media/v4l2-core/videobuf2-dma-contig.c index ae35d25..fd56f25
> 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -162,9 +162,6 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long
> size, gfp_t gfp_flags) if (!buf)
> return ERR_PTR(-ENOMEM);
>
> - /* align image size to PAGE_SIZE */
> - size = PAGE_ALIGN(size);
> -
> buf->vaddr = dma_alloc_coherent(dev, size, &buf->dma_addr,
> GFP_KERNEL | gfp_flags);
> if (!buf->vaddr) {
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index 59522b2..16ae3dc 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> @@ -55,7 +55,8 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
> long size, gfp_t gfp_fla buf->write = 0;
> buf->offset = 0;
> buf->sg_desc.size = size;
> - buf->sg_desc.num_pages = (size + PAGE_SIZE - 1) >> PAGE_SHIFT;
> + /* size is already page aligned */
> + buf->sg_desc.num_pages = size >> PAGE_SHIFT;
>
> buf->sg_desc.sglist = vzalloc(buf->sg_desc.num_pages *
> sizeof(*buf->sg_desc.sglist));
--
Regards,
Laurent Pinchart
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
