Re: [PATCH v7 18/20] tracing/remotes: Add trace_remote cmdline options

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] tracing: missing validation and truncation of buf_size in trace_remote
- [Low] tracing: data race in trace_remote cmdline parsing due to missing WRITE_ONCE
--

commit 6359480e2f0f5e3b0022cfdf6db7a85cba580136
Author: Vincent Donnefort <vdonnefort@google.com>

tracing/remotes: Add trace_remote cmdline options

This patch adds a command-line interface to configure trace remotes upon
registration. It parses options such as dump_on_panic, dmesg, buf_size,
and poll parameters, and enables tracing automatically when events are
activated.

> diff --git a/kernel/trace/trace_remote.c b/kernel/trace/trace_remote.c
> index b2547eb117f85..de41f36e9a0e3 100644
> --- a/kernel/trace/trace_remote.c
> +++ b/kernel/trace/trace_remote.c

[ ... ]

> +static bool trace_remote_apply_cmdline_opts(struct trace_remote *remote, char *cmdline)
> +{
> +	bool dmesg_on = false;
> +	char *opt;
> +
> +	while ((opt = strsep(&cmdline, "^"))) {
> +		if (!*opt)
> +			continue;
> +
> +		if (!strcmp(opt, "dump_on_panic")) {
> +			WRITE_ONCE(remote->panic_on, true);
> +		} else if (!strcmp(opt, "dmesg")) {
> +			dmesg_on = true;
> +		} else if (!strncmp(opt, "buf_size=", 9)) {
> +			WRITE_ONCE(remote->trace_buffer_size, memparse(opt + 9, NULL));

[Severity: Medium]
Is it possible for this to silently truncate large buffer sizes on 32-bit
architectures?

Since memparse() returns a 64-bit unsigned long long, but
remote->trace_buffer_size is an unsigned long (32 bits on 32-bit
architectures), values like 4G might truncate.

Additionally, what happens if a privileged user specifies buf_size=0 on
the command line? It seems this bypasses validation and might cause
undefined behavior in ring buffer allocation or remote driver
initialization.

> +		} else if (!strncmp(opt, "poll=", 5)) {
> +			unsigned int poll_ms;
> +
> +			if (!kstrtouint(opt + 5, 10, &poll_ms) && poll_ms > 0)
> +				remote->poll_ms = poll_ms;

[Severity: Low]
Should this assignment use WRITE_ONCE()?

Concurrent lockless readers like poll_ms_show() access this variable via
READ_ONCE(). A plain assignment here without WRITE_ONCE() violates kernel
concurrency rules and could trigger a KCSAN data race warning if accessed
while the boot cmdline is being applied during remote registration.

> +			else
> +				pr_warn("Invalid trace remote poll '%s'\n", opt);
> +		} else {
> +			pr_warn("Unknown trace remote option '%s'\n", opt);
> +		}
> +	}
> +
> +	return dmesg_on;
> +}

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