Re: [PATCH v6 06/18] tracing/remotes: Add dmesg tracefs file

[email protected]
Newsgroups org.kernel.vger.linux-trace-kernel
Message-ID <[email protected]>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Infinite loop/stuck iterator in `trace_remote_do_dmesg()` when `CONFIG_PRINTK` is disabled.
--

--- Patch [6]: [PATCH v6 06/18] tracing/remotes: Add dmesg tracefs file ---
commit dcbacd33ccf7193b3ba94b1298b0c78d75002d92
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..3a5e0d84c1722 100644
> --- a/kernel/trace/trace_remote.c
> +++ b/kernel/trace/trace_remote.c

[ ... ]

> @@ -882,6 +908,87 @@ static const struct file_operations trace_fops = {
> +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", seq_buf_str(&iter->seq.seq)))
> +			break;

[Severity: Medium]
Does this cause an infinite loop if CONFIG_PRINTK is disabled?

When CONFIG_PRINTK is not set, pr_info() evaluates to 0. If that happens,
!pr_info() will always evaluate to true, causing the loop to break early
before trace_remote_iter_move() is called. 

Could this leave the iterator stuck on the same event indefinitely during
the next polling intervals?

> +
> +		trace_remote_iter_move(iter);
> +
> +		if (!(--max_events))
> +			break;
> +	}
> +
> +	trace_remote_iter_read_finished(iter);
> +
> +	return !max_events;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.