tracing: Fixes for 6.15

tracing fixes for 6.15

- Fix build error when CONFIG_PROBE_EVENTS_BTF_ARGS is not enabled

The tracing of arguments in the function tracer depends on some functions that are only defined when PROBE_EVENTS_BTF_ARGS is enabled. In fact, PROBE_EVENTS_BTF_ARGS also depends on all the same configs as the function argument tracing requires. Just have the function argument tracing depend on PROBE_EVENTS_BTF_ARGS.

- Free module_delta for persistent ring buffer instance

When an instance holds the persistent ring buffer, it allocates a helper array to hold the deltas between where modules are loaded on the last boot and the current boot. This array needs to be freed when the instance is freed.

- Add cond_reschd() to loop in ftrace_graph_set_hash()

The hash functions in ftrace loop over every function that can be enabled by ftrace. This can be 50,000 functions or more. This loop is known to trigger soft lockup warnings and requires a cond_resched(). The loop in ftrace_graph_set_hash() was missing it.

- Fix the event format verifier to include "%*p.." arguments

To prevent events from dereferencing stale pointers that can happen if a trace event uses a dereferece pointer to something that was not copied into the ring buffer and can be freed by the time the trace is read, a verifier is called. At boot or module load, the verifier scans the print format string for pointers that can be dereferenced and it checks the arguments to make sure they do not contain something that can be freed. The "%*p" was not handled, which would add another argument and cause the verifier to not only not verify this pointer, but it will look at the wrong argument for every pointer after that.

git://git.kernel.org/pub/scm/linux/kernel/git/trace/linux-trace.git
trace/fixes

Head SHA1: 1bf4f0161e70a33c602732c5e0f0fdb268f130e1

Steven Rostedt (3):
ftrace: Have tracing function args depend on PROBE_EVENTS_BTF_ARGS
tracing: Free module_delta on freeing of persistent ring buffer
tracing: Verify event formats that have "%*p.."

zhoumin (1):
ftrace: Add cond_resched() to ftrace_graph_set_hash()

----

 kernel/trace/Kconfig                       | 3 +--
 kernel/trace/ftrace.c                      | 1 +
 kernel/trace/trace.c                       | 1 +
 kernel/trace/trace_events.c                | 7 +++++++
 samples/trace_events/trace-events-sample.h | 8 ++++++--
 5 files changed, 16 insertions(+), 4 deletions(-)

From: Steven Rostedt <[email protected]>

The option PROBE_EVENTS_BTF_ARGS enables the functions btf_find_func_proto() and btf_get_func_param() which are used by the function argument tracing code. The option FUNCTION_TRACE_ARGS was dependent on the same configs that PROBE_EVENTS_BTF_ARGS was dependent on, but it was also dependent on PROBE_EVENTS_BTF_ARGS. In fact, if PROBE_EVENTS_BTF_ARGS is supported then FUNCTION_TRACE_ARGS is supported.

Just make FUNCTION_TRACE_ARGS depend on PROBE_EVENTS_BTF_ARGS.

---
 kernel/trace/Kconfig | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

index 033fba0633cf..a3f35c7d83b6 100644
@@ -265,8 +265,7 @@ config FUNCTION_GRAPH_RETADDR
 
 config FUNCTION_TRACE_ARGS
        bool
-	depends on HAVE_FUNCTION_ARG_ACCESS_API
-	depends on DEBUG_INFO_BTF
+	depends on PROBE_EVENTS_BTF_ARGS
 	default y
 	help
 	  If supported with function argument access API and BTF, then

2.47.2

Thanks

From: Steven Rostedt <[email protected]>

If a persistent ring buffer is created, a "module_delta" array is also allocated to hold the module deltas of loaded modules that match modules in the scratch area. If this buffer gets freed, the module_delta array is not freed and causes a memory leak.

---
 kernel/trace/trace.c | 1 +
 1 file changed, 1 insertion(+)

index 103b193875b3..de6d7f0e6206 100644
@@ -9609,6 +9609,7 @@ static void free_trace_buffers(struct trace_array *tr)
 		return;
 
 	free_trace_buffer(&tr->array_buffer);
+	kfree(tr->module_delta);
 
 #ifdef CONFIG_TRACER_MAX_TRACE
 	free_trace_buffer(&tr->max_buffer);

2.47.2

From: zhoumin <[email protected]>

When the kernel contains a large number of functions that can be traced, the loop in ftrace_graph_set_hash() may take a lot of time to execute. This may trigger the softlockup watchdog.

Add cond_resched() within the loop to allow the kernel to remain responsive even when processing a large number of functions.

This matches the cond_resched() that is used in other locations of the code that iterates over all functions that can be traced.

---
 kernel/trace/ftrace.c | 1 +
 1 file changed, 1 insertion(+)

index 92015de6203d..1a48aedb5255 100644
@@ -6855,6 +6855,7 @@ ftrace_graph_set_hash(struct ftrace_hash *hash, char *buffer)
 				}
 			}
 		}
+		cond_resched();
 	} while_for_each_ftrace_rec();
 
 	return fail ? -EINVAL : 0;

2.47.2

From: Steven Rostedt <[email protected]>

The trace event verifier checks the formats of trace events to make sure that they do not point at memory that is not in the trace event itself or in data that will never be freed. If an event references data that was allocated when the event triggered and that same data is freed before the event is read, then the kernel can crash by reading freed memory.

The verifier runs at boot up (or module load) and scans the print formats of the events and checks their arguments to make sure that dereferenced pointers are safe. If the format uses "%*p.." the verifier will ignore it, and that could be dangerous. Cover this case as well.

Also add to the sample code a use case of "%*pbl".

---
 kernel/trace/trace_events.c                | 7 +++++++
 samples/trace_events/trace-events-sample.h | 8 ++++++--
 2 files changed, 13 insertions(+), 2 deletions(-)

index 8638b7f7ff85..069e92856bda 100644
@@ -470,6 +470,7 @@ static void test_event_printk(struct trace_event_call *call)
 			case '%':
 				continue;
 			case 'p':
+ do_pointer:
 				/* Find dereferencing fields */
 				switch (fmt[i + 1]) {
 				case 'B': case 'R': case 'r':

@@ -498,6 +499,12 @@ static void test_event_printk(struct trace_event_call *call)
 						continue;
 					if (fmt[i + j] == '*') {
 						star = true;
+						/* Handle %*pbl case */
+						if (!j && fmt[i + 1] == 'p') {
+							arg++;
+							i++;
+							goto do_pointer;
+						}
 						continue;
 					}
 					if ((fmt[i + j] == 's')) {

index 999f78d380ae..1a05fc153353 100644
@@ -319,7 +319,8 @@ TRACE_EVENT(foo_bar,
 		__assign_cpumask(cpum, cpumask_bits(mask));
 	),
 
-	TP_printk("foo %s %d %s %s %s %s %s %s (%s) (%s) %s", __entry->foo, __entry->bar,
+	TP_printk("foo %s %d %s %s %s %s %s %s (%s) (%s) %s [%d] %*pbl",
+		  __entry->foo, __entry->bar,
 
 /*
  * Notice here the use of some helper functions. This includes:

@@ -370,7 +371,10 @@ TRACE_EVENT(foo_bar,
 
 		  __get_str(str), __get_str(lstr),
 		  __get_bitmask(cpus), __get_cpumask(cpum),
-		  __get_str(vstr))
+		  __get_str(vstr),
+		  __get_dynamic_array_len(cpus),
+		  __get_dynamic_array_len(cpus),
+		  __get_dynamic_array(cpus))
 );
 
 /*

2.47.2