From nobody Sat Feb 7 17:19:47 2026 Received: from mail-pl1-f201.google.com (mail-pl1-f201.google.com [209.85.214.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EECD02EC096 for ; Fri, 30 Jan 2026 07:48:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769759333; cv=none; b=ksFvgZ90ayQMUH1jAtznrH3S0VlRe+lcR6qaINtj8M9AQYi+OZP5sFWLbUE3eKXfa0juEyhhVfzSrNtf/UZLlKsBqNOReDy7Z9W2RQfKDt30K8qf1NeUzhrH9JXGg7sRsH+FH2TOaY8NjkTm9UnNsdSUOyj1z1ngRwAxMCtm4xI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1769759333; c=relaxed/simple; bh=p4RI/PJNo9P2zoz4qNZVHYn11CSHjlAh2ePSn95HglE=; h=Date:Mime-Version:Message-ID:Subject:From:To:Cc:Content-Type; b=ldADjJfyWoJuom19KYlPIoLIvtKNTFPCP0htczkZS10MKY6yXe2ZiM9DFeHopVpBZmNy4nNziAlYzxc/VCUmLnQejU5/YHXgACdn5sM4AGzKhYMztWAuk7ef2oasmXx8MUP1tSejuqIRFfKf+luU9tQ2ZKPqg4vM6oIk6MPzEkM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=flex--guanyulin.bounces.google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=CIMFr94a; arc=none smtp.client-ip=209.85.214.201 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--guanyulin.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="CIMFr94a" Received: by mail-pl1-f201.google.com with SMTP id d9443c01a7336-2a7701b6328so43787575ad.2 for ; Thu, 29 Jan 2026 23:48:50 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1769759330; x=1770364130; darn=vger.kernel.org; h=cc:to:from:subject:message-id:mime-version:date:from:to:cc:subject :date:message-id:reply-to; bh=ubwoGcMvFrz/YoKawnL1KLVWp0+NDz5W2QGY4b7pS8w=; b=CIMFr94a38nYU0TPcZkC1HtZEz7iqDL405VIpITH9+GpTvg6P/NO7lC46HnKw40zzb HN7tbrQkH4WvKBUc0MgaT4cEtAEpiG6UKG9asJjz6ixyW2AhL7TmzoZ/88tQmcEVOTzF YYRZ1V6wjpCmG8mkVtzo7YGUIxk17497LSaOSYbgUiK0Jeusd74qrRXIfrL5Uzg2oozD ADzLccvqhQcnYRPlSM2NoBqUtQUM9dfhZDSL9okli0MwCnNtfu1niULYl7Q0pXY915Hl vW8yKlswl8Ay1FLr7NX3pI47vQA0UTmR6GSgJu8ubrR8flinkZYr8Pz9UfTe9LWBYmwA OcJA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1769759330; x=1770364130; h=cc:to:from:subject:message-id:mime-version:date:x-gm-message-state :from:to:cc:subject:date:message-id:reply-to; bh=ubwoGcMvFrz/YoKawnL1KLVWp0+NDz5W2QGY4b7pS8w=; b=uMlv4y6V+r3NnR/c+fbTv2i+kqnTtLk0w9hfyHt/5zsM9KOtZ0fDrf2ED4Qo+R3ypE Xzghwh7vx1xIOt7J0mlG78iKrxtL0x4yuOdmmMOLDhXN4GlydZdcSfK3NGUyoXXSJOOR 3Zu4Hdcg1DghL2sOxhl2yKP4XDGDB9/0Cnrdb3TnK7nbMaJnm/SLXPZ7Cda70wGGGa6d cz/k5UZWpEDYRiVs46LKkQxemOrIuLxih9nCC0kxq136iXH7F9zMhEi9DeG9LpHZYGx5 Mir57KekiAWXK96eahASxEOonfe9eYRbsicUXMEO2gQOR0FZmm+nrLh+/LeA1mTziEJ/ h4lA== X-Forwarded-Encrypted: i=1; AJvYcCUJ0aORkw/jOsx2HxSkAA4IRTZQaVxsvRXDCYgvMF8KP5OZLDX3s4YCoJ7dNlxv85lMYclzEGSZyWRQDok=@vger.kernel.org X-Gm-Message-State: AOJu0Yzz9k2yMb5x+RrP64DCqvF7srmHe+f1eLRr55GFEIgoUz61hdgj +stzM6zST56HXMEx5ZSpm2TxAjwdMhncLF4aqTrdzUgMjI6zwioSLmkjguWl7tN291SQGJhvA4h Zu60KLNs7QbVRcqGk6Q== X-Received: from plsl6.prod.google.com ([2002:a17:903:2446:b0:2a3:1bf9:d25]) (user=guanyulin job=prod-delivery.src-stubby-dispatcher) by 2002:a17:903:2d0:b0:2a7:cf36:819 with SMTP id d9443c01a7336-2a8d9a5edc8mr22929475ad.44.1769759330394; Thu, 29 Jan 2026 23:48:50 -0800 (PST) Date: Fri, 30 Jan 2026 07:47:46 +0000 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 X-Mailer: git-send-email 2.53.0.rc1.225.gd81095ad13-goog Message-ID: <20260130074746.287750-1-guanyulin@google.com> Subject: [RFC PATCH] usb: host: xhci-sideband: fix deadlock in unregister path From: Guan-Yu Lin To: gregkh@linuxfoundation.org, mathias.nyman@intel.com, stern@rowland.harvard.edu, wesley.cheng@oss.qualcomm.com Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org, Guan-Yu Lin , stable@vger.kernel.org Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" When a USB device is disconnected or a driver is unbound, the USB core invokes the driver's disconnect callback while holding the udev device lock. If the driver calls xhci_sideband_unregister(), it eventually reaches usb_offload_put(), which attempts to acquire the same udev lock, resulting in a self-deadlock. Introduce lockless variants __usb_offload_get() and __usb_offload_put() to allow modifying the offload usage count when the device lock is already held. These helpers use device_lock_assert() to ensure callers meet the locking requirements. Update the xHCI sideband implementation to use these lockless variants in the unregister and interrupter removal paths. For public sideband APIs that can be called from other contexts, wrap the calls with explicit udev locking to maintain synchronization. Cc: stable@vger.kernel.org Fixes: ef82a4803aab ("xhci: sideband: add api to trace sideband usage") Signed-off-by: Guan-Yu Lin --- Hi Maintainers, In the current implementation, xhci_sideband_unregister() is only invoked when a driver manages a USB interface via snd_usb_platform_ops callbacks (.connect_cb and .disconnect_cb). In these contexts, the parent USB device lock is already held by the USB core. However, the original design of usb_offload_put() attempts to re-acquire the same USB device lock, leading to a recursive locking scenario. This patch resolves the issue by: 1. Refactoring Lock Ownership: Shifting lock acquisition responsibility to the upper-level USB driver. This ensures usb_offload_put() can be safely called from contexts where the lock is already held. 2. Standardizing the Lock Hierarchy: To prevent potential ABBA deadlocks, the locking sequence is unified across the driver. The functions xhci_sideband_create_interrupter() and xhci_sideband_remove_interrupter() have been updated to strictly follow the same hierarchy. --- drivers/usb/core/offload.c | 92 +++++++++++++++++++++----------- drivers/usb/host/xhci-sideband.c | 48 +++++++++++------ include/linux/usb.h | 6 +++ 3 files changed, 100 insertions(+), 46 deletions(-) diff --git a/drivers/usb/core/offload.c b/drivers/usb/core/offload.c index 7c699f1b8d2b..bc5f337fb355 100644 --- a/drivers/usb/core/offload.c +++ b/drivers/usb/core/offload.c @@ -13,44 +13,59 @@ #include "usb.h" =20 /** - * usb_offload_get - increment the offload_usage of a USB device + * __usb_offload_get - increment the offload_usage of a USB device * @udev: the USB device to increment its offload_usage * - * Incrementing the offload_usage of a usb_device indicates that offload is - * enabled on this usb_device; that is, another entity is actively handlin= g USB - * transfers. This information allows the USB driver to adjust its power - * management policy based on offload activity. + * This is the lockless version of usb_offload_get. The caller must hold + * @udev's device lock. * * Return: 0 on success. A negative error code otherwise. */ -int usb_offload_get(struct usb_device *udev) +int __usb_offload_get(struct usb_device *udev) { int ret; =20 - usb_lock_device(udev); - if (udev->state =3D=3D USB_STATE_NOTATTACHED) { - usb_unlock_device(udev); + device_lock_assert(&udev->dev); + + if (udev->state =3D=3D USB_STATE_NOTATTACHED) return -ENODEV; - } =20 if (udev->state =3D=3D USB_STATE_SUSPENDED || - udev->offload_at_suspend) { - usb_unlock_device(udev); + udev->offload_at_suspend) return -EBUSY; - } =20 /* * offload_usage could only be modified when the device is active, since * it will alter the suspend flow of the device. */ ret =3D usb_autoresume_device(udev); - if (ret < 0) { - usb_unlock_device(udev); + if (ret < 0) return ret; - } =20 udev->offload_usage++; usb_autosuspend_device(udev); + + return ret; +} +EXPORT_SYMBOL_GPL(__usb_offload_get); + +/** + * usb_offload_get - increment the offload_usage of a USB device + * @udev: the USB device to increment its offload_usage + * + * Incrementing the offload_usage of a usb_device indicates that offload is + * enabled on this usb_device; that is, another entity is actively handlin= g USB + * transfers. This information allows the USB driver to adjust its power + * management policy based on offload activity. + * + * Return: 0 on success. A negative error code otherwise. + */ +int usb_offload_get(struct usb_device *udev) +{ + int ret; + + usb_lock_device(udev); + ret =3D __usb_offload_get(udev); usb_unlock_device(udev); =20 return ret; @@ -58,45 +73,60 @@ int usb_offload_get(struct usb_device *udev) EXPORT_SYMBOL_GPL(usb_offload_get); =20 /** - * usb_offload_put - drop the offload_usage of a USB device + * __usb_offload_put - drop the offload_usage of a USB device * @udev: the USB device to drop its offload_usage * - * The inverse operation of usb_offload_get, which drops the offload_usage= of - * a USB device. This information allows the USB driver to adjust its power - * management policy based on offload activity. + * This is the lockless version of usb_offload_put. The caller must hold + * @udev's device lock. * * Return: 0 on success. A negative error code otherwise. */ -int usb_offload_put(struct usb_device *udev) +int __usb_offload_put(struct usb_device *udev) { int ret; =20 - usb_lock_device(udev); - if (udev->state =3D=3D USB_STATE_NOTATTACHED) { - usb_unlock_device(udev); + device_lock_assert(&udev->dev); + + if (udev->state =3D=3D USB_STATE_NOTATTACHED) return -ENODEV; - } =20 if (udev->state =3D=3D USB_STATE_SUSPENDED || - udev->offload_at_suspend) { - usb_unlock_device(udev); + udev->offload_at_suspend) return -EBUSY; - } =20 /* * offload_usage could only be modified when the device is active, since * it will alter the suspend flow of the device. */ ret =3D usb_autoresume_device(udev); - if (ret < 0) { - usb_unlock_device(udev); + if (ret < 0) return ret; - } =20 /* Drop the count when it wasn't 0, ignore the operation otherwise. */ if (udev->offload_usage) udev->offload_usage--; usb_autosuspend_device(udev); + + return ret; +} +EXPORT_SYMBOL_GPL(__usb_offload_put); + +/** + * usb_offload_put - drop the offload_usage of a USB device + * @udev: the USB device to drop its offload_usage + * + * The inverse operation of usb_offload_get, which drops the offload_usage= of + * a USB device. This information allows the USB driver to adjust its power + * management policy based on offload activity. + * + * Return: 0 on success. A negative error code otherwise. + */ +int usb_offload_put(struct usb_device *udev) +{ + int ret; + + usb_lock_device(udev); + ret =3D __usb_offload_put(udev); usb_unlock_device(udev); =20 return ret; diff --git a/drivers/usb/host/xhci-sideband.c b/drivers/usb/host/xhci-sideb= and.c index 2bd77255032b..c37c3f3adf4a 100644 --- a/drivers/usb/host/xhci-sideband.c +++ b/drivers/usb/host/xhci-sideband.c @@ -89,7 +89,7 @@ __xhci_sideband_remove_endpoint(struct xhci_sideband *sb,= struct xhci_virt_ep *e sb->eps[ep->ep_index] =3D NULL; } =20 -/* Caller must hold sb->mutex */ +/* Caller must hold sb->mutex and udev device lock */ static void __xhci_sideband_remove_interrupter(struct xhci_sideband *sb) { @@ -102,10 +102,10 @@ __xhci_sideband_remove_interrupter(struct xhci_sideba= nd *sb) =20 xhci_remove_secondary_interrupter(xhci_to_hcd(sb->xhci), sb->ir); sb->ir =3D NULL; - udev =3D sb->vdev->udev; =20 - if (udev->state !=3D USB_STATE_NOTATTACHED) - usb_offload_put(udev); + udev =3D interface_to_usbdev(sb->intf); + device_lock_assert(&udev->dev); + __usb_offload_put(udev); } =20 /* sideband api functions */ @@ -334,25 +334,36 @@ xhci_sideband_create_interrupter(struct xhci_sideband= *sb, int num_seg, if (!sb || !sb->xhci) return -ENODEV; =20 - guard(mutex)(&sb->mutex); + udev =3D interface_to_usbdev(sb->intf); + usb_lock_device(udev); + mutex_lock(&sb->mutex); =20 - if (!sb->vdev) - return -ENODEV; + if (!sb->vdev) { + ret =3D -ENODEV; + goto unlock; + } =20 - if (sb->ir) - return -EBUSY; + if (sb->ir) { + ret =3D -EBUSY; + goto unlock; + } =20 sb->ir =3D xhci_create_secondary_interrupter(xhci_to_hcd(sb->xhci), num_seg, imod_interval, intr_num); - if (!sb->ir) - return -ENOMEM; + if (!sb->ir) { + ret =3D -ENOMEM; + goto unlock; + } =20 - udev =3D sb->vdev->udev; - ret =3D usb_offload_get(udev); + ret =3D __usb_offload_get(udev); =20 sb->ir->ip_autoclear =3D ip_autoclear; =20 +unlock: + mutex_unlock(&sb->mutex); + usb_unlock_device(udev); + return ret; } EXPORT_SYMBOL_GPL(xhci_sideband_create_interrupter); @@ -367,12 +378,19 @@ EXPORT_SYMBOL_GPL(xhci_sideband_create_interrupter); void xhci_sideband_remove_interrupter(struct xhci_sideband *sb) { - if (!sb) + struct usb_device *udev; + + if (!sb || !sb->vdev) return; =20 - guard(mutex)(&sb->mutex); + udev =3D interface_to_usbdev(sb->intf); + usb_lock_device(udev); + mutex_lock(&sb->mutex); =20 __xhci_sideband_remove_interrupter(sb); + + mutex_unlock(&sb->mutex); + usb_unlock_device(udev); } EXPORT_SYMBOL_GPL(xhci_sideband_remove_interrupter); =20 diff --git a/include/linux/usb.h b/include/linux/usb.h index fbfcc70b07fb..19c84c05f376 100644 --- a/include/linux/usb.h +++ b/include/linux/usb.h @@ -846,11 +846,17 @@ static inline void usb_mark_last_busy(struct usb_devi= ce *udev) #endif =20 #if IS_ENABLED(CONFIG_USB_XHCI_SIDEBAND) +int __usb_offload_get(struct usb_device *udev); +int __usb_offload_put(struct usb_device *udev); int usb_offload_get(struct usb_device *udev); int usb_offload_put(struct usb_device *udev); bool usb_offload_check(struct usb_device *udev); #else =20 +static inline int __usb_offload_get(struct usb_device *udev) +{ return 0; } +static inline int __usb_offload_put(struct usb_device *udev) +{ return 0; } static inline int usb_offload_get(struct usb_device *udev) { return 0; } static inline int usb_offload_put(struct usb_device *udev) --=20 2.53.0.rc1.225.gd81095ad13-goog