* [PATCH v2 0/2] videobuf2: Fix kernel memory overwriting @ 2016-05-12 12:14 Sakari Ailus 2016-05-12 12:14 ` [PATCH v2 1/2] vb2: core: Skip planes array verification if pb is NULL Sakari Ailus 2016-05-12 12:14 ` [PATCH v2 2/2] [media] videobuf2-v4l2: Verify planes array in buffer dequeueing Sakari Ailus 0 siblings, 2 replies; 5+ messages in thread From: Sakari Ailus @ 2016-05-12 12:14 UTC (permalink / raw) To: linux-media; +Cc: mchehab, hverkuil, david, linux-kernel Hi folks, There was a bug in the first version of the set --- the pb argument to __vb2_get_done_vb() is NULL in cases which did not get tested the last time. The second patch in the set was reverted for that reason. This set contains a patch originally from David and again the second reverted patch to fix the memory corruption issue. The previous set can be found here: <URL:http://www.spinics.net/lists/linux-media/msg99336.html> The patches have been tested with V4L2 streaming and file I/O API but not with DVB. -- Kind regards, Sakari ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] vb2: core: Skip planes array verification if pb is NULL 2016-05-12 12:14 [PATCH v2 0/2] videobuf2: Fix kernel memory overwriting Sakari Ailus @ 2016-05-12 12:14 ` Sakari Ailus 2016-05-13 9:09 ` Hans Verkuil 2016-05-12 12:14 ` [PATCH v2 2/2] [media] videobuf2-v4l2: Verify planes array in buffer dequeueing Sakari Ailus 1 sibling, 1 reply; 5+ messages in thread From: Sakari Ailus @ 2016-05-12 12:14 UTC (permalink / raw) To: linux-media; +Cc: mchehab, hverkuil, david, linux-kernel, Sakari Ailus 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] vb2: core: Skip planes array verification if pb is NULL 2016-05-12 12:14 ` [PATCH v2 1/2] vb2: core: Skip planes array verification if pb is NULL Sakari Ailus @ 2016-05-13 9:09 ` Hans Verkuil 2016-05-13 9:38 ` Sakari Ailus 0 siblings, 1 reply; 5+ messages in thread From: Hans Verkuil @ 2016-05-13 9:09 UTC (permalink / raw) To: Sakari Ailus, linux-media; +Cc: mchehab, david, linux-kernel 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); > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] vb2: core: Skip planes array verification if pb is NULL 2016-05-13 9:09 ` Hans Verkuil @ 2016-05-13 9:38 ` Sakari Ailus 0 siblings, 0 replies; 5+ messages in thread From: Sakari Ailus @ 2016-05-13 9:38 UTC (permalink / raw) To: Hans Verkuil, linux-media; +Cc: mchehab, david, linux-kernel 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] [media] videobuf2-v4l2: Verify planes array in buffer dequeueing 2016-05-12 12:14 [PATCH v2 0/2] videobuf2: Fix kernel memory overwriting Sakari Ailus 2016-05-12 12:14 ` [PATCH v2 1/2] vb2: core: Skip planes array verification if pb is NULL Sakari Ailus @ 2016-05-12 12:14 ` Sakari Ailus 1 sibling, 0 replies; 5+ messages in thread From: Sakari Ailus @ 2016-05-12 12:14 UTC (permalink / raw) To: linux-media; +Cc: mchehab, hverkuil, david, linux-kernel, Sakari Ailus 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 <sakari.ailus@linux.intel.com> Acked-by: Hans Verkuil <hans.verkuil@cisco.com> Cc: stable@vger.kernel.org # for v4.4 and later Signed-off-by: Mauro Carvalho Chehab <mchehab@osg.samsung.com> --- drivers/media/v4l2-core/videobuf2-v4l2.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/drivers/media/v4l2-core/videobuf2-v4l2.c b/drivers/media/v4l2-core/videobuf2-v4l2.c index 0b1b8c7..7f366f1 100644 --- a/drivers/media/v4l2-core/videobuf2-v4l2.c +++ b/drivers/media/v4l2-core/videobuf2-v4l2.c @@ -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, -- 2.1.4 ^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2016-05-13 9:38 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2016-05-12 12:14 [PATCH v2 0/2] videobuf2: Fix kernel memory overwriting Sakari Ailus 2016-05-12 12:14 ` [PATCH v2 1/2] vb2: core: Skip planes array verification if pb is NULL Sakari Ailus 2016-05-13 9:09 ` Hans Verkuil 2016-05-13 9:38 ` Sakari Ailus 2016-05-12 12:14 ` [PATCH v2 2/2] [media] videobuf2-v4l2: Verify planes array in buffer dequeueing Sakari Ailus
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.