linux-kernel - Re: [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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250910020306.1d77d7e5.michal.pecio@gmail.com>
Date: Wed, 10 Sep 2025 02:03:06 +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: Re: [PATCH 0/1] usb: xhci: Queue URB_ZERO_PACKET as one TD

On Wed, 10 Sep 2025 01:57:39 +0300, Mathias Nyman wrote:
> On 9.9.2025 20.38, Michal Pecio wrote:
> > On Tue, 9 Sep 2025 16:04:33 +0300, Mathias Nyman wrote:  
> >> Adding the zero-length TRB to the original TD when we need to send a
> >> zero-length packet would simplify things, and I would otherwise fully
> >> support this, but the xHCI spec is pretty clear that it requires a
> >> dedicated TD for zero-length transactions.  
> > 
> > You are right of course, an empty TRB in a TD would simply send no
> > data, or maybe it's a TRB Error, I'm not sure.
> > 
> > But this is not what this patch is about - the trick is to use an
> > *unchained* TRB, which is a separate TD from HW's perspective, and
> > to count it as part of the same TD from the driver's perspective.  
> 
> Ok, I see.
> The whole TD without completion flag does worry me a bit.
>
> We need to make sure stop/stald mid TD cases work, and  urb length is
> set correctly.

It looks odd, but I can't find anything wrong.

4.10.4 discusses what happens when IOC is clear on the last TRB of
a TD so it looks like this is allowed.

If the first TD halts or stops before completion then it doesn't
matter that we cleared its IOC. Everything works as before, except
that Set TR Deq will skip both TDs and the URB will be given back.

If the first TD completes, the xHC silently moves to the second TD.

No matter what happens to the second TD, URB length is calculated:

       if (ep_trb == td->end_trb)	/* end_trb is second TD */
                td->urb->actual_length = requested - remaining;

where 'requested' is full URB and 'remaining' is TRB transfer residue
which should be zero because buffer length is zero. Looks OK.

And curiously, this behavior too is no different from what exists now.
I see nothing stopping the second TD from overwriting whatever was set
when the first TD generated its event (success or otherwise).

Regards,
Michal