linux-kernel - [PATCH 0/1] usb: xhci: Queue URB_ZERO

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250908130128.7ed81912.michal.pecio@gmail.com>
Date: Mon, 8 Sep 2025 13:01:28 +0200
From: Michal Pecio <michal.pecio@...il.com>
To: Mathias Nyman <mathias.nyman@...ux.intel.com>
Cc: linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [PATCH 0/1] usb: xhci: Queue URB_ZERO_PACKET as one TD

Hi Mathias,

I wanted to finish and send v2 of "Simplify TD cancellation and drop
some states" over the weekend, but I encountered an annoying roadblock
and I need your input.

Basically, there is a bug: URB_ZERO_PACKET is queued as two TDs, and
when the first TD halts, the driver simply advances to the second one.

I found that extending the event handler to cover this case requires
multiple changes:

1. obviously, all TDs must be cancelled, not just the current one
2. they may be given back in weird order (waiting for Set Deq), so
   we need to store the status on urb_priv and override td->status
3. xhci_invalidate_cancelled_tds() would need to recognize multiple
   halted TDs on the same URB as valid

This is doable, and I have already implemented most of it in that
series, but there is an alternative solution: simply stop worrying
about halted multi-TD URBs, because this is the only case and it
can be converted to pretend that it's just one TD per URB as usual.

If you are OK with this patch, cancellation logic will be simpler,
because this time there really are no remaining cases of multi-TD
URBs except isochronous. This is clear in xhci_urb_enqueue():

        if (usb_endpoint_xfer_isoc(&urb->ep->desc))
                num_tds = urb->number_of_packets;
        else         
                num_tds = 1;

Regards,
Michal