usb: xhci: handle_cmd_completion() walks an empty cmd_list -- the only guard is a struct-layout coincidence (hardening)
From: 徐晗翔
Date: Fri Oct 30 2026 - 07:42:58 EST
handle_cmd_completion() takes the head of xhci->cmd_list with a bare
list_first_entry() and no empty-list check (xhci-ring.c:1845). Several
legal paths empty that list while command-completion events are still
sitting in the event ring (command timeout during removal, abort
without a stop event, xhci_hc_died), and the event-handling loop does
not re-check -- it can even drop and re-acquire xhci->lock mid-loop
(handle_port_event around usb_hcd_poll_rh_status). When a stale
completion event is then processed, list_first_entry() fabricates a
"command" pointer 40 bytes before the list head, i.e. inside struct
xhci_hcd itself, and the handler dereferences it. Today that is
harmless by a genuine coincidence of struct layout, which I verified
field-by-field on the running kernel: the fabricated pointer's
command_trb field lands exactly on xhci->cmd_ring_state (+4 bytes of
padding), so it always reads a small integer (1/2/4) that can never
equal the dequeue pointer, and the existing mismatch check at :1849
returns. But nothing documents or asserts this; any future field
addition or reorder in xhci_hcd or xhci_command silently removes the
shield, and the first stops after that are list_del() poisoning the
real cmd_list head and complete() called on the cmd_ring pointer.
This is a hardening report: please make the guard explicit.
The detailed description:
* How the list can be empty while events remain (Linux 7.3)
- The emptiers (all under xhci->lock, completing and kfreeing every
pending command): the timeout+REMOVING branch
(xhci-ring.c:1787-1792 -- XHCI_STATE_REMOVING is set by
xhci-pci.c:730 before usb_remove_hcd), the abort path without a
stop event (:542-544), xhci_hc_died() (:1389-1402), and
xhci_mem_cleanup() (xhci-mem.c:1949).
- The consumer does not re-check: the event loop tests DYING/HALTED
once at entry (xhci-ring.c:3107-3115) and then processes events
in a while loop that can drop the lock (:2173-2176).
- The DMA match check cannot reject these: the emptiers do not
advance the dequeue pointer, and the abort flow has the hardware
emit completions (including no-op forms, :455-489) that match the
dequeue TRB, so :1833-1843 passes.
* Why it is harmless today -- and only today
struct xhci_command (xhci.h:537-551): command_trb at offset 32,
cmd_list at 40, completion at 24. An empty list makes
list_first_entry() return &xhci->cmd_list - 40. In struct xhci_hcd
(xhci.h:1536-1548) the fields land as:
garbage->command_trb -> xhci_hcd+152 == cmd_ring_state (+pad):
always 1/2/4 (RUNNING/ABORTED/STOPPED),
never a valid TRB address, so the
:1849 mismatch check returns. This is
the ONLY thing standing guard.
garbage->completion -> xhci_hcd+144 == the cmd_ring pointer:
complete() would write through it.
garbage->cmd_list -> the cmd_list head itself: list_del()
would poison and self-link the head,
crashing the next queue_command.
I verified every offset at runtime (module attached): xbits.c
prints offsetof() for both structs from a module built against the
same tree/ABI, e.g. cmd_ring_state off=152, cmd_list off=160,
command_trb off=32 -- the aliases above are exact, including the
small-integer property of cmd_ring_state. (One audit-stage typo --
the landing of garbage->status -- was corrected by the same
measurement: it is xhci_hcd+128, not the interrupters pointer;
status is only reached after a breakthrough and carries no
primitive, so the conclusion is unchanged.)
* What is being asked
Not a vulnerability claim -- the current layout blocks it
deterministically. The ask is to stop depending on the layout:
if (list_empty(&xhci->cmd_list)) {
xhci_warn(xhci, "command completion with empty list\n");
return;
}
cmd = list_first_entry(&xhci->cmd_list, ...);
placed after the existing DMA-mismatch check in
handle_cmd_completion(). (Alternatively a
lockdep/WARN-style assert documenting the invariant.)
* Attachment
xbits.c -- the layout printer used for the runtime verification
(insmod it on any xhci system; it prints the offsets and the two
alias conclusions). Full serial output of the run:
see the attached evidence excerpt.
Upstream status: torvalds master xhci-ring.c:1852 has the same bare
list_first_entry; unfixed.
Regards,
Hanxiang Xu
xuhx@xxxxxxxxxxx
// SPDX-License-Identifier: GPL-2.0
/*
* xbits.c - BUG-3 layout verification: compile-time offsets of the exact
* structures from the audited tree, built against the exact verification
* kernel configuration.
*
* Audit case: drivers/usb/host manual review 2026-10-29, BUG-3.
*
* When cmd_list is empty, list_first_entry() yields
* garbage = &xhci->cmd_list - offsetof(struct xhci_command, cmd_list)
* and every garbage->field access lands at a fixed offset relative to
* &xhci->cmd_list inside struct xhci_hcd. This module prints the full
* offset map so the audit's "which real xhci_hcd field does each garbage
* field alias" table can be checked against the binary truth:
*
* garbage->command_trb @ &cmd_list - 8 (must alias cmd_ring_state+pad)
* garbage->completion @ &cmd_list - 16 (must alias cmd_ring)
* garbage->status @ &cmd_list - 32 (must alias interrupters)
*/
#define pr_fmt(fmt) "XBITS: " fmt
#include <linux/module.h>
#include <linux/init.h>
#include <linux/kernel.h>
#include <linux/stddef.h>
#include "../../linux/drivers/usb/host/xhci.h"
static int __init xbits_init(void)
{
pr_info("struct xhci_command (size=%zu):\n", sizeof(struct xhci_command));
pr_info(" in_ctx off=%zu\n", offsetof(struct xhci_command, in_ctx));
pr_info(" status off=%zu\n", offsetof(struct xhci_command, status));
pr_info(" comp_param off=%zu\n", offsetof(struct xhci_command, comp_param));
pr_info(" slot_id off=%zu\n", offsetof(struct xhci_command, slot_id));
pr_info(" completion off=%zu\n", offsetof(struct xhci_command, completion));
pr_info(" command_trb off=%zu\n", offsetof(struct xhci_command, command_trb));
pr_info(" cmd_list off=%zu\n", offsetof(struct xhci_command, cmd_list));
pr_info(" timeout_ms off=%zu\n", offsetof(struct xhci_command, timeout_ms));
pr_info("struct xhci_hcd (size=%zu) around cmd_list:\n",
sizeof(struct xhci_hcd));
pr_info(" interrupters off=%zu\n", offsetof(struct xhci_hcd, interrupters));
pr_info(" cmd_ring off=%zu\n", offsetof(struct xhci_hcd, cmd_ring));
pr_info(" cmd_ring_state off=%zu\n", offsetof(struct xhci_hcd, cmd_ring_state));
pr_info(" cmd_list off=%zu\n", offsetof(struct xhci_hcd, cmd_list));
pr_info(" cmd_ring_reserved_trbs off=%zu\n",
offsetof(struct xhci_hcd, cmd_ring_reserved_trbs));
pr_info(" current_cmd off=%zu\n", offsetof(struct xhci_hcd, current_cmd));
pr_info(" lock off=%zu\n", offsetof(struct xhci_hcd, lock));
pr_info(" mutex off=%zu\n", offsetof(struct xhci_hcd, mutex));
/* the BUG-3 aliasing table, computed from the real offsets */
{
long cl = offsetof(struct xhci_hcd, cmd_list);
long g_trb = cl - offsetof(struct xhci_command, cmd_list)
+ offsetof(struct xhci_command, command_trb);
long g_cmp = cl - offsetof(struct xhci_command, cmd_list)
+ offsetof(struct xhci_command, completion);
long g_st = cl - offsetof(struct xhci_command, cmd_list)
+ offsetof(struct xhci_command, status);
pr_info("ALIAS: garbage->command_trb sits at xhci_hcd+%ld (cmd_ring_state at +%zu => %s)\n",
g_trb, offsetof(struct xhci_hcd, cmd_ring_state),
g_trb == (long)offsetof(struct xhci_hcd, cmd_ring_state)
? "CONFIRMED small-int field" : "NOT the small-int field");
pr_info("ALIAS: garbage->completion sits at xhci_hcd+%ld (cmd_ring at +%zu => %s)\n",
g_cmp, offsetof(struct xhci_hcd, cmd_ring),
g_cmp == (long)offsetof(struct xhci_hcd, cmd_ring)
? "CONFIRMED cmd_ring pointer" : "NOT cmd_ring");
pr_info("ALIAS: garbage->status sits at xhci_hcd+%ld (interrupters at +%zu => %s)\n",
g_st, offsetof(struct xhci_hcd, interrupters),
g_st == (long)offsetof(struct xhci_hcd, interrupters)
? "CONFIRMED interrupters pointer" : "NOT interrupters");
}
return 0;
}
static void __exit xbits_exit(void) { }
module_init(xbits_init);
module_exit(xbits_exit);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("BUG-3 struct layout offset verification");