Re: [PATCH v5 2/3] media: uvcvideo: add UVC_QUIRK_CTRL_THROTTLE for fragile firmware

Jeffrey Hein
2026-04-09T07:51:29Z

Hi Ricardo,

On Thu, 9 Apr 2026 at 08:45, Ricardo Ribalda <ribalda@chromium.org> wrote:
> A usb device shall not be able crash the whole USB host. I believe
> that you already captured some logs and the USB guys are looking into
> it. I'd really like to hear what they have to say after reviewing
> them.

Agreed -- a single device shouldn't be able to take down the host.
Alan Stern raised the same point and asked whether xhci-hcd should
handle this. Michal Pecio noted that the stop-endpoint command timeout
is a controller-side failure and asked for dynamic debug traces on
newer kernels and non-Intel hardware.

I provided the 6.17 traces. The result: the stress test (control
transfers only) now passes 50/50 thanks to xHCI error handling
improvements between 6.8 and 6.17. But starting a video stream still
triggers hc_died() -- the firmware fails to disable LPM during stream
setup, the endpoint stalls, and the stop-endpoint command times out.

So there's an open question on the xHCI side about whether the
controller could recover from stop-endpoint timeouts instead of
killing the HC. The UVC quirks are defense-in-depth -- they prevent
the firmware from reaching the failure state that triggers the timeout
in the first place.

I'm also planning to test on additional Intel machines (and non-Intel
if I can source one) to determine whether the stop-endpoint timeout
is controller-specific, per Michal's request.

> Why don't do you do the rate-limit in __uvc_query_ctrl()?

Good point -- moved it there in v6. This covers all callers including
uvc_set_video_ctrl() which bypasses uvc_query_ctrl() for probe/commit.

> Are you sure that you only have to limit UVC_SET_CUR?

I haven't been able to isolate the crash to a specific query direction
-- our testing shows it's the sustained transfer rate that matters. v6
throttles all query types in __uvc_query_ctrl() to be safe.

v6 posted with these changes.

Thanks,
JP

