* Re: [PATCH v2 5/5] media: uvcvideo: Abort uvc_v4l2_open if video device is unregistered
@ 2020-09-09 10:52 kernel test robot
0 siblings, 0 replies; 3+ messages in thread
From: kernel test robot @ 2020-09-09 10:52 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 7753 bytes --]
CC: kbuild-all(a)lists.01.org
In-Reply-To: <20200908194557.198335-6-linux@roeck-us.net>
References: <20200908194557.198335-6-linux@roeck-us.net>
TO: Guenter Roeck <linux@roeck-us.net>
TO: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
CC: Mauro Carvalho Chehab <mchehab@kernel.org>
CC: linux-media(a)vger.kernel.org
CC: Sakari Ailus <sakari.ailus@iki.fi>
CC: linux-uvc-devel(a)lists.sourceforge.net
CC: linux-usb(a)vger.kernel.org
CC: linux-kernel(a)vger.kernel.org
CC: Guenter Roeck <linux@roeck-us.net>
CC: Alan Stern <stern@rowland.harvard.edu>
CC: Hans Verkuil <hverkuil@xs4all.nl>
Hi Guenter,
I love your patch! Perhaps something to improve:
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on v5.9-rc4 next-20200908]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Guenter-Roeck/media-uvcvideo-Fix-race-conditions/20200909-121927
base: git://linuxtv.org/media_tree.git master
:::::: branch date: 7 hours ago
:::::: commit date: 7 hours ago
config: x86_64-randconfig-m001-20200909 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/media/usb/uvc/uvc_v4l2.c:553 uvc_v4l2_open() warn: possible memory leak of 'handle'
# https://github.com/0day-ci/linux/commit/50911975ff9b21d08ff5408e496683b8ac567b1c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Guenter-Roeck/media-uvcvideo-Fix-race-conditions/20200909-121927
git checkout 50911975ff9b21d08ff5408e496683b8ac567b1c
vim +/handle +553 drivers/media/usb/uvc/uvc_v4l2.c
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 525
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 526 /* ------------------------------------------------------------------------
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 527 * V4L2 file operations
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 528 */
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 529
bec43661b1dc00 drivers/media/video/uvc/uvc_v4l2.c Hans Verkuil 2008-12-30 530 static int uvc_v4l2_open(struct file *file)
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 531 {
35f02a681b72ec drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-06-28 532 struct uvc_streaming *stream;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 533 struct uvc_fh *handle;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 534 int ret = 0;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 535
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 536 uvc_trace(UVC_TRACE_CALLS, "uvc_v4l2_open\n");
35f02a681b72ec drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-06-28 537 stream = video_drvdata(file);
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 538
35f02a681b72ec drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-06-28 539 ret = usb_autopm_get_interface(stream->dev->intf);
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 540 if (ret < 0)
716fdee110ceb8 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-09-29 541 return ret;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 542
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 543 /* Create the device handle. */
f14d4988c28e52 drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2018-01-16 544 handle = kzalloc(sizeof(*handle), GFP_KERNEL);
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 545 if (handle == NULL) {
35f02a681b72ec drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-06-28 546 usb_autopm_put_interface(stream->dev->intf);
716fdee110ceb8 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-09-29 547 return -ENOMEM;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 548 }
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 549
17706f5653a90f drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2013-04-25 550 mutex_lock(&stream->dev->lock);
50911975ff9b21 drivers/media/usb/uvc/uvc_v4l2.c Guenter Roeck 2020-09-08 551 if (!video_is_registered(&stream->vdev)) {
50911975ff9b21 drivers/media/usb/uvc/uvc_v4l2.c Guenter Roeck 2020-09-08 552 mutex_unlock(&stream->dev->lock);
50911975ff9b21 drivers/media/usb/uvc/uvc_v4l2.c Guenter Roeck 2020-09-08 @553 return -ENODEV;
50911975ff9b21 drivers/media/usb/uvc/uvc_v4l2.c Guenter Roeck 2020-09-08 554 }
17706f5653a90f drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2013-04-25 555 if (stream->dev->users == 0) {
17706f5653a90f drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2013-04-25 556 ret = uvc_status_start(stream->dev, GFP_KERNEL);
35f02a681b72ec drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-06-28 557 if (ret < 0) {
17706f5653a90f drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2013-04-25 558 mutex_unlock(&stream->dev->lock);
a82a45f65377b0 drivers/media/usb/uvc/uvc_v4l2.c Oliver Neukum 2013-01-10 559 usb_autopm_put_interface(stream->dev->intf);
04a37e0f32f988 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-05-19 560 kfree(handle);
716fdee110ceb8 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-09-29 561 return ret;
04a37e0f32f988 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-05-19 562 }
04a37e0f32f988 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-05-19 563 }
04a37e0f32f988 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-05-19 564
17706f5653a90f drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2013-04-25 565 stream->dev->users++;
17706f5653a90f drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2013-04-25 566 mutex_unlock(&stream->dev->lock);
17706f5653a90f drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2013-04-25 567
d8da7513bcf983 drivers/media/usb/uvc/uvc_v4l2.c Hans Verkuil 2015-03-09 568 v4l2_fh_init(&handle->vfh, &stream->vdev);
b4012002f3a398 drivers/media/video/uvc/uvc_v4l2.c Hans de Goede 2012-04-08 569 v4l2_fh_add(&handle->vfh);
8e113595edf074 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-07-01 570 handle->chain = stream->chain;
35f02a681b72ec drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-06-28 571 handle->stream = stream;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 572 handle->state = UVC_HANDLE_PASSIVE;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 573 file->private_data = handle;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 574
716fdee110ceb8 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-09-29 575 return 0;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 576 }
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 577
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 49876 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2 5/5] media: uvcvideo: Abort uvc_v4l2_open if video device is unregistered
2020-09-08 19:45 ` [PATCH v2 5/5] media: uvcvideo: Abort uvc_v4l2_open if video device is unregistered Guenter Roeck
@ 2020-09-09 12:19 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2020-09-09 12:19 UTC (permalink / raw)
To: kbuild
[-- Attachment #1: Type: text/plain, Size: 6515 bytes --]
Hi Guenter,
url: https://github.com/0day-ci/linux/commits/Guenter-Roeck/media-uvcvideo-Fix-race-conditions/20200909-121927
base: git://linuxtv.org/media_tree.git master
config: x86_64-randconfig-m001-20200909 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
smatch warnings:
drivers/media/usb/uvc/uvc_v4l2.c:553 uvc_v4l2_open() warn: possible memory leak of 'handle'
# https://github.com/0day-ci/linux/commit/50911975ff9b21d08ff5408e496683b8ac567b1c
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Guenter-Roeck/media-uvcvideo-Fix-race-conditions/20200909-121927
git checkout 50911975ff9b21d08ff5408e496683b8ac567b1c
vim +/handle +553 drivers/media/usb/uvc/uvc_v4l2.c
bec43661b1dc00 drivers/media/video/uvc/uvc_v4l2.c Hans Verkuil 2008-12-30 530 static int uvc_v4l2_open(struct file *file)
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 531 {
35f02a681b72ec drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-06-28 532 struct uvc_streaming *stream;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 533 struct uvc_fh *handle;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 534 int ret = 0;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 535
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 536 uvc_trace(UVC_TRACE_CALLS, "uvc_v4l2_open\n");
35f02a681b72ec drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-06-28 537 stream = video_drvdata(file);
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 538
35f02a681b72ec drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-06-28 539 ret = usb_autopm_get_interface(stream->dev->intf);
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 540 if (ret < 0)
716fdee110ceb8 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-09-29 541 return ret;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 542
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 543 /* Create the device handle. */
f14d4988c28e52 drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2018-01-16 544 handle = kzalloc(sizeof(*handle), GFP_KERNEL);
^^^^^^^^^^^^^^^^
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 545 if (handle == NULL) {
35f02a681b72ec drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-06-28 546 usb_autopm_put_interface(stream->dev->intf);
716fdee110ceb8 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-09-29 547 return -ENOMEM;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 548 }
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 549
17706f5653a90f drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2013-04-25 550 mutex_lock(&stream->dev->lock);
50911975ff9b21 drivers/media/usb/uvc/uvc_v4l2.c Guenter Roeck 2020-09-08 551 if (!video_is_registered(&stream->vdev)) {
50911975ff9b21 drivers/media/usb/uvc/uvc_v4l2.c Guenter Roeck 2020-09-08 552 mutex_unlock(&stream->dev->lock);
50911975ff9b21 drivers/media/usb/uvc/uvc_v4l2.c Guenter Roeck 2020-09-08 @553 return -ENODEV;
^^^^^^^^^^^^^^
kfree(handle);
50911975ff9b21 drivers/media/usb/uvc/uvc_v4l2.c Guenter Roeck 2020-09-08 554 }
17706f5653a90f drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2013-04-25 555 if (stream->dev->users == 0) {
17706f5653a90f drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2013-04-25 556 ret = uvc_status_start(stream->dev, GFP_KERNEL);
35f02a681b72ec drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-06-28 557 if (ret < 0) {
17706f5653a90f drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2013-04-25 558 mutex_unlock(&stream->dev->lock);
a82a45f65377b0 drivers/media/usb/uvc/uvc_v4l2.c Oliver Neukum 2013-01-10 559 usb_autopm_put_interface(stream->dev->intf);
04a37e0f32f988 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-05-19 560 kfree(handle);
716fdee110ceb8 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-09-29 561 return ret;
04a37e0f32f988 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-05-19 562 }
04a37e0f32f988 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-05-19 563 }
04a37e0f32f988 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-05-19 564
17706f5653a90f drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2013-04-25 565 stream->dev->users++;
17706f5653a90f drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2013-04-25 566 mutex_unlock(&stream->dev->lock);
17706f5653a90f drivers/media/usb/uvc/uvc_v4l2.c Laurent Pinchart 2013-04-25 567
d8da7513bcf983 drivers/media/usb/uvc/uvc_v4l2.c Hans Verkuil 2015-03-09 568 v4l2_fh_init(&handle->vfh, &stream->vdev);
b4012002f3a398 drivers/media/video/uvc/uvc_v4l2.c Hans de Goede 2012-04-08 569 v4l2_fh_add(&handle->vfh);
8e113595edf074 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-07-01 570 handle->chain = stream->chain;
35f02a681b72ec drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-06-28 571 handle->stream = stream;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 572 handle->state = UVC_HANDLE_PASSIVE;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 573 file->private_data = handle;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 574
716fdee110ceb8 drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2009-09-29 575 return 0;
c0efd232929c2c drivers/media/video/uvc/uvc_v4l2.c Laurent Pinchart 2008-06-30 576 }
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org
_______________________________________________
kbuild mailing list -- kbuild(a)lists.01.org
To unsubscribe send an email to kbuild-leave(a)lists.01.org
[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 49876 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH v2 5/5] media: uvcvideo: Abort uvc_v4l2_open if video device is unregistered
2020-09-08 19:45 [PATCH v2 0/5] media: uvcvideo: Fix race conditions Guenter Roeck
@ 2020-09-08 19:45 ` Guenter Roeck
2020-09-09 12:19 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Guenter Roeck @ 2020-09-08 19:45 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Mauro Carvalho Chehab, Sakari Ailus, linux-uvc-devel, linux-usb,
linux-media, linux-kernel, Guenter Roeck, Alan Stern,
Hans Verkuil
uvc_v4l2_open() acquires the uvc device mutex. After doing so,
it does not check if the video device is still registered. This may
result in race conditions and can result in an attempt to submit an urb
to a disconnected USB interface (from uvc_status_start).
The problem was observed after adding a call to msleep() just before
acquiring the mutex and disconnecting the camera during the sleep.
Check if the video device is still registered after acquiring the mutex
to avoid the problem. In the release function, only call uvc_status_stop()
if the video device is still registered. If the video device has been
unregistered, the urb associated with uvc status has already been killed
in uvc_status_unregister(). Trying to kill it again won't do any good
and might have unexpected side effects.
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Guenter Roeck <linux@roeck-us.net>
---
v2: Fix typo in description. The USB message is sent from uvc_status_start()
in the open function, not from uvc_v4l2_open().
drivers/media/usb/uvc/uvc_v4l2.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c
index 7e5e583dae5e..1f7d454557b9 100644
--- a/drivers/media/usb/uvc/uvc_v4l2.c
+++ b/drivers/media/usb/uvc/uvc_v4l2.c
@@ -548,6 +548,10 @@ static int uvc_v4l2_open(struct file *file)
}
mutex_lock(&stream->dev->lock);
+ if (!video_is_registered(&stream->vdev)) {
+ mutex_unlock(&stream->dev->lock);
+ return -ENODEV;
+ }
if (stream->dev->users == 0) {
ret = uvc_status_start(stream->dev, GFP_KERNEL);
if (ret < 0) {
@@ -590,7 +594,7 @@ static int uvc_v4l2_release(struct file *file)
file->private_data = NULL;
mutex_lock(&stream->dev->lock);
- if (--stream->dev->users == 0)
+ if (--stream->dev->users == 0 && video_is_registered(&stream->vdev))
uvc_status_stop(stream->dev);
mutex_unlock(&stream->dev->lock);
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-09 12:19 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-09 10:52 [PATCH v2 5/5] media: uvcvideo: Abort uvc_v4l2_open if video device is unregistered kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2020-09-08 19:45 [PATCH v2 0/5] media: uvcvideo: Fix race conditions Guenter Roeck
2020-09-08 19:45 ` [PATCH v2 5/5] media: uvcvideo: Abort uvc_v4l2_open if video device is unregistered Guenter Roeck
2020-09-09 12:19 ` Dan Carpenter
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.