linux-kernel - Re: mplayer v4l hangs in 2.6.25.2/4 (likely regression)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:	Tue, 20 May 2008 09:53:52 -0700
From:	Arjan van de Ven <arjan@...radead.org>
To:	Mauro Carvalho Chehab <mchehab@...radead.org>
Cc:	"koos vriezen" <koos.vriezen@...il.com>,
	linux-kernel@...r.kernel.org, torvalds@...ux-foundation.org,
	Krufky <mkrufky@...radead.org>, stable@...nel.org
Subject: Re: mplayer v4l hangs in 2.6.25.2/4 (likely regression)

On Tue, 20 May 2008 08:49:11 -0300
Mauro Carvalho Chehab <mchehab@...radead.org> wrote:

> > Mauro: will you send this on to Linus or should this go direct?
> 
> The fix looks sane. I prefer to just remove the lock call from bttv,
> instead of calling  __videobuf_mmap_setup() and make this symbol
> global. The better is to avoid locking inside the drivers, except
> during the interrupts, and inside videbuf code. This is what we're
> doing on the other drivers.
> 
> So, it would be better if you could change your patch to remove the
> lock/unlock at bttv-driver handling for this ioctl.

but it's more complex than I want to deal with right now.. lets changing
the locking in .27 instead rather than during an -rc
> 
> To solve the bug, both ways work, so:
> 
> Acked-by: Mauro Carvalho Chehab <mchehab@...radead.org>
> 
> Could you please send it to Linus and to -stable for me? Otherwise,
> I'll do this by Monday.
> 

Sure

Linus, please apply this bugfix:


From: Arjan van de Ven <arjan@...ux.intel.com>
Subject: [PATCH] Fix a deadlock in the bttv driver

vidiocgmbuf() does this:
        mutex_lock(&fh->cap.vb_lock);
        retval = videobuf_mmap_setup(&fh->cap, gbuffers, gbufsize,
                                     V4L2_MEMORY_MMAP);

and videobuf_mmap_setup() then just does
        mutex_lock(&q->vb_lock);
        ret = __videobuf_mmap_setup(q, bcount, bsize, memory);
        mutex_unlock(&q->vb_lock);

which is an obvious double-take deadlock.

This patch fixes this by having vidiocgmbuf() just call the __videobuf_mmap_setup
function instead.

Acked-by: Mauro Carvalho Chehab <mchehab@...radead.org> 
Reported-by: Koos Vriezen <koos.vriezen@...il.com>
Signed-off-by: Arjan van de Ven <arjan@...ux.intel.com>
---
 drivers/media/video/bt8xx/bttv-driver.c |    2 +-
 drivers/media/video/videobuf-core.c     |    3 ++-
 include/media/videobuf-core.h           |    3 +++
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/bt8xx/bttv-driver.c b/drivers/media/video/bt8xx/bttv-driver.c
index 2ca3e9c..0165aac 100644
--- a/drivers/media/video/bt8xx/bttv-driver.c
+++ b/drivers/media/video/bt8xx/bttv-driver.c
@@ -2613,7 +2613,7 @@ static int vidiocgmbuf(struct file *file, void *priv, struct video_mbuf *mbuf)
 	struct bttv_fh *fh = priv;
 
 	mutex_lock(&fh->cap.vb_lock);
-	retval = videobuf_mmap_setup(&fh->cap, gbuffers, gbufsize,
+	retval = __videobuf_mmap_setup(&fh->cap, gbuffers, gbufsize,
 				     V4L2_MEMORY_MMAP);
 	if (retval < 0) {
 		mutex_unlock(&fh->cap.vb_lock);
diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c
index 982f446..0a88c44 100644
--- a/drivers/media/video/videobuf-core.c
+++ b/drivers/media/video/videobuf-core.c
@@ -331,7 +331,7 @@ int videobuf_mmap_free(struct videobuf_queue *q)
 }
 
 /* Locking: Caller holds q->vb_lock */
-static int __videobuf_mmap_setup(struct videobuf_queue *q,
+int __videobuf_mmap_setup(struct videobuf_queue *q,
 			unsigned int bcount, unsigned int bsize,
 			enum v4l2_memory memory)
 {
@@ -1129,6 +1129,7 @@ EXPORT_SYMBOL_GPL(videobuf_read_stream);
 EXPORT_SYMBOL_GPL(videobuf_read_one);
 EXPORT_SYMBOL_GPL(videobuf_poll_stream);
 
+EXPORT_SYMBOL_GPL(__videobuf_mmap_setup);
 EXPORT_SYMBOL_GPL(videobuf_mmap_setup);
 EXPORT_SYMBOL_GPL(videobuf_mmap_free);
 EXPORT_SYMBOL_GPL(videobuf_mmap_mapper);
diff --git a/include/media/videobuf-core.h b/include/media/videobuf-core.h
index 5b39a22..874f134 100644
--- a/include/media/videobuf-core.h
+++ b/include/media/videobuf-core.h
@@ -237,6 +237,9 @@ unsigned int videobuf_poll_stream(struct file *file,
 int videobuf_mmap_setup(struct videobuf_queue *q,
 			unsigned int bcount, unsigned int bsize,
 			enum v4l2_memory memory);
+int __videobuf_mmap_setup(struct videobuf_queue *q,
+			unsigned int bcount, unsigned int bsize,
+			enum v4l2_memory memory);
 int videobuf_mmap_free(struct videobuf_queue *q);
 int videobuf_mmap_mapper(struct videobuf_queue *q,
 			 struct vm_area_struct *vma);
-- 
1.5.4.5

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/