Hi Laurent, Thank you for the review. Please refer to the comments below.
On 10/11/2012 11:36 PM, Laurent Pinchart wrote:
Hi Tomasz,
Thanks for the patch.
On Wednesday 10 October 2012 16:46:41 Tomasz Stanislawski wrote:
From: Marek Szyprowski m.szyprowski@samsung.com
The DMA transfer must be aligned to a specific value. If userptr is not aligned to DMA requirements then unexpected corruptions of the memory may occur before or after a buffer. To prevent such situations, all unligned userptr buffers are rejected at VIDIOC_QBUF.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Acked-by: Hans Verkuil hans.verkuil@cisco.com
drivers/media/v4l2-core/videobuf2-dma-contig.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c index 2d661fd..571a919 100644 --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c @@ -493,6 +493,18 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr, struct vm_area_struct *vma; struct sg_table *sgt; unsigned long contig_size;
- unsigned long dma_align = dma_get_cache_alignment();
- /* Only cache aligned DMA transfers are reliable */
- if (!IS_ALIGNED(vaddr | size, dma_align)) {
pr_debug("user data must be aligned to %lu bytes\n", dma_align);return ERR_PTR(-EINVAL);- }
Looks good to me.
- if (!size) {
pr_debug("size is zero\n");return ERR_PTR(-EINVAL);- }
Can this happen ? The vb2 core already has
/* Check if the provided plane buffer is large enough */ if (planes[plane].length < q->plane_sizes[plane]) { ret = -EINVAL; goto err; }Unless queue_setup sets plane_sizes to 0 we can't reach vb2_dc_get_userptr.
Yes.. unfortunately, some drivers set plane_size to 0 at queue_setup. Especially, if REQBUFS is called before any S_FMT. Maybe it is just a driver bug.
However, VB2 makes no sanity check if plane_sizes[] is zero. I was not able to find in Documentation nor code comments any explicit statement that plane_size cannot be zero.
Therefore I have to reject reject a 0-bytes-long user pointer at vb2_dc_get_userptr before creating an empty scatterlist and passing it to the DMA layer.
Regards, Tomasz Stanislawski
buf = kzalloc(sizeof *buf, GFP_KERNEL); if (!buf)