mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Stefan Richter <stefanr@s5r6.in-berlin.de>
To: linux1394-devel@lists.sourceforge.net
Cc: bugme-daemon@bugzilla.kernel.org, linux-kernel@vger.kernel.org,
	Johannes Weiner <hannes@saeurebad.de>
Subject: [Bug 11823][PATCH] ieee1394: dv1394: fix possible deadlock in multithreaded clients
Date: Sun, 26 Oct 2008 12:03:37 +0100 (CET)	[thread overview]
Message-ID: <tkrat.306c405611bf6756@s5r6.in-berlin.de> (raw)
In-Reply-To: <4902E80B.7090805@s5r6.in-berlin.de>

Fix a possible though highly unlikely deadlock:

Thread A:                  Thread B:
 - acquire mmap_sem         - dv1394_ioctl/read/write()
 - dv1394_mmap()            - acquire video->mtx
 - acquire video->mtx       - copy_to/from_user(), possible page fault:
                              acquire mmap_sem

The simplest fix is to use mutex_trylock() instead of mutex_lock() in
dv1394_mmap().  This changes the behavior under contention in a way
which is visible to userspace clients.  However, my guess is that no
clients exist which use mmap vs. ioctl/read/write on the dv1394
character device file interface in concurrent threads.

Reported-by: Johannes Weiner <hannes@saeurebad.de>
Signed-off-by: Stefan Richter <stefanr@s5r6.in-berlin.de>
---
 drivers/ieee1394/dv1394.c |   10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

Index: linux/drivers/ieee1394/dv1394.c
===================================================================
--- linux.orig/drivers/ieee1394/dv1394.c
+++ linux/drivers/ieee1394/dv1394.c
@@ -1270,8 +1270,14 @@ static int dv1394_mmap(struct file *file
 	struct video_card *video = file_to_video_card(file);
 	int retval = -EINVAL;
 
-	/* serialize mmap */
-	mutex_lock(&video->mtx);
+	/*
+	 * We cannot use the blocking variant mutex_lock here because .mmap
+	 * is called with mmap_sem held, while .ioctl, .read, .write acquire
+	 * video->mtx and subsequently call copy_to/from_user which will
+	 * grab mmap_sem in case of a page fault.
+	 */
+	if (!mutex_trylock(&video->mtx))
+		return -EAGAIN;
 
 	if ( ! video_card_initialized(video) ) {
 		retval = do_dv1394_init_default(video);

-- 
Stefan Richter
-=====-==--- =-=- ==-=-
http://arcgraph.de/sr/


           reply	other threads:[~2008-10-26 11:08 UTC|newest]

Thread overview: expand[flat|nested]  mbox.gz  Atom feed
 [parent not found: <4902E80B.7090805@s5r6.in-berlin.de>]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=tkrat.306c405611bf6756@s5r6.in-berlin.de \
    --to=stefanr@s5r6.in-berlin.de \
    --cc=bugme-daemon@bugzilla.kernel.org \
    --cc=hannes@saeurebad.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux1394-devel@lists.sourceforge.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®