diff mbox series

USB: Fix race condition during UVC webcam disconnect

Message ID 20230720113142.3070583-1-aman.deep@samsung.com (mailing list archive)
State New, archived
Headers show
Series USB: Fix race condition during UVC webcam disconnect | expand

Commit Message

Aman Deep July 20, 2023, 11:31 a.m. UTC
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 <anuj01.gupta@samsung.com>
Signed-off-by: Aman Deep <aman.deep@samsung.com>
---
 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(-)

Comments

Greg KH July 20, 2023, 1:22 p.m. UTC | #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 <anuj01.gupta@samsung.com>
> Signed-off-by: Aman Deep <aman.deep@samsung.com>

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
Alan Stern July 20, 2023, 2:55 p.m. UTC | #2
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 <anuj01.gupta@samsung.com>
> Signed-off-by: Aman Deep <aman.deep@samsung.com>
> ---
>  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
>
Dan Carpenter July 27, 2023, 5:13 a.m. UTC | #3
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/202307270834.rpaexQSs-lkp@intel.com/config)
compiler: hppa-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230727/202307270834.rpaexQSs-lkp@intel.com/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 <lkp@intel.com>
| Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
| Closes: https://lore.kernel.org/r/202307270834.rpaexQSs-lkp@intel.com/

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?
diff mbox series

Patch

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;
 }