Skip to contentSkip to conversation

xhci: check device notification type before forwarding wake event

PATCH In review [email protected] 2 participants 2 messages Sep 29 – Sep 30

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/1 xhci: check device notification type before forwarding wake event

From: Hongyu Xie <[email protected]>

The xHCI driver programs the Device Notification Control register to only enable the Function Wake device notification (N1), so any Device Notification Event TRB received is expected to be a function wake notification. handle_device_notification() does however not check the Notification Type field of the event (xHCI 1.2 section 6.4.2.7, DW0 bits 7:4), and forwards every device notification event as a function wake.

A host controller that delivers an unexpected notification type (e.g. due to broken firmware or emulation) would trigger a spurious wake notification on the parent hub.

Parse the notification type and drop events other than Function Wake with a warning, mirroring the slot ID validation in the same function.

DEV_NOTE_FWAKE is the DNCTRL register bit for notification type 1 (N1), while the event TRB carries the notification type value itself, so add a separate DEV_NOTE_TYPE_FWAKE constant for the comparison.

---
 drivers/usb/host/xhci-ring.c | 9 +++++++++
 drivers/usb/host/xhci.h      | 5 +++++
 2 files changed, 14 insertions(+)
drivers/usb/host/xhci-ring.c +9 -0
index ec278a9f9540..af5d93a4e586 100644
@@ -1952,6 +1952,7 @@ static void handle_device_notification(struct xhci_hcd *xhci,
 		union xhci_trb *event)
 {
 	u32 slot_id;
+	u32 type;
 	struct usb_device *udev;
 
 	slot_id = TRB_TO_SLOT_ID(le32_to_cpu(event->generic.field[3]));
@@ -1961,6 +1962,14 @@ static void handle_device_notification(struct xhci_hcd *xhci,
 		return;
 	}
 
+	/* xHCI 1.2 6.4.2.7: Notification Type is DW0 bits 7:4 */
+	type = TRB_TO_DEV_NOTE_TYPE(le32_to_cpu(event->generic.field[0]));
+	if (type != DEV_NOTE_TYPE_FWAKE) {
+		xhci_warn(xhci, "Unsupported device notification type %u for slot ID %u\n",
+				type, slot_id);
+		return;
+	}
+
 	xhci_dbg(xhci, "Device Wake Notification event for slot ID %u\n",
 			slot_id);
 	udev = xhci->devs[slot_id]->udev;
drivers/usb/host/xhci.h +5 -0
index c7bfa7f028d3..ec4bfeb4887c 100644
@@ -187,6 +187,8 @@ struct xhci_op_regs {
  * SW does need to pay attention to function wake notifications.
  */
 #define	DEV_NOTE_FWAKE		BIT(1)
+/* Notification Type value carried by a Device Notification Event TRB (6.4.2.7) */
+#define	DEV_NOTE_TYPE_FWAKE	1
 
 /* CRCR - Command Ring Control Register - cmd_ring bitmasks */
 /* bit 0 - Cycle bit indicates the ownership of the command ring */
@@ -996,6 +998,9 @@ enum xhci_ep_reset_type {
 #define TRB_TO_PACKET_TYPE(p)		((p) & 0x1f)
 #define TRB_TO_ROOTHUB_PORT(p)		(((p) & (0xff << 24)) >> 24)
 
+/* Device Notification Event TRB fields, 6.4.2.7 */
+#define TRB_TO_DEV_NOTE_TYPE(p)		(((p) & (0xf << 4)) >> 4)
+
 enum xhci_setup_dev {
 	SETUP_CONTEXT_ONLY,
 	SETUP_CONTEXT_ADDRESS,
Signature · 1 line
2.32.0
#1 lore raw
Quoted 14 lines · On 9/29/26 14:28, [email protected] wrote:

From: Hongyu Xie <[email protected]>

The xHCI driver programs the Device Notification Control register to only enable the Function Wake device notification (N1), so any Device Notification Event TRB received is expected to be a function wake notification. handle_device_notification() does however not check the Notification Type field of the event (xHCI 1.2 section 6.4.2.7, DW0 bits 7:4), and forwards every device notification event as a function wake.

A host controller that delivers an unexpected notification type (e.g. due to broken firmware or emulation) would trigger a spurious wake notification on the parent hub.

Quoted 8 lines

Parse the notification type and drop events other than Function Wake with a warning, mirroring the slot ID validation in the same function.

DEV_NOTE_FWAKE is the DNCTRL register bit for notification type 1 (N1), while the event TRB carries the notification type value itself, so add a separate DEV_NOTE_TYPE_FWAKE constant for the comparison.

Added, but changed warning to a debug message

Thanks
Mathias