Skip to content

Commit

Permalink
tracing: Have process_string() also allow arrays
Browse files Browse the repository at this point in the history
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/[email protected]/

Cc: [email protected]
Cc: Masami Hiramatsu <[email protected]>
Cc: Mathieu Desnoyers <[email protected]>
Link: https://lore.kernel.org/[email protected]
Fixes: 65a25d9 ("tracing: Add "%s" check in test_event_printk()")
Reported-by: Genes Lists <[email protected]>
Tested-by: Gene C <[email protected]>
Signed-off-by: Steven Rostedt (Google) <[email protected]>
  • Loading branch information
rostedt committed Dec 31, 2024
1 parent fc033cf commit afc6717
Showing 1 changed file with 12 additions and 0 deletions.
12 changes: 12 additions & 0 deletions kernel/trace/trace_events.c
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,18 @@ static bool process_string(const char *fmt, int len, struct trace_event_call *ca
s = r + 1;
} while (s < e);

/*
* Check for arrays. If the argument has: foo[REC->val]
* then it is very likely that foo is an array of strings
* that are safe to use.
*/
r = strstr(s, "[");
if (r && r < e) {
r = strstr(r, "REC->");
if (r && r < e)
return true;
}

/*
* If there's any strings in the argument consider this arg OK as it
* could be: REC->field ? "foo" : "bar" and we don't want to get into
Expand Down

0 comments on commit afc6717

Please sign in to comment.