| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20250408121817.6ae8defd@foxbook>
Date: Tue, 8 Apr 2025 12:18:17 +0200
From: Michał Pecio <michal.pecio@...il.com>
To: Alan Stern <stern@...land.harvard.edu>, Mathias Nyman
<mathias.nyman@...el.com>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Paul Menzel <pmenzel@...gen.mpg.de>, Mathias Nyman
<mathias.nyman@...ux.intel.com>, linux-usb@...r.kernel.org, LKML
<linux-kernel@...r.kernel.org>
Subject: [PATCH RFC RFT] usb: hcd: Add a usb_device argument to
hc_driver.endpoint_reset()
xHCI needs usb_device in this callback so it employed some hacks that
proved unreliable in the long term and made the code a little tricky.
Make USB core supply it directly and simplify xhci_endpoint_reset().
Use xhci_check_args() to prevent resetting emulated endpoints of root
hubs and to deduplicate argument validation and improve debuggability.
Update ehci_endpoint_reset(), which is the only other such callback,
to accept (and ignore) the new argument.
This fixes the root cause of a 6.15-rc1 regression reported by Paul,
which I was able to reproduce locally. It also solves the general
problem of xhci_endpoint_reset() becoming a no-op after device reset
or changing configuration or altsetting. Although nobody complained
because halted endpoints are reset automatically by xhci_hcd, it was
a bug - sometimes class drivers want to reset not halted endpoints.
Reported-by: Paul Menzel <pmenzel@...gen.mpg.de>
Closes: https://lore.kernel.org/linux-usb/c279bd85-3069-4841-b1be-20507ac9f2d7@molgen.mpg.de/
Signed-off-by: Michal Pecio <michal.pecio@...il.com>
---
Is such change acceptable to interested parties?
It solves the problem completely for me, because as Alan said,
core calls endpoint_reset() after installing a new config or alt
to notify HCDs that ignore reset_device(), and also those which
implement it incompletely, I guess ;)
Unlike clearing EP_STALLED on reset_device() or drop_endpoint(),
this also fixes cases when another STALL happens after device
reset and the device is not reset again. For example, I see that
when I insert a card after the original problem happens.
At this point I can insert or remove the card, plug or unplug
the reader and reload ums-realtek in any order, it all works.
Paul, could you check if this patch works on your hardware too?
drivers/usb/core/hcd.c | 2 +-
drivers/usb/host/ehci-hcd.c | 3 ++-
drivers/usb/host/xhci.c | 27 ++++++++-------------------
include/linux/usb/hcd.h | 2 +-
4 files changed, 12 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
index a63c793bac21..d2433807a397 100644
--- a/drivers/usb/core/hcd.c
+++ b/drivers/usb/core/hcd.c
@@ -1986,7 +1986,7 @@ void usb_hcd_reset_endpoint(struct usb_device *udev,
struct usb_hcd *hcd = bus_to_hcd(udev->bus);
if (hcd->driver->endpoint_reset)
- hcd->driver->endpoint_reset(hcd, ep);
+ hcd->driver->endpoint_reset(hcd, udev, ep);
else {
int epnum = usb_endpoint_num(&ep->desc);
int is_out = usb_endpoint_dir_out(&ep->desc);
diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 6d1d190c914d..813cdedb14ab 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1044,7 +1044,8 @@ ehci_endpoint_disable (struct usb_hcd *hcd, struct usb_host_endpoint *ep)
}
static void
-ehci_endpoint_reset(struct usb_hcd *hcd, struct usb_host_endpoint *ep)
+ehci_endpoint_reset(struct usb_hcd *hcd, struct usb_device *udev,
+ struct usb_host_endpoint *ep)
{
struct ehci_hcd *ehci = hcd_to_ehci(hcd);
struct ehci_qh *qh;
diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 0452b8d65832..5bf89ba7e2b8 100644
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3161,11 +3161,10 @@ static void xhci_endpoint_disable(struct usb_hcd *hcd,
* resume. A new vdev will be allocated later by xhci_discover_or_reset_device()
*/
-static void xhci_endpoint_reset(struct usb_hcd *hcd,
+static void xhci_endpoint_reset(struct usb_hcd *hcd, struct usb_device *udev,
struct usb_host_endpoint *host_ep)
{
struct xhci_hcd *xhci;
- struct usb_device *udev;
struct xhci_virt_device *vdev;
struct xhci_virt_ep *ep;
struct xhci_input_control_ctx *ctrl_ctx;
@@ -3175,7 +3174,12 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
u32 ep_flag;
int err;
+ err = xhci_check_args(hcd, udev, host_ep, 1, true, __func__);
+ if (err <= 0)
+ return;
+
xhci = hcd_to_xhci(hcd);
+ vdev = xhci->devs[udev->slot_id];
ep_index = xhci_get_endpoint_index(&host_ep->desc);
/*
@@ -3185,28 +3189,13 @@ static void xhci_endpoint_reset(struct usb_hcd *hcd,
*/
if (usb_endpoint_xfer_control(&host_ep->desc) && ep_index == 0) {
- udev = container_of(host_ep, struct usb_device, ep0);
- if (udev->speed != USB_SPEED_FULL || !udev->slot_id)
- return;
-
- vdev = xhci->devs[udev->slot_id];
- if (!vdev || vdev->udev != udev)
- return;
-
- xhci_check_ep0_maxpacket(xhci, vdev);
+ if (udev->speed == USB_SPEED_FULL)
+ xhci_check_ep0_maxpacket(xhci, vdev);
/* Nothing else should be done here for ep0 during ep reset */
return;
}
- if (!host_ep->hcpriv)
- return;
- udev = (struct usb_device *) host_ep->hcpriv;
- vdev = xhci->devs[udev->slot_id];
-
- if (!udev->slot_id || !vdev)
- return;
-
ep = &vdev->eps[ep_index];
spin_lock_irqsave(&xhci->lock, flags);
diff --git a/include/linux/usb/hcd.h b/include/linux/usb/hcd.h
index ac95e7c89df5..179c85337eff 100644
--- a/include/linux/usb/hcd.h
+++ b/include/linux/usb/hcd.h
@@ -304,7 +304,7 @@ struct hc_driver {
/* (optional) reset any endpoint state such as sequence number
and current window */
- void (*endpoint_reset)(struct usb_hcd *hcd,
+ void (*endpoint_reset)(struct usb_hcd *hcd, struct usb_device *udev,
struct usb_host_endpoint *ep);
/* root hub support */
--
2.48.1
Powered by blists - more mailing lists