Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1400003 > unrolled thread
| Started by | Sakari Ailus <sakari.ailus@linux.intel.com> |
|---|---|
| First post | 2016-05-12 14:20 +0200 |
| Last post | 2016-05-13 11:40 +0200 |
| Articles | 3 — 2 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
[PATCH v2 1/2] vb2: core: Skip planes array verification if pb is NULL Sakari Ailus <sakari.ailus@linux.intel.com> - 2016-05-12 14:20 +0200
Re: [PATCH v2 1/2] vb2: core: Skip planes array verification if pb is NULL Hans Verkuil <hverkuil@xs4all.nl> - 2016-05-13 11:10 +0200
Re: [PATCH v2 1/2] vb2: core: Skip planes array verification if pb is NULL Sakari Ailus <sakari.ailus@linux.intel.com> - 2016-05-13 11:40 +0200
| From | Sakari Ailus <sakari.ailus@linux.intel.com> |
|---|---|
| Date | 2016-05-12 14:20 +0200 |
| Subject | [PATCH v2 1/2] vb2: core: Skip planes array verification if pb is NULL |
| Message-ID | <rxXKW-5Dw-17@gated-at.bofh.it> |
An earlier patch fixing an input validation issue introduced another
issue: vb2_core_dqbuf() is called with pb argument value NULL in some
cases, causing a NULL pointer dereference. Fix this by skipping the
verification as there's nothing to verify.
Signed-off-by: David R <david@unsolicited.net>
Use if () instead of ? :; it's nicer that way. Improve the comment in the
code as well.
Fixes: e7e0c3e26587 ("[media] videobuf2-core: Check user space planes array in dqbuf")
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Cc: stable@vger.kernel.org # for v4.4 and later
---
drivers/media/v4l2-core/videobuf2-core.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 9fbcb67..633fc1a 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1648,7 +1648,7 @@ static int __vb2_get_done_vb(struct vb2_queue *q, struct vb2_buffer **vb,
void *pb, int nonblocking)
{
unsigned long flags;
- int ret;
+ int ret = 0;
/*
* Wait for at least one buffer to become available on the done_list.
@@ -1664,10 +1664,12 @@ static int __vb2_get_done_vb(struct vb2_queue *q, struct vb2_buffer **vb,
spin_lock_irqsave(&q->done_lock, flags);
*vb = list_first_entry(&q->done_list, struct vb2_buffer, done_entry);
/*
- * Only remove the buffer from done_list if v4l2_buffer can handle all
- * the planes.
+ * Only remove the buffer from done_list if all planes can be
+ * handled. Some cases such as V4L2 file I/O and DVB have pb
+ * == NULL; skip the check then as there's nothing to verify.
*/
- ret = call_bufop(q, verify_planes_array, *vb, pb);
+ if (pb)
+ ret = call_bufop(q, verify_planes_array, *vb, pb);
if (!ret)
list_del(&(*vb)->done_entry);
spin_unlock_irqrestore(&q->done_lock, flags);
--
2.1.4
[toc] | [next] | [standalone]
| From | Hans Verkuil <hverkuil@xs4all.nl> |
|---|---|
| Date | 2016-05-13 11:10 +0200 |
| Subject | Re: [PATCH v2 1/2] vb2: core: Skip planes array verification if pb is NULL |
| Message-ID | <ryhgC-qB-7@gated-at.bofh.it> |
| In reply to | #1400003 |
On 05/12/2016 02:14 PM, Sakari Ailus wrote:
> An earlier patch fixing an input validation issue introduced another
> issue: vb2_core_dqbuf() is called with pb argument value NULL in some
> cases, causing a NULL pointer dereference. Fix this by skipping the
> verification as there's nothing to verify.
>
> Signed-off-by: David R <david@unsolicited.net>
>
> Use if () instead of ? :; it's nicer that way. Improve the comment in the
> code as well.
This comment doesn't seem applicable to this patch.
Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com>
Hans
>
> Fixes: e7e0c3e26587 ("[media] videobuf2-core: Check user space planes array in dqbuf")
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Cc: stable@vger.kernel.org # for v4.4 and later
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 10 ++++++----
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index 9fbcb67..633fc1a 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1648,7 +1648,7 @@ static int __vb2_get_done_vb(struct vb2_queue *q, struct vb2_buffer **vb,
> void *pb, int nonblocking)
> {
> unsigned long flags;
> - int ret;
> + int ret = 0;
>
> /*
> * Wait for at least one buffer to become available on the done_list.
> @@ -1664,10 +1664,12 @@ static int __vb2_get_done_vb(struct vb2_queue *q, struct vb2_buffer **vb,
> spin_lock_irqsave(&q->done_lock, flags);
> *vb = list_first_entry(&q->done_list, struct vb2_buffer, done_entry);
> /*
> - * Only remove the buffer from done_list if v4l2_buffer can handle all
> - * the planes.
> + * Only remove the buffer from done_list if all planes can be
> + * handled. Some cases such as V4L2 file I/O and DVB have pb
> + * == NULL; skip the check then as there's nothing to verify.
> */
> - ret = call_bufop(q, verify_planes_array, *vb, pb);
> + if (pb)
> + ret = call_bufop(q, verify_planes_array, *vb, pb);
> if (!ret)
> list_del(&(*vb)->done_entry);
> spin_unlock_irqrestore(&q->done_lock, flags);
>
[toc] | [prev] | [next] | [standalone]
| From | Sakari Ailus <sakari.ailus@linux.intel.com> |
|---|---|
| Date | 2016-05-13 11:40 +0200 |
| Subject | Re: [PATCH v2 1/2] vb2: core: Skip planes array verification if pb is NULL |
| Message-ID | <ryhJE-Hg-15@gated-at.bofh.it> |
| In reply to | #1400626 |
Hi Hans, Hans Verkuil wrote: > On 05/12/2016 02:14 PM, Sakari Ailus wrote: >> An earlier patch fixing an input validation issue introduced another >> issue: vb2_core_dqbuf() is called with pb argument value NULL in some >> cases, causing a NULL pointer dereference. Fix this by skipping the >> verification as there's nothing to verify. >> >> Signed-off-by: David R <david@unsolicited.net> >> >> Use if () instead of ? :; it's nicer that way. Improve the comment in the >> code as well. > > This comment doesn't seem applicable to this patch. Compared to the original patch. I can sure drop the comment as well, it's not that important. > > Reviewed-by: Hans Verkuil <hans.verkuil@cisco.com> Thanks! -- Sakari Ailus sakari.ailus@linux.intel.com
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web