Re: [PATCH] ring-buffer: Annotate the speculative iterator event copy

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Low] ring-buffer: unannotated speculative reads in
  rb_iter_head_event()
--

commit a821b9e5782f64ce12600813e637e78c35dbbee8
Author: Zhu Jun <zhujun2@cmss.chinamobile.com>

ring-buffer: Annotate the speculative iterator event copy

This commit wraps the speculative memcpy() in rb_iter_head_event() with
data_race() to silence KCSAN warnings when a concurrent writer overwrites
the page being read by a non-consuming iterator.

> diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
> index e760d2fe386fb..6e51a7a75d6b1 100644
> --- a/kernel/trace/ring_buffer.c
> +++ b/kernel/trace/ring_buffer.c
> @@ -3562,7 +3562,8 @@ rb_iter_head_event(struct ring_buffer_iter *iter)
>  		/* Writer corrupted the read? */
>  		goto reset;
>  
> -	memcpy(iter->event, event, length);
> +	/* The page stamp below detects a concurrent overwrite of this copy. */
> +	data_race(memcpy(iter->event, event, length));

[Severity: Low]
Can KCSAN still trigger on the adjacent speculative reads of event and
iter_head_page->page->time_stamp in rb_iter_head_event()?

kernel/trace/ring_buffer.c:rb_iter_head_event() {
    ...
	event = __rb_page_index(iter_head_page, iter->head);
	length = rb_event_length(event);
    ...
	data_race(memcpy(iter->event, event, length));
	smp_rmb();

	/* Make sure the page didn't change since we read this */
	if (iter->page_stamp != iter_head_page->page->time_stamp ||
    ...
}

Before the annotated memcpy(), rb_event_length(event) reads
event->type_len, event->time_delta, and event->array[0] from the same
shared page while a concurrent writer updates the header in
rb_update_event(). Because rb_event_length() and rb_event_data_length()
read the event->type_len bitfield via plain loads across multiple checks,
the compiler can also reload inconsistent values if a writer modifies the
header mid-evaluation. Likewise, the plain loads of
iter_head_page->page->time_stamp in the validation check and reset path
race with the writer updating tail_page->page->time_stamp in
__rb_reserve_next().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111084201.80984-1-zhujun2@cmss.chinamobile.com?part=1