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
>
> > --- Patch [6]: [PATCH v4 06/18] tracing/remotes: Add dmesg tracefs file ---
> > commit b6a4c19257d91807c612baa19691b2d602bb0670
> > Author: Vincent Donnefort <[email protected]>
> >
> > 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.
> >
> > > 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
> >
> > > [ ... ]
> >
> > > @@ -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()
>
>
> >
> > 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?
> >
> > > + 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
