[REVIEW PATCH FOR v3.19 1/1] vb2: Fix dma_dir setting for dma-contig mem type

[REVIEW PATCH FOR v3.19 1/1] vb2: Fix dma_dir setting for dma-contig mem type

[Sakari Ailus]

The last argument of vb2_dc_get_user_pages() is of type enum
dma_data_direction, but the caller, vb2_dc_get_userptr() passes a value
which is the result of comparison dma_dir == DMA_FROM_DEVICE. This results
in the write parameter to get_user_pages() being zero in all cases, i.e.
that the caller has no intent to write there.

This was broken by patch "vb2: replace 'write' by 'dma_dir'".

Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index b481d20..69e0483 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -632,8 +632,7 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	}
 
 	/* extract page list from userspace mapping */
-	ret = vb2_dc_get_user_pages(start, pages, n_pages, vma,
-				    dma_dir == DMA_FROM_DEVICE);
+	ret = vb2_dc_get_user_pages(start, pages, n_pages, vma, dma_dir);
 	if (ret) {
 		unsigned long pfn;
 		if (vb2_dc_get_user_pfn(start, n_pages, vma, &pfn) == 0) {
-- 
2.1.0.231.g7484e3b

Re: [REVIEW PATCH FOR v3.19 1/1] vb2: Fix dma_dir setting for dma-contig mem type

[Sakari Ailus]

On Fri, Feb 13, 2015 at 09:42:37AM +0200, Sakari Ailus wrote:
> The last argument of vb2_dc_get_user_pages() is of type enum
> dma_data_direction, but the caller, vb2_dc_get_userptr() passes a value
> which is the result of comparison dma_dir == DMA_FROM_DEVICE. This results
> in the write parameter to get_user_pages() being zero in all cases, i.e.
> that the caller has no intent to write there.
> 
> This was broken by patch "vb2: replace 'write' by 'dma_dir'".
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Well, v3.19 is out so this is material for the stable branch. I'll submit a
pull request once I get some acks.

This is quite bad indeed, as writing somewhere you're not expected to write
may well lead to memory corruption, which is exactly the case here.

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	XMPP: sailus@retiisi.org.uk

Re: [REVIEW PATCH FOR v3.19 1/1] vb2: Fix dma_dir setting for dma-contig mem type

[Hans Verkuil]

On 02/13/2015 08:42 AM, Sakari Ailus wrote:
> The last argument of vb2_dc_get_user_pages() is of type enum
> dma_data_direction, but the caller, vb2_dc_get_userptr() passes a value
> which is the result of comparison dma_dir == DMA_FROM_DEVICE. This results
> in the write parameter to get_user_pages() being zero in all cases, i.e.
> that the caller has no intent to write there.
> 
> This was broken by patch "vb2: replace 'write' by 'dma_dir'".
> 
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>

Yuck.

Acked-by: Hans Verkuil <hans.verkuil@cisco.com>

Regards,

	Hans

> ---
>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> index b481d20..69e0483 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -632,8 +632,7 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
>  	}
>  
>  	/* extract page list from userspace mapping */
> -	ret = vb2_dc_get_user_pages(start, pages, n_pages, vma,
> -				    dma_dir == DMA_FROM_DEVICE);
> +	ret = vb2_dc_get_user_pages(start, pages, n_pages, vma, dma_dir);
>  	if (ret) {
>  		unsigned long pfn;
>  		if (vb2_dc_get_user_pfn(start, n_pages, vma, &pfn) == 0) {
>