>
> On Wed, Apr 8, 2026 at 11:45 PM Ricardo Ribalda <ribalda@chromium.org> wrote:
>>
>> Hi JP
>>
>> On Tue, 31 Mar 2026 at 02:38, JP Hein <jp@jphein.com> wrote:
>> >
>> > Some USB webcams have firmware that crashes when it receives rapid
>> > consecutive UVC control transfers (SET_CUR). The Razer Kiyo Pro
>> > (1532:0e05) is one such device -- after several hundred rapid control
>> > changes over a few seconds, the device stops responding entirely,
>> > triggering an xHCI stop-endpoint command timeout that causes the host
>> > controller to be declared dead, disconnecting every USB device on the
>> > bus.
>>
>> A usb device shall not be able crash the whole USB host. I believe
>> that you already captured some logs and the USB guys are looking into
>> it. I'd really like to hear what they have to say after reviewing
>> them.
>>
>> >
>> > The failure is amplified by the standard UVC error-code query: when a
>> > SET_CUR fails with EPIPE, the driver sends a second transfer (GET_CUR
>> > on UVC_VC_REQUEST_ERROR_CODE_CONTROL) to read the UVC error code. On a
>> > device that is already stalling, this second transfer pushes the
>> > firmware into a full lockup.
>> >
>> > Introduce UVC_QUIRK_CTRL_THROTTLE (0x00080000) to address both issues:
>> >
>> > - Enforce a minimum 50ms interval between SET_CUR control transfers,
>> > preventing the rapid-fire pattern that overwhelms the firmware.
>> > 50ms allows up to 20 control changes per second, which is sufficient
>> > for interactive slider adjustments while keeping the device stable.
>> >
>> > - Skip the UVC_VC_REQUEST_ERROR_CODE_CONTROL query after EPIPE errors
>> > on devices with this quirk. EPIPE is returned directly without the
>> > follow-up query that would amplify the failure.
>> >
>> > The UVC control path is serialized by ctrl_mutex, so last_ctrl_set_jiffies
>> > does not require additional locking.
>> >
>> > Cc: stable@vger.kernel.org
>> > Signed-off-by: JP Hein <jp@jphein.com>
>> > ---
>> > drivers/media/usb/uvc/uvc_video.c | 32 +++++++++++++++++++++++++++++++
>> > drivers/media/usb/uvc/uvcvideo.h | 3 +++
>> > 2 files changed, 35 insertions(+)
>> >
>> > diff --git a/drivers/media/usb/uvc/uvc_video.c b/drivers/media/usb/uvc/uvc_video.c
>> > index 40c76c051..9f402f55e 100644
>> > --- a/drivers/media/usb/uvc/uvc_video.c
>> > +++ b/drivers/media/usb/uvc/uvc_video.c
>> > @@ -75,8 +75,30 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
>> > u8 error;
>> > u8 tmp;
>> >
>>
>> Why don't do you do the rate-limit in __uvc_query_ctrl()?
>>
>> Are you sure that you only have to limit UVC_SET_CUR?
>>
>> > + /*
>> > + * Rate-limit SET_CUR operations for devices with fragile firmware.
>> > + * The Razer Kiyo Pro locks up under sustained rapid SET_CUR
>> > + * transfers (hundreds without delay), crashing the xHCI controller.
>> > + */
>> > + if (query == UVC_SET_CUR &&
>> > + (dev->quirks & UVC_QUIRK_CTRL_THROTTLE)) {
>> > + unsigned long min_interval = msecs_to_jiffies(50);
>> > +
>> > + if (dev->last_ctrl_set_jiffies &&
>> > + time_before(jiffies,
>> > + dev->last_ctrl_set_jiffies + min_interval)) {
>> > + unsigned long elapsed = dev->last_ctrl_set_jiffies +
>> > + min_interval - jiffies;
>> > + msleep(jiffies_to_msecs(elapsed));
>> > + }
>> > + }
>> > +
>> > ret = __uvc_query_ctrl(dev, query, unit, intfnum, cs, data, size,
>> > UVC_CTRL_CONTROL_TIMEOUT);
>> > +
>> > + if (query == UVC_SET_CUR &&
>> > + (dev->quirks & UVC_QUIRK_CTRL_THROTTLE))
>> > + dev->last_ctrl_set_jiffies = jiffies;
>> > if (likely(ret == size))
>> > return 0;
>> >
>> > @@ -108,6 +130,16 @@ int uvc_query_ctrl(struct uvc_device *dev, u8 query, u8 unit,
>> > return ret < 0 ? ret : -EPIPE;
>> > }
>> >
>> > + /*
>> > + * Skip the error code query for devices that crash under load.
>> > + * The standard error-code query (GET_CUR on
>> > + * UVC_VC_REQUEST_ERROR_CODE_CONTROL) sends a second USB transfer to
>> > + * a device that is already stalling, which can amplify the failure
>> > + * into a full firmware lockup and xHCI controller death.
>> > + */
>> > + if (dev->quirks & UVC_QUIRK_CTRL_THROTTLE)
>> > + return -EPIPE;
>> > +
>> > /* Reuse data[0] to request the error code. */
>> > tmp = *(u8 *)data;
>> >
>> > diff --git a/drivers/media/usb/uvc/uvcvideo.h b/drivers/media/usb/uvc/uvcvideo.h
>> > index 8480d65ec..cafc71457 100644
>> > --- a/drivers/media/usb/uvc/uvcvideo.h
>> > +++ b/drivers/media/usb/uvc/uvcvideo.h
>> > @@ -81,6 +81,7 @@
>> > #define UVC_QUIRK_INVALID_DEVICE_SOF 0x00010000
>> > #define UVC_QUIRK_MJPEG_NO_EOF 0x00020000
>> > #define UVC_QUIRK_MSXU_META 0x00040000
>> > +#define UVC_QUIRK_CTRL_THROTTLE 0x00080000
>> >
>> > /* Format flags */
>> > #define UVC_FMT_FLAG_COMPRESSED 0x00000001
>> > @@ -579,6 +580,8 @@ struct uvc_device {
>> > struct usb_interface *intf;
>> > unsigned long warnings;
>> > u32 quirks;
>> > + /* Control transfer throttling (UVC_QUIRK_CTRL_THROTTLE) */
>> > + unsigned long last_ctrl_set_jiffies;
>> > int intfnum;
>> > char name[32];
>> >
>> > --
>> > 2.43.0
>> >
>>
>>
>> --
>> Ricardo Ribalda