Revert "[media] videobuf2-v4l2: Verify planes array in buffer dequeueing"

Message ID 575aa5711a62f79c5f973011b415403fd3d3b7c7.1462984023.git.mchehab@osg.samsung.com (mailing list archive)
State Accepted, archived
Headers

Commit Message

Mauro Carvalho Chehab May 11, 2016, 4:27 p.m. UTC
This patch causes a Kernel panic when called on a DVB driver.

This reverts commit 2c1f6951a8a82e6de0d82b1158b5e493fc6c54ab.

Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: Hans Verkuil <hans.verkuil@cisco.com>
Cc: stable@vgar.kernel.org
Fixes: 2c1f6951a8a8 ("[media] videobuf2-v4l2: Verify planes array in buffer dequeueing")
Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
---
 drivers/media/v4l2-core/videobuf2-v4l2.c | 6 ------
 1 file changed, 6 deletions(-)
  

Comments

Nicolas Dufresne May 12, 2016, 6:59 a.m. UTC | #1
Le mercredi 11 mai 2016 à 13:27 -0300, Mauro Carvalho Chehab a écrit :
> This patch causes a Kernel panic when called on a DVB driver.
> 
> This reverts commit 2c1f6951a8a82e6de0d82b1158b5e493fc6c54ab.

Seems rather tricky, since this commit fixed a possible (user induced)
buffer overflow according to Sakari comment. Would be nice to fix and
resubmit.

> 
> Cc: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: stable@vgar.kernel.org
> Fixes: 2c1f6951a8a8 ("[media] videobuf2-v4l2: Verify planes array in
> buffer dequeueing")
> Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
> ---
>  drivers/media/v4l2-core/videobuf2-v4l2.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c
> b/drivers/media/v4l2-core/videobuf2-v4l2.c
> index 7f366f1b0377..0b1b8c7b6ce5 100644
> --- a/drivers/media/v4l2-core/videobuf2-v4l2.c
> +++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
> @@ -74,11 +74,6 @@ static int __verify_planes_array(struct vb2_buffer
> *vb, const struct v4l2_buffer
>  	return 0;
>  }
>  
> -static int __verify_planes_array_core(struct vb2_buffer *vb, const
> void *pb)
> -{
> -	return __verify_planes_array(vb, pb);
> -}
> -
>  /**
>   * __verify_length() - Verify that the bytesused value for each
> plane fits in
>   * the plane length and that the data offset doesn't exceed the
> bytesused value.
> @@ -442,7 +437,6 @@ static int __fill_vb2_buffer(struct vb2_buffer
> *vb,
>  }
>  
>  static const struct vb2_buf_ops v4l2_buf_ops = {
> -	.verify_planes_array	= __verify_planes_array_core,
>  	.fill_user_buffer	= __fill_v4l2_buffer,
>  	.fill_vb2_buffer	= __fill_vb2_buffer,
>  	.copy_timestamp		= __copy_timestamp,
--
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
  
Sakari Ailus May 12, 2016, 7:05 a.m. UTC | #2
Hi Nicolas,

Nicolas Dufresne wrote:
> Le mercredi 11 mai 2016 à 13:27 -0300, Mauro Carvalho Chehab a écrit :
>> This patch causes a Kernel panic when called on a DVB driver.
>>
>> This reverts commit 2c1f6951a8a82e6de0d82b1158b5e493fc6c54ab.
> 
> Seems rather tricky, since this commit fixed a possible (user induced)
> buffer overflow according to Sakari comment. Would be nice to fix and
> resubmit.

I have updated patches here:

<URL:https://git.linuxtv.org/sailus/media_tree.git/log/?h=vb2-overwrite-fix-error-on-fixes-v2>

These are tested on V4L2 streaming API only so far, I'll test file I/O
today but with DVB I'd need some help with testing. I'd very much
appreciate test reports if someone has a chance to test the two patches
with a DVB adapter using VB2.

Thanks.
  

Patch

diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c b/drivers/media/v4l2-core/videobuf2-v4l2.c
index 7f366f1b0377..0b1b8c7b6ce5 100644
--- a/drivers/media/v4l2-core/videobuf2-v4l2.c
+++ b/drivers/media/v4l2-core/videobuf2-v4l2.c
@@ -74,11 +74,6 @@  static int __verify_planes_array(struct vb2_buffer *vb, const struct v4l2_buffer
 	return 0;
 }
 
-static int __verify_planes_array_core(struct vb2_buffer *vb, const void *pb)
-{
-	return __verify_planes_array(vb, pb);
-}
-
 /**
  * __verify_length() - Verify that the bytesused value for each plane fits in
  * the plane length and that the data offset doesn't exceed the bytesused value.
@@ -442,7 +437,6 @@  static int __fill_vb2_buffer(struct vb2_buffer *vb,
 }
 
 static const struct vb2_buf_ops v4l2_buf_ops = {
-	.verify_planes_array	= __verify_planes_array_core,
 	.fill_user_buffer	= __fill_v4l2_buffer,
 	.fill_vb2_buffer	= __fill_vb2_buffer,
 	.copy_timestamp		= __copy_timestamp,