Re: [PATCH v4 06/18] tracing/remotes: Add dmesg tracefs file
From: Vincent Donnefort <hidden>
Date: 2026-08-11 09:09:36
Also in:
lkml
On Mon, Aug 10, 2026 at 06:42:16PM -0400, Steven Rostedt wrote:
Adding Sashiko review: https://sashiko.dev/#/patchset/20260804140333.797210-2-vdonnefort%40google.comquoted
--- Patch [6]: [PATCH v4 06/18] tracing/remotes: Add dmesg tracefs file ---commit b6a4c19257d91807c612baa19691b2d602bb0670 Author: Vincent Donnefort [off-list ref] tracing/remotes: Add dmesg tracefs file When enabled, the dmesg tracefs file enables the redirection of all events to dmesg. This is similar to tp_printk.quoted
diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c index 1fbe2f3f20e99..58a38f3d49024 100644 --- a/kernel/trace/trace_remote.c +++ b/kernel/trace/trace_remote.cquoted
[ ... ]quoted
@@ -882,6 +908,87 @@ static const struct file_operations trace_fops = { .release = trace_release, }; +static bool trace_remote_do_dmesg(struct trace_remote *remote) +{ + struct trace_remote_iterator *iter = smp_load_acquire(&remote->dmesg); + unsigned int max_events = 1000; + + if (!iter) + return false; + + trace_remote_iter_read_start(iter); + + while (trace_remote_iter_read_event(iter)) { + trace_seq_init(&iter->seq); + + trace_remote_iter_print_event(iter); + trace_seq_putc(&iter->seq, 0); + if (!pr_info("%s", iter->seq.buffer))If the remote trace event exceeds the PAGE_SIZE sequence buffer, does trace_seq_putc() in trace_remote_do_dmesg() fail to append the null byte? Since iter->seq.buffer might not be null-terminated in this case, could this lead to an out-of-bounds read when pr_info() prints the string?Yeah, we may want to have it be: if (!pr_info("%s", seq_buf_str(&iter->seq)) Which will make sure the buffer is nul terminated.
ack, and same for trace_remote_panic_handler()
quoted
This isn't a bug introduced by this patch, but the same pattern appears to exist in the pre-existing trace_remote_panic_handler() with pr_emerg(). If the kernel is built with CONFIG_PRINTK disabled, pr_info() evaluates to 0. Will this cause the loop in trace_remote_do_dmesg() to break without calling trace_remote_iter_move(), leaving the event unconsumed and permanently stalling the workqueue?quoted
+ break; + + trace_remote_iter_move(iter); + + if (!(--max_events)) + break; + } + + trace_remote_iter_read_finished(iter); + + return !max_events; +}While reviewing this, I also noticed a pre-existing issue in trace_pipe_read() when handling oversized events: kernel/trace/trace_remote.c:trace_pipe_read() { ... if (trace_remote_iter_print_event(iter)) { iter->seq.seq.len = prev_len; break; } ... } This is a pre-existing issue, but if an oversized event causes trace_remote_iter_print_event() to return -EOVERFLOW, will this break the loop without calling trace_remote_iter_move()? Since the event is left unconsumed in the buffer, could this lead to an uninterruptible infinite loop (hard lockup) when reading the trace pipe?I'll let you answer the above. -- Steve
I don't believe this can realistically happen, trace_seq buffer is 8K and remote events are really small. -- Vincent