linux-kernel - Re: [PATCH 1/2] usb: xhci: Fix a format bug

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] [day] [month] [year] [list]

Message-ID: <20251031185413.2eeefa2b.michal.pecio@gmail.com>
Date: Fri, 31 Oct 2025 18:54:13 +0100
From: Michal Pecio <michal.pecio@...il.com>
To: Mathias Nyman <mathias.nyman@...ux.intel.com>
Cc: Mathias Nyman <mathias.nyman@...el.com>, Greg Kroah-Hartman
 <gregkh@...uxfoundation.org>, linux-usb@...r.kernel.org,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] usb: xhci: Fix a format bug

On Fri, 31 Oct 2025 17:00:42 +0200, Mathias Nyman wrote:
> On 10/16/25 19:28, Michal Pecio wrote:
> > The width of 'addr' depends on kernel configuration and gibberish is
> > printed in traces and dynamic debug on some 32 bit systems like ARM:
> > 
> >    Removing canceled TD starting at 0xf9c96eb0 (dma) in stream 0 URB 54e247b5
> >    Set TR Deq ptr 0x205400000000000, cycle 0
> > 
> >    Successful Set TR Deq Ptr cmd, deq = @f9c96ef0
> > 
> > Fix it by casting to 64 bits. No effect on unaffected systems.
> > Remove the newline which casuses an empty line to appear next.
> > 
> > Fixes: d1dbfb942c33 ("xhci: introduce a new move_dequeue_past_td() function to replace old code.")
> > Cc: stable@...r.kernel.org
> > Signed-off-by: Michal Pecio <michal.pecio@...il.com>
> > ---
> >   drivers/usb/host/xhci-ring.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
> > index c7f658d446cd..6d799a5a062d 100644
> > --- a/drivers/usb/host/xhci-ring.c
> > +++ b/drivers/usb/host/xhci-ring.c
> > @@ -776,7 +776,7 @@ static int xhci_move_dequeue_past_td(struct xhci_hcd *xhci,
> >   	ep->queued_deq_ptr = new_deq;
> >   
> >   	xhci_dbg_trace(xhci, trace_xhci_dbg_cancel_urb,
> > -		       "Set TR Deq ptr 0x%llx, cycle %u\n", addr, new_cycle);
> > +		       "Set TR Deq ptr 0x%llx, cycle %u", (u64) addr, new_cycle);  
> 
> Why not %pad and &addr instead?

I thought this would be worth fixing in stable and %pad annoyingly
doesn't support precision modifiers. So using it would be
- functional change (implicit %.16 padding) on non-broken 64 bit systems
- difference from other related formats which I am not updating here

I can do a v2, though I think such change would make more sense as
a separate non-stable commit updating more of related messages.

And I admit that I am not a fan of %pad in general, it's a hack which
defeats compiler type checks and allows passing any invalid type as
long as it's passed by reference. Probably made sense when review was
the only means of catching format bugs, but not sure today.

Patch 2/2 makes such bugs a build warning on affected systems. With
%pad, &addr you can change addr to u32, u64 or 'struct xhci_hcd' and
it will build cleanly on every platform, then print garbage.

Regards,
Michal