[PATCH] media: vb2: add a check if queued userptr buffer is large enough

[PATCH] media: vb2: add a check if queued userptr buffer is large enough

[Marek Szyprowski]

Videobuf2 accepted any userptr buffer without verifying if its size is
large enough to store the video data from the driver. The driver reports
the minimal size of video data once in queue_setup and expects that
videobuf2 provides buffers that match these requirements. This patch
adds the required check.

Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Pawel Osciak <pawel@osciak.com>
---
 drivers/media/video/videobuf2-core.c |   41 +++++++++++++++++++--------------
 include/media/videobuf2-core.h       |    1 +
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 3015e60..c360627 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -43,8 +43,7 @@ module_param(debug, int, 0644);
 /**
  * __vb2_buf_mem_alloc() - allocate video memory for the given buffer
  */
-static int __vb2_buf_mem_alloc(struct vb2_buffer *vb,
-				unsigned long *plane_sizes)
+static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
 {
 	struct vb2_queue *q = vb->vb2_queue;
 	void *mem_priv;
@@ -53,13 +52,13 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb,
 	/* Allocate memory for all planes in this buffer */
 	for (plane = 0; plane < vb->num_planes; ++plane) {
 		mem_priv = call_memop(q, plane, alloc, q->alloc_ctx[plane],
-					plane_sizes[plane]);
+				      q->plane_sizes[plane]);
 		if (IS_ERR_OR_NULL(mem_priv))
 			goto free;
 
 		/* Associate allocator private data with this plane */
 		vb->planes[plane].mem_priv = mem_priv;
-		vb->v4l2_planes[plane].length = plane_sizes[plane];
+		vb->v4l2_planes[plane].length = q->plane_sizes[plane];
 	}
 
 	return 0;
@@ -141,8 +140,7 @@ static void __setup_offsets(struct vb2_queue *q)
  * Returns the number of buffers successfully allocated.
  */
 static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
