media: video: usbvision: add mutex_unlock() to error paths
PATCH 1/1 media: video: usbvision: add mutex_unlock() to error paths
There are a few error paths which don't unlock the usbvision->lock.
So I've added mutex_unlock() calls to fix those paths.
--- drivers/media/video/usbvision/usbvision-video.c | 2 ++ 1 file changed, 2 insertions(+)
drivers/media/video/usbvision/usbvision-video.c +2 -0
@@ -1290,6 +1290,7 @@ static int usbvision_radio_open(struct i errCode =usbvision_set_alternate (usbvision);if (errCode <0 ) { usbvision->last_error = errCode; +mutex_unlock (&usbvision->lock);return -EBUSY ; }
@@ -1806,6 +1807,7 @@ static int __devinit usbvision_probe(str usbvision->num_alt,GFP_KERNEL);if (usbvision->alt_max_pkt_size ==NULL ) {err ("usbvision: out of memory!\n" ); +mutex_unlock (&usbvision->lock);return -ENOMEM ; }
Signature · 1 line
--
Quoted 23 lines · On Tue, 11 Dec 2007 17:56:50 -0800 Daniel Walker <[email protected]> wrote:
There are a few error paths which don't unlock the
usbvision->lock.So I've added
mutex_unlock()calls to fix those paths.Signed-off-by Daniel Walker <[email protected]>--- drivers/media/video/usbvision/usbvision-video.c | 2 ++ 1 file changed, 2 insertions(+)Index:
linux-2.6.23/drivers/media/video/usbvision/usbvision-video.c
===================================================================drivers/media/video/usbvision/usbvision-video.c +1
-0@@ -1290,6 +1290,7 @@ static int usbvision_radio_open(struct i errCode =usbvision_set_alternate (usbvision);if (errCode <0 ) { usbvision->last_error = errCode; +mutex_unlock (&usbvision->lock);return -EBUSY ; }
drivers/media/video/usbvision/usbvision-video.c static int __devinit usbvision_probe(str · line 1813
Quoted 7 lines
drivers/media/video/usbvision/usbvision-video.c +1
-0@@ -1806,6 +1807,7 @@ static int __devinit usbvision_probe(str usbvision->num_alt,GFP_KERNEL);if (usbvision->alt_max_pkt_size ==NULL ) {err ("usbvision: out of memory!\n" ); +mutex_unlock (&usbvision->lock);return -ENOMEM ; }
Well yes. But the bug which you've found is a *direct* consequence of a coding mistake in those functions: they have multiple deeply-nested return' statements. This is a common cause of locking errors and resource leaks and is why we prefer the goto place-which-unwinds' approach.
We can easily fix that in one case, at least.
One does wonder whether usbvision_radio_open() should be returning the usbvision_set_alternate() result to the caller here, rather than overwriting it with -EBUSY.
drivers/media/video/usbvision/usbvision-video.c renamed from drivers/media/video/usbvision/usbvision-video.c~media-video-usbvision-add-mutex_unlock-to-error-paths-fix +3 -2
@@ -1290,8 +1290,8 @@ static int usbvision_radio_open(struct i errCode =usbvision_set_alternate (usbvision);if (errCode <0 ) { usbvision->last_error = errCode; -mutex_unlock (&usbvision->lock); -return -EBUSY ; + errCode = -EBUSY ; + goto out; }// If so far no errors then we shall start the radio
@@ -1308,6 +1308,7 @@ static int usbvision_radio_open(struct i usbvision->initialized =0 ; } } +out:mutex_unlock (&usbvision->lock);return errCode; }
_