Commit 98633495 authored Feb 20, 2025 by Steven Rostedt Committed by Pu Lehui Feb 20, 2025

tracing: Have process_string() also allow arrays

stable inclusion
from stable-v6.6.70
commit a64e5295ebc4afdefe69cdf16cc286a60ff8ba4b
category: bugfix
bugzilla: https://gitee.com/src-openeuler/kernel/issues/IBJ6PG
CVE: CVE-2024-57930

Reference: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?id=a64e5295ebc4

--------------------------------

commit afc6717628f959941d7b33728570568b4af1c4b8 upstream.

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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Pu Lehui <pulehui@huawei.com>

parent c43e2bbf

kernel/trace/trace_events.c

+12 −0

Original line number	Diff line number	Diff line
		@@ -360,6 +360,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