-			     unsigned int num_buffers, unsigned int num_planes,
-			     unsigned long plane_sizes[])
+			     unsigned int num_buffers, unsigned int num_planes)
 {
 	unsigned int buffer;
 	struct vb2_buffer *vb;
@@ -169,7 +167,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
 
 		/* Allocate video buffer memory for the MMAP type */
 		if (memory == V4L2_MEMORY_MMAP) {
-			ret = __vb2_buf_mem_alloc(vb, plane_sizes);
+			ret = __vb2_buf_mem_alloc(vb);
 			if (ret) {
 				dprintk(1, "Failed allocating memory for "
 						"buffer %d\n", buffer);
@@ -454,7 +452,6 @@ static bool __buffers_in_use(struct vb2_queue *q)
 int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 {
 	unsigned int num_buffers, num_planes;
-	unsigned long plane_sizes[VIDEO_MAX_PLANES];
 	int ret = 0;
 
 	if (q->fileio) {
@@ -516,7 +513,7 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 	 * Make sure the requested values and current defaults are sane.
 	 */
 	num_buffers = min_t(unsigned int, req->count, VIDEO_MAX_FRAME);
-	memset(plane_sizes, 0, sizeof(plane_sizes));
+	memset(q->plane_sizes, 0, sizeof(q->plane_sizes));
 	memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
 	q->memory = req->memory;
 
@@ -525,13 +522,12 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 	 * Driver also sets the size and allocator context for each plane.
 	 */
 	ret = call_qop(q, queue_setup, q, &num_buffers, &num_planes,
-		       plane_sizes, q->alloc_ctx);
+		       q->plane_sizes, q->alloc_ctx);
 	if (ret)
 		return ret;
 
 	/* Finally, allocate buffers and video memory */
-	ret = __vb2_queue_alloc(q, req->memory, num_buffers, num_planes,
-				plane_sizes);
+	ret = __vb2_queue_alloc(q, req->memory, num_buffers, num_planes);
 	if (ret == 0) {
 		dprintk(1, "Memory allocation failed\n");
 		return -ENOMEM;
@@ -545,7 +541,7 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
 
 		orig_num_buffers = num_buffers = ret;
 		ret = call_qop(q, queue_setup, q, &num_buffers, &num_planes,
-			       plane_sizes, q->alloc_ctx);
+			       q->plane_sizes, q->alloc_ctx);
 		if (ret)
 			goto free_mem;
 
@@ -745,12 +741,20 @@ static int __qbuf_userptr(struct vb2_buffer *vb, struct v4l2_buffer *b)
 		dprintk(3, "qbuf: userspace address for plane %d changed, "
 				"reacquiring memory\n", plane);
 
+		/* Check if the provided plane buffer is large enough */
+		if (planes[plane].length < q->plane_sizes[plane]) {
+			ret = EINVAL;
+			goto err;
+		}
+
 		/* Release previously acquired memory if present */
 		if (vb->planes[plane].mem_priv)
 			call_memop(q, plane, put_userptr,
 					vb->planes[plane].mem_priv);
 
 		vb->planes[plane].mem_priv = NULL;
+		vb->v4l2_planes[plane].m.userptr = 0;
+		vb->v4l2_planes[plane].length = 0;
 
 		/* Acquire each plane's memory */
 		if (q->mem_ops->get_userptr) {
@@ -788,10 +792,13 @@ static int __qbuf_userptr(struct vb2_buffer *vb, struct v4l2_buffer *b)
 	return 0;
 err:
 	/* In case of errors, release planes that were already acquired */
-	for (; plane > 0; --plane) {
-		call_memop(q, plane, put_userptr,
-				vb->planes[plane - 1].mem_priv);
-		vb->planes[plane - 1].mem_priv = NULL;
+	for (plane = 0; plane < vb->num_planes; ++plane) {
+		if (vb->planes[plane].mem_priv)
+			call_memop(q, plane, put_userptr,
+				   vb->planes[plane].mem_priv);
+		vb->planes[plane].mem_priv = NULL;
+		vb->v4l2_planes[plane].m.userptr = 0;
+		vb->v4l2_planes[plane].length = 0;
 	}
 
 	return ret;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index f87472a..496d6e5 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -276,6 +276,7 @@ struct vb2_queue {
 	wait_queue_head_t		done_wq;
 
 	void				*alloc_ctx[VIDEO_MAX_PLANES];
+	unsigned long			plane_sizes[VIDEO_MAX_PLANES];
 
 	unsigned int			streaming:1;
 
-- 
1.7.1.569.g6f426

Re: [PATCH] media: vb2: add a check if queued userptr buffer is large enough

[Hans Verkuil]

Just one comment:

On Wednesday, August 10, 2011 10:21:28 Marek Szyprowski wrote:
> Videobuf2 accepted any userptr buffer without verifying if its size is
> large enough to store the video data from the driver. The driver reports
> the minimal size of video data once in queue_setup and expects that
> videobuf2 provides buffers that match these requirements. This patch
> adds the required check.
> 
> Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> CC: Pawel Osciak <pawel@osciak.com>
> ---
>  drivers/media/video/videobuf2-core.c |   41 
+++++++++++++++++++--------------
>  include/media/videobuf2-core.h       |    1 +
>  2 files changed, 25 insertions(+), 17 deletions(-)
> 

<snip>

> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index f87472a..496d6e5 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -276,6 +276,7 @@ struct vb2_queue {
>  	wait_queue_head_t		done_wq;
>  
>  	void				*alloc_ctx[VIDEO_MAX_PLANES];
> +	unsigned long			plane_sizes[VIDEO_MAX_PLANES];

Why unsigned long when it is a u32 in struct v4l2_plane_pix_format?

unsigned long is 64 bit on a 64-bit OS, so that seems wasteful to me.

Regards,

	Hans

>  
>  	unsigned int			streaming:1;
>  
> -- 
> 1.7.1.569.g6f426
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 

RE: [PATCH] media: vb2: add a check if queued userptr buffer is large enough

[Marek Szyprowski]

Hello,

On Wednesday, August 10, 2011 10:46 AM Hans Verkuil wrote:

> Just one comment:
> 
> On Wednesday, August 10, 2011 10:21:28 Marek Szyprowski wrote:
> > Videobuf2 accepted any userptr buffer without verifying if its size is
> > large enough to store the video data from the driver. The driver reports
> > the minimal size of video data once in queue_setup and expects that
> > videobuf2 provides buffers that match these requirements. This patch
> > adds the required check.
> >
> > Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > CC: Pawel Osciak <pawel@osciak.com>
> > ---
> >  drivers/media/video/videobuf2-core.c |   41
> +++++++++++++++++++--------------
> >  include/media/videobuf2-core.h       |    1 +
> >  2 files changed, 25 insertions(+), 17 deletions(-)
> >
> 
> <snip>
> 
> > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> > index f87472a..496d6e5 100644
> > --- a/include/media/videobuf2-core.h
> > +++ b/include/media/videobuf2-core.h
> > @@ -276,6 +276,7 @@ struct vb2_queue {
> >  	wait_queue_head_t		done_wq;
> >
> >  	void				*alloc_ctx[VIDEO_MAX_PLANES];
> > +	unsigned long			plane_sizes[VIDEO_MAX_PLANES];
> 
> Why unsigned long when it is a u32 in struct v4l2_plane_pix_format?
> 
> unsigned long is 64 bit on a 64-bit OS, so that seems wasteful to me.

u32 type should be used in places where the exact size really matters,
like strictly defined io structures passed to userspace or structures that
are used for accessing hardware registers directly. For all other cases,
like temporary storage of some values, the cpu native types should be used.
Looks at the whole vb2 code - u32 type is not used in any single place.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center

Re: [PATCH] media: vb2: add a check if queued userptr buffer is large enough

[Hans Verkuil]

On Wednesday, August 10, 2011 11:08:20 Marek Szyprowski wrote:
> Hello,
> 
> On Wednesday, August 10, 2011 10:46 AM Hans Verkuil wrote:
> 
> > Just one comment:
> > 
> > On Wednesday, August 10, 2011 10:21:28 Marek Szyprowski wrote:
> > > Videobuf2 accepted any userptr buffer without verifying if its size is
> > > large enough to store the video data from the driver. The driver reports
> > > the minimal size of video data once in queue_setup and expects that
> > > videobuf2 provides buffers that match these requirements. This patch
> > > adds the required check.
> > >
> > > Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > > CC: Pawel Osciak <pawel@osciak.com>
> > > ---
> > >  drivers/media/video/videobuf2-core.c |   41
> > +++++++++++++++++++--------------
> > >  include/media/videobuf2-core.h       |    1 +
> > >  2 files changed, 25 insertions(+), 17 deletions(-)
> > >
> > 
> > <snip>
> > 
> > > diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-
core.h
> > > index f87472a..496d6e5 100644
> > > --- a/include/media/videobuf2-core.h
> > > +++ b/include/media/videobuf2-core.h
> > > @@ -276,6 +276,7 @@ struct vb2_queue {
> > >  	wait_queue_head_t		done_wq;
> > >
> > >  	void				*alloc_ctx[VIDEO_MAX_PLANES];
> > > +	unsigned long			plane_sizes[VIDEO_MAX_PLANES];
> > 
> > Why unsigned long when it is a u32 in struct v4l2_plane_pix_format?
> > 
> > unsigned long is 64 bit on a 64-bit OS, so that seems wasteful to me.
> 
> u32 type should be used in places where the exact size really matters,
> like strictly defined io structures passed to userspace or structures that
> are used for accessing hardware registers directly. For all other cases,
> like temporary storage of some values, the cpu native types should be used.
> Looks at the whole vb2 code - u32 type is not used in any single place.

You can also change unsigned long to unsigned int as that is always 32 bits.

I don't mind one way or another.

Regards,

	Hans

> 
> Best regards
> -- 
> Marek Szyprowski
> Samsung Poland R&D Center
> 
> 

Re: [PATCH] media: vb2: add a check if queued userptr buffer is large enough

[Laurent Pinchart]

HI Marek,

Thanks for the patch.

On Wednesday 10 August 2011 10:21:28 Marek Szyprowski wrote:
> Videobuf2 accepted any userptr buffer without verifying if its size is
> large enough to store the video data from the driver. The driver reports
> the minimal size of video data once in queue_setup and expects that
> videobuf2 provides buffers that match these requirements. This patch
> adds the required check.
> 
> Reported-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> CC: Pawel Osciak <pawel@osciak.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Just one small comment below though.

> ---
>  drivers/media/video/videobuf2-core.c |   41
> +++++++++++++++++++-------------- include/media/videobuf2-core.h       |  
>  1 +
>  2 files changed, 25 insertions(+), 17 deletions(-)

[snip]

> diff --git a/include/media/videobuf2-core.h
> b/include/media/videobuf2-core.h index f87472a..496d6e5 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -276,6 +276,7 @@ struct vb2_queue {
>  	wait_queue_head_t		done_wq;
> 
>  	void				*alloc_ctx[VIDEO_MAX_PLANES];
> +	unsigned long			plane_sizes[VIDEO_MAX_PLANES];

What about unsigned int ? That 4GB should be more than enough :-)

>  	unsigned int			streaming:1;

-- 
Regards,

Laurent Pinchart

Re: [PATCH] media: vb2: add a check if queued userptr buffer is large enough

[Tomasz Stanislawski]

Hi Marek,

On 08/10/2011 10:21 AM, Marek Szyprowski wrote:
> Videobuf2 accepted any userptr buffer without verifying if its size is
> large enough to store the video data from the driver. The driver reports
> the minimal size of video data once in queue_setup and expects that
> videobuf2 provides buffers that match these requirements. This patch
> adds the required check.
>
> Reported-by: Laurent Pinchart<laurent.pinchart@ideasonboard.com>
> Signed-off-by: Marek Szyprowski<m.szyprowski@samsung.com>
> Signed-off-by: Kyungmin Park<kyungmin.park@samsung.com>
> CC: Pawel Osciak<pawel@osciak.com>
> ---
>   drivers/media/video/videobuf2-core.c |   41 +++++++++++++++++++--------------
>   include/media/videobuf2-core.h       |    1 +
>   2 files changed, 25 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
> index 3015e60..c360627 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -43,8 +43,7 @@ module_param(debug, int, 0644);
>   /**
>    * __vb2_buf_mem_alloc() - allocate video memory for the given buffer
>    */
> -static int __vb2_buf_mem_alloc(struct vb2_buffer *vb,
> -				unsigned long *plane_sizes)
> +static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>   {
>   	struct vb2_queue *q = vb->vb2_queue;
>   	void *mem_priv;
> @@ -53,13 +52,13 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb,
>   	/* Allocate memory for all planes in this buffer */
>   	for (plane = 0; plane<  vb->num_planes; ++plane) {
>   		mem_priv = call_memop(q, plane, alloc, q->alloc_ctx[plane],
> -					plane_sizes[plane]);
> +				      q->plane_sizes[plane]);
>   		if (IS_ERR_OR_NULL(mem_priv))
>   			goto free;
>
>   		/* Associate allocator private data with this plane */
>   		vb->planes[plane].mem_priv = mem_priv;
> -		vb->v4l2_planes[plane].length = plane_sizes[plane];
> +		vb->v4l2_planes[plane].length = q->plane_sizes[plane];
>   	}
>
>   	return 0;
> @@ -141,8 +140,7 @@ static void __setup_offsets(struct vb2_queue *q)
>    * Returns the number of buffers successfully allocated.
>    */
>   static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
> -			     unsigned int num_buffers, unsigned int num_planes,
> -			     unsigned long plane_sizes[])
> +			     unsigned int num_buffers, unsigned int num_planes)
>   {
>   	unsigned int buffer;
>   	struct vb2_buffer *vb;
> @@ -169,7 +167,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
>
>   		/* Allocate video buffer memory for the MMAP type */
>   		if (memory == V4L2_MEMORY_MMAP) {
> -			ret = __vb2_buf_mem_alloc(vb, plane_sizes);
> +			ret = __vb2_buf_mem_alloc(vb);
>   			if (ret) {
>   				dprintk(1, "Failed allocating memory for "
>   						"buffer %d\n", buffer);
> @@ -454,7 +452,6 @@ static bool __buffers_in_use(struct vb2_queue *q)
>   int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>   {
>   	unsigned int num_buffers, num_planes;
> -	unsigned long plane_sizes[VIDEO_MAX_PLANES];
>   	int ret = 0;
>
>   	if (q->fileio) {
> @@ -516,7 +513,7 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>   	 * Make sure the requested values and current defaults are sane.
>   	 */
>   	num_buffers = min_t(unsigned int, req->count, VIDEO_MAX_FRAME);
> -	memset(plane_sizes, 0, sizeof(plane_sizes));
> +	memset(q->plane_sizes, 0, sizeof(q->plane_sizes));
>   	memset(q->alloc_ctx, 0, sizeof(q->alloc_ctx));
>   	q->memory = req->memory;
>
> @@ -525,13 +522,12 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>   	 * Driver also sets the size and allocator context for each plane.
>   	 */
>   	ret = call_qop(q, queue_setup, q,&num_buffers,&num_planes,
> -		       plane_sizes, q->alloc_ctx);
> +		       q->plane_sizes, q->alloc_ctx);
>   	if (ret)
>   		return ret;
>
>   	/* Finally, allocate buffers and video memory */
> -	ret = __vb2_queue_alloc(q, req->memory, num_buffers, num_planes,
> -				plane_sizes);
> +	ret = __vb2_queue_alloc(q, req->memory, num_buffers, num_planes);
>   	if (ret == 0) {
>   		dprintk(1, "Memory allocation failed\n");
>   		return -ENOMEM;
> @@ -545,7 +541,7 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>
>   		orig_num_buffers = num_buffers = ret;
>   		ret = call_qop(q, queue_setup, q,&num_buffers,&num_planes,
> -			       plane_sizes, q->alloc_ctx);
> +			       q->plane_sizes, q->alloc_ctx);
>   		if (ret)
>   			goto free_mem;
>
> @@ -745,12 +741,20 @@ static int __qbuf_userptr(struct vb2_buffer *vb, struct v4l2_buffer *b)
>   		dprintk(3, "qbuf: userspace address for plane %d changed, "
>   				"reacquiring memory\n", plane);
>
> +		/* Check if the provided plane buffer is large enough */
> +		if (planes[plane].length<  q->plane_sizes[plane]) {
> +			ret = EINVAL;

You should return -EINVAL, not EINVAL. Returning the positive number is 
a success.

Best regards,
Tomasz Stanislawski

> +			goto err;
> +		}
> +
>   		/* Release previously acquired memory if present */
>   		if (vb->planes[plane].mem_priv)
>   			call_memop(q, plane, put_userptr,
>   					vb->planes[plane].mem_priv);
>
>   		vb->planes[plane].mem_priv = NULL;
> +		vb->v4l2_planes[plane].m.userptr = 0;
> +		vb->v4l2_planes[plane].length = 0;
>
>   		/* Acquire each plane's memory */
>   		if (q->mem_ops->get_userptr) {
> @@ -788,10 +792,13 @@ static int __qbuf_userptr(struct vb2_buffer *vb, struct v4l2_buffer *b)
>   	return 0;
>   err:
>   	/* In case of errors, release planes that were already acquired */
> -	for (; plane>  0; --plane) {
> -		call_memop(q, plane, put_userptr,
> -				vb->planes[plane - 1].mem_priv);
> -		vb->planes[plane - 1].mem_priv = NULL;
> +	for (plane = 0; plane<  vb->num_planes; ++plane) {
> +		if (vb->planes[plane].mem_priv)
> +			call_memop(q, plane, put_userptr,
> +				   vb->planes[plane].mem_priv);
> +		vb->planes[plane].mem_priv = NULL;
> +		vb->v4l2_planes[plane].m.userptr = 0;
> +		vb->v4l2_planes[plane].length = 0;
>   	}
>
>   	return ret;
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index f87472a..496d6e5 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -276,6 +276,7 @@ struct vb2_queue {
>   	wait_queue_head_t		done_wq;
>
>   	void				*alloc_ctx[VIDEO_MAX_PLANES];
> +	unsigned long			plane_sizes[VIDEO_MAX_PLANES];
>
>   	unsigned int			streaming:1;
>

[PATCH] media: vb2: fix incorrect return value

[Marek Szyprowski]

This patch fixes incorrect return value. Errors should be returned
as negative numbers.

Reported-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/media/video/videobuf2-core.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index 6687ac3..3f5c7a3 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -751,7 +751,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, struct v4l2_buffer *b)
 
 		/* Check if the provided plane buffer is large enough */
 		if (planes[plane].length < q->plane_sizes[plane]) {
-			ret = EINVAL;
+			ret = -EINVAL;
 			goto err;
 		}
 
-- 
1.7.1.569.g6f426

Re: [PATCH] media: vb2: fix incorrect return value

[Pawel Osciak]

On Mon, Oct 3, 2011 at 00:21, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> This patch fixes incorrect return value. Errors should be returned
> as negative numbers.
>
> Reported-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/media/video/videobuf2-core.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
> index 6687ac3..3f5c7a3 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -751,7 +751,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, struct v4l2_buffer *b)
>
>                /* Check if the provided plane buffer is large enough */
>                if (planes[plane].length < q->plane_sizes[plane]) {
> -                       ret = EINVAL;
> +                       ret = -EINVAL;
>                        goto err;
>                }
>
> --
> 1.7.1.569.g6f426
>
>


Acked-by: Pawel Osciak <pawel@osciak.com>

-- 
Best regards,
Pawel Osciak