Thread (11 messages) flat view 11 messages, 4 authors, 3d ago

Re: [BUG/PATCH] drivers/usb/mtu3: Work around mtu3_log_ep double-indirection issue

From: "Paul E. McKenney" <paulmck@kernel.org>
Date: 2026-09-09 15:14:49
Also in: linux-mediatek, linux-usb, lkml

On Wed, Sep 09, 2026 at 09:34:17AM -0400, Steven Rostedt wrote:
On Tue, 8 Sep 2026 11:08:33 -0700
"Paul E. McKenney" [off-list ref] wrote:
quoted
quoted
quoted
Apparently, the code should instead create another entry in
TP_STRUCT__entry(), and do the double-indirection TP_fast_assign() instead
of TP_printk().  But simply removing the offending double indirection
in TP_printk() gets this splat out of the way of other debugging.  
Indeed, splat has gone after applying the patch.

Thanks
Vladimir  
Does this less hacky patch do the trick?
Nope!

quoted
diff --git a/drivers/usb/mtu3/mtu3_trace.h b/drivers/usb/mtu3/mtu3_trace.h
index 89870175d63561..6477ad3ddc66f6 100644
--- a/drivers/usb/mtu3/mtu3_trace.h
+++ b/drivers/usb/mtu3/mtu3_trace.h
@@ -224,6 +224,7 @@ DECLARE_EVENT_CLASS(mtu3_log_ep,
 		__field(unsigned int, flags)
 		__field(unsigned int, direction)
 		__field(struct mtu3_gpd_ring *, gpd_ring)
+		__field(dma_addr_t *, gpd_ring_dma)
 	),
 	TP_fast_assign(
 		__assign_str(name);
@@ -235,12 +236,13 @@ DECLARE_EVENT_CLASS(mtu3_log_ep,
 		__entry->flags = mep->flags;
 		__entry->direction = mep->is_in;
 		__entry->gpd_ring = &mep->gpd_ring;
+		__entry->gpd_ring_dma = &mep->gpd_ring->dma;
You are still saving the address of some memory into the ring buffer.

quoted
 	),
 	TP_printk("%s: type %s maxp %d slot %d mult %d burst %d ring %p/%pad flags %c:%c%c%c:%c",
                                                                        ^^^^

That %pad dereferences the pointer passed to it.
quoted
 		__get_str(name), usb_ep_type_string(__entry->type),
 		__entry->maxp, __entry->slot,
 		__entry->mult, __entry->maxburst,
-		__entry->gpd_ring, &__entry->gpd_ring->dma,
+		__entry->gpd_ring, __entry->gpd_ring_dma,
That will read the address saved in the ring buffer and dereference it.

Remember, the above TP_fast_assign() logic gets executed when the
tracepoint is triggered. The TP_printk() is executed when the user reads
the trace buffer. That could be seconds, minutes, hours, days, even months
later!

You can't trust that the memory you are dereferencing will not be freed
when the user reads the trace.

The original patch is not hacky. It is actually the correct way of handling
this.
Very well, "git revert" followed by "git cherry-pick" of the original.
Or someone can feel free to pull in the original from earlier in this
thread.

Either way, thank you!

							Thanx, Paul
-- Steve

quoted
 		__entry->flags & MTU3_EP_ENABLED ? 'E' : 'e',
 		__entry->flags & MTU3_EP_STALL ? 'S' : 's',
 		__entry->flags & MTU3_EP_WEDGE ? 'W' : 'w',
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help