In the bug happened during uvc webcam disconect,there is race
between stopping a video transfer and usb disconnect.This issue is
reproduced in my system running Linux kernel when UVC webcam play is
stopped and UVC webcam is disconnected at the same time. This causes the
below backtrace:
[2-3496.7275] PC is at 0xbf418000+0x2d8 [usbcore]
[2-3496.7275] LR is at 0x00000005
[2-3496.7275] pc : [<bf4182d8>]((usb_ifnum_to_if
</drivers/usb/core/usb.c:283
[usbcore.ko]>)) lr : [<00000005>]() psr: 20000013
[2-3496.7275] Function entered at [<bf4182a4>]((usb_ifnum_to_if
</drivers/usb/core/usb.c:275
[usbcore.ko]>)) (0xbf418000+0x2a4 [usbcore]) from
[<bf423974>]((usb_hcd_alloc_bandwidth
</drivers/usb/core/hcd.c:1947
[usbcore.ko]>)) (0xbf418000+0xb974 [usbcore])
[2-3496.7275] Function entered at [<bf423738>]((usb_hcd_alloc_bandwidth
</drivers/usb/core/hcd.c:1876
[usbcore.ko]>)) (0xbf418000+0xb738 [usbcore]) from
[<bf426ca0>]((usb_set_interface
</drivers/usb/core/message.c:1461
[usbcore.ko]>)) (0xbf418000+0xeca0 [usbcore])
[2-3496.7275] Function entered at [<bf426b9c>]((usb_set_interface
</drivers/usb/core/message.c:1385
[usbcore.ko]>)) (0xbf418000+0xeb9c [usbcore]) from
[<bf9c4dd4>]((uvc_video_clock_cleanup
</drivers/media/usb/uvc/uvc_video.c:598
uvc_video_stop_streaming
</drivers/media/usb/uvc/uvc_video.c:2221
[uvcvideo.ko]>)) (0xbf9bd000+0x7dd4 [uvcvideo])
[2-3496.7275] Function entered at [<bf9c4d98>]((uvc_video_stop_streaming
</drivers/media/usb/uvc/uvc_video.c:2200
[uvcvideo.ko]>)) (0xbf9bd000+0x7d98 [uvcvideo]) from
[<bf9bfab8>]((spin_lock_irq
</include/linux/spinlock.h:363
uvc_stop_streaming
</drivers/media/usb/uvc/uvc_queue.c:194
[uvcvideo.ko]>)) (0xbf9bd000+0x2ab8 [uvcvideo])
[2-3496.7276] Function entered at [<bf9bfa94>]((uvc_stop_streaming
</drivers/media/usb/uvc/uvc_queue.c:186
[uvcvideo.ko]>)) (0xbf9bd000+0x2a94 [uvcvideo]) from
[<be307150>]((__read_once_size
</include/linux/compiler.h:290
(discriminator 1) __vb2_queue_cancel
</drivers/media/common/videobuf2/videobuf2-core.c:1893
(discriminator 1) [videobuf2_common.ko]>)) (0xbe306000+0x1150
[videobuf2_common])
[2-3496.7276] Function entered at [<be307120>]((__vb2_queue_cancel
</drivers/media/common/videobuf2/videobuf2-core.c:1877
[videobuf2_common.ko]>)) (0xbe306000+0x1120 [videobuf2_common]) from
[<be308894>]((vb2_core_streamoff
</drivers/media/common/videobuf2/videobuf2-core.c:2053
This below solution patch fixes this race condition at USB core level
occurring during UVC webcam device disconnect.
Signed-off-by: Anuj Gupta <[email protected]>
Signed-off-by: Aman Deep <[email protected]>
---
drivers/usb/core/hcd.c | 7 ++++++-
drivers/usb/core/message.c | 4 ++++
drivers/usb/core/usb.c | 9 ++++++---
3 files changed, 16 insertions(+), 4 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index 8300baedafd2..a06452cbbaa4 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1931,7 +1931,12 @@ int usb_hcd_alloc_bandwidth(struct usb_device *udev,
}
}
if (cur_alt && new_alt) {
- struct usb_interface *iface = usb_ifnum_to_if(udev,
+ struct usb_interface *iface;
+
+ if (udev->state == USB_STATE_NOTATTACHED)
+ return -ENODEV;
+
+ iface = usb_ifnum_to_if(udev,
cur_alt->desc.bInterfaceNumber);
if (!iface)
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index b5811620f1de..f31c7287dc01 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1575,7 +1575,11 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate)
for (i = 0; i < iface->cur_altsetting->desc.bNumEndpoints; i++)
iface->cur_altsetting->endpoint[i].streams = 0;
+ if (dev->state == USB_STATE_NOTATTACHED)
+ return -ENODEV;
+
ret = usb_hcd_alloc_bandwidth(dev, NULL, iface->cur_altsetting, alt);
+
if (ret < 0) {
dev_info(&dev->dev, "Not enough bandwidth for altsetting %d\n",
alternate);
diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
index 901ec732321c..6fb8b14469ae 100644
--- a/drivers/usb/core/usb.c
+++ b/drivers/usb/core/usb.c
@@ -352,10 +352,13 @@ struct usb_interface *usb_ifnum_to_if(const struct usb_device *dev,
if (!config)
return NULL;
- for (i = 0; i < config->desc.bNumInterfaces; i++)
- if (config->interface[i]->altsetting[0]
- .desc.bInterfaceNumber == ifnum)
+ for (i = 0; i < config->desc.bNumInterfaces; i++) {
+ if (config->interface[i] &&
+ config->interface[i]->altsetting[0]
+ .desc.bInterfaceNumber == ifnum) {
return config->interface[i];
+ }
+ }
return NULL;
}
--
2.34.1
On Thu, Jul 20, 2023 at 05:01:42PM +0530, Aman Deep wrote:
> In the bug happened during uvc webcam disconect,there is race
> between stopping a video transfer and usb disconnect.This issue is
> reproduced in my system running Linux kernel when UVC webcam play is
> stopped and UVC webcam is disconnected at the same time. This causes the
> below backtrace:
>
> [2-3496.7275] PC is at 0xbf418000+0x2d8 [usbcore]
> [2-3496.7275] LR is at 0x00000005
> [2-3496.7275] pc : [<bf4182d8>]((usb_ifnum_to_if
> </drivers/usb/core/usb.c:283
> [usbcore.ko]>)) lr : [<00000005>]() psr: 20000013
> [2-3496.7275] Function entered at [<bf4182a4>]((usb_ifnum_to_if
> </drivers/usb/core/usb.c:275
> [usbcore.ko]>)) (0xbf418000+0x2a4 [usbcore]) from
> [<bf423974>]((usb_hcd_alloc_bandwidth
> </drivers/usb/core/hcd.c:1947
> [usbcore.ko]>)) (0xbf418000+0xb974 [usbcore])
> [2-3496.7275] Function entered at [<bf423738>]((usb_hcd_alloc_bandwidth
> </drivers/usb/core/hcd.c:1876
> [usbcore.ko]>)) (0xbf418000+0xb738 [usbcore]) from
> [<bf426ca0>]((usb_set_interface
> </drivers/usb/core/message.c:1461
> [usbcore.ko]>)) (0xbf418000+0xeca0 [usbcore])
> [2-3496.7275] Function entered at [<bf426b9c>]((usb_set_interface
> </drivers/usb/core/message.c:1385
> [usbcore.ko]>)) (0xbf418000+0xeb9c [usbcore]) from
> [<bf9c4dd4>]((uvc_video_clock_cleanup
> </drivers/media/usb/uvc/uvc_video.c:598
> uvc_video_stop_streaming
> </drivers/media/usb/uvc/uvc_video.c:2221
> [uvcvideo.ko]>)) (0xbf9bd000+0x7dd4 [uvcvideo])
> [2-3496.7275] Function entered at [<bf9c4d98>]((uvc_video_stop_streaming
> </drivers/media/usb/uvc/uvc_video.c:2200
> [uvcvideo.ko]>)) (0xbf9bd000+0x7d98 [uvcvideo]) from
> [<bf9bfab8>]((spin_lock_irq
> </include/linux/spinlock.h:363
> uvc_stop_streaming
> </drivers/media/usb/uvc/uvc_queue.c:194
> [uvcvideo.ko]>)) (0xbf9bd000+0x2ab8 [uvcvideo])
> [2-3496.7276] Function entered at [<bf9bfa94>]((uvc_stop_streaming
> </drivers/media/usb/uvc/uvc_queue.c:186
> [uvcvideo.ko]>)) (0xbf9bd000+0x2a94 [uvcvideo]) from
> [<be307150>]((__read_once_size
> </include/linux/compiler.h:290
> (discriminator 1) __vb2_queue_cancel
> </drivers/media/common/videobuf2/videobuf2-core.c:1893
> (discriminator 1) [videobuf2_common.ko]>)) (0xbe306000+0x1150
> [videobuf2_common])
> [2-3496.7276] Function entered at [<be307120>]((__vb2_queue_cancel
> </drivers/media/common/videobuf2/videobuf2-core.c:1877
> [videobuf2_common.ko]>)) (0xbe306000+0x1120 [videobuf2_common]) from
> [<be308894>]((vb2_core_streamoff
> </drivers/media/common/videobuf2/videobuf2-core.c:2053
Odd wrapping, please fix.
>
> This below solution patch fixes this race condition at USB core level
> occurring during UVC webcam device disconnect.
>
> Signed-off-by: Anuj Gupta <[email protected]>
> Signed-off-by: Aman Deep <[email protected]>
What commit id does this fix? SHould this go to the stable trees?
> ---
> drivers/usb/core/hcd.c | 7 ++++++-
> drivers/usb/core/message.c | 4 ++++
> drivers/usb/core/usb.c | 9 ++++++---
> 3 files changed, 16 insertions(+), 4 deletions(-)
Why are you making changes to the core USB stack for a driver bug?
>
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 8300baedafd2..a06452cbbaa4 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -1931,7 +1931,12 @@ int usb_hcd_alloc_bandwidth(struct usb_device *udev,
> }
> }
> if (cur_alt && new_alt) {
> - struct usb_interface *iface = usb_ifnum_to_if(udev,
> + struct usb_interface *iface;
> +
> + if (udev->state == USB_STATE_NOTATTACHED)
> + return -ENODEV;
> +
> + iface = usb_ifnum_to_if(udev,
> cur_alt->desc.bInterfaceNumber);
>
> if (!iface)
> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index b5811620f1de..f31c7287dc01 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1575,7 +1575,11 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate)
> for (i = 0; i < iface->cur_altsetting->desc.bNumEndpoints; i++)
> iface->cur_altsetting->endpoint[i].streams = 0;
>
> + if (dev->state == USB_STATE_NOTATTACHED)
> + return -ENODEV;
> +
> ret = usb_hcd_alloc_bandwidth(dev, NULL, iface->cur_altsetting, alt);
> +
Why the extra line?
And why can't the state change right after you check for it? What
happens if the device is unattached right here?
> if (ret < 0) {
> dev_info(&dev->dev, "Not enough bandwidth for altsetting %d\n",
> alternate);
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 901ec732321c..6fb8b14469ae 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -352,10 +352,13 @@ struct usb_interface *usb_ifnum_to_if(const struct usb_device *dev,
>
> if (!config)
> return NULL;
> - for (i = 0; i < config->desc.bNumInterfaces; i++)
> - if (config->interface[i]->altsetting[0]
> - .desc.bInterfaceNumber == ifnum)
> + for (i = 0; i < config->desc.bNumInterfaces; i++) {
> + if (config->interface[i] &&
> + config->interface[i]->altsetting[0]
> + .desc.bInterfaceNumber == ifnum) {
> return config->interface[i];
I don't understand this change, what does it do?
Your changelog does not say why you are doing any of this, only that
"there is a problem", please explain this better when you resubmit this.
thanks,
greg k-h
On Thu, Jul 20, 2023 at 05:01:42PM +0530, Aman Deep wrote:
> In the bug happened during uvc webcam disconect,there is race
> between stopping a video transfer and usb disconnect.This issue is
> reproduced in my system running Linux kernel when UVC webcam play is
> stopped and UVC webcam is disconnected at the same time. This causes the
> below backtrace:
>
> [2-3496.7275] PC is at 0xbf418000+0x2d8 [usbcore]
> [2-3496.7275] LR is at 0x00000005
> [2-3496.7275] pc : [<bf4182d8>]((usb_ifnum_to_if
> </drivers/usb/core/usb.c:283
> [usbcore.ko]>)) lr : [<00000005>]() psr: 20000013
> [2-3496.7275] Function entered at [<bf4182a4>]((usb_ifnum_to_if
> </drivers/usb/core/usb.c:275
> [usbcore.ko]>)) (0xbf418000+0x2a4 [usbcore]) from
> [<bf423974>]((usb_hcd_alloc_bandwidth
> </drivers/usb/core/hcd.c:1947
> [usbcore.ko]>)) (0xbf418000+0xb974 [usbcore])
> [2-3496.7275] Function entered at [<bf423738>]((usb_hcd_alloc_bandwidth
> </drivers/usb/core/hcd.c:1876
> [usbcore.ko]>)) (0xbf418000+0xb738 [usbcore]) from
> [<bf426ca0>]((usb_set_interface
> </drivers/usb/core/message.c:1461
> [usbcore.ko]>)) (0xbf418000+0xeca0 [usbcore])
> [2-3496.7275] Function entered at [<bf426b9c>]((usb_set_interface
> </drivers/usb/core/message.c:1385
> [usbcore.ko]>)) (0xbf418000+0xeb9c [usbcore]) from
> [<bf9c4dd4>]((uvc_video_clock_cleanup
> </drivers/media/usb/uvc/uvc_video.c:598
> uvc_video_stop_streaming
> </drivers/media/usb/uvc/uvc_video.c:2221
> [uvcvideo.ko]>)) (0xbf9bd000+0x7dd4 [uvcvideo])
> [2-3496.7275] Function entered at [<bf9c4d98>]((uvc_video_stop_streaming
> </drivers/media/usb/uvc/uvc_video.c:2200
> [uvcvideo.ko]>)) (0xbf9bd000+0x7d98 [uvcvideo]) from
> [<bf9bfab8>]((spin_lock_irq
> </include/linux/spinlock.h:363
> uvc_stop_streaming
> </drivers/media/usb/uvc/uvc_queue.c:194
> [uvcvideo.ko]>)) (0xbf9bd000+0x2ab8 [uvcvideo])
> [2-3496.7276] Function entered at [<bf9bfa94>]((uvc_stop_streaming
> </drivers/media/usb/uvc/uvc_queue.c:186
> [uvcvideo.ko]>)) (0xbf9bd000+0x2a94 [uvcvideo]) from
> [<be307150>]((__read_once_size
> </include/linux/compiler.h:290
> (discriminator 1) __vb2_queue_cancel
> </drivers/media/common/videobuf2/videobuf2-core.c:1893
> (discriminator 1) [videobuf2_common.ko]>)) (0xbe306000+0x1150
> [videobuf2_common])
> [2-3496.7276] Function entered at [<be307120>]((__vb2_queue_cancel
> </drivers/media/common/videobuf2/videobuf2-core.c:1877
> [videobuf2_common.ko]>)) (0xbe306000+0x1120 [videobuf2_common]) from
> [<be308894>]((vb2_core_streamoff
> </drivers/media/common/videobuf2/videobuf2-core.c:2053
This backtrace doesn't show what actually caused the bug. You should
have included several lines from the system log _preceding_ the
backtrace. Without those lines, we can only guess at what the problem
was.
> This below solution patch fixes this race condition at USB core level
> occurring during UVC webcam device disconnect.
How can a race in the UVC driver be fixed by changing the USB core? It
seems obvious that the only way to fix such a race is by changing the
UVC driver.
> Signed-off-by: Anuj Gupta <[email protected]>
> Signed-off-by: Aman Deep <[email protected]>
> ---
> drivers/usb/core/hcd.c | 7 ++++++-
> drivers/usb/core/message.c | 4 ++++
> drivers/usb/core/usb.c | 9 ++++++---
> 3 files changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
> index 8300baedafd2..a06452cbbaa4 100644
> --- a/drivers/usb/core/hcd.c
> +++ b/drivers/usb/core/hcd.c
> @@ -1931,7 +1931,12 @@ int usb_hcd_alloc_bandwidth(struct usb_device *udev,
> }
> }
> if (cur_alt && new_alt) {
> - struct usb_interface *iface = usb_ifnum_to_if(udev,
> + struct usb_interface *iface;
> +
> + if (udev->state == USB_STATE_NOTATTACHED)
> + return -ENODEV;
What will happen if the state changes to USB_STATE_NOTATTACHED at this
point, after that test was made?
> +
> + iface = usb_ifnum_to_if(udev,
> cur_alt->desc.bInterfaceNumber);
>
> if (!iface)
> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index b5811620f1de..f31c7287dc01 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1575,7 +1575,11 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate)
> for (i = 0; i < iface->cur_altsetting->desc.bNumEndpoints; i++)
> iface->cur_altsetting->endpoint[i].streams = 0;
>
> + if (dev->state == USB_STATE_NOTATTACHED)
> + return -ENODEV;
Same question: What happens if the state changes right here?
> +
> ret = usb_hcd_alloc_bandwidth(dev, NULL, iface->cur_altsetting, alt);
> +
> if (ret < 0) {
> dev_info(&dev->dev, "Not enough bandwidth for altsetting %d\n",
> alternate);
> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
> index 901ec732321c..6fb8b14469ae 100644
> --- a/drivers/usb/core/usb.c
> +++ b/drivers/usb/core/usb.c
> @@ -352,10 +352,13 @@ struct usb_interface *usb_ifnum_to_if(const struct usb_device *dev,
>
> if (!config)
> return NULL;
> - for (i = 0; i < config->desc.bNumInterfaces; i++)
> - if (config->interface[i]->altsetting[0]
> - .desc.bInterfaceNumber == ifnum)
> + for (i = 0; i < config->desc.bNumInterfaces; i++) {
> + if (config->interface[i] &&
This new test can fail only if the routine is called after (or while)
the device is unconfigured or removed. But if a driver makes such a
call then the driver is buggy. The proper solution is to fix the
driver, not add this test here.
Besides, this test has the same problem as the others you added above.
What happens if config->interface[i] gets set to NULL right here?
Alan Stern
> + config->interface[i]->altsetting[0]
> + .desc.bInterfaceNumber == ifnum) {
> return config->interface[i];
> + }
> + }
>
> return NULL;
> }
> --
> 2.34.1
>
Hi Aman,
kernel test robot noticed the following build warnings:
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Aman-Deep/USB-Fix-race-condition-during-UVC-webcam-disconnect/20230720-202046
base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
patch link: https://lore.kernel.org/r/20230720113142.3070583-1-aman.deep%40samsung.com
patch subject: [PATCH] USB: Fix race condition during UVC webcam disconnect
config: parisc-randconfig-m041-20230726 (https://download.01.org/0day-ci/archive/20230727/[email protected]/config)
compiler: hppa-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230727/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Reported-by: Dan Carpenter <[email protected]>
| Closes: https://lore.kernel.org/r/[email protected]/
smatch warnings:
drivers/usb/core/message.c:1668 usb_set_interface() warn: inconsistent returns 'hcd->bandwidth_mutex'.
vim +1668 drivers/usb/core/message.c
^1da177e4c3f41 Linus Torvalds 2005-04-16 1528 int usb_set_interface(struct usb_device *dev, int interface, int alternate)
^1da177e4c3f41 Linus Torvalds 2005-04-16 1529 {
^1da177e4c3f41 Linus Torvalds 2005-04-16 1530 struct usb_interface *iface;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1531 struct usb_host_interface *alt;
3f0479e00a3fca Sarah Sharp 2009-12-03 1532 struct usb_hcd *hcd = bus_to_hcd(dev->bus);
7a7b562d08ad6d Hans de Goede 2013-11-08 1533 int i, ret, manual = 0;
3e35bf39e0b909 Greg Kroah-Hartman 2008-01-30 1534 unsigned int epaddr;
3e35bf39e0b909 Greg Kroah-Hartman 2008-01-30 1535 unsigned int pipe;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1536
^1da177e4c3f41 Linus Torvalds 2005-04-16 1537 if (dev->state == USB_STATE_SUSPENDED)
^1da177e4c3f41 Linus Torvalds 2005-04-16 1538 return -EHOSTUNREACH;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1539
^1da177e4c3f41 Linus Torvalds 2005-04-16 1540 iface = usb_ifnum_to_if(dev, interface);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1541 if (!iface) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 1542 dev_dbg(&dev->dev, "selecting invalid interface %d\n",
^1da177e4c3f41 Linus Torvalds 2005-04-16 1543 interface);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1544 return -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1545 }
e534c5b831c8b8 Alan Stern 2011-07-01 1546 if (iface->unregistering)
e534c5b831c8b8 Alan Stern 2011-07-01 1547 return -ENODEV;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1548
^1da177e4c3f41 Linus Torvalds 2005-04-16 1549 alt = usb_altnum_to_altsetting(iface, alternate);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1550 if (!alt) {
385f690bc058ba Thadeu Lima de Souza Cascardo 2010-01-17 1551 dev_warn(&dev->dev, "selecting invalid altsetting %d\n",
3b6004f3b5a8b4 Greg Kroah-Hartman 2008-08-14 1552 alternate);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1553 return -EINVAL;
^1da177e4c3f41 Linus Torvalds 2005-04-16 1554 }
f9a5b4f58b280c Mathias Nyman 2018-09-03 1555 /*
f9a5b4f58b280c Mathias Nyman 2018-09-03 1556 * usb3 hosts configure the interface in usb_hcd_alloc_bandwidth,
f9a5b4f58b280c Mathias Nyman 2018-09-03 1557 * including freeing dropped endpoint ring buffers.
f9a5b4f58b280c Mathias Nyman 2018-09-03 1558 * Make sure the interface endpoints are flushed before that
f9a5b4f58b280c Mathias Nyman 2018-09-03 1559 */
f9a5b4f58b280c Mathias Nyman 2018-09-03 1560 usb_disable_interface(dev, iface, false);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1561
3f0479e00a3fca Sarah Sharp 2009-12-03 1562 /* Make sure we have enough bandwidth for this alternate interface.
3f0479e00a3fca Sarah Sharp 2009-12-03 1563 * Remove the current alt setting and add the new alt setting.
3f0479e00a3fca Sarah Sharp 2009-12-03 1564 */
d673bfcbfffdeb Sarah Sharp 2010-10-15 1565 mutex_lock(hcd->bandwidth_mutex);
8306095fd2c110 Sarah Sharp 2012-05-02 1566 /* Disable LPM, and re-enable it once the new alt setting is installed,
8306095fd2c110 Sarah Sharp 2012-05-02 1567 * so that the xHCI driver can recalculate the U1/U2 timeouts.
8306095fd2c110 Sarah Sharp 2012-05-02 1568 */
8306095fd2c110 Sarah Sharp 2012-05-02 1569 if (usb_disable_lpm(dev)) {
1ccc417e6c3201 Joe Perches 2017-12-05 1570 dev_err(&iface->dev, "%s Failed to disable LPM\n", __func__);
8306095fd2c110 Sarah Sharp 2012-05-02 1571 mutex_unlock(hcd->bandwidth_mutex);
8306095fd2c110 Sarah Sharp 2012-05-02 1572 return -ENOMEM;
8306095fd2c110 Sarah Sharp 2012-05-02 1573 }
7a7b562d08ad6d Hans de Goede 2013-11-08 1574 /* Changing alt-setting also frees any allocated streams */
7a7b562d08ad6d Hans de Goede 2013-11-08 1575 for (i = 0; i < iface->cur_altsetting->desc.bNumEndpoints; i++)
7a7b562d08ad6d Hans de Goede 2013-11-08 1576 iface->cur_altsetting->endpoint[i].streams = 0;
7a7b562d08ad6d Hans de Goede 2013-11-08 1577
4682bbb9e2f196 Aman Deep 2023-07-20 1578 if (dev->state == USB_STATE_NOTATTACHED)
4682bbb9e2f196 Aman Deep 2023-07-20 1579 return -ENODEV;
mutex_unlock(hcd->bandwidth_mutex); before returning
4682bbb9e2f196 Aman Deep 2023-07-20 1580
3f0479e00a3fca Sarah Sharp 2009-12-03 1581 ret = usb_hcd_alloc_bandwidth(dev, NULL, iface->cur_altsetting, alt);
4682bbb9e2f196 Aman Deep 2023-07-20 1582
3f0479e00a3fca Sarah Sharp 2009-12-03 1583 if (ret < 0) {
3f0479e00a3fca Sarah Sharp 2009-12-03 1584 dev_info(&dev->dev, "Not enough bandwidth for altsetting %d\n",
3f0479e00a3fca Sarah Sharp 2009-12-03 1585 alternate);
8306095fd2c110 Sarah Sharp 2012-05-02 1586 usb_enable_lpm(dev);
d673bfcbfffdeb Sarah Sharp 2010-10-15 1587 mutex_unlock(hcd->bandwidth_mutex);
3f0479e00a3fca Sarah Sharp 2009-12-03 1588 return ret;
3f0479e00a3fca Sarah Sharp 2009-12-03 1589 }
3f0479e00a3fca Sarah Sharp 2009-12-03 1590
392e1d9817d002 Alan Stern 2008-03-11 1591 if (dev->quirks & USB_QUIRK_NO_SET_INTF)
392e1d9817d002 Alan Stern 2008-03-11 1592 ret = -EPIPE;
392e1d9817d002 Alan Stern 2008-03-11 1593 else
297e84c04d76b9 Greg Kroah-Hartman 2020-09-14 1594 ret = usb_control_msg_send(dev, 0,
297e84c04d76b9 Greg Kroah-Hartman 2020-09-14 1595 USB_REQ_SET_INTERFACE,
297e84c04d76b9 Greg Kroah-Hartman 2020-09-14 1596 USB_RECIP_INTERFACE, alternate,
ddd1198e3e0935 Oliver Neukum 2020-09-23 1597 interface, NULL, 0, 5000,
ddd1198e3e0935 Oliver Neukum 2020-09-23 1598 GFP_NOIO);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1599
^1da177e4c3f41 Linus Torvalds 2005-04-16 1600 /* 9.4.10 says devices don't need this and are free to STALL the
^1da177e4c3f41 Linus Torvalds 2005-04-16 1601 * request if the interface only has one alternate setting.
^1da177e4c3f41 Linus Torvalds 2005-04-16 1602 */
^1da177e4c3f41 Linus Torvalds 2005-04-16 1603 if (ret == -EPIPE && iface->num_altsetting == 1) {
^1da177e4c3f41 Linus Torvalds 2005-04-16 1604 dev_dbg(&dev->dev,
^1da177e4c3f41 Linus Torvalds 2005-04-16 1605 "manual set_interface for iface %d, alt %d\n",
^1da177e4c3f41 Linus Torvalds 2005-04-16 1606 interface, alternate);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1607 manual = 1;
297e84c04d76b9 Greg Kroah-Hartman 2020-09-14 1608 } else if (ret) {
3f0479e00a3fca Sarah Sharp 2009-12-03 1609 /* Re-instate the old alt setting */
3f0479e00a3fca Sarah Sharp 2009-12-03 1610 usb_hcd_alloc_bandwidth(dev, NULL, alt, iface->cur_altsetting);
8306095fd2c110 Sarah Sharp 2012-05-02 1611 usb_enable_lpm(dev);
d673bfcbfffdeb Sarah Sharp 2010-10-15 1612 mutex_unlock(hcd->bandwidth_mutex);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1613 return ret;
3f0479e00a3fca Sarah Sharp 2009-12-03 1614 }
d673bfcbfffdeb Sarah Sharp 2010-10-15 1615 mutex_unlock(hcd->bandwidth_mutex);
^1da177e4c3f41 Linus Torvalds 2005-04-16 1616
^1da177e4c3f41 Linus Torvalds 2005-04-16 1617 /* FIXME drivers shouldn't need to replicate/bugfix the logic here
^1da177e4c3f41 Linus Torvalds 2005-04-16 1618 * when they implement async or easily-killable versions of this or
^1da177e4c3f41 Linus Torvalds 2005-04-16 1619 * other "should-be-internal" functions (like clear_halt).
^1da177e4c3f41 Linus Torvalds 2005-04-16 1620 * should hcd+usbcore postprocess control requests?
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Thanks for your analysis.
and i am sorry for late reply due to some unavoidable circumstances.
My replies are inline.
>On Thu, Jul 20, 2023 at 05:01:42PM +0530, Aman Deep wrote:
>> In the bug happened during uvc webcam disconect,there is race
>> between stopping a video transfer and usb disconnect. This issue is
>> reproduced in my system running Linux kernel when UVC webcam play is
>> stopped and UVC webcam is disconnected at the same time. This causes the
>> below backtrace:
>>
>> [2-3496.7275] PC is at 0xbf418000+0x2d8 [usbcore]
>> [2-3496.7275] LR is at 0x00000005
>> [2-3496.7275] pc : [<bf4182d8>]((usb_ifnum_to_if
>> </drivers/usb/core/usb.c:283
>> [usbcore.ko]>)) lr : [<00000005>]() psr: 20000013
>> [2-3496.7275] Function entered at [<bf4182a4>]((usb_ifnum_to_if
>> </drivers/usb/core/usb.c:275
>> [usbcore.ko]>)) (0xbf418000+0x2a4 [usbcore]) from
>> [<bf423974>]((usb_hcd_alloc_bandwidth
>> </drivers/usb/core/hcd.c:1947
>> [usbcore.ko]>)) (0xbf418000+0xb974 [usbcore])
>> [2-3496.7275] Function entered at [<bf423738>]((usb_hcd_alloc_bandwidth
>> </drivers/usb/core/hcd.c:1876
>> [usbcore.ko]>)) (0xbf418000+0xb738 [usbcore]) from
>> [<bf426ca0>]((usb_set_interface
>> </drivers/usb/core/message.c:1461
>> [usbcore.ko]>)) (0xbf418000+0xeca0 [usbcore])
>> [2-3496.7275] Function entered at [<bf426b9c>]((usb_set_interface
>> </drivers/usb/core/message.c:1385
>> [usbcore.ko]>)) (0xbf418000+0xeb9c [usbcore]) from
>> [<bf9c4dd4>]((uvc_video_clock_cleanup
>> </drivers/media/usb/uvc/uvc_video.c:598
>> uvc_video_stop_streaming
>> </drivers/media/usb/uvc/uvc_video.c:2221
>> [uvcvideo.ko]>)) (0xbf9bd000+0x7dd4 [uvcvideo])
>> [2-3496.7275] Function entered at [<bf9c4d98>]((uvc_video_stop_streaming
>> </drivers/media/usb/uvc/uvc_video.c:2200
>> [uvcvideo.ko]>)) (0xbf9bd000+0x7d98 [uvcvideo]) from
>> [<bf9bfab8>]((spin_lock_irq
>> </include/linux/spinlock.h:363
>> uvc_stop_streaming
>> </drivers/media/usb/uvc/uvc_queue.c:194
>> [uvcvideo.ko]>)) (0xbf9bd000+0x2ab8 [uvcvideo])
>> [2-3496.7276] Function entered at [<bf9bfa94>]((uvc_stop_streaming
>> </drivers/media/usb/uvc/uvc_queue.c:186
>> [uvcvideo.ko]>)) (0xbf9bd000+0x2a94 [uvcvideo]) from
>> [<be307150>]((__read_once_size
>> </include/linux/compiler.h:290
>> (discriminator 1) __vb2_queue_cancel
>> </drivers/media/common/videobuf2/videobuf2-core.c:1893
>> (discriminator 1) [videobuf2_common.ko]>)) (0xbe306000+0x1150
>> [videobuf2_common])
>> [2-3496.7276] Function entered at [<be307120>]((__vb2_queue_cancel
>> </drivers/media/common/videobuf2/videobuf2-core.c:1877
>> [videobuf2_common.ko]>)) (0xbe306000+0x1120 [videobuf2_common]) from
>> [<be308894>]((vb2_core_streamoff
>> </drivers/media/common/videobuf2/videobuf2-core.c:2053
>
>Odd wrapping, please fix.
Sorry for odd wrapping.
This is below complete backtrace for crash problem:
[1-221.1821] [ msg: 4788] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[1-221.1821] [ msg: 4788] pgd = e136ded4
[1-221.1821] [ msg: 4788] [00000000] *pgd=26210003, *pmd=00000000
[1-221.1821] [ msg: 4788]
[1-221.1821] [ msg: 4788] Die cpu info :
[1-221.1821] [ msg: 4788] Internal error: Oops: 206 [#1] PREEMPT SMP ARM
[1-221.1821] [ msg: 4788] CPU: 1 PID: 4788 Comm: msg(muse-server) Kdump: loaded Tainted: PO 5.4.77 #1 PPID: 1 PComm: systemd preempt_count: 0x0
[1-221.1821] [ msg: 4788] Hardware name: Novatek Cortex-A53
[1-221.1822] [ msg: 4788] PC is at usb_ifnum_to_if+0x30/0x74 [usbcore]
[1-221.1822] [ msg: 4788] LR is at 0x5
[1-221.1822] [ msg: 4788] pc : [<bede1300>] lr : [<00000005>] psr: 20000113
[1-221.1822] [ msg: 4788] sp : ca443c18 ip : ca443c28 fp : ca443c24
[1-221.1822] [ msg: 4788] r10: e668b6c8 r9 : 00000000 r8 : e668b7e0
[1-221.1822] [ msg: 4788] r7 : e7b78880 r6 : bf1d9db0 r5 : e668b6c8 r4 : e690c000
[1-221.1822] [ msg: 4788] r3 : 00002000 r2 : e696ac40 r1 : 00000001 r0 : 00000000
[1-221.1822] [ msg: 4788] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[1-221.1822] [ msg: 4788] Control: 30c5383d Table: 261f8a80 DAC: e45d65d5
[1-221.1822] [ msg: 4788] Process msg (pid: 4788, stack limit = 0xa0153238)
[1-221.1822] [ msg: 4788] Stack: (0xca443c18 to 0xca444000)
[1-221.1822] [ msg: 4788] 3c00: ca443c64 ca443c28
[1-221.1822] [ msg: 4788] 3c20: bedee6e4 bede12dc 00000000 bee0ae78 ca443c54 ca443c40 c083c894 e7b78880
[1-221.1822] [ msg: 4788] 3c40: e6b88340 00000000 bee0ae78 00000001 e690c000 e668b6c8 ca443cb4 ca443c68
[1-221.1822] [ msg: 4788] 3c60: bedf22ac bedee64c e5cf1508 e5cf1508 e5cf0000 e5cf0330 00000001 e5cf0330
[1-221.1822] [ msg: 4788] 3c80: ca443ca4 ca443c90 c083c894 e5cf0000 e5cf0330 00000001 e5cf0330 00000000
[1-221.1822] [ msg: 4788] 3ca0: 00000001 c08d1b3c ca443ccc ca443cb8 bfb3f958 bedf1ff4 e5cf0330 e5cf0330
[1-221.1822] [ msg: 4788] 3cc0: ca443ce4 ca443cd0 bfb3a024 bfb3f8a8 e5cf0330 e5cf0330 ca443d14 ca443ce8
[1-221.1823] [ msg: 4788] 3ce0: be3661e0 bfb3a004 00000001 e5cf0330 e5cf0330 00000001 c05d6260 00000000
[1-221.1823] [ msg: 4788] 3d00: 00000001 c08d1b3c ca443d2c ca443d18 be367994 be3661b4 e5cf0484 e5cf0330
[1-221.1823] [ msg: 4788] 3d20: ca443d3c ca443d30 be37e3e4 be367978 ca443d5c ca443d40 bfb3a518 be37e3cc
[1-221.1823] [ msg: 4788] 3d40: e5cf030c e5cf0000 00000001 c05d6260 ca443d7c ca443d60 bfb3b628 bfb3a4f0
[1-221.1823] [ msg: 4788] 3d60: bfb3b5e8 40045613 00000000 c05d6260 ca443d94 ca443d80 c05d6288 bfb3b5f4
[1-221.1823] [ msg: 4788] 3d80: e5cf0010 40045613 ca443dfc ca443d98 c05d9b84 c05d626c 00000068 ca443deb
[1-221.1823] [ msg: 4788] 3da0: c08d1b3c 00000001 ca443e24 bfb44680 00000000 e2fa3780 c01a926c 031e1090
[1-221.1823] [ msg: 4788] 3dc0: ca443df4 ffffffff c01e0048 0000072c 000012b4 00000000 40045613 00000000
[1-221.1823] [ msg: 4788] 3de0: 00000000 00000001 00000004 ca443e24 ca443ed4 ca443e00 c05db320 c05d9a04
[1-221.1823] [ msg: 4788] 3e00: 00000000 00000000 c05d99f8 e77a6700 ab8fd26c 00000000 00000000 00000000
[1-221.1823] [ msg: 4788] 3e20: ca443f60 00000001 ca443ee0 00000000 ca443e9c ca443e40 c02390a8 be211e84
[1-221.1823] [ msg: 4788] 3e40: 00000000 00000001 e2861600 00000000 00000000 00000000 00000000 00000000
[1-221.1823] [ msg: 4788] 3e60: 00000000 00000000 00000000 c03681bc 00000008 00000000 ca443ee0 c0bbd748
[1-221.1823] [ msg: 4788] 3e80: 00000000 c0be9a14 ca443ef4 00000002 ca443ed4 ca443ea0 c03681bc c036790c
[1-221.1823] [ msg: 4788] 3ea0: ca443ef4 c0bbd748 e2861600 c05db7dc e6695448 40045613 ab8fd26c e77a6700
[1-221.1823] [ msg: 4788] 3ec0: 00000021 00000036 ca443ee4 ca443ed8 c05db7fc c05db0f8 ca443efc ca443ee8
[1-221.1823] [ msg: 4788] 3ee0: c05d4728 c05db7e8 ab8fd26c e6695448 ca443f6c ca443f00 c02506a0 c05d46e8
[1-221.1823] [ msg: 4788] 3f00: ca443f04 c08a7a00 00000000 00000000 00000000 00000000 00000000 00000000
[1-221.1823] [ msg: 4788] 3f20: 00000000 00000000 00000000 00000000 ab8fd26c c0abb6ec ab8fd26c e77a6700
[1-221.1823] [ msg: 4788] 3f40: ca443f6c e77a6701 00000000 40045613 ab8fd26c e77a6700 00000021 00000036
[1-221.1824] [ msg: 4788] 3f60: ca443f94 ca443f70 c0250b3c c02502fc 00000000 000006f7 00000000 00000036
[1-221.1824] [ msg: 4788] 3f80: c000924c ca442000 ca443fa4 ca443f98 c0250b78 c0250adc 00000000 ca443fa8
[1-221.1824] [ msg: 4788] 3fa0: c0009230 c0250b6c 00000000 000006f7 00000021 40045613 ab8fd26c 00000021
[1-221.1824] [ msg: 4788] 3fc0: 00000000 000006f7 00000000 00000036 abb79e30 00000000 00000001 abb79e28
[1-221.1824] [ msg: 4788] 3fe0: aeca607c ab8fd24c aec8e749 b5f1ed1c 20000010 00000021 00000000 00000000
[1-221.1824] [ msg: 4788] Backtrace:
[1-221.1824] [ msg: 4788] [<bede12d0>] (usb_ifnum_to_if [usbcore]) from [<bedee6e4>] (usb_hcd_alloc_bandwidth+0xa4/0x564 [usbcore])
[1-221.1824] [ msg: 4788] [<bedee640>] (usb_hcd_alloc_bandwidth [usbcore]) from [<bedf22ac>] (usb_set_interface+0x2c4/0x61c [usbcore])
[1-221.1824] [ msg: 4788] r10:e668b6c8 r9:e690c000 r8:00000001 r7:bee0ae78 r6:00000000 r5:e6b88340
[1-221.1824] [ msg: 4788] r4:e7b78880
[1-221.1825] [ msg: 4788] [<bedf1fe8>] (usb_set_interface [usbcore]) from [<bfb3f958>] (uvc_video_stop_streaming+0xbc/0xc4 [uvcvideo])
[1-221.1825] [ msg: 4788] r10:c08d1b3c r9:00000001 r8:00000000 r7:e5cf0330 r6:00000001 r5:e5cf0330
[1-221.1825] [ msg: 4788] r4:e5cf0000
[1-221.1825] [ msg: 4788] [<bfb3f89c>] (uvc_video_stop_streaming [uvcvideo]) from [<bfb3a024>] (uvc_stop_streaming+0x2c/0x50 [uvcvideo])
[1-221.1825] [ msg: 4788] r5:e5cf0330 r4:e5cf0330
[1-221.1825] [ msg: 4788] [<bfb39ff8>] (uvc_stop_streaming [uvcvideo]) from [<be3661e0>] (__vb2_queue_cancel+0x38/0x290 [videobuf2_common])
[1-221.1825] [ msg: 4788] r5:e5cf0330 r4:e5cf0330
[1-221.1825] [ msg: 4788] [<be3661a8>] (__vb2_queue_cancel [videobuf2_common]) from [<be367994>] (vb2_core_streamoff+0x28/0xb4 [videobuf2_common])
[1-221.1825] [ msg: 4788] r10:c08d1b3c r9:00000001 r8:00000000 r7:c05d6260 r6:00000001 r5:e5cf0330
[1-221.1825] [ msg: 4788] r4:e5cf0330 r3:00000001
[1-221.1825] [ msg: 4788] [<be36796c>] (vb2_core_streamoff [videobuf2_common]) from [<be37e3e4>] (vb2_streamoff+0x24/0x60 [videobuf2_v4l2])
[1-221.1825] [ msg: 4788] r5:e5cf0330 r4:e5cf0484
[1-221.1825] [ msg: 4788] [<be37e3c0>] (vb2_streamoff [videobuf2_v4l2]) from [<bfb3a518>] (uvc_queue_streamoff+0x34/0x48 [uvcvideo])
[1-221.1825] [ msg: 4788] [<bfb3a4e4>] (uvc_queue_streamoff [uvcvideo]) from [<bfb3b628>] (uvc_ioctl_streamoff+0x40/0x58 [uvcvideo])
[1-221.1826] [ msg: 4788] r7:c05d6260 r6:00000001 r5:e5cf0000 r4:e5cf030c
[1-221.1826] [ msg: 4788] [<bfb3b5e8>] (uvc_ioctl_streamoff [uvcvideo]) from [<c05d6288>] (v4l_streamoff+0x28/0x2c)
[1-221.1826] [ msg: 4788] r7:c05d6260 r6:00000000 r5:40045613 r4:bfb3b5e8
[1-221.1826] [ msg: 4788] [<c05d6260>] (v4l_streamoff) from [<c05d9b84>] (__video_do_ioctl+0x18c/0x400)
[1-221.1826] [ msg: 4788] r5:40045613 r4:e5cf0010
[1-221.1826] [ msg: 4788] [<c05d99f8>] (__video_do_ioctl) from [<c05db320>] (video_usercopy+0x234/0x6f0)
[1-221.1826] [ msg: 4788] r10:ca443e24 r9:00000004 r8:00000001 r7:00000000 r6:00000000 r5:40045613
[1-221.1826] [ msg: 4788] r4:00000000
[1-221.1826] [ msg: 4788] [<c05db0ec>] (video_usercopy) from [<c05db7fc>] (video_ioctl2+0x20/0x24)
[1-221.1826] [ msg: 4788] r10:00000036 r9:00000021 r8:e77a6700 r7:ab8fd26c r6:40045613 r5:e6695448
[1-221.1826] [ msg: 4788] r4:c05db7dc
[1-221.1826] [ msg: 4788] [<c05db7dc>] (video_ioctl2) from [<c05d4728>] (v4l2_ioctl+0x4c/0x60)
[1-221.1826] [ msg: 4788] [<c05d46dc>] (v4l2_ioctl) from [<c02506a0>] (do_vfs_ioctl+0x3b0/0x7e0)
[1-221.1826] [ msg: 4788] r5:e6695448 r4:ab8fd26c
[1-221.1826] [ msg: 4788] [<c02502f0>] (do_vfs_ioctl) from [<c0250b3c>] (ksys_ioctl+0x6c/0x90)
[1-221.1826] [ msg: 4788] r10:00000036 r9:00000021 r8:e77a6700 r7:ab8fd26c r6:40045613 r5:00000000
[1-221.1826] [ msg: 4788] r4:e77a6701
[1-221.1826] [ msg: 4788] [<c0250ad0>] (ksys_ioctl) from [<c0250b78>] (sys_ioctl+0x18/0x1c)
[1-221.1826] [ msg: 4788] r9:ca442000 r8:c000924c r7:00000036 r6:00000000 r5:000006f7 r4:00000000
[1-221.1827] [ msg: 4788] [<c0250b60>] (sys_ioctl) from [<c0009230>] (__sys_trace_return+0x0/0x10)
>>
>> This below solution patch fixes this race condition at USB core level
>> occurring during UVC webcam device disconnect.
>>
>> Signed-off-by: Anuj Gupta <[email protected]>
>> Signed-off-by: Aman Deep <[email protected]>
>
>What commit id does this fix? SHould this go to the stable trees?
This issue is present in stable trees and i have found few other online references for this issue reported before.
https://lore.kernel.org/all/CANiDSCsSn=UJfCt6shy8htGXAPyeEceVzKva3eD+YxhC3YVmxA@mail.gmail.com/t/
https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1827452
>> ---
>> drivers/usb/core/hcd.c | 7 ++++++-
>> drivers/usb/core/message.c | 4 ++++
>> drivers/usb/core/usb.c | 9 ++++++---
>> 3 files changed, 16 insertions(+), 4 deletions(-)
>
>Why are you making changes to the core USB stack for a driver bug?
I thought this issue can occur with other devices in simillar race conditions so i thought it will be fixed for all drivers.
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 8300baedafd2.. a06452cbbaa4 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -1931,7 +1931,12 @@ int usb_hcd_alloc_bandwidth(struct usb_device *udev,
>> }
>> }
>> if (cur_alt && new_alt) {
>> - struct usb_interface *iface = usb_ifnum_to_if(udev,
>> + struct usb_interface *iface;
>> +
>> + if (udev->state == USB_STATE_NOTATTACHED)
>> + return -ENODEV;
>> +
>> + iface = usb_ifnum_to_if(udev,
>> cur_alt->desc.bInterfaceNumber);
>>
>> if (!iface)
>> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
>> index b5811620f1de..f31c7287dc01 100644
>> --- a/drivers/usb/core/message.c
>> +++ b/drivers/usb/core/message.c
>> @@ -1575,7 +1575,11 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate)
>> for (i = 0; i < iface->cur_altsetting->desc.bNumEndpoints; i++)
>> iface->cur_altsetting->endpoint[i].streams = 0;
>>
>> + if (dev->state == USB_STATE_NOTATTACHED)
>> + return -ENODEV;
>> +
>> ret = usb_hcd_alloc_bandwidth(dev, NULL, iface->cur_altsetting, alt);
>> +
>
>Why the extra line?
This is done to provide more checking for disconnected device case.
>
>And why can't the state change right after you check for it? What
>happens if the device is unattached right here?
>
Please suggest if we need to add locking mechanism to cover such cases.
i will try accordingly.
>> if (ret < 0) {
>> dev_info(&dev->dev, "Not enough bandwidth for altsetting %d\n",
>> alternate);
>> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
>> index 901ec732321c..6fb8b14469ae 100644
>> --- a/drivers/usb/core/usb.c
>> +++ b/drivers/usb/core/usb.c
>> @@ -352,10 +352,13 @@ struct usb_interface *usb_ifnum_to_if(const struct usb_device *dev,
>>
>> if (!config)
>> return NULL;
>> - for (i = 0; i < config->desc.bNumInterfaces; i++)
>> - if (config->interface[i]->altsetting[0]
>> - .desc.bInterfaceNumber == ifnum)
>> + for (i = 0; i < config->desc.bNumInterfaces; i++) {
>> + if (config->interface[i] &&
>> + config->interface[i]->altsetting[0]
>> + .desc.bInterfaceNumber == ifnum) {
>> return config->interface[i];
>
>I don't understand this change, what does it do?
>
>
>Your changelog does not say why you are doing any of this, only that
>"there is a problem", please explain this better when you resubmit this.
>
>thanks,
>
>greg k-h
This is just for more NULL checking for pointers.
If you suggest, i can remove it.
Thanks,
Aman Deep
Thanks for your detailed analysis.
Sorry for late reply and my answers are inline.
>On Thu, Jul 20, 2023 at 05:01:42PM +0530, Aman Deep wrote:
>> In the bug happened during uvc webcam disconect,there is race
>> between stopping a video transfer and usb disconnect. This issue is
>> reproduced in my system running Linux kernel when UVC webcam play is
>> stopped and UVC webcam is disconnected at the same time. This causes the
>> below backtrace:
>>
>> [2-3496.7275] PC is at 0xbf418000+0x2d8 [usbcore]
>> [2-3496.7275] LR is at 0x00000005
>> [2-3496.7275] pc : [<bf4182d8>]((usb_ifnum_to_if
>> </drivers/usb/core/usb.c:283
>> [usbcore.ko]>)) lr : [<00000005>]() psr: 20000013
>> [2-3496.7275] Function entered at [<bf4182a4>]((usb_ifnum_to_if
>> </drivers/usb/core/usb.c:275
>> [usbcore.ko]>)) (0xbf418000+0x2a4 [usbcore]) from
>> [<bf423974>]((usb_hcd_alloc_bandwidth
>> </drivers/usb/core/hcd.c:1947
>> [usbcore.ko]>)) (0xbf418000+0xb974 [usbcore])
>> [2-3496.7275] Function entered at [<bf423738>]((usb_hcd_alloc_bandwidth
>> </drivers/usb/core/hcd.c:1876
>> [usbcore.ko]>)) (0xbf418000+0xb738 [usbcore]) from
>> [<bf426ca0>]((usb_set_interface
>> </drivers/usb/core/message.c:1461
>> [usbcore.ko]>)) (0xbf418000+0xeca0 [usbcore])
>> [2-3496.7275] Function entered at [<bf426b9c>]((usb_set_interface
>> </drivers/usb/core/message.c:1385
>> [usbcore.ko]>)) (0xbf418000+0xeb9c [usbcore]) from
>> [<bf9c4dd4>]((uvc_video_clock_cleanup
>> </drivers/media/usb/uvc/uvc_video.c:598
>> uvc_video_stop_streaming
>> </drivers/media/usb/uvc/uvc_video.c:2221
>> [uvcvideo.ko]>)) (0xbf9bd000+0x7dd4 [uvcvideo])
>> [2-3496.7275] Function entered at [<bf9c4d98>]((uvc_video_stop_streaming
>> </drivers/media/usb/uvc/uvc_video.c:2200
>> [uvcvideo.ko]>)) (0xbf9bd000+0x7d98 [uvcvideo]) from
>> [<bf9bfab8>]((spin_lock_irq
>> </include/linux/spinlock.h:363
>> uvc_stop_streaming
>> </drivers/media/usb/uvc/uvc_queue.c:194
>> [uvcvideo.ko]>)) (0xbf9bd000+0x2ab8 [uvcvideo])
>> [2-3496.7276] Function entered at [<bf9bfa94>]((uvc_stop_streaming
>> </drivers/media/usb/uvc/uvc_queue.c:186
>> [uvcvideo.ko]>)) (0xbf9bd000+0x2a94 [uvcvideo]) from
>> [<be307150>]((__read_once_size
>> </include/linux/compiler.h:290
>> (discriminator 1) __vb2_queue_cancel
>> </drivers/media/common/videobuf2/videobuf2-core.c:1893
>> (discriminator 1) [videobuf2_common.ko]>)) (0xbe306000+0x1150
>> [videobuf2_common])
>> [2-3496.7276] Function entered at [<be307120>]((__vb2_queue_cancel
>> </drivers/media/common/videobuf2/videobuf2-core.c:1877
>> [videobuf2_common.ko]>)) (0xbe306000+0x1120 [videobuf2_common]) from
>> [<be308894>]((vb2_core_streamoff
>> </drivers/media/common/videobuf2/videobuf2-core.c:2053
>
>This backtrace doesn't show what actually caused the bug. You should
>have included several lines from the system log _preceding_ the
>backtrace. Without those lines, we can only guess at what the problem
>was.
This is below complete backtrace for crash problem:
[1-221.1821] [ msg: 4788] Unable to handle kernel NULL pointer dereference at virtual address 00000000
[1-221.1821] [ msg: 4788] pgd = e136ded4
[1-221.1821] [ msg: 4788] [00000000] *pgd=26210003, *pmd=00000000
[1-221.1821] [ msg: 4788]
[1-221.1821] [ msg: 4788] Die cpu info :
[1-221.1821] [ msg: 4788] Internal error: Oops: 206 [#1] PREEMPT SMP ARM
[1-221.1821] [ msg: 4788] CPU: 1 PID: 4788 Comm: msg(muse-server) Kdump: loaded Tainted: PO 5.4.77 #1 PPID: 1 PComm: systemd preempt_count: 0x0
[1-221.1821] [ msg: 4788] Hardware name: Novatek Cortex-A53
[1-221.1822] [ msg: 4788] PC is at usb_ifnum_to_if+0x30/0x74 [usbcore]
[1-221.1822] [ msg: 4788] LR is at 0x5
[1-221.1822] [ msg: 4788] pc : [<bede1300>] lr : [<00000005>] psr: 20000113
[1-221.1822] [ msg: 4788] sp : ca443c18 ip : ca443c28 fp : ca443c24
[1-221.1822] [ msg: 4788] r10: e668b6c8 r9 : 00000000 r8 : e668b7e0
[1-221.1822] [ msg: 4788] r7 : e7b78880 r6 : bf1d9db0 r5 : e668b6c8 r4 : e690c000
[1-221.1822] [ msg: 4788] r3 : 00002000 r2 : e696ac40 r1 : 00000001 r0 : 00000000
[1-221.1822] [ msg: 4788] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
[1-221.1822] [ msg: 4788] Control: 30c5383d Table: 261f8a80 DAC: e45d65d5
[1-221.1822] [ msg: 4788] Process msg (pid: 4788, stack limit = 0xa0153238)
[1-221.1822] [ msg: 4788] Stack: (0xca443c18 to 0xca444000)
[1-221.1822] [ msg: 4788] 3c00: ca443c64 ca443c28
[1-221.1822] [ msg: 4788] 3c20: bedee6e4 bede12dc 00000000 bee0ae78 ca443c54 ca443c40 c083c894 e7b78880
[1-221.1822] [ msg: 4788] 3c40: e6b88340 00000000 bee0ae78 00000001 e690c000 e668b6c8 ca443cb4 ca443c68
[1-221.1822] [ msg: 4788] 3c60: bedf22ac bedee64c e5cf1508 e5cf1508 e5cf0000 e5cf0330 00000001 e5cf0330
[1-221.1822] [ msg: 4788] 3c80: ca443ca4 ca443c90 c083c894 e5cf0000 e5cf0330 00000001 e5cf0330 00000000
[1-221.1822] [ msg: 4788] 3ca0: 00000001 c08d1b3c ca443ccc ca443cb8 bfb3f958 bedf1ff4 e5cf0330 e5cf0330
[1-221.1822] [ msg: 4788] 3cc0: ca443ce4 ca443cd0 bfb3a024 bfb3f8a8 e5cf0330 e5cf0330 ca443d14 ca443ce8
[1-221.1823] [ msg: 4788] 3ce0: be3661e0 bfb3a004 00000001 e5cf0330 e5cf0330 00000001 c05d6260 00000000
[1-221.1823] [ msg: 4788] 3d00: 00000001 c08d1b3c ca443d2c ca443d18 be367994 be3661b4 e5cf0484 e5cf0330
[1-221.1823] [ msg: 4788] 3d20: ca443d3c ca443d30 be37e3e4 be367978 ca443d5c ca443d40 bfb3a518 be37e3cc
[1-221.1823] [ msg: 4788] 3d40: e5cf030c e5cf0000 00000001 c05d6260 ca443d7c ca443d60 bfb3b628 bfb3a4f0
[1-221.1823] [ msg: 4788] 3d60: bfb3b5e8 40045613 00000000 c05d6260 ca443d94 ca443d80 c05d6288 bfb3b5f4
[1-221.1823] [ msg: 4788] 3d80: e5cf0010 40045613 ca443dfc ca443d98 c05d9b84 c05d626c 00000068 ca443deb
[1-221.1823] [ msg: 4788] 3da0: c08d1b3c 00000001 ca443e24 bfb44680 00000000 e2fa3780 c01a926c 031e1090
[1-221.1823] [ msg: 4788] 3dc0: ca443df4 ffffffff c01e0048 0000072c 000012b4 00000000 40045613 00000000
[1-221.1823] [ msg: 4788] 3de0: 00000000 00000001 00000004 ca443e24 ca443ed4 ca443e00 c05db320 c05d9a04
[1-221.1823] [ msg: 4788] 3e00: 00000000 00000000 c05d99f8 e77a6700 ab8fd26c 00000000 00000000 00000000
[1-221.1823] [ msg: 4788] 3e20: ca443f60 00000001 ca443ee0 00000000 ca443e9c ca443e40 c02390a8 be211e84
[1-221.1823] [ msg: 4788] 3e40: 00000000 00000001 e2861600 00000000 00000000 00000000 00000000 00000000
[1-221.1823] [ msg: 4788] 3e60: 00000000 00000000 00000000 c03681bc 00000008 00000000 ca443ee0 c0bbd748
[1-221.1823] [ msg: 4788] 3e80: 00000000 c0be9a14 ca443ef4 00000002 ca443ed4 ca443ea0 c03681bc c036790c
[1-221.1823] [ msg: 4788] 3ea0: ca443ef4 c0bbd748 e2861600 c05db7dc e6695448 40045613 ab8fd26c e77a6700
[1-221.1823] [ msg: 4788] 3ec0: 00000021 00000036 ca443ee4 ca443ed8 c05db7fc c05db0f8 ca443efc ca443ee8
[1-221.1823] [ msg: 4788] 3ee0: c05d4728 c05db7e8 ab8fd26c e6695448 ca443f6c ca443f00 c02506a0 c05d46e8
[1-221.1823] [ msg: 4788] 3f00: ca443f04 c08a7a00 00000000 00000000 00000000 00000000 00000000 00000000
[1-221.1823] [ msg: 4788] 3f20: 00000000 00000000 00000000 00000000 ab8fd26c c0abb6ec ab8fd26c e77a6700
[1-221.1823] [ msg: 4788] 3f40: ca443f6c e77a6701 00000000 40045613 ab8fd26c e77a6700 00000021 00000036
[1-221.1824] [ msg: 4788] 3f60: ca443f94 ca443f70 c0250b3c c02502fc 00000000 000006f7 00000000 00000036
[1-221.1824] [ msg: 4788] 3f80: c000924c ca442000 ca443fa4 ca443f98 c0250b78 c0250adc 00000000 ca443fa8
[1-221.1824] [ msg: 4788] 3fa0: c0009230 c0250b6c 00000000 000006f7 00000021 40045613 ab8fd26c 00000021
[1-221.1824] [ msg: 4788] 3fc0: 00000000 000006f7 00000000 00000036 abb79e30 00000000 00000001 abb79e28
[1-221.1824] [ msg: 4788] 3fe0: aeca607c ab8fd24c aec8e749 b5f1ed1c 20000010 00000021 00000000 00000000
[1-221.1824] [ msg: 4788] Backtrace:
[1-221.1824] [ msg: 4788] [<bede12d0>] (usb_ifnum_to_if [usbcore]) from [<bedee6e4>] (usb_hcd_alloc_bandwidth+0xa4/0x564 [usbcore])
[1-221.1824] [ msg: 4788] [<bedee640>] (usb_hcd_alloc_bandwidth [usbcore]) from [<bedf22ac>] (usb_set_interface+0x2c4/0x61c [usbcore])
[1-221.1824] [ msg: 4788] r10:e668b6c8 r9:e690c000 r8:00000001 r7:bee0ae78 r6:00000000 r5:e6b88340
[1-221.1824] [ msg: 4788] r4:e7b78880
[1-221.1825] [ msg: 4788] [<bedf1fe8>] (usb_set_interface [usbcore]) from [<bfb3f958>] (uvc_video_stop_streaming+0xbc/0xc4 [uvcvideo])
[1-221.1825] [ msg: 4788] r10:c08d1b3c r9:00000001 r8:00000000 r7:e5cf0330 r6:00000001 r5:e5cf0330
[1-221.1825] [ msg: 4788] r4:e5cf0000
[1-221.1825] [ msg: 4788] [<bfb3f89c>] (uvc_video_stop_streaming [uvcvideo]) from [<bfb3a024>] (uvc_stop_streaming+0x2c/0x50 [uvcvideo])
[1-221.1825] [ msg: 4788] r5:e5cf0330 r4:e5cf0330
[1-221.1825] [ msg: 4788] [<bfb39ff8>] (uvc_stop_streaming [uvcvideo]) from [<be3661e0>] (__vb2_queue_cancel+0x38/0x290 [videobuf2_common])
[1-221.1825] [ msg: 4788] r5:e5cf0330 r4:e5cf0330
[1-221.1825] [ msg: 4788] [<be3661a8>] (__vb2_queue_cancel [videobuf2_common]) from [<be367994>] (vb2_core_streamoff+0x28/0xb4 [videobuf2_common])
[1-221.1825] [ msg: 4788] r10:c08d1b3c r9:00000001 r8:00000000 r7:c05d6260 r6:00000001 r5:e5cf0330
[1-221.1825] [ msg: 4788] r4:e5cf0330 r3:00000001
[1-221.1825] [ msg: 4788] [<be36796c>] (vb2_core_streamoff [videobuf2_common]) from [<be37e3e4>] (vb2_streamoff+0x24/0x60 [videobuf2_v4l2])
[1-221.1825] [ msg: 4788] r5:e5cf0330 r4:e5cf0484
[1-221.1825] [ msg: 4788] [<be37e3c0>] (vb2_streamoff [videobuf2_v4l2]) from [<bfb3a518>] (uvc_queue_streamoff+0x34/0x48 [uvcvideo])
[1-221.1825] [ msg: 4788] [<bfb3a4e4>] (uvc_queue_streamoff [uvcvideo]) from [<bfb3b628>] (uvc_ioctl_streamoff+0x40/0x58 [uvcvideo])
[1-221.1826] [ msg: 4788] r7:c05d6260 r6:00000001 r5:e5cf0000 r4:e5cf030c
[1-221.1826] [ msg: 4788] [<bfb3b5e8>] (uvc_ioctl_streamoff [uvcvideo]) from [<c05d6288>] (v4l_streamoff+0x28/0x2c)
[1-221.1826] [ msg: 4788] r7:c05d6260 r6:00000000 r5:40045613 r4:bfb3b5e8
[1-221.1826] [ msg: 4788] [<c05d6260>] (v4l_streamoff) from [<c05d9b84>] (__video_do_ioctl+0x18c/0x400)
[1-221.1826] [ msg: 4788] r5:40045613 r4:e5cf0010
[1-221.1826] [ msg: 4788] [<c05d99f8>] (__video_do_ioctl) from [<c05db320>] (video_usercopy+0x234/0x6f0)
[1-221.1826] [ msg: 4788] r10:ca443e24 r9:00000004 r8:00000001 r7:00000000 r6:00000000 r5:40045613
[1-221.1826] [ msg: 4788] r4:00000000
[1-221.1826] [ msg: 4788] [<c05db0ec>] (video_usercopy) from [<c05db7fc>] (video_ioctl2+0x20/0x24)
[1-221.1826] [ msg: 4788] r10:00000036 r9:00000021 r8:e77a6700 r7:ab8fd26c r6:40045613 r5:e6695448
[1-221.1826] [ msg: 4788] r4:c05db7dc
[1-221.1826] [ msg: 4788] [<c05db7dc>] (video_ioctl2) from [<c05d4728>] (v4l2_ioctl+0x4c/0x60)
[1-221.1826] [ msg: 4788] [<c05d46dc>] (v4l2_ioctl) from [<c02506a0>] (do_vfs_ioctl+0x3b0/0x7e0)
[1-221.1826] [ msg: 4788] r5:e6695448 r4:ab8fd26c
[1-221.1826] [ msg: 4788] [<c02502f0>] (do_vfs_ioctl) from [<c0250b3c>] (ksys_ioctl+0x6c/0x90)
[1-221.1826] [ msg: 4788] r10:00000036 r9:00000021 r8:e77a6700 r7:ab8fd26c r6:40045613 r5:00000000
[1-221.1826] [ msg: 4788] r4:e77a6701
[1-221.1826] [ msg: 4788] [<c0250ad0>] (ksys_ioctl) from [<c0250b78>] (sys_ioctl+0x18/0x1c)
[1-221.1826] [ msg: 4788] r9:ca442000 r8:c000924c r7:00000036 r6:00000000 r5:000006f7 r4:00000000
[1-221.1827] [ msg: 4788] [<c0250b60>] (sys_ioctl) from [<c0009230>] (__sys_trace_return+0x0/0x10)
>> This below solution patch fixes this race condition at USB core level
>> occurring during UVC webcam device disconnect.
>
>How can a race in the UVC driver be fixed by changing the USB core? It
>seems obvious that the only way to fix such a race is by changing the
>UVC driver.
I think solution fixed at USB core level avoids race condition for all kind of devices and drivers.
>> Signed-off-by: Anuj Gupta <[email protected]>
>> Signed-off-by: Aman Deep <[email protected]>
>> ---
>> drivers/usb/core/hcd.c | 7 ++++++-
>> drivers/usb/core/message.c | 4 ++++
>> drivers/usb/core/usb.c | 9 ++++++---
>> 3 files changed, 16 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
>> index 8300baedafd2.. a06452cbbaa4 100644
>> --- a/drivers/usb/core/hcd.c
>> +++ b/drivers/usb/core/hcd.c
>> @@ -1931,7 +1931,12 @@ int usb_hcd_alloc_bandwidth(struct usb_device *udev,
>> }
>> }
>> if (cur_alt && new_alt) {
>> - struct usb_interface *iface = usb_ifnum_to_if(udev,
>> + struct usb_interface *iface;
>> +
>> + if (udev->state == USB_STATE_NOTATTACHED)
>> + return -ENODEV;
>
>What will happen if the state changes to USB_STATE_NOTATTACHED at this
>point, after that test was made?
Please suggest if i should add some propr locking mechanism to avoid this.
I will add accordingly.
>> +
>> + iface = usb_ifnum_to_if(udev,
>> cur_alt->desc.bInterfaceNumber);
>>
>> if (!iface)
>> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
>> index b5811620f1de.. f31c7287dc01 100644
>> --- a/drivers/usb/core/message.c
>> +++ b/drivers/usb/core/message.c
>> @@ -1575,7 +1575,11 @@ int usb_set_interface(struct usb_device *dev, int interface, int alternate)
>> for (i = 0; i < iface->cur_altsetting->desc.bNumEndpoints; i++)
>> iface->cur_altsetting->endpoint[i].streams = 0;
>>
>> + if (dev->state == USB_STATE_NOTATTACHED)
>> + return -ENODEV;
>
>Same question: What happens if the state changes right here?
yes, please suggest more required changes for it.
>> +
>> ret = usb_hcd_alloc_bandwidth(dev, NULL, iface->cur_altsetting, alt);
>> +
>> if (ret < 0) {
>> dev_info(&dev->dev, "Not enough bandwidth for altsetting %d\n",
>> alternate);
>> diff --git a/drivers/usb/core/usb.c b/drivers/usb/core/usb.c
>> index 901ec732321c.. 6fb8b14469ae 100644
>> --- a/drivers/usb/core/usb.c
>> +++ b/drivers/usb/core/usb.c
>> @@ -352,10 +352,13 @@ struct usb_interface *usb_ifnum_to_if(const struct usb_device *dev,
>>
>> if (!config)
>> return NULL;
>> - for (i = 0; i < config->desc.bNumInterfaces; i++)
>> - if (config->interface[i]->altsetting[0]
>> - .desc.bInterfaceNumber == ifnum)
>> + for (i = 0; i < config->desc.bNumInterfaces; i++) {
>> + if (config->interface[i] &&
>
>This new test can fail only if the routine is called after (or while)
>the device is unconfigured or removed. But if a driver makes such a
>call then the driver is buggy. The proper solution is to fix the
>driver, not add this test here.
>
>Besides, this test has the same problem as the others you added above.
>What happens if config->interface[i] gets set to NULL right here?
>
>Alan Stern
>
All above changes resolve existing issue and at USB core level, it will avoid simillar race conditions for all others.
>> + config->interface[i]->altsetting[0]
>> + .desc.bInterfaceNumber == ifnum) {
>> return config->interface[i];
>> + }
>> + }
>>
>> return NULL;
>> }
>> --
>> 2.34.1
>>
Thanks Den Carpenter for information.
yes, i will modify new patch version accordingly to add for mutex_unlock(hcd->bandwidth_mutex); before returning.
>Hi Aman,
>
>kernel test robot noticed the following build warnings:
>
>https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
>url: https://protect2.fireeye.com/v1/url?k=c33c7aba-a2b76f89-c33df1f5-000babff9bb7-672d2cfefe2327a5&q=1&e=0dbea670-3cf3-4a45-a999->157e4e0dcad9&u=https%3A%2F%2Fgithub.com%2Fintel-lab-lkp%2Flinux%2Fcommits%2FAman-Deep%2FUSB-Fix-race-condition-during-UVC-webcam-disconnect%2F20230720->202046
>base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing
>patch link: https://lore.kernel.org/r/20230720113142.3070583-1-aman.deep%40samsung.com
>patch subject: [PATCH] USB: Fix race condition during UVC webcam disconnect
>config: parisc-randconfig-m041-20230726 (https://download.01.org/0day-ci/archive/20230727/[email protected]/config)
>compiler: hppa-linux-gcc (GCC) 12.3.0
>reproduce: (https://download.01.org/0day-ci/archive/20230727/[email protected]/reproduce)
>
>If you fix the issue in a separate patch/commit (i.e. not just a new version of
>the same patch/commit), kindly add following tags
>| Reported-by: kernel test robot <[email protected]>
>| Reported-by: Dan Carpenter <[email protected]>
>| Closes: https://lore.kernel.org/r/[email protected]/
we will add it when creating new patch version.
>
>smatch warnings:
>drivers/usb/core/message.c:1668 usb_set_interface() warn: inconsistent returns 'hcd->bandwidth_mutex'.
>
>vim +1668 drivers/usb/core/message.c
>
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1528 int usb_set_interface(struct usb_device *dev, int interface, int alternate)
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1529 {
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1530 struct usb_interface *iface;
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1531 struct usb_host_interface *alt;
>3f0479e00a3fca Sarah Sharp 2009-12-03 1532 struct usb_hcd *hcd = bus_to_hcd(dev->bus);
>7a7b562d08ad6d Hans de Goede 2013-11-08 1533 int i, ret, manual = 0;
>3e35bf39e0b909 Greg Kroah-Hartman 2008-01-30 1534 unsigned int epaddr;
>3e35bf39e0b909 Greg Kroah-Hartman 2008-01-30 1535 unsigned int pipe;
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1536
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1537 if (dev->state == USB_STATE_SUSPENDED)
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1538 return -EHOSTUNREACH;
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1539
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1540 iface = usb_ifnum_to_if(dev, interface);
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1541 if (!iface) {
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1542 dev_dbg(&dev->dev, "selecting invalid interface %d\n",
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1543 interface);
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1544 return -EINVAL;
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1545 }
>e534c5b831c8b8 Alan Stern 2011-07-01 1546 if (iface->unregistering)
>e534c5b831c8b8 Alan Stern 2011-07-01 1547 return -ENODEV;
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1548
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1549 alt = usb_altnum_to_altsetting(iface, alternate);
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1550 if (!alt) {
>385f690bc058ba Thadeu Lima de Souza Cascardo 2010-01-17 1551 dev_warn(&dev->dev, "selecting invalid altsetting %d\n",
>3b6004f3b5a8b4 Greg Kroah-Hartman 2008-08-14 1552 alternate);
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1553 return -EINVAL;
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1554 }
>f9a5b4f58b280c Mathias Nyman 2018-09-03 1555 /*
>f9a5b4f58b280c Mathias Nyman 2018-09-03 1556 * usb3 hosts configure the interface in usb_hcd_alloc_bandwidth,
>f9a5b4f58b280c Mathias Nyman 2018-09-03 1557 * including freeing dropped endpoint ring buffers.
>f9a5b4f58b280c Mathias Nyman 2018-09-03 1558 * Make sure the interface endpoints are flushed before that
>f9a5b4f58b280c Mathias Nyman 2018-09-03 1559 */
>f9a5b4f58b280c Mathias Nyman 2018-09-03 1560 usb_disable_interface(dev, iface, false);
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1561
>3f0479e00a3fca Sarah Sharp 2009-12-03 1562 /* Make sure we have enough bandwidth for this alternate interface.
>3f0479e00a3fca Sarah Sharp 2009-12-03 1563 * Remove the current alt setting and add the new alt setting.
>3f0479e00a3fca Sarah Sharp 2009-12-03 1564 */
>d673bfcbfffdeb Sarah Sharp 2010-10-15 1565 mutex_lock(hcd->bandwidth_mutex);
>8306095fd2c110 Sarah Sharp 2012-05-02 1566 /* Disable LPM, and re-enable it once the new alt setting is installed,
>8306095fd2c110 Sarah Sharp 2012-05-02 1567 * so that the xHCI driver can recalculate the U1/U2 timeouts.
>8306095fd2c110 Sarah Sharp 2012-05-02 1568 */
>8306095fd2c110 Sarah Sharp 2012-05-02 1569 if (usb_disable_lpm(dev)) {
>1ccc417e6c3201 Joe Perches 2017-12-05 1570 dev_err(&iface->dev, "%s Failed to disable LPM\n", __func__);
>8306095fd2c110 Sarah Sharp 2012-05-02 1571 mutex_unlock(hcd->bandwidth_mutex);
>8306095fd2c110 Sarah Sharp 2012-05-02 1572 return -ENOMEM;
>8306095fd2c110 Sarah Sharp 2012-05-02 1573 }
>7a7b562d08ad6d Hans de Goede 2013-11-08 1574 /* Changing alt-setting also frees any allocated streams */
>7a7b562d08ad6d Hans de Goede 2013-11-08 1575 for (i = 0; i < iface->cur_altsetting->desc.bNumEndpoints; i++)
>7a7b562d08ad6d Hans de Goede 2013-11-08 1576 iface->cur_altsetting->endpoint[i].streams = 0;
>7a7b562d08ad6d Hans de Goede 2013-11-08 1577
>4682bbb9e2f196 Aman Deep 2023-07-20 1578 if (dev->state == USB_STATE_NOTATTACHED)
>4682bbb9e2f196 Aman Deep 2023-07-20 1579 return -ENODEV;
>
>
>mutex_unlock(hcd->bandwidth_mutex); before returning
>
>4682bbb9e2f196 Aman Deep 2023-07-20 1580
>3f0479e00a3fca Sarah Sharp 2009-12-03 1581 ret = usb_hcd_alloc_bandwidth(dev, NULL, iface->cur_altsetting, alt);
>4682bbb9e2f196 Aman Deep 2023-07-20 1582
>3f0479e00a3fca Sarah Sharp 2009-12-03 1583 if (ret < 0) {
>3f0479e00a3fca Sarah Sharp 2009-12-03 1584 dev_info(&dev->dev, "Not enough bandwidth for altsetting %d\n",
>3f0479e00a3fca Sarah Sharp 2009-12-03 1585 alternate);
>8306095fd2c110 Sarah Sharp 2012-05-02 1586 usb_enable_lpm(dev);
>d673bfcbfffdeb Sarah Sharp 2010-10-15 1587 mutex_unlock(hcd->bandwidth_mutex);
>3f0479e00a3fca Sarah Sharp 2009-12-03 1588 return ret;
>3f0479e00a3fca Sarah Sharp 2009-12-03 1589 }
>3f0479e00a3fca Sarah Sharp 2009-12-03 1590
>392e1d9817d002 Alan Stern 2008-03-11 1591 if (dev->quirks & USB_QUIRK_NO_SET_INTF)
>392e1d9817d002 Alan Stern 2008-03-11 1592 ret = -EPIPE;
>392e1d9817d002 Alan Stern 2008-03-11 1593 else
>297e84c04d76b9 Greg Kroah-Hartman 2020-09-14 1594 ret = usb_control_msg_send(dev, 0,
>297e84c04d76b9 Greg Kroah-Hartman 2020-09-14 1595 USB_REQ_SET_INTERFACE,
>297e84c04d76b9 Greg Kroah-Hartman 2020-09-14 1596 USB_RECIP_INTERFACE, alternate,
>ddd1198e3e0935 Oliver Neukum 2020-09-23 1597 interface, NULL, 0, 5000,
>ddd1198e3e0935 Oliver Neukum 2020-09-23 1598 GFP_NOIO);
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1599
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1600 /* 9.4.10 says devices don't need this and are free to STALL the
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1601 * request if the interface only has one alternate setting.
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1602 */
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1603 if (ret == -EPIPE && iface->num_altsetting == 1) {
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1604 dev_dbg(&dev->dev,
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1605 "manual set_interface for iface %d, alt %d\n",
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1606 interface, alternate);
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1607 manual = 1;
>297e84c04d76b9 Greg Kroah-Hartman 2020-09-14 1608 } else if (ret) {
>3f0479e00a3fca Sarah Sharp 2009-12-03 1609 /* Re-instate the old alt setting */
>3f0479e00a3fca Sarah Sharp 2009-12-03 1610 usb_hcd_alloc_bandwidth(dev, NULL, alt, iface->cur_altsetting);
>8306095fd2c110 Sarah Sharp 2012-05-02 1611 usb_enable_lpm(dev);
>d673bfcbfffdeb Sarah Sharp 2010-10-15 1612 mutex_unlock(hcd->bandwidth_mutex);
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1613 return ret;
>3f0479e00a3fca Sarah Sharp 2009-12-03 1614 }
>d673bfcbfffdeb Sarah Sharp 2010-10-15 1615 mutex_unlock(hcd->bandwidth_mutex);
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1616
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1617 /* FIXME drivers shouldn't need to replicate/bugfix the logic here
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1618 * when they implement async or easily-killable versions of this or
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1619 * other "should-be-internal" functions (like clear_halt).
>^1da177e4c3f41 Linus Torvalds 2005-04-16 1620 * should hcd+usbcore postprocess control requests?
>
>--
>0-DAY CI Kernel Test Service
>https://protect2.fireeye.com/v1/url?k=f58bda35-9400cf06-f58a517a-000babff9bb7-d72ab9bab3df6b4a&q=1&e=0dbea670-3cf3-4a45-a999->157e4e0dcad9&u=https%3A%2F%2Fgithub.com%2Fintel%2Flkp-tests%2Fwiki
>
>
Thanks,
Aman
On 28.07.23 06:59, AMAN DEEP wrote:
Hi,
> [1-221.1822] [ msg: 4788] PC is at usb_ifnum_to_if+0x30/0x74 [usbcore]
This has to fail if the device is gone, but the question is why the driver
is doing this. Hence we need to look at the backtrace.
> [1-221.1822] [ msg: 4788] LR is at 0x5
> [1-221.1822] [ msg: 4788] pc : [<bede1300>] lr : [<00000005>] psr: 20000113
> [1-221.1822] [ msg: 4788] sp : ca443c18 ip : ca443c28 fp : ca443c24
> [1-221.1822] [ msg: 4788] r10: e668b6c8 r9 : 00000000 r8 : e668b7e0
> [1-221.1822] [ msg: 4788] r7 : e7b78880 r6 : bf1d9db0 r5 : e668b6c8 r4 : e690c000
> [1-221.1822] [ msg: 4788] r3 : 00002000 r2 : e696ac40 r1 : 00000001 r0 : 00000000
> [1-221.1822] [ msg: 4788] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> [1-221.1822] [ msg: 4788] Control: 30c5383d Table: 261f8a80 DAC: e45d65d5
> [1-221.1822] [ msg: 4788] Process msg (pid: 4788, stack limit = 0xa0153238)
> [1-221.1822] [ msg: 4788] Stack: (0xca443c18 to 0xca444000)
> [1-221.1822] [ msg: 4788] 3c00: ca443c64 ca443c28
> [1-221.1822] [ msg: 4788] 3c20: bedee6e4 bede12dc 00000000 bee0ae78 ca443c54 ca443c40 c083c894 e7b78880
> [1-221.1822] [ msg: 4788] 3c40: e6b88340 00000000 bee0ae78 00000001 e690c000 e668b6c8 ca443cb4 ca443c68
> [1-221.1822] [ msg: 4788] 3c60: bedf22ac bedee64c e5cf1508 e5cf1508 e5cf0000 e5cf0330 00000001 e5cf0330
> [1-221.1822] [ msg: 4788] 3c80: ca443ca4 ca443c90 c083c894 e5cf0000 e5cf0330 00000001 e5cf0330 00000000
> [1-221.1822] [ msg: 4788] 3ca0: 00000001 c08d1b3c ca443ccc ca443cb8 bfb3f958 bedf1ff4 e5cf0330 e5cf0330
> [1-221.1822] [ msg: 4788] 3cc0: ca443ce4 ca443cd0 bfb3a024 bfb3f8a8 e5cf0330 e5cf0330 ca443d14 ca443ce8
> [1-221.1823] [ msg: 4788] 3ce0: be3661e0 bfb3a004 00000001 e5cf0330 e5cf0330 00000001 c05d6260 00000000
> [1-221.1823] [ msg: 4788] 3d00: 00000001 c08d1b3c ca443d2c ca443d18 be367994 be3661b4 e5cf0484 e5cf0330
> [1-221.1823] [ msg: 4788] 3d20: ca443d3c ca443d30 be37e3e4 be367978 ca443d5c ca443d40 bfb3a518 be37e3cc
> [1-221.1823] [ msg: 4788] 3d40: e5cf030c e5cf0000 00000001 c05d6260 ca443d7c ca443d60 bfb3b628 bfb3a4f0
> [1-221.1823] [ msg: 4788] 3d60: bfb3b5e8 40045613 00000000 c05d6260 ca443d94 ca443d80 c05d6288 bfb3b5f4
> [1-221.1823] [ msg: 4788] 3d80: e5cf0010 40045613 ca443dfc ca443d98 c05d9b84 c05d626c 00000068 ca443deb
> [1-221.1823] [ msg: 4788] 3da0: c08d1b3c 00000001 ca443e24 bfb44680 00000000 e2fa3780 c01a926c 031e1090
> [1-221.1823] [ msg: 4788] 3dc0: ca443df4 ffffffff c01e0048 0000072c 000012b4 00000000 40045613 00000000
> [1-221.1823] [ msg: 4788] 3de0: 00000000 00000001 00000004 ca443e24 ca443ed4 ca443e00 c05db320 c05d9a04
> [1-221.1823] [ msg: 4788] 3e00: 00000000 00000000 c05d99f8 e77a6700 ab8fd26c 00000000 00000000 00000000
> [1-221.1823] [ msg: 4788] 3e20: ca443f60 00000001 ca443ee0 00000000 ca443e9c ca443e40 c02390a8 be211e84
> [1-221.1823] [ msg: 4788] 3e40: 00000000 00000001 e2861600 00000000 00000000 00000000 00000000 00000000
> [1-221.1823] [ msg: 4788] 3e60: 00000000 00000000 00000000 c03681bc 00000008 00000000 ca443ee0 c0bbd748
> [1-221.1823] [ msg: 4788] 3e80: 00000000 c0be9a14 ca443ef4 00000002 ca443ed4 ca443ea0 c03681bc c036790c
> [1-221.1823] [ msg: 4788] 3ea0: ca443ef4 c0bbd748 e2861600 c05db7dc e6695448 40045613 ab8fd26c e77a6700
> [1-221.1823] [ msg: 4788] 3ec0: 00000021 00000036 ca443ee4 ca443ed8 c05db7fc c05db0f8 ca443efc ca443ee8
> [1-221.1823] [ msg: 4788] 3ee0: c05d4728 c05db7e8 ab8fd26c e6695448 ca443f6c ca443f00 c02506a0 c05d46e8
> [1-221.1823] [ msg: 4788] 3f00: ca443f04 c08a7a00 00000000 00000000 00000000 00000000 00000000 00000000
> [1-221.1823] [ msg: 4788] 3f20: 00000000 00000000 00000000 00000000 ab8fd26c c0abb6ec ab8fd26c e77a6700
> [1-221.1823] [ msg: 4788] 3f40: ca443f6c e77a6701 00000000 40045613 ab8fd26c e77a6700 00000021 00000036
> [1-221.1824] [ msg: 4788] 3f60: ca443f94 ca443f70 c0250b3c c02502fc 00000000 000006f7 00000000 00000036
> [1-221.1824] [ msg: 4788] 3f80: c000924c ca442000 ca443fa4 ca443f98 c0250b78 c0250adc 00000000 ca443fa8
> [1-221.1824] [ msg: 4788] 3fa0: c0009230 c0250b6c 00000000 000006f7 00000021 40045613 ab8fd26c 00000021
> [1-221.1824] [ msg: 4788] 3fc0: 00000000 000006f7 00000000 00000036 abb79e30 00000000 00000001 abb79e28
> [1-221.1824] [ msg: 4788] 3fe0: aeca607c ab8fd24c aec8e749 b5f1ed1c 20000010 00000021 00000000 00000000
> [1-221.1824] [ msg: 4788] Backtrace:
> [1-221.1824] [ msg: 4788] [<bede12d0>] (usb_ifnum_to_if [usbcore]) from [<bedee6e4>] (usb_hcd_alloc_bandwidth+0xa4/0x564 [usbcore])
> [1-221.1824] [ msg: 4788] [<bedee640>] (usb_hcd_alloc_bandwidth [usbcore]) from [<bedf22ac>] (usb_set_interface+0x2c4/0x61c [usbcore])
This is the proximate cause.
> [1-221.1824] [ msg: 4788] r10:e668b6c8 r9:e690c000 r8:00000001 r7:bee0ae78 r6:00000000 r5:e6b88340
> [1-221.1824] [ msg: 4788] r4:e7b78880
> [1-221.1825] [ msg: 4788] [<bedf1fe8>] (usb_set_interface [usbcore]) from [<bfb3f958>] (uvc_video_stop_streaming+0xbc/0xc4 [uvcvideo])
> [1-221.1825] [ msg: 4788] r10:c08d1b3c r9:00000001 r8:00000000 r7:e5cf0330 r6:00000001 r5:e5cf0330
> [1-221.1825] [ msg: 4788] r4:e5cf0000
> [1-221.1825] [ msg: 4788] [<bfb3f89c>] (uvc_video_stop_streaming [uvcvideo]) from [<bfb3a024>] (uvc_stop_streaming+0x2c/0x50 [uvcvideo])
triggered from here
> [1-221.1826] [ msg: 4788] [<bfb3b5e8>] (uvc_ioctl_streamoff [uvcvideo]) from [<c05d6288>] (v4l_streamoff+0x28/0x2c)
> [1-221.1826] [ msg: 4788] r7:c05d6260 r6:00000000 r5:40045613 r4:bfb3b5e8
User space is trying to execute an ioctl() on a device whose
disconnect() method has run. A driver has to either prevent or fail such calls.
> I thought this issue can occur with other devices in simillar race conditions so i thought it will be fixed for all drivers.
No, this will not work. You are failing to take into consideration
that the life time of the device is different from its association
with a particular device driver.
> Please suggest if we need to add locking mechanism to cover such cases.
> i will try accordingly.
For the reason I stated above this is not fixable with locking
at this level. The test for the device state is the wrong test.
Consequently no amount of locking can correct that. The conditions
only happen to conincide because your testing replicates the most
common code path. It is not the only one.
You need to fix uvc_disconnect()
HTH
Oliver
Hi Oliver,
Thanks for analysis. I got your point about changes required in UVC driver to handle this race condition.
Alan and Greg also pointed it out, so i tried to handle this race condition in UVC driver.
I am testing my new changes done in only UVC driver to resolve this issue.
After checking new patch for this race condition, I will share it.
>>On 28.07.23 06:59, AMAN DEEP wrote:
>
>Hi,
>
>> [1-221.1822] [ msg: 4788] PC is at usb_ifnum_to_if+0x30/0x74 [usbcore]
>
>This has to fail if the device is gone, but the question is why the driver
>is doing this. Hence we need to look at the backtrace.
>
>> [1-221.1822] [ msg: 4788] LR is at 0x5
>> [1-221.1822] [ msg: 4788] pc : [<bede1300>] lr : [<00000005>] psr: 20000113
>> [1-221.1822] [ msg: 4788] sp : ca443c18 ip : ca443c28 fp : ca443c24
>> [1-221.1822] [ msg: 4788] r10: e668b6c8 r9 : 00000000 r8 : e668b7e0
>> [1-221.1822] [ msg: 4788] r7 : e7b78880 r6 : bf1d9db0 r5 : e668b6c8 r4 : e690c000
>> [1-221.1822] [ msg: 4788] r3 : 00002000 r2 : e696ac40 r1 : 00000001 r0 : 00000000
>> [1-221.1822] [ msg: 4788] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
>> [1-221.1822] [ msg: 4788] Control: 30c5383d Table: 261f8a80 DAC: e45d65d5
>> [1-221.1822] [ msg: 4788] Process msg (pid: 4788, stack limit = 0xa0153238)
>> [1-221.1822] [ msg: 4788] Stack: (0xca443c18 to 0xca444000)
>> [1-221.1822] [ msg: 4788] 3c00: ca443c64 ca443c28
>> [1-221.1822] [ msg: 4788] 3c20: bedee6e4 bede12dc 00000000 bee0ae78 ca443c54 ca443c40 c083c894 e7b78880
>> [1-221.1822] [ msg: 4788] 3c40: e6b88340 00000000 bee0ae78 00000001 e690c000 e668b6c8 ca443cb4 ca443c68
>> [1-221.1822] [ msg: 4788] 3c60: bedf22ac bedee64c e5cf1508 e5cf1508 e5cf0000 e5cf0330 00000001 e5cf0330
>> [1-221.1822] [ msg: 4788] 3c80: ca443ca4 ca443c90 c083c894 e5cf0000 e5cf0330 00000001 e5cf0330 00000000
>> [1-221.1822] [ msg: 4788] 3ca0: 00000001 c08d1b3c ca443ccc ca443cb8 bfb3f958 bedf1ff4 e5cf0330 e5cf0330
>> [1-221.1822] [ msg: 4788] 3cc0: ca443ce4 ca443cd0 bfb3a024 bfb3f8a8 e5cf0330 e5cf0330 ca443d14 ca443ce8
>> [1-221.1823] [ msg: 4788] 3ce0: be3661e0 bfb3a004 00000001 e5cf0330 e5cf0330 00000001 c05d6260 00000000
>> [1-221.1823] [ msg: 4788] 3d00: 00000001 c08d1b3c ca443d2c ca443d18 be367994 be3661b4 e5cf0484 e5cf0330
>> [1-221.1823] [ msg: 4788] 3d20: ca443d3c ca443d30 be37e3e4 be367978 ca443d5c ca443d40 bfb3a518 be37e3cc
>> [1-221.1823] [ msg: 4788] 3d40: e5cf030c e5cf0000 00000001 c05d6260 ca443d7c ca443d60 bfb3b628 bfb3a4f0
>> [1-221.1823] [ msg: 4788] 3d60: bfb3b5e8 40045613 00000000 c05d6260 ca443d94 ca443d80 c05d6288 bfb3b5f4
>> [1-221.1823] [ msg: 4788] 3d80: e5cf0010 40045613 ca443dfc ca443d98 c05d9b84 c05d626c 00000068 ca443deb
>> [1-221.1823] [ msg: 4788] 3da0: c08d1b3c 00000001 ca443e24 bfb44680 00000000 e2fa3780 c01a926c 031e1090
>> [1-221.1823] [ msg: 4788] 3dc0: ca443df4 ffffffff c01e0048 0000072c 000012b4 00000000 40045613 00000000
>> [1-221.1823] [ msg: 4788] 3de0: 00000000 00000001 00000004 ca443e24 ca443ed4 ca443e00 c05db320 c05d9a04
>> [1-221.1823] [ msg: 4788] 3e00: 00000000 00000000 c05d99f8 e77a6700 ab8fd26c 00000000 00000000 00000000
>> [1-221.1823] [ msg: 4788] 3e20: ca443f60 00000001 ca443ee0 00000000 ca443e9c ca443e40 c02390a8 be211e84
>> [1-221.1823] [ msg: 4788] 3e40: 00000000 00000001 e2861600 00000000 00000000 00000000 00000000 00000000
>> [1-221.1823] [ msg: 4788] 3e60: 00000000 00000000 00000000 c03681bc 00000008 00000000 ca443ee0 c0bbd748
>> [1-221.1823] [ msg: 4788] 3e80: 00000000 c0be9a14 ca443ef4 00000002 ca443ed4 ca443ea0 c03681bc c036790c
>> [1-221.1823] [ msg: 4788] 3ea0: ca443ef4 c0bbd748 e2861600 c05db7dc e6695448 40045613 ab8fd26c e77a6700
>> [1-221.1823] [ msg: 4788] 3ec0: 00000021 00000036 ca443ee4 ca443ed8 c05db7fc c05db0f8 ca443efc ca443ee8
>> [1-221.1823] [ msg: 4788] 3ee0: c05d4728 c05db7e8 ab8fd26c e6695448 ca443f6c ca443f00 c02506a0 c05d46e8
>> [1-221.1823] [ msg: 4788] 3f00: ca443f04 c08a7a00 00000000 00000000 00000000 00000000 00000000 00000000
>> [1-221.1823] [ msg: 4788] 3f20: 00000000 00000000 00000000 00000000 ab8fd26c c0abb6ec ab8fd26c e77a6700
>> [1-221.1823] [ msg: 4788] 3f40: ca443f6c e77a6701 00000000 40045613 ab8fd26c e77a6700 00000021 00000036
>> [1-221.1824] [ msg: 4788] 3f60: ca443f94 ca443f70 c0250b3c c02502fc 00000000 000006f7 00000000 00000036
>> [1-221.1824] [ msg: 4788] 3f80: c000924c ca442000 ca443fa4 ca443f98 c0250b78 c0250adc 00000000 ca443fa8
>> [1-221.1824] [ msg: 4788] 3fa0: c0009230 c0250b6c 00000000 000006f7 00000021 40045613 ab8fd26c 00000021
>> [1-221.1824] [ msg: 4788] 3fc0: 00000000 000006f7 00000000 00000036 abb79e30 00000000 00000001 abb79e28
>> [1-221.1824] [ msg: 4788] 3fe0: aeca607c ab8fd24c aec8e749 b5f1ed1c 20000010 00000021 00000000 00000000
>> [1-221.1824] [ msg: 4788] Backtrace:
>> [1-221.1824] [ msg: 4788] [<bede12d0>] (usb_ifnum_to_if [usbcore]) from [<bedee6e4>] (usb_hcd_alloc_bandwidth+0xa4/0x564 [usbcore])
>> [1-221.1824] [ msg: 4788] [<bedee640>] (usb_hcd_alloc_bandwidth [usbcore]) from [<bedf22ac>] (usb_set_interface+0x2c4/0x61c [usbcore])
>
>This is the proximate cause.
>
>> [1-221.1824] [ msg: 4788] r10:e668b6c8 r9:e690c000 r8:00000001 r7:bee0ae78 r6:00000000 r5:e6b88340
>> [1-221.1824] [ msg: 4788] r4:e7b78880
>> [1-221.1825] [ msg: 4788] [<bedf1fe8>] (usb_set_interface [usbcore]) from [<bfb3f958>] (uvc_video_stop_streaming+0xbc/0xc4 [uvcvideo])
>> [1-221.1825] [ msg: 4788] r10:c08d1b3c r9:00000001 r8:00000000 r7:e5cf0330 r6:00000001 r5:e5cf0330
>> [1-221.1825] [ msg: 4788] r4:e5cf0000
>> [1-221.1825] [ msg: 4788] [<bfb3f89c>] (uvc_video_stop_streaming [uvcvideo]) from [<bfb3a024>] (uvc_stop_streaming+0x2c/0x50 [uvcvideo])
>
>triggered from here
>> [1-221.1826] [ msg: 4788] [<bfb3b5e8>] (uvc_ioctl_streamoff [uvcvideo]) from [<c05d6288>] (v4l_streamoff+0x28/0x2c)
>> [1-221.1826] [ msg: 4788] r7:c05d6260 r6:00000000 r5:40045613 r4:bfb3b5e8
>
>User space is trying to execute an ioctl() on a device whose
>disconnect() method has run. A driver has to either prevent or fail such calls.
>
>> I thought this issue can occur with other devices in simillar race conditions so i thought it will be fixed for all drivers.
>
>No, this will not work. You are failing to take into consideration
>that the life time of the device is different from its association
>with a particular device driver.
>
>> Please suggest if we need to add locking mechanism to cover such cases.
>> i will try accordingly.
>
>For the reason I stated above this is not fixable with locking
>at this level. The test for the device state is the wrong test.
>Consequently no amount of locking can correct that. The conditions
>only happen to conincide because your testing replicates the most
>common code path. It is not the only one.
>
>You need to fix uvc_disconnect()
>
> HTH
> Oliver
>
Thanks,
Aman Deep
Hi Aman
Could you check if this patchset fixes the problem for you
https://patchwork.linuxtv.org/project/linux-media/list/?series=10038 ?
Regards!
On Wed, 2 Aug 2023 at 09:56, AMAN DEEP <[email protected]> wrote:
>
>
> Hi Oliver,
>
>
> Thanks for analysis. I got your point about changes required in UVC driver to handle this race condition.
> Alan and Greg also pointed it out, so i tried to handle this race condition in UVC driver.
> I am testing my new changes done in only UVC driver to resolve this issue.
> After checking new patch for this race condition, I will share it.
>
>
> >>On 28.07.23 06:59, AMAN DEEP wrote:
> >
> >Hi,
> >
> >> [1-221.1822] [ msg: 4788] PC is at usb_ifnum_to_if+0x30/0x74 [usbcore]
> >
> >This has to fail if the device is gone, but the question is why the driver
> >is doing this. Hence we need to look at the backtrace.
> >
> >> [1-221.1822] [ msg: 4788] LR is at 0x5
> >> [1-221.1822] [ msg: 4788] pc : [<bede1300>] lr : [<00000005>] psr: 20000113
> >> [1-221.1822] [ msg: 4788] sp : ca443c18 ip : ca443c28 fp : ca443c24
> >> [1-221.1822] [ msg: 4788] r10: e668b6c8 r9 : 00000000 r8 : e668b7e0
> >> [1-221.1822] [ msg: 4788] r7 : e7b78880 r6 : bf1d9db0 r5 : e668b6c8 r4 : e690c000
> >> [1-221.1822] [ msg: 4788] r3 : 00002000 r2 : e696ac40 r1 : 00000001 r0 : 00000000
> >> [1-221.1822] [ msg: 4788] Flags: nzCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
> >> [1-221.1822] [ msg: 4788] Control: 30c5383d Table: 261f8a80 DAC: e45d65d5
> >> [1-221.1822] [ msg: 4788] Process msg (pid: 4788, stack limit = 0xa0153238)
> >> [1-221.1822] [ msg: 4788] Stack: (0xca443c18 to 0xca444000)
> >> [1-221.1822] [ msg: 4788] 3c00: ca443c64 ca443c28
> >> [1-221.1822] [ msg: 4788] 3c20: bedee6e4 bede12dc 00000000 bee0ae78 ca443c54 ca443c40 c083c894 e7b78880
> >> [1-221.1822] [ msg: 4788] 3c40: e6b88340 00000000 bee0ae78 00000001 e690c000 e668b6c8 ca443cb4 ca443c68
> >> [1-221.1822] [ msg: 4788] 3c60: bedf22ac bedee64c e5cf1508 e5cf1508 e5cf0000 e5cf0330 00000001 e5cf0330
> >> [1-221.1822] [ msg: 4788] 3c80: ca443ca4 ca443c90 c083c894 e5cf0000 e5cf0330 00000001 e5cf0330 00000000
> >> [1-221.1822] [ msg: 4788] 3ca0: 00000001 c08d1b3c ca443ccc ca443cb8 bfb3f958 bedf1ff4 e5cf0330 e5cf0330
> >> [1-221.1822] [ msg: 4788] 3cc0: ca443ce4 ca443cd0 bfb3a024 bfb3f8a8 e5cf0330 e5cf0330 ca443d14 ca443ce8
> >> [1-221.1823] [ msg: 4788] 3ce0: be3661e0 bfb3a004 00000001 e5cf0330 e5cf0330 00000001 c05d6260 00000000
> >> [1-221.1823] [ msg: 4788] 3d00: 00000001 c08d1b3c ca443d2c ca443d18 be367994 be3661b4 e5cf0484 e5cf0330
> >> [1-221.1823] [ msg: 4788] 3d20: ca443d3c ca443d30 be37e3e4 be367978 ca443d5c ca443d40 bfb3a518 be37e3cc
> >> [1-221.1823] [ msg: 4788] 3d40: e5cf030c e5cf0000 00000001 c05d6260 ca443d7c ca443d60 bfb3b628 bfb3a4f0
> >> [1-221.1823] [ msg: 4788] 3d60: bfb3b5e8 40045613 00000000 c05d6260 ca443d94 ca443d80 c05d6288 bfb3b5f4
> >> [1-221.1823] [ msg: 4788] 3d80: e5cf0010 40045613 ca443dfc ca443d98 c05d9b84 c05d626c 00000068 ca443deb
> >> [1-221.1823] [ msg: 4788] 3da0: c08d1b3c 00000001 ca443e24 bfb44680 00000000 e2fa3780 c01a926c 031e1090
> >> [1-221.1823] [ msg: 4788] 3dc0: ca443df4 ffffffff c01e0048 0000072c 000012b4 00000000 40045613 00000000
> >> [1-221.1823] [ msg: 4788] 3de0: 00000000 00000001 00000004 ca443e24 ca443ed4 ca443e00 c05db320 c05d9a04
> >> [1-221.1823] [ msg: 4788] 3e00: 00000000 00000000 c05d99f8 e77a6700 ab8fd26c 00000000 00000000 00000000
> >> [1-221.1823] [ msg: 4788] 3e20: ca443f60 00000001 ca443ee0 00000000 ca443e9c ca443e40 c02390a8 be211e84
> >> [1-221.1823] [ msg: 4788] 3e40: 00000000 00000001 e2861600 00000000 00000000 00000000 00000000 00000000
> >> [1-221.1823] [ msg: 4788] 3e60: 00000000 00000000 00000000 c03681bc 00000008 00000000 ca443ee0 c0bbd748
> >> [1-221.1823] [ msg: 4788] 3e80: 00000000 c0be9a14 ca443ef4 00000002 ca443ed4 ca443ea0 c03681bc c036790c
> >> [1-221.1823] [ msg: 4788] 3ea0: ca443ef4 c0bbd748 e2861600 c05db7dc e6695448 40045613 ab8fd26c e77a6700
> >> [1-221.1823] [ msg: 4788] 3ec0: 00000021 00000036 ca443ee4 ca443ed8 c05db7fc c05db0f8 ca443efc ca443ee8
> >> [1-221.1823] [ msg: 4788] 3ee0: c05d4728 c05db7e8 ab8fd26c e6695448 ca443f6c ca443f00 c02506a0 c05d46e8
> >> [1-221.1823] [ msg: 4788] 3f00: ca443f04 c08a7a00 00000000 00000000 00000000 00000000 00000000 00000000
> >> [1-221.1823] [ msg: 4788] 3f20: 00000000 00000000 00000000 00000000 ab8fd26c c0abb6ec ab8fd26c e77a6700
> >> [1-221.1823] [ msg: 4788] 3f40: ca443f6c e77a6701 00000000 40045613 ab8fd26c e77a6700 00000021 00000036
> >> [1-221.1824] [ msg: 4788] 3f60: ca443f94 ca443f70 c0250b3c c02502fc 00000000 000006f7 00000000 00000036
> >> [1-221.1824] [ msg: 4788] 3f80: c000924c ca442000 ca443fa4 ca443f98 c0250b78 c0250adc 00000000 ca443fa8
> >> [1-221.1824] [ msg: 4788] 3fa0: c0009230 c0250b6c 00000000 000006f7 00000021 40045613 ab8fd26c 00000021
> >> [1-221.1824] [ msg: 4788] 3fc0: 00000000 000006f7 00000000 00000036 abb79e30 00000000 00000001 abb79e28
> >> [1-221.1824] [ msg: 4788] 3fe0: aeca607c ab8fd24c aec8e749 b5f1ed1c 20000010 00000021 00000000 00000000
> >> [1-221.1824] [ msg: 4788] Backtrace:
> >> [1-221.1824] [ msg: 4788] [<bede12d0>] (usb_ifnum_to_if [usbcore]) from [<bedee6e4>] (usb_hcd_alloc_bandwidth+0xa4/0x564 [usbcore])
> >> [1-221.1824] [ msg: 4788] [<bedee640>] (usb_hcd_alloc_bandwidth [usbcore]) from [<bedf22ac>] (usb_set_interface+0x2c4/0x61c [usbcore])
> >
> >This is the proximate cause.
> >
> >> [1-221.1824] [ msg: 4788] r10:e668b6c8 r9:e690c000 r8:00000001 r7:bee0ae78 r6:00000000 r5:e6b88340
> >> [1-221.1824] [ msg: 4788] r4:e7b78880
> >> [1-221.1825] [ msg: 4788] [<bedf1fe8>] (usb_set_interface [usbcore]) from [<bfb3f958>] (uvc_video_stop_streaming+0xbc/0xc4 [uvcvideo])
> >> [1-221.1825] [ msg: 4788] r10:c08d1b3c r9:00000001 r8:00000000 r7:e5cf0330 r6:00000001 r5:e5cf0330
> >> [1-221.1825] [ msg: 4788] r4:e5cf0000
> >> [1-221.1825] [ msg: 4788] [<bfb3f89c>] (uvc_video_stop_streaming [uvcvideo]) from [<bfb3a024>] (uvc_stop_streaming+0x2c/0x50 [uvcvideo])
> >
> >triggered from here
> >> [1-221.1826] [ msg: 4788] [<bfb3b5e8>] (uvc_ioctl_streamoff [uvcvideo]) from [<c05d6288>] (v4l_streamoff+0x28/0x2c)
> >> [1-221.1826] [ msg: 4788] r7:c05d6260 r6:00000000 r5:40045613 r4:bfb3b5e8
> >
> >User space is trying to execute an ioctl() on a device whose
> >disconnect() method has run. A driver has to either prevent or fail such calls.
> >
> >> I thought this issue can occur with other devices in simillar race conditions so i thought it will be fixed for all drivers.
> >
> >No, this will not work. You are failing to take into consideration
> >that the life time of the device is different from its association
> >with a particular device driver.
> >
> >> Please suggest if we need to add locking mechanism to cover such cases.
> >> i will try accordingly.
> >
> >For the reason I stated above this is not fixable with locking
> >at this level. The test for the device state is the wrong test.
> >Consequently no amount of locking can correct that. The conditions
> >only happen to conincide because your testing replicates the most
> >common code path. It is not the only one.
> >
> >You need to fix uvc_disconnect()
> >
> > HTH
> > Oliver
> >
>
> Thanks,
> Aman Deep
>
--
Ricardo Ribalda