All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: linux-media@vger.kernel.org
Cc: Huber Andreas <hobrom@corax.at>,
	Mauro Carvalho Chehab <mchehab@redhat.com>,
	Hans Verkuil <hverkuil@xs4all.nl>,
	linux-kernel@vger.kernel.org, Ben Hutchings <ben@decadent.org.uk>,
	Steven Toth <stoth@kernellabs.com>
Subject: [PATCH 3/7] [media] cx88: hold device lock during sub-driver initialization
Date: Mon, 4 Apr 2011 22:27:13 -0500	[thread overview]
Message-ID: <20110405032713.GD4498@elie> (raw)
In-Reply-To: <20110405032014.GA4498@elie>

cx8802_blackbird_probe makes a device node for the mpeg sub-device
before it has been added to dev->drvlist.  If the device is opened
during that time, the open succeeds but request_acquire cannot be
called, so the reference count remains zero.  Later, when the device
is closed, the reference count becomes negative --- uh oh.

Close the race by holding core->lock during probe and not releasing
until the device is in drvlist and initialization finished.
Previously the BKL prevented this race.

Reported-by: Andreas Huber <hobrom@gmx.at>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
 drivers/media/video/cx88/cx88-blackbird.c |    2 --
 drivers/media/video/cx88/cx88-mpeg.c      |    5 ++---
 drivers/media/video/cx88/cx88.h           |    7 ++-----
 3 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/media/video/cx88/cx88-blackbird.c b/drivers/media/video/cx88/cx88-blackbird.c
index a6f7d53..f637d34 100644
--- a/drivers/media/video/cx88/cx88-blackbird.c
+++ b/drivers/media/video/cx88/cx88-blackbird.c
@@ -1335,11 +1335,9 @@ static int cx8802_blackbird_probe(struct cx8802_driver *drv)
 	blackbird_register_video(dev);
 
 	/* initial device configuration: needed ? */
-	mutex_lock(&dev->core->lock);
 //	init_controls(core);
 	cx88_set_tvnorm(core,core->tvnorm);
 	cx88_video_mux(core,0);
-	mutex_unlock(&dev->core->lock);
 
 	return 0;
 
diff --git a/drivers/media/video/cx88/cx88-mpeg.c b/drivers/media/video/cx88/cx88-mpeg.c
index 9147c16..497f26f 100644
--- a/drivers/media/video/cx88/cx88-mpeg.c
+++ b/drivers/media/video/cx88/cx88-mpeg.c
@@ -709,18 +709,17 @@ int cx8802_register_driver(struct cx8802_driver *drv)
 		drv->request_release = cx8802_request_release;
 		memcpy(driver, drv, sizeof(*driver));
 
+		mutex_lock(&drv->core->lock);
 		err = drv->probe(driver);
 		if (err == 0) {
 			i++;
-			mutex_lock(&drv->core->lock);
 			list_add_tail(&driver->drvlist, &dev->drvlist);
-			mutex_unlock(&drv->core->lock);
 		} else {
 			printk(KERN_ERR
 			       "%s/2: cx8802 probe failed, err = %d\n",
 			       dev->core->name, err);
 		}
-
+		mutex_unlock(&drv->core->lock);
 	}
 
 	return i ? 0 : -ENODEV;
diff --git a/drivers/media/video/cx88/cx88.h b/drivers/media/video/cx88/cx88.h
index 9731daa..3d32f4a 100644
--- a/drivers/media/video/cx88/cx88.h
+++ b/drivers/media/video/cx88/cx88.h
@@ -505,13 +505,10 @@ struct cx8802_driver {
 	int (*suspend)(struct pci_dev *pci_dev, pm_message_t state);
 	int (*resume)(struct pci_dev *pci_dev);
 
+	/* Callers to the following functions must hold core->lock */
+
 	/* MPEG 8802 -> mini driver - Driver probe and configuration */
-
-	/* Caller must _not_ hold core->lock */
 	int (*probe)(struct cx8802_driver *drv);
-
-	/* Callers to the following functions must hold core->lock */
-
 	int (*remove)(struct cx8802_driver *drv);
 
 	/* MPEG 8802 -> mini driver - Access for hardware control */
-- 
1.7.5.rc0


  parent reply	other threads:[~2011-04-05  3:27 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20110327150610.4029.95961.reportbug@xen.corax.at>
2011-03-27 15:28 ` [linux-dvb] cx88-blackbird broken (since 2.6.37) Jonathan Nieder
2011-04-02  9:38   ` [RFC/PATCH 0/3] locking fixes for cx88 Jonathan Nieder
2011-04-02  9:41     ` [PATCH 1/3] [media] cx88: protect per-device driver list with device lock Jonathan Nieder
2011-04-02  9:41     ` [PATCH 2/3] [media] cx88: fix locking of sub-driver operations Jonathan Nieder
2011-04-02  9:44     ` [PATCH 3/3] [media] cx88: use a mutex to protect cx8802_devlist Jonathan Nieder
     [not found]       ` <4D971B8D.4040305@corax.at>
2011-04-02 19:29         ` Jonathan Nieder
2011-04-02 20:13           ` Andreas Huber
2011-04-04  2:12             ` Andreas Huber
2011-04-05  1:16               ` Jonathan Nieder
2011-04-02 14:05     ` [RFC/PATCH 0/3] locking fixes for cx88 Andreas Huber
2011-04-02 15:19     ` Ben Hutchings
2011-04-02 18:30       ` Jonathan Nieder
2011-04-05  3:20     ` [RFC/PATCH v2 0/7] " Jonathan Nieder
2011-04-05  3:22       ` [PATCH 1/7] [media] cx88: protect per-device driver list with device lock Jonathan Nieder
2011-04-05  3:25       ` [PATCH 2/7] [media] cx88: fix locking of sub-driver operations Jonathan Nieder
2011-04-05  3:27       ` Jonathan Nieder [this message]
2011-04-05  3:27       ` [PATCH 4/7] [media] cx88: use a mutex to protect cx8802_devlist Jonathan Nieder
2011-04-05  3:29       ` [PATCH 5/7] [media] cx88: gracefully reject attempts to use unregistered cx88-blackbird driver Jonathan Nieder
2011-04-05  3:30       ` [PATCH 6/7] [media] cx88: don't use atomic_t for core->mpeg_users Jonathan Nieder
2011-04-05  3:31       ` [PATCH 7/7] [mpeg] cx88: don't use atomic_t for core->users Jonathan Nieder
2011-04-05  3:41         ` Jonathan Nieder
2011-04-05 18:17       ` [RFC/PATCH v2 0/7] locking fixes for cx88 Jonathan Nieder
2011-05-01  9:17 [PATCH v2.6.38 resend 0/7] cx88 deadlock and data races Jonathan Nieder
2011-05-01  9:29 ` [PATCH 3/7] [media] cx88: hold device lock during sub-driver initialization Jonathan Nieder

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=20110405032713.GD4498@elie \
    --to=jrnieder@gmail.com \
    --cc=ben@decadent.org.uk \
    --cc=hobrom@corax.at \
    --cc=hverkuil@xs4all.nl \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=mchehab@redhat.com \
    --cc=stoth@kernellabs.com \
    /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 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.