Re: [PATCH v6 18/18] Documentation/kernel-parameters: Add trace_remote

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] The `dump_on_panic` command-line option silently fails to register the panic handler if the remote buffer was already loaded by a preceding parameter.

Pre-existing issues:
- [High] `trace_pipe_read` loops infinitely inside the kernel if an event cannot fit into the trace_seq buffer.
--

--- Patch [18]: [PATCH v6 18/18] Documentation/kernel-parameters: Add trace_remote ---
commit fb8043141866facf6319346314d1e6305e601a6e
Author: Vincent Donnefort <vdonnefort@google.com>

Documentation/kernel-parameters: Add trace_remote

The trace_remote parameter allows configuring a trace remote on
registration. The syntax is similar to trace_instance.

> diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
> index e6724f947170d..34847463d9e43 100644
> --- a/kernel/trace/trace_remote.c
> +++ b/kernel/trace/trace_remote.c

[ ... ]

> +static void trace_remote_apply_cmdline_opts(struct trace_remote *remote, char *cmdline)
> +{
> +	bool dmesg_on = false;
> +	char *opt;
> +	int ret;
> +
> +	while ((opt = strsep(&cmdline, "^"))) {
> +		if (!*opt)
> +			continue;
> +
> +		if (!strcmp(opt, "dump_on_panic")) {
> +			remote->panic_on = true;
> +		} else if (!strcmp(opt, "dmesg")) {

[Severity: Medium]
If the remote buffer was already loaded by a preceding parameter on the
command line (e.g., via an earlier trace_remote= that specified dmesg or
enabled an event), does setting remote->panic_on = true here silently fail to
register the panic handler?

This differs from the tracefs write paths which dynamically register the panic
handler if the remote is already loaded. Could this cause the request to dump
the trace buffer on panic to be silently ignored despite the configuration
reporting it as enabled?

[ ... ]

kernel/trace/trace_remote.c:trace_pipe_read() {
    ...
	while (trace_remote_iter_read_event(iter)) {
		int prev_len = iter->seq.seq.len;

		if (trace_remote_iter_print_event(iter)) {
			iter->seq.seq.len = prev_len;
			break;
		}

		trace_remote_iter_move(iter);
	}
    ...
}

[Severity: High]
This is a pre-existing issue, but if a trace remote generates an oversized
event whose formatted string exceeds PAGE_SIZE, trace_remote_iter_print_event()
returns -EOVERFLOW and the loop breaks without calling trace_remote_iter_move().

Since the unconsumed oversized event remains in the ring buffer, will
ring_buffer_wait() return immediately on the next read, causing the task to
re-read the same event, fail to print it, break, and repeat infinitely?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817135517.3919534-1-vdonnefort@google.com?part=18