From: Simon Arlott <simon@fire.lp0.eu>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Mauro <mchehab@infradead.org>,
Andreas Oberritter <obi@linuxtv.org>,
Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
linux-dvb@linuxtv.org
Subject: [PATCH] dvb-core: Fix several locking related problems.
Date: Sun, 04 Mar 2007 17:45:54 +0000 [thread overview]
Message-ID: <45EB05D2.90608@simon.arlott.org.uk> (raw)
In-Reply-To: <45EAE894.1010000@linuxtv.org>
Fix several instances of dvb-core functions using mutex_lock_interruptible and
returning -ERESTARTSYS where the calling function will either never retry or
never check the return value.
These cause a race condition with dvb_dmxdev_filter_free and dvb_dvr_release,
both of which are filesystem release functions whose return value is ignored
and will never be retried. When this happens it becomes impossible to open dvr0
again (-EBUSY) since it has not been released properly.
Signed-off-by: Simon Arlott <simon@fire.lp0.eu>
---
On 04/03/07 15:41, Andreas Oberritter wrote:
> please send an updated patch together with
> Signed-off-by line to Mauro <mchehab@infradead.org> and ask him to apply
> it for inclusion into the -mm tree for further testing.
Unless there are other -mm trees I've not heard about, presumably I should just
do this myself. Doesn't linux-dvb have it's own development tree this would
get better tested in?
The dvb_dvr_release change has been working for me for 6 months and the
dvb_dmxdev_filter_free (dvb_dmxdev_filter_free) change looks equivalent.
See http://www.linuxtv.org/pipermail/linux-dvb/2007-February/016120.html for
an example of the bug before and after fixing.
All the other changes run ok for me but should have lockdep enabled when
testing (if there's a possible deadlock somewhere, using _interruptible will
hide it).
drivers/media/dvb/dvb-core/dmxdev.c | 12 +++---------
drivers/media/dvb/dvb-core/dvb_demux.c | 21 +++++++--------------
drivers/media/dvb/dvb-core/dvbdev.c | 9 +++------
3 files changed, 13 insertions(+), 29 deletions(-)
diff --git a/drivers/media/dvb/dvb-core/dmxdev.c b/drivers/media/dvb/dvb-core/dmxdev.c
index fc77de4..a5c0e1a 100644
--- a/drivers/media/dvb/dvb-core/dmxdev.c
+++ b/drivers/media/dvb/dvb-core/dmxdev.c
@@ -180,8 +180,7 @@ static int dvb_dvr_release(struct inode *inode, struct file *file)
struct dvb_device *dvbdev = file->private_data;
struct dmxdev *dmxdev = dvbdev->priv;
- if (mutex_lock_interruptible(&dmxdev->mutex))
- return -ERESTARTSYS;
+ mutex_lock(&dmxdev->mutex);
if ((file->f_flags & O_ACCMODE) == O_WRONLY) {
dmxdev->demux->disconnect_frontend(dmxdev->demux);
@@ -673,13 +672,8 @@ static int dvb_demux_open(struct inode *inode, struct file *file)
static int dvb_dmxdev_filter_free(struct dmxdev *dmxdev,
struct dmxdev_filter *dmxdevfilter)
{
- if (mutex_lock_interruptible(&dmxdev->mutex))
- return -ERESTARTSYS;
-
- if (mutex_lock_interruptible(&dmxdevfilter->mutex)) {
- mutex_unlock(&dmxdev->mutex);
- return -ERESTARTSYS;
- }
+ mutex_lock(&dmxdev->mutex);
+ mutex_lock(&dmxdevfilter->mutex);
dvb_dmxdev_filter_stop(dmxdevfilter);
dvb_dmxdev_filter_reset(dmxdevfilter);
diff --git a/drivers/media/dvb/dvb-core/dvb_demux.c b/drivers/media/dvb/dvb-core/dvb_demux.c
index fcff5ea..6d8d1c3 100644
--- a/drivers/media/dvb/dvb-core/dvb_demux.c
+++ b/drivers/media/dvb/dvb-core/dvb_demux.c
@@ -673,8 +673,7 @@ static int dmx_ts_feed_stop_filtering(struct dmx_ts_feed *ts_feed)
struct dvb_demux *demux = feed->demux;
int ret;
- if (mutex_lock_interruptible(&demux->mutex))
- return -ERESTARTSYS;
+ mutex_lock(&demux->mutex);
if (feed->state < DMX_STATE_GO) {
mutex_unlock(&demux->mutex);
@@ -748,8 +747,7 @@ static int dvbdmx_release_ts_feed(struct dmx_demux *dmx,
struct dvb_demux *demux = (struct dvb_demux *)dmx;
struct dvb_demux_feed *feed = (struct dvb_demux_feed *)ts_feed;
- if (mutex_lock_interruptible(&demux->mutex))
- return -ERESTARTSYS;
+ mutex_lock(&demux->mutex);
if (feed->state == DMX_STATE_FREE) {
mutex_unlock(&demux->mutex);
@@ -916,8 +914,7 @@ static int dmx_section_feed_stop_filtering(struct dmx_section_feed *feed)
struct dvb_demux *dvbdmx = dvbdmxfeed->demux;
int ret;
- if (mutex_lock_interruptible(&dvbdmx->mutex))
- return -ERESTARTSYS;
+ mutex_lock(&dvbdmx->mutex);
if (!dvbdmx->stop_feed) {
mutex_unlock(&dvbdmx->mutex);
@@ -942,8 +939,7 @@ static int dmx_section_feed_release_filter(struct dmx_section_feed *feed,
struct dvb_demux_feed *dvbdmxfeed = (struct dvb_demux_feed *)feed;
struct dvb_demux *dvbdmx = dvbdmxfeed->demux;
- if (mutex_lock_interruptible(&dvbdmx->mutex))
- return -ERESTARTSYS;
+ mutex_lock(&dvbdmx->mutex);
if (dvbdmxfilter->feed != dvbdmxfeed) {
mutex_unlock(&dvbdmx->mutex);
@@ -1016,8 +1012,7 @@ static int dvbdmx_release_section_feed(struct dmx_demux *demux,
struct dvb_demux_feed *dvbdmxfeed = (struct dvb_demux_feed *)feed;
struct dvb_demux *dvbdmx = (struct dvb_demux *)demux;
- if (mutex_lock_interruptible(&dvbdmx->mutex))
- return -ERESTARTSYS;
+ mutex_lock(&dvbdmx->mutex);
if (dvbdmxfeed->state == DMX_STATE_FREE) {
mutex_unlock(&dvbdmx->mutex);
@@ -1126,8 +1121,7 @@ static int dvbdmx_connect_frontend(struct dmx_demux *demux,
if (demux->frontend)
return -EINVAL;
- if (mutex_lock_interruptible(&dvbdemux->mutex))
- return -ERESTARTSYS;
+ mutex_lock(&dvbdemux->mutex);
demux->frontend = frontend;
mutex_unlock(&dvbdemux->mutex);
@@ -1138,8 +1132,7 @@ static int dvbdmx_disconnect_frontend(struct dmx_demux *demux)
{
struct dvb_demux *dvbdemux = (struct dvb_demux *)demux;
- if (mutex_lock_interruptible(&dvbdemux->mutex))
- return -ERESTARTSYS;
+ mutex_lock(&dvbdemux->mutex);
demux->frontend = NULL;
mutex_unlock(&dvbdemux->mutex);
diff --git a/drivers/media/dvb/dvb-core/dvbdev.c b/drivers/media/dvb/dvb-core/dvbdev.c
index b4553ae..1e0f01a 100644
--- a/drivers/media/dvb/dvb-core/dvbdev.c
+++ b/drivers/media/dvb/dvb-core/dvbdev.c
@@ -203,8 +203,7 @@ int dvb_register_device(struct dvb_adapter *adap, struct dvb_device **pdvbdev,
int id;
- if (mutex_lock_interruptible(&dvbdev_register_lock))
- return -ERESTARTSYS;
+ mutex_lock(&dvbdev_register_lock);
if ((id = dvbdev_get_free_id (adap, type)) < 0){
mutex_unlock(&dvbdev_register_lock);
@@ -294,8 +293,7 @@ int dvb_register_adapter(struct dvb_adapter *adap, const char *name, struct modu
{
int num;
- if (mutex_lock_interruptible(&dvbdev_register_lock))
- return -ERESTARTSYS;
+ mutex_lock(&dvbdev_register_lock);
if ((num = dvbdev_get_free_adapter_num ()) < 0) {
mutex_unlock(&dvbdev_register_lock);
@@ -323,8 +321,7 @@ EXPORT_SYMBOL(dvb_register_adapter);
int dvb_unregister_adapter(struct dvb_adapter *adap)
{
- if (mutex_lock_interruptible(&dvbdev_register_lock))
- return -ERESTARTSYS;
+ mutex_lock(&dvbdev_register_lock);
list_del (&adap->list_head);
mutex_unlock(&dvbdev_register_lock);
return 0;
--
1.5.0.1
--
Simon Arlott
next parent reply other threads:[~2007-03-04 17:46 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <45DCB3A3.3060705@simon.arlott.org.uk>
[not found] ` <45E08886.50606@linuxtv.org>
[not found] ` <45E08BFD.3080203@simon.arlott.org.uk>
[not found] ` <45E9D923.7040707@simon.arlott.org.uk>
[not found] ` <45EAE894.1010000@linuxtv.org>
2007-03-04 17:45 ` Simon Arlott [this message]
2007-03-10 1:49 ` [PATCH] dvb-core: Fix several locking related problems Johannes Stezenbach
2007-03-10 9:23 ` Mauro Carvalho Chehab
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=45EB05D2.90608@simon.arlott.org.uk \
--to=simon@fire.lp0.eu \
--cc=akpm@linux-foundation.org \
--cc=linux-dvb@linuxtv.org \
--cc=linux-kernel@vger.kernel.org \
--cc=mchehab@infradead.org \
--cc=obi@linuxtv.org \
/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.