| Message ID | 1376772531-8963-1-git-send-email-khoroshilov@ispras.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Em Sat, 17 Aug 2013 23:48:51 +0300 Alexey Khoroshilov <khoroshilov@ispras.ru> escreveu: > dmx_section_feed_release_filter() locks dvbdmx->mutex and > if the feed is still filtering, it calls feed->stop_filtering(feed). > stop_filtering() is implemented by dmx_section_feed_stop_filtering() > that first of all try to lock the same mutex: dvbdmx->mutex. > That leads to a deadlock. > > It does not happen often in practice because all callers of > release_filter() stop filtering by themselves. > So the problem can happen in case of race condition only. > > The patch proposes to unlock dvbdmx->mutex before call feed->stop_filtering(feed) > and recheck feed->is_filtering after reacquiring mutex. > > Found by Linux Driver Verification project (linuxtesting.org). > > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> > --- > drivers/media/dvb-core/dvb_demux.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/dvb-core/dvb_demux.c b/drivers/media/dvb-core/dvb_demux.c > index 3485655..9d517af 100644 > --- a/drivers/media/dvb-core/dvb_demux.c > +++ b/drivers/media/dvb-core/dvb_demux.c > @@ -1027,8 +1027,11 @@ static int dmx_section_feed_release_filter(struct dmx_section_feed *feed, > return -EINVAL; > } > > - if (feed->is_filtering) > + while (feed->is_filtering) { Changing from if to while here can cause a dead lock, as, if the device got removed, dmx_section_feed_stop_filtering() won't touch feed->is_filtering. So, the loop will happen forever. Except for that the patch looks correct on my eyes. > + mutex_unlock(&dvbdmx->mutex); Please add a small comment about why the mutex needs to be unlocked there, as this looks ugly, and likely deserve some cleanups in the future (as both the spinclock and the mutex are taken on both callback and at the release filter function). > feed->stop_filtering(feed); > + mutex_lock(&dvbdmx->mutex); > + } > > spin_lock_irq(&dvbdmx->lock); > f = dvbdmxfeed->filter; Thanks! Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/media/dvb-core/dvb_demux.c b/drivers/media/dvb-core/dvb_demux.c index 3485655..9d517af 100644 --- a/drivers/media/dvb-core/dvb_demux.c +++ b/drivers/media/dvb-core/dvb_demux.c @@ -1027,8 +1027,11 @@ static int dmx_section_feed_release_filter(struct dmx_section_feed *feed, return -EINVAL; } - if (feed->is_filtering) + while (feed->is_filtering) { + mutex_unlock(&dvbdmx->mutex); feed->stop_filtering(feed); + mutex_lock(&dvbdmx->mutex); + } spin_lock_irq(&dvbdmx->lock); f = dvbdmxfeed->filter;
dmx_section_feed_release_filter() locks dvbdmx->mutex and if the feed is still filtering, it calls feed->stop_filtering(feed). stop_filtering() is implemented by dmx_section_feed_stop_filtering() that first of all try to lock the same mutex: dvbdmx->mutex. That leads to a deadlock. It does not happen often in practice because all callers of release_filter() stop filtering by themselves. So the problem can happen in case of race condition only. The patch proposes to unlock dvbdmx->mutex before call feed->stop_filtering(feed) and recheck feed->is_filtering after reacquiring mutex. Found by Linux Driver Verification project (linuxtesting.org). Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru> --- drivers/media/dvb-core/dvb_demux.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)