[PATCH REPOST] usb: udc: Make tracing aware that usb_ep_alloc_request() can return NULL

[PATCH REPOST] usb: udc: Make tracing aware that usb_ep_alloc_request() can return NULL

[Sebastian Andrzej Siewior]

The alloc_request callback of the EP can return NULL if it was not able
to allocate a request. This NULL pointer will then be dereferenced by
tracing in order to show details about it.

Let tracing check the req pointer and if NULL just write 0.

Fixes: 5e42d710a108c ("usb: gadget: add tracepoints to the gadget API")
Reported-by: syzbot+de553c19cb054f174a35@syzkaller.appspotmail.com
Closes: https://lore.kernel.org/all/6aab0160.e91c2013.3bdb06.0005.GAE@google.com/
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 drivers/usb/gadget/udc/trace.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/gadget/udc/trace.h b/drivers/usb/gadget/udc/trace.h
index fa3e6ddf0a128..05533eb1bae0c 100644
--- a/drivers/usb/gadget/udc/trace.h
+++ b/drivers/usb/gadget/udc/trace.h
@@ -239,15 +239,15 @@ DECLARE_EVENT_CLASS(udc_log_req,
 	),
 	TP_fast_assign(
 		__assign_str(name);
-		__entry->length = req->length;
-		__entry->actual = req->actual;
-		__entry->num_sgs = req->num_sgs;
-		__entry->num_mapped_sgs = req->num_mapped_sgs;
-		__entry->stream_id = req->stream_id;
-		__entry->no_interrupt = req->no_interrupt;
-		__entry->zero = req->zero;
-		__entry->short_not_ok = req->short_not_ok;
-		__entry->status = req->status;
+		__entry->length = req ? req->length : 0;
+		__entry->actual = req ? req->actual : 0;
+		__entry->num_sgs = req ? req->num_sgs : 0;
+		__entry->num_mapped_sgs = req ? req->num_mapped_sgs : 0;
+		__entry->stream_id = req ? req->stream_id : 0;
+		__entry->no_interrupt = req ? req->no_interrupt : 0;
+		__entry->zero = req ? req->zero : 0;
+		__entry->short_not_ok = req ? req->short_not_ok : 0;
+		__entry->status = req ? req->status : 0;
 		__entry->ret = ret;
 		__entry->req = req;
 	),
-- 
2.55.0

Re: [PATCH REPOST] usb: udc: Make tracing aware that usb_ep_alloc_request() can return NULL

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] usb: use-after-free in usb_ep_queue() and usb_ep_dequeue() tracepoints
--

commit 1373e903abc2252ba5340f5231a6fb6306db9019
Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

usb: udc: Make tracing aware that usb_ep_alloc_request() can return NULL

This commit updates the udc_log_req trace event class to handle NULL
request pointers by checking the pointer before accessing its fields. This
prevents a NULL pointer dereference when usb_ep_alloc_request() fails and
returns NULL.

Link: https://lore.kernel.org/all/6aab0160.e91c2013.3bdb06.0005.GAE@google.com/

