Thread (25 messages) flat view 25 messages, 2 authors, 22h ago

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.com
quoted
--- 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.c
quoted
[ ... ]
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help