Skip to content

Commit 9b7bdf6

Browse files
committed
tracing: Have trace_printk not use binary prints if boot buffer
If the persistent boot mapped ring buffer is used for trace_printk(), force it to not use the binary versions. trace_printk() by default uses bin_printf() that only saves the pointer to the format and not the format itself inside the ring buffer. But for a persistent buffer that is read after reboot, the pointers to the format strings may not be the same, or worse, not even exist! Instead, just force the more robust, but slower, version that does the formatting before saving into the ring buffer. The boot mapped buffer can now be used for trace_printk and friends! Using the trace_printk() and the persistent buffer was used to debug the issue with the osnoise tracer: Link: https://lore.kernel.org/all/[email protected]/ Cc: Masami Hiramatsu <[email protected]> Cc: Mark Rutland <[email protected]> Cc: Mathieu Desnoyers <[email protected]> Cc: Andrew Morton <[email protected]> Cc: Vincent Donnefort <[email protected]> Cc: Joel Fernandes <[email protected]> Cc: Ingo Molnar <[email protected]> Cc: Peter Zijlstra <[email protected]> Cc: Thomas Gleixner <[email protected]> Cc: Vineeth Pillai <[email protected]> Cc: Beau Belgrave <[email protected]> Cc: Alexander Graf <[email protected]> Cc: Baoquan He <[email protected]> Cc: Borislav Petkov <[email protected]> Cc: "Paul E. McKenney" <[email protected]> Cc: David Howells <[email protected]> Cc: Mike Rapoport <[email protected]> Cc: Dave Hansen <[email protected]> Cc: Tony Luck <[email protected]> Cc: Guenter Roeck <[email protected]> Cc: Ross Zwisler <[email protected]> Cc: Kees Cook <[email protected]> Cc: Alexander Aring <[email protected]> Cc: "Luis Claudio R. Goncalves" <[email protected]> Cc: Tomas Glozar <[email protected]> Cc: John Kacur <[email protected]> Cc: Clark Williams <[email protected]> Cc: Linus Torvalds <[email protected]> Cc: "Jonathan Corbet" <[email protected]> Link: https://lore.kernel.org/[email protected] Signed-off-by: Steven Rostedt (Google) <[email protected]>
1 parent ddb8ea9 commit 9b7bdf6

File tree

4 files changed

+36
-20
lines changed

4 files changed

+36
-20
lines changed

Documentation/admin-guide/kernel-parameters.txt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6751,8 +6751,6 @@
67516751
traceoff - Have the tracing instance tracing disabled after it is created.
67526752
traceprintk - Have trace_printk() write into this trace instance
67536753
(note, "printk" and "trace_printk" can also be used)
6754-
Currently, traceprintk flag cannot be used for memory
6755-
mapped ring buffers as described below.
67566754

67576755
trace_instance=foo^traceoff^traceprintk,sched,irq
67586756

@@ -6785,7 +6783,7 @@
67856783
mix with events of the current boot (unless you are debugging a random crash
67866784
at boot up).
67876785

6788-
reserve_mem=12M:4096:trace trace_instance=boot_map^traceoff@trace,sched,irq
6786+
reserve_mem=12M:4096:trace trace_instance=boot_map^traceoff^traceprintk@trace,sched,irq
67896787

67906788

67916789
trace_options=[option-list]

kernel/trace/trace.c

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -502,6 +502,17 @@ static struct trace_array global_trace = {
502502

503503
static struct trace_array *printk_trace = &global_trace;
504504

505+
static __always_inline bool printk_binsafe(struct trace_array *tr)
506+
{
507+
/*
508+
* The binary format of traceprintk can cause a crash if used
509+
* by a buffer from another boot. Force the use of the
510+
* non binary version of trace_printk if the trace_printk
511+
* buffer is a boot mapped ring buffer.
512+
*/
513+
return !(tr->flags & TRACE_ARRAY_FL_BOOT);
514+
}
515+
505516
void trace_set_ring_buffer_expanded(struct trace_array *tr)
506517
{
507518
if (!tr)
@@ -1130,14 +1141,17 @@ EXPORT_SYMBOL_GPL(__trace_puts);
11301141
*/
11311142
int __trace_bputs(unsigned long ip, const char *str)
11321143
{
1133-
struct trace_array *tr = printk_trace;
1144+
struct trace_array *tr = READ_ONCE(printk_trace);
11341145
struct ring_buffer_event *event;
11351146
struct trace_buffer *buffer;
11361147
struct bputs_entry *entry;
11371148
unsigned int trace_ctx;
11381149
int size = sizeof(struct bputs_entry);
11391150
int ret = 0;
11401151

1152+
if (!printk_binsafe(tr))
1153+
return __trace_puts(ip, str, strlen(str));
1154+
11411155
if (!(tr->trace_flags & TRACE_ITER_PRINTK))
11421156
return 0;
11431157

@@ -3247,12 +3261,15 @@ int trace_vbprintk(unsigned long ip, const char *fmt, va_list args)
32473261
struct trace_event_call *call = &event_bprint;
32483262
struct ring_buffer_event *event;
32493263
struct trace_buffer *buffer;
3250-
struct trace_array *tr = printk_trace;
3264+
struct trace_array *tr = READ_ONCE(printk_trace);
32513265
struct bprint_entry *entry;
32523266
unsigned int trace_ctx;
32533267
char *tbuffer;
32543268
int len = 0, size;
32553269

3270+
if (!printk_binsafe(tr))
3271+
return trace_vprintk(ip, fmt, args);
3272+
32563273
if (unlikely(tracing_selftest_running || tracing_disabled))
32573274
return 0;
32583275

@@ -10560,20 +10577,17 @@ __init static void enable_instances(void)
1056010577
if (traceoff)
1056110578
tracer_tracing_off(tr);
1056210579

10563-
if (traceprintk) {
10564-
/*
10565-
* The binary format of traceprintk can cause a crash if used
10566-
* by a buffer from another boot. Do not allow it for the
10567-
* memory mapped ring buffers.
10568-
*/
10569-
if (start)
10570-
pr_warn("Tracing: WARNING: memory mapped ring buffers cannot be used for trace_printk\n");
10571-
else
10572-
printk_trace = tr;
10573-
}
10580+
if (traceprintk)
10581+
printk_trace = tr;
1057410582

10575-
/* Only allow non mapped buffers to be deleted */
10576-
if (!start)
10583+
/*
10584+
* If start is set, then this is a mapped buffer, and
10585+
* cannot be deleted by user space, so keep the reference
10586+
* to it.
10587+
*/
10588+
if (start)
10589+
tr->flags |= TRACE_ARRAY_FL_BOOT;
10590+
else
1057710591
trace_array_put(tr);
1057810592

1057910593
while ((tok = strsep(&curr_str, ","))) {

kernel/trace/trace.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,8 @@ struct trace_array {
429429
};
430430

431431
enum {
432-
TRACE_ARRAY_FL_GLOBAL = (1 << 0)
432+
TRACE_ARRAY_FL_GLOBAL = BIT(0),
433+
TRACE_ARRAY_FL_BOOT = BIT(1),
433434
};
434435

435436
extern struct list_head ftrace_trace_arrays;

kernel/trace/trace_output.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1591,10 +1591,13 @@ static enum print_line_t trace_print_print(struct trace_iterator *iter,
15911591
{
15921592
struct print_entry *field;
15931593
struct trace_seq *s = &iter->seq;
1594+
unsigned long ip;
15941595

15951596
trace_assign_type(field, iter->ent);
15961597

1597-
seq_print_ip_sym(s, field->ip, flags);
1598+
ip = field->ip + iter->tr->text_delta;
1599+
1600+
seq_print_ip_sym(s, ip, flags);
15981601
trace_seq_printf(s, ": %s", field->buf);
15991602

16001603
return trace_handle_return(s);

0 commit comments

Comments
 (0)