Re: [PATCH v2] tracing: Keep copying markers when one instance is stopped

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] tracing: successful marker write overwritten by tail error in marker_copies
--

commit 2345280f3b3625fa6e36116fc50947e792b25719
Author: Jaewon Jung <jaewon8868@gmail.com>

tracing: Keep copying markers when one instance is stopped

This commit removes the early break on write errors in tracing_mark_write()
and tracing_mark_raw_write() when iterating over marker_copies so that
stopped instances do not prevent remaining instances from receiving markers.

> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 08614af8fad1c..0b66c55d378b1 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -6330,8 +6330,6 @@ tracing_mark_write(struct file *filp, const char __user *ubuf,
>  		guard(rcu)();
>  		list_for_each_entry_rcu(tr, &marker_copies, marker_list) {
>  			written = write_marker_to_buffer(tr, buf, cnt, ip);
> -			if (written < 0)
> -				break;
>  		}
>  	} else {
>  		written = write_marker_to_buffer(tr, buf, cnt, ip);
> @@ -6409,8 +6407,6 @@ tracing_mark_raw_write(struct file *filp, const char __user *ubuf,
>  		guard(rcu)();
>  		list_for_each_entry_rcu(tr, &marker_copies, marker_list) {
>  			written = write_raw_marker_to_buffer(tr, buf, cnt);
> -			if (written < 0)
> -				break;

[Severity: Medium]
As reported by Steven Rostedt on v1 [1], should errors from stopped
instances in marker_copies be prevented from overwriting a successful write
return value?

When instances enable copy_trace_marker, update_marker_trace() adds each
instance to the head of marker_copies:

kernel/trace/trace.c:update_marker_trace() {
	if (enabled) {
		if (tr->trace_flags & TRACE_ITER(COPY_MARKER))
			return false;

		list_add_rcu(&tr->marker_list, &marker_copies);
		tr->trace_flags |= TRACE_ITER(COPY_MARKER);
		return true;
	}

This leaves earlier-enabled instances and global_trace at the tail of
marker_copies.

Because tracing_mark_write() and tracing_mark_raw_write() unconditionally
assign written on every iteration of marker_copies, if an earlier instance
in the list is active and receives the marker (setting written = cnt), but
the last instance in marker_copies has tracing disabled,
write_marker_to_buffer() or write_raw_marker_to_buffer() will return -EBADF
on that tail instance. Won't that overwrite written and cause the syscall
to return -EBADF to userspace even though the marker was successfully
written to the active instance(s)?

[1] https://lore.kernel.org/r/20261102091751.2ee7c4d3@gandalf.local.home

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103005219.3236380-1-jaewon8868@gmail.com?part=1