diff mbox series

tracing: Use a copy of the va_list for __assign_vstr()

Message ID 20220719182004.21daa83e@gandalf.local.home
State New
Headers show
Series tracing: Use a copy of the va_list for __assign_vstr() | expand

Commit Message

Steven Rostedt July 19, 2022, 10:20 p.m. UTC 
From: "Steven Rostedt (Google)" <rostedt@goodmis.org>

If an instance of tracing enables the same trace event as another
instance, or the top level instance, or even perf, then the va_list passed
into some tracepoints can be used more than once.

As va_list can only be traversed once, this can cause issues:

 # cat /sys/kernel/tracing/instances/qla2xxx/trace
             cat-56106   [012] ..... 2419873.470098: ql_dbg_log: qla2xxx [0000:05:00.0]-1054:14:  Entered (null).
             cat-56106   [012] ..... 2419873.470101: ql_dbg_log: qla2xxx [0000:05:00.0]-1000:14:  Entered ×+<96>²Ü<98>^H.
             cat-56106   [012] ..... 2419873.470102: ql_dbg_log: qla2xxx [0000:05:00.0]-1006:14:  Prepare to issue mbox cmd=0xde589000.

 # cat /sys/kernel/tracing/trace
             cat-56106   [012] ..... 2419873.470097: ql_dbg_log: qla2xxx [0000:05:00.0]-1054:14:  Entered qla2x00_get_firmware_state.
             cat-56106   [012] ..... 2419873.470100: ql_dbg_log: qla2xxx [0000:05:00.0]-1000:14:  Entered qla2x00_mailbox_command.
             cat-56106   [012] ..... 2419873.470102: ql_dbg_log: qla2xxx [0000:05:00.0]-1006:14:  Prepare to issue mbox cmd=0x69.

The instance version is corrupted because the top level instance iterated
the va_list first.

Use va_copy() in the __assign_vstr() macro to make sure that each trace
event for each use case gets a fresh va_list.

Link: https://lore.kernel.org/all/259d53a5-958e-6508-4e45-74dba2821242@marvell.com/

Reported-by: Arun Easi <aeasi@marvell.com>
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---

 This means that the __vstring/__assign_vstr() series, with this patch is
 actually a bug fix and not a clean up. These will probably need to go
 to stable after they hit Linus's tree. I'll still wait till the merge
 window as it's not far away and I'd like these to sit in linux-next
 for a bit too.

 include/trace/stages/stage6_event_callback.h | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)
diff mbox series

Patch

diff --git a/include/trace/stages/stage6_event_callback.h b/include/trace/stages/stage6_event_callback.h
index 0f51f6b3ab70..3c554a585320 100644
--- a/include/trace/stages/stage6_event_callback.h
+++ b/include/trace/stages/stage6_event_callback.h
@@ -40,7 +40,12 @@ 
 
 #undef __assign_vstr
 #define __assign_vstr(dst, fmt, va)					\
-	vsnprintf(__get_str(dst), TRACE_EVENT_STR_MAX, fmt, *(va))
+	do {								\
+		va_list __cp_va;					\
+		va_copy(__cp_va, *(va));				\
+		vsnprintf(__get_str(dst), TRACE_EVENT_STR_MAX, fmt, __cp_va); \
+		va_end(__cp_va);					\
+	} while (0)
 
 #undef __bitmask
 #define __bitmask(item, nr_bits) __dynamic_array(unsigned long, item, -1)