From: Steven Rostedt <rostedt@goodmis.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: LKML <linux-kernel@vger.kernel.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Mathieu Desnoyers <mathieu.desnoyers@efficios.com>,
Gene C <arch@sapience.com>
Subject: [GIT PULL] tracing: Fix trace event check for 6.13
Date: Tue, 31 Dec 2024 08:56:22 -0500 [thread overview]
Message-ID: <20241231085622.35f69353@gandalf.local.home> (raw)
Linus,
Tracing fix for 6.13:
- Fix trace event string check when dealing with array of strings
The xe_bo_move event has a field that indexes into an array of
strings. The TP_fast_assign() added the index into the ring buffer
and the TP_printk() had a "%s" that referenced the array using the
index in the ring buffer. This is a legitimate use of "%s" in
trace events. But this triggered a false positive in the
test_event_printk() at boot saying that the string was dangerous.
Change the check to allow arrays using fields in the ring buffer
as an index to be considered a safe string.
Please pull the latest trace-v6.13-rc5 tree, which can be found at:
git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
trace-v6.13-rc5
Tag SHA1: ab6af83399fec949e9ac8072301ec4d81d82a6e9
Head SHA1: afc6717628f959941d7b33728570568b4af1c4b8
Steven Rostedt (1):
tracing: Have process_string() also allow arrays
----
kernel/trace/trace_events.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
---------------------------
commit afc6717628f959941d7b33728570568b4af1c4b8
Author: Steven Rostedt <rostedt@goodmis.org>
Date: Tue Dec 31 00:06:46 2024 -0500
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: 65a25d9f7ac02 ("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>
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index 1545cc8b49d0..770e7ed91716 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -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
next reply other threads:[~2024-12-31 13:55 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-12-31 13:56 Steven Rostedt [this message]
2024-12-31 14:09 ` Sasha Levin
2024-12-31 14:16 ` Steven Rostedt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20241231085622.35f69353@gandalf.local.home \
--to=rostedt@goodmis.org \
--cc=arch@sapience.com \
--cc=linux-kernel@vger.kernel.org \
--cc=mathieu.desnoyers@efficios.com \
--cc=mhiramat@kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®