FAILED: patch "[PATCH] [media] videobuf2-v4l2: Verify planes array in buffer" failed to apply to 4.4-stable tree

PATCH [email protected] 2 participants 3 messages May 2, 2016

Subsystems (3)

Stable branch Media (V4L/DVB) VIDEOBUF2 framework

lore

Share & tools

lore

[email protected]

author maintainer

2016-05-02 02:58 UTC #0 lore raw

The patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

Quoted 1 line

From 2c1f6951a8a82e6de0d82b1158b5e493fc6c54ab Mon Sep 17 00:00:00 2001

From: Sakari Ailus <[email protected]>
Date: Sun, 3 Apr 2016 16:31:03 -0300
Subject: [PATCH] [media] videobuf2-v4l2: Verify planes array in buffer
dequeueing

When a buffer is being dequeued using VIDIOC_DQBUF IOCTL, the exact buffer which will be dequeued is not known until the buffer has been removed from the queue. The number of planes is specific to a buffer, not to the queue.

This does lead to the situation where multi-plane buffers may be requested and queued with n planes, but VIDIOC_DQBUF IOCTL may be passed an argument struct with fewer planes.

__fill_v4l2_buffer() however uses the number of planes from the dequeued videobuf2 buffer, overwriting kernel memory (the m.planes array allocated in video_usercopy() in v4l2-ioctl.c) if the user provided fewer planes than the dequeued buffer had. Oops!

Fixes b0e0e1f83de3 ("[media] media: videobuf2: Prepare to divide videobuf2")

Signed-off-by Sakari Ailus <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Cc [email protected] # for v4.4 and later

Signed-off-by Mauro Carvalho Chehab <[email protected]>

drivers/media/v4l2-core/videobuf2-v4l2.c +6 -0

index 91f552124050..8da7470ca364 100644
@@ -74,6 +74,11 @@ 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.

@@ -437,6 +442,7 @@ 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,

Sakari Ailus

Acked-by Hans Verkuil

2016-05-02 08:11 UTC #1 lore raw

Hi Greg,

Quoted 5 lines · On Sun, May 01, 2016 at 07:58:08PM -0700, [email protected] wrote:

The patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

This patch depends on "[media] videobuf2-core: Check user space planes array in dqbuf" . The patch is below (submitted to stable as well):

Quoted 1 line

From ae72317024907367adaf597127f8afc6acfb7f20 Mon Sep 17 00:00:00 2001

From: Sakari Ailus <[email protected]>
Date: Sun, 3 Apr 2016 22:15:00 +0300
Subject: [PATCH 1/1] videobuf2-core: Check user space planes array in dqbuf

The number of planes in videobuf2 is specific to a buffer. In order to verify that the planes array provided by the user is long enough, a new vb2_buf_op is required.

Call __verify_planes_array() when the dequeued buffer is known. Return an error to the caller if there was one, otherwise remove the buffer from the done list.

Signed-off-by Sakari Ailus <[email protected]>

Acked-by Hans Verkuil <[email protected]>

Cc [email protected] # for v4.4 and later

---
 drivers/media/v4l2-core/videobuf2-core.c | 10 +++++-----
 include/media/videobuf2-core.h           |  4 ++++
 2 files changed, 9 insertions(+), 5 deletions(-)

drivers/media/v4l2-core/videobuf2-core.c +5 -5

index 5d016f4..2169544 100644
@@ -1645,7 +1645,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking)
  * Will sleep if required for nonblocking == false.
  */
 static int __vb2_get_done_vb(struct vb2_queue *q, struct vb2_buffer **vb,
-				int nonblocking)
+			     void *pb, int nonblocking)
 {
 	unsigned long flags;
 	int ret;

@@ -1666,10 +1666,10 @@ static int __vb2_get_done_vb(struct vb2_queue *q, struct vb2_buffer **vb,
 	/*
 	 * Only remove the buffer from done_list if v4l2_buffer can handle all
 	 * the planes.
-	 * Verifying planes is NOT necessary since it already has been checked
-	 * before the buffer is queued/prepared. So it can never fail.
 	 */
-	list_del(&(*vb)->done_entry);
+	ret = call_bufop(q, verify_planes_array, *vb, pb);
+	if (!ret)
+		list_del(&(*vb)->done_entry);
 	spin_unlock_irqrestore(&q->done_lock, flags);
 
 	return ret;

@@ -1748,7 +1748,7 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
 	struct vb2_buffer *vb = NULL;
 	int ret;
 
-	ret = __vb2_get_done_vb(q, &vb, nonblocking);
+	ret = __vb2_get_done_vb(q, &vb, pb, nonblocking);
 	if (ret < 0)
 		return ret;

include/media/videobuf2-core.h +4 -0

index 8a0f55b..e2b1773 100644
@@ -375,6 +375,9 @@ struct vb2_ops {
 /**
  * struct vb2_ops - driver-specific callbacks
  *
+ * @verify_planes_array:Verify that a given user space structure contains
+ *			enough planes for the buffer. This is called
+ *			for each dequeued buffer.
  * @fill_user_buffer:	given a vb2_buffer fill in the userspace structure.
  *			For V4L2 this is a struct v4l2_buffer.
  * @fill_vb2_buffer:	given a userspace structure, fill in the vb2_buffer.

@@ -384,6 +387,7 @@ struct vb2_ops {
  *			the vb2_buffer struct.
  */
 struct vb2_buf_ops {
+	int (*verify_planes_array)(struct vb2_buffer *vb, const void *pb);
 	void (*fill_user_buffer)(struct vb2_buffer *vb, void *pb);
 	int (*fill_vb2_buffer)(struct vb2_buffer *vb, const void *pb,
 				struct vb2_plane *planes);

Signature · 7 lines

2.1.4

-- 
Kind regards,

Sakari Ailus
[email protected]

Greg KH

author maintainer

2016-05-02 15:54 UTC #2 lore raw

Quoted 11 lines · Sakari Ailus

Hi Greg,
Quoted 5 lines · On Sun, May 01, 2016 at 07:58:08PM -0700, [email protected] wrote:
The patch below does not apply to the 4.4-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
This patch depends on "[media] videobuf2-core: Check user space planes array in dqbuf" . The patch is below (submitted to stable as well):

The patch below doesn't apply to 4.4-stable :(

I have fixed it up by hand now, let's see if this works...

thanks,

greg k-h