From: Steven Rostedt <[email protected]>

Currently on boot up and when modules are loaded, the trace event
infrastructure will examine the TP_printk's of every event looking to see
if it dereferences pointers on the ring buffer via printk formats like
"%pB" and such. What it doesn't do is check if the arguments themselves
do a dereference from a pointer.

This was brought with a fix[1] to the fsl_edma event that had in the
arguments of the TP_printk(): "__entry->edma->membase"

The __entry->edma is a pointer saved in the ring buffer. The dereference
from TP_printk() happens when the user reads the "trace" file which can be
seconds, minutes, hours, days, weeks, or even months later! There is no
guarantee that the __entry->edma pointer will still be pointing to what it
was when it was recorded, and could crash the kernel when a user reads the
event.

Add logic to the test_event_printk() that also checks for this case and
warn if the event dereferences a pointer from the ring buffer.

[1] https://lore.kernel.org/all/[email protected]/

Signed-off-by: Steven Rostedt <[email protected]>
---
 kernel/trace/trace_events.c | 35 +++++++++++++++++++++++++++++------
 1 file changed, 29 insertions(+), 6 deletions(-)

diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
index c46e623e7e0d..3b52bfd8b300 100644
--- a/kernel/trace/trace_events.c
+++ b/kernel/trace/trace_events.c
@@ -400,6 +400,31 @@ static bool process_string(const char *fmt, int len, 
struct trace_event_call *ca
        return true;
 }
 
+static void test_double_dereference(const char *str, int len,
+                                   struct trace_event_call *call)
+{
+       const char *ptr;
+       const char *end = str + len;
+
+       ptr = strstr(str, "REC->");
+
+       while (ptr && ptr < end) {
+
+               ptr += 5;
+               for (; ptr < end; ptr++) {
+                       if (ptr[0] == '-' && ptr[1] == '>') {
+                               WARN_ONCE(1, "Event %s has double dereference 
in TP_printk: %.*s\n",
+                                         trace_event_name(call), len, str);
+                               return;
+                       }
+                       if (!isalnum(*ptr) && *ptr != '_')
+                               break;
+               }
+
+               ptr = strstr(ptr, "REC->");
+       }
+}
+
 static void handle_dereference_arg(const char *arg_str, u64 string_flags, int 
len,
                                   u64 *dereference_flags, int arg,
                                   struct trace_event_call *call)
@@ -459,12 +484,6 @@ static void test_event_printk(struct trace_event_call 
*call)
                                if (in_quote) {
                                        arg = 0;
                                        first = false;
-                                       /*
-                                        * If there was no %p* uses
-                                        * the fmt is OK.
-                                        */
-                                       if (!dereference_flags)
-                                               return;
                                }
                        }
                        if (in_quote) {
@@ -576,6 +595,8 @@ static void test_event_printk(struct trace_event_call *call)
                                continue;
                        }
 
+                       test_double_dereference(fmt + start_arg, e - start_arg, 
call);
+
                        if (dereference_flags & (1ULL << arg)) {
                                handle_dereference_arg(fmt + start_arg, 
string_flags,
                                                       e - start_arg,
@@ -589,6 +610,8 @@ static void test_event_printk(struct trace_event_call *call)
                }
        }
 
+       test_double_dereference(fmt + start_arg, i - start_arg, call);
+
        if (dereference_flags & (1ULL << arg)) {
                handle_dereference_arg(fmt + start_arg, string_flags,
                                       i - start_arg,
-- 
2.53.0


Reply via email to