* [PATCH] videobuf2-core: take mmap_sem before calling __qbuf_userptr
@ 2014-08-22 18:01 Hans Verkuil
2014-08-22 18:58 ` Laurent Pinchart
2014-08-22 19:34 ` Jan Kara
0 siblings, 2 replies; 4+ messages in thread
From: Hans Verkuil @ 2014-08-22 18:01 UTC (permalink / raw)
To: LMML; +Cc: Jan Kara, m.szyprowski, pawel, Laurent Pinchart
Commit f035eb4e976ef5a059e30bc91cfd310ff030a7d3 (videobuf2: fix lockdep warning)
unfortunately removed the mmap_sem lock that is needed around the call to
__qbuf_userptr. Amazingly nobody noticed this until Jan Kara pointed this out
to me.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Reported-by: Jan Kara <jack@suse.cz>
---
drivers/media/v4l2-core/videobuf2-core.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 5b808e2..2f6ac7e 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1591,6 +1591,7 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
{
struct vb2_queue *q = vb->vb2_queue;
+ struct rw_semaphore *mmap_sem;
int ret;
ret = __verify_length(vb, b);
@@ -1627,7 +1628,9 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
ret = __qbuf_mmap(vb, b);
break;
case V4L2_MEMORY_USERPTR:
+ down_read(mmap_sem);
ret = __qbuf_userptr(vb, b);
+ up_read(mmap_sem);
break;
case V4L2_MEMORY_DMABUF:
ret = __qbuf_dmabuf(vb, b);
--
2.0.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] videobuf2-core: take mmap_sem before calling __qbuf_userptr
2014-08-22 18:01 [PATCH] videobuf2-core: take mmap_sem before calling __qbuf_userptr Hans Verkuil
@ 2014-08-22 18:58 ` Laurent Pinchart
2014-08-22 19:34 ` Jan Kara
1 sibling, 0 replies; 4+ messages in thread
From: Laurent Pinchart @ 2014-08-22 18:58 UTC (permalink / raw)
To: Hans Verkuil; +Cc: LMML, Jan Kara, m.szyprowski, pawel
Hi Hans,
Thank you for the patch.
On Friday 22 August 2014 18:01:09 Hans Verkuil wrote:
> Commit f035eb4e976ef5a059e30bc91cfd310ff030a7d3 (videobuf2: fix lockdep
> warning) unfortunately removed the mmap_sem lock that is needed around the
> call to __qbuf_userptr. Amazingly nobody noticed this until Jan Kara
> pointed this out to me.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Reported-by: Jan Kara <jack@suse.cz>
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c index 5b808e2..2f6ac7e 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -1591,6 +1591,7 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
> static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer
> *b) {
> struct vb2_queue *q = vb->vb2_queue;
> + struct rw_semaphore *mmap_sem;
I don't want to to be included in the category of the "amazing reviewers who
don't notice obvious issues" twice in a row, so I'll point out the obvious
lack of initialization of the mmap_sem variable :-)
> int ret;
>
> ret = __verify_length(vb, b);
> @@ -1627,7 +1628,9 @@ static int __buf_prepare(struct vb2_buffer *vb, const
> struct v4l2_buffer *b) ret = __qbuf_mmap(vb, b);
> break;
> case V4L2_MEMORY_USERPTR:
> + down_read(mmap_sem);
> ret = __qbuf_userptr(vb, b);
> + up_read(mmap_sem);
> break;
> case V4L2_MEMORY_DMABUF:
> ret = __qbuf_dmabuf(vb, b);
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] videobuf2-core: take mmap_sem before calling __qbuf_userptr
2014-08-22 18:01 [PATCH] videobuf2-core: take mmap_sem before calling __qbuf_userptr Hans Verkuil
2014-08-22 18:58 ` Laurent Pinchart
@ 2014-08-22 19:34 ` Jan Kara
2014-08-22 23:24 ` Hans Verkuil
1 sibling, 1 reply; 4+ messages in thread
From: Jan Kara @ 2014-08-22 19:34 UTC (permalink / raw)
To: Hans Verkuil; +Cc: LMML, Jan Kara, m.szyprowski, pawel, Laurent Pinchart
On Fri 22-08-14 18:01:09, Hans Verkuil wrote:
> Commit f035eb4e976ef5a059e30bc91cfd310ff030a7d3 (videobuf2: fix lockdep warning)
> unfortunately removed the mmap_sem lock that is needed around the call to
> __qbuf_userptr. Amazingly nobody noticed this until Jan Kara pointed this out
> to me.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Reported-by: Jan Kara <jack@suse.cz>
...
> @@ -1627,7 +1628,9 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
> ret = __qbuf_mmap(vb, b);
> break;
> case V4L2_MEMORY_USERPTR:
I guess you are missing something like:
mmap_sem = ¤t->mm->mmap_sem;
Honza
> + down_read(mmap_sem);
> ret = __qbuf_userptr(vb, b);
> + up_read(mmap_sem);
> break;
> case V4L2_MEMORY_DMABUF:
> ret = __qbuf_dmabuf(vb, b);
> --
> 2.0.1
>
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] videobuf2-core: take mmap_sem before calling __qbuf_userptr
2014-08-22 19:34 ` Jan Kara
@ 2014-08-22 23:24 ` Hans Verkuil
0 siblings, 0 replies; 4+ messages in thread
From: Hans Verkuil @ 2014-08-22 23:24 UTC (permalink / raw)
To: Jan Kara; +Cc: LMML, m.szyprowski, pawel, Laurent Pinchart
On 08/22/2014 07:34 PM, Jan Kara wrote:
> On Fri 22-08-14 18:01:09, Hans Verkuil wrote:
>> Commit f035eb4e976ef5a059e30bc91cfd310ff030a7d3 (videobuf2: fix lockdep warning)
>> unfortunately removed the mmap_sem lock that is needed around the call to
>> __qbuf_userptr. Amazingly nobody noticed this until Jan Kara pointed this out
>> to me.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Reported-by: Jan Kara <jack@suse.cz>
> ...
>> @@ -1627,7 +1628,9 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>> ret = __qbuf_mmap(vb, b);
>> break;
>> case V4L2_MEMORY_USERPTR:
> I guess you are missing something like:
>
> mmap_sem = ¤t->mm->mmap_sem;
Insert <red face>.
Oops, I'll repost.
Regards,
Hans
>
> Honza
>> + down_read(mmap_sem);
>> ret = __qbuf_userptr(vb, b);
>> + up_read(mmap_sem);
>> break;
>> case V4L2_MEMORY_DMABUF:
>> ret = __qbuf_dmabuf(vb, b);
>> --
>> 2.0.1
>>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-08-22 23:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-08-22 18:01 [PATCH] videobuf2-core: take mmap_sem before calling __qbuf_userptr Hans Verkuil
2014-08-22 18:58 ` Laurent Pinchart
2014-08-22 19:34 ` Jan Kara
2014-08-22 23:24 ` Hans Verkuil
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.