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@xxxxxxxxxxx>
>>
>> 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@xxxxxxxxxxx>
>> Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx>
>> ---
>> 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)
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/dri-devel