> diff --git a/drivers/usb/gadget/udc/trace.h b/drivers/usb/gadget/udc/trace.h
> index fa3e6ddf0a128..05533eb1bae0c 100644
> --- a/drivers/usb/gadget/udc/trace.h
> +++ b/drivers/usb/gadget/udc/trace.h
> @@ -239,15 +239,15 @@ DECLARE_EVENT_CLASS(udc_log_req,
>  	),
>  	TP_fast_assign(
>  		__assign_str(name);
> -		__entry->length = req->length;
> -		__entry->actual = req->actual;
> -		__entry->num_sgs = req->num_sgs;
> -		__entry->num_mapped_sgs = req->num_mapped_sgs;
> -		__entry->stream_id = req->stream_id;
> -		__entry->no_interrupt = req->no_interrupt;
> -		__entry->zero = req->zero;
> -		__entry->short_not_ok = req->short_not_ok;
> -		__entry->status = req->status;
> +		__entry->length = req ? req->length : 0;
> +		__entry->actual = req ? req->actual : 0;

[Severity: High]
While this successfully avoids a dereference when req is NULL, does this
still leave a use-after-free when tracing usb_ep_queue() or
usb_ep_dequeue()?

If a gadget driver submits a request via usb_ep_queue(), the UDC driver can
complete it synchronously and return ownership to the gadget driver. For
instance, in dummy_queue():

    usb_gadget_giveback_request(_ep, _req);

If the gadget driver's completion handler then frees the request, control
eventually returns to usb_ep_queue() which invokes the tracepoint:

    ret = ep->ops->queue(ep, req, gfp_flags);
out:
    trace_usb_ep_queue(ep, req, ret);

Since trace_usb_ep_queue() shares this udc_log_req event class, won't req
point to freed memory, causing a use-after-free when evaluating
req->length? A similar scenario can occur in usb_ep_dequeue().

> +		__entry->num_sgs = req ? req->num_sgs : 0;
> +		__entry->num_mapped_sgs = req ? req->num_mapped_sgs : 0;
> +		__entry->stream_id = req ? req->stream_id : 0;
> +		__entry->no_interrupt = req ? req->no_interrupt : 0;
> +		__entry->zero = req ? req->zero : 0;
> +		__entry->short_not_ok = req ? req->short_not_ok : 0;
> +		__entry->status = req ? req->status : 0;
>  		__entry->ret = ret;
>  		__entry->req = req;
>  	),

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012094911.wHPc_9lJ@linutronix.de?part=1

Re: [PATCH REPOST] usb: udc: Make tracing aware that usb_ep_alloc_request() can return NULL

[Greg KH]

On Mon, Oct 12, 2026 at 11:49:11AM +0200, Sebastian Andrzej Siewior wrote:
> The alloc_request callback of the EP can return NULL if it was not able
> to allocate a request. This NULL pointer will then be dereferenced by
> tracing in order to show details about it.

When does that happen?

> Let tracing check the req pointer and if NULL just write 0.

Why not fix up the calling point and if req is NULL, not call the
tracepoint as odds are something went really wrong at that point in
time, right?

thanks,

greg k-h

Re: [PATCH REPOST] usb: udc: Make tracing aware that usb_ep_alloc_request() can return NULL

[Sebastian Andrzej Siewior]

- Felipe (bounce)

On 2026-10-12 13:26:18 [+0200], Greg KH wrote:
> On Mon, Oct 12, 2026 at 11:49:11AM +0200, Sebastian Andrzej Siewior wrote:
> > The alloc_request callback of the EP can return NULL if it was not able
> > to allocate a request. This NULL pointer will then be dereferenced by
> > tracing in order to show details about it.
> 
> When does that happen?

In usb_ep_alloc_request() when ep->ops->alloc_request() return NULL.

> > Let tracing check the req pointer and if NULL just write 0.
> 
> Why not fix up the calling point and if req is NULL, not call the
> tracepoint as odds are something went really wrong at that point in
> time, right?

usb_ep_alloc_request() uses it as:
	trace_usb_ep_alloc_request(ep, req, req ? 0 : -ENOMEM);

so if the allocations fails the entry in the trace buffer will be 
"EPNAME: req NULL … --> -12" aka ENOMEM.
If that trace point is skipped then we have nothing.

Now sashiko pointed out that in usb_ep_queue() that request can be
completed immediately (instead queueing it) and then
usb_gadget_giveback_request() could release it before the tracepoint is
using `req'.

"Immediate" completion could happen for ep0 handling/
composite_setup_complete() but this does not do it. 
loopback_complete() does free `req' for error case (such as
-ECONNABORTED) but this should not happen on enqueue. And if it does,
that memory is still mapped but maybe with bogus content.

This makes me think that it would be better to have copy of `req' on
stack for the tracing case.

> thanks,
> 
> greg k-h

Sebastian