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 <[email protected]> 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/[email protected] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
