Skip to content

Commit afc6717

Browse files
committed
tracing: Have process_string() also allow arrays
In order to catch a common bug where a TRACE_EVENT() TP_fast_assign() assigns an address of an allocated string to the ring buffer and then references it in TP_printk(), which can be executed hours later when the string is free, the function test_event_printk() runs on all events as they are registered to make sure there's no unwanted dereferencing. It calls process_string() to handle cases in TP_printk() format that has "%s". It returns whether or not the string is safe. But it can have some false positives. For instance, xe_bo_move() has: TP_printk("move_lacks_source:%s, migrate object %p [size %zu] from %s to %s device_id:%s", __entry->move_lacks_source ? "yes" : "no", __entry->bo, __entry->size, xe_mem_type_to_name[__entry->old_placement], xe_mem_type_to_name[__entry->new_placement], __get_str(device_id)) Where the "%s" references into xe_mem_type_to_name[]. This is an array of pointers that should be safe for the event to access. Instead of flagging this as a bad reference, if a reference points to an array, where the record field is the index, consider it safe. Link: https://lore.kernel.org/all/9dee19b6185d325d0e6fa5f7cbba81d007d99166.camel@sapience.com/ Cc: stable@vger.kernel.org Cc: Masami Hiramatsu <mhiramat@kernel.org> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com> Link: https://lore.kernel.org/20241231000646.324fb5f7@gandalf.local.home Fixes: 65a25d9 ("tracing: Add "%s" check in test_event_printk()") Reported-by: Genes Lists <lists@sapience.com> Tested-by: Gene C <arch@sapience.com> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
1 parent fc033cf commit afc6717

File tree

1 file changed

+12
-0
lines changed

1 file changed

+12
-0
lines changed

kernel/trace/trace_events.c

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,18 @@ static bool process_string(const char *fmt, int len, struct trace_event_call *ca
364364
s = r + 1;
365365
} while (s < e);
366366

367+
/*
368+
* Check for arrays. If the argument has: foo[REC->val]
369+
* then it is very likely that foo is an array of strings
370+
* that are safe to use.
371+
*/
372+
r = strstr(s, "[");
373+
if (r && r < e) {
374+
r = strstr(r, "REC->");
375+
if (r && r < e)
376+
return true;
377+
}
378+
367379
/*
368380
* If there's any strings in the argument consider this arg OK as it
369381
* could be: REC->field ? "foo" : "bar" and we don't want to get into

0 commit comments

Comments
 (0)