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

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

[Zhu Jun]

A non-consuming iterator speculatively copies an event while a writer
may overwrite its page. The reader lock excludes other readers, but not
writers, so KCSAN reports the concurrent memcpy() and event reservation
accesses.

The iterator already validates the page stamp and committed size after
the copy and retries if the page changed. This makes the race intentional
and the KCSAN report a false positive. Mark the speculative copy with
data_race() to document the allowed access and suppress the report.

Signed-off-by: Zhu Jun <zhujun2@cmss.chinamobile.com>
---
Changes in v2:
- Describe this as an intentional-race annotation suppressing a KCSAN
  false positive.
- Drop the Fixes tag; the existing iterator validation already handles
  concurrent writers correctly.
- No code changes.

 kernel/trace/ring_buffer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/trace/ring_buffer.c b/kernel/trace/ring_buffer.c
index 75cd8df23c1b..950c42f1e849 100644
--- a/kernel/trace/ring_buffer.c
+++ b/kernel/trace/ring_buffer.c
@@ -3566,7 +3566,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));
 	/*
 	 * If the page stamp is still the same after this rmb() then the
 	 * event was safely copied without the writer entering the page.
-- 
2.51.0

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

[sashiko-bot]

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 b8f65c61949524a718551f053209be2d46d549d1
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 suppress data race reports when a concurrent writer
overwrites the buffer page.

> 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));
>  	/*
>  	 * If the page stamp is still the same after this rmb() then the
>  	 * event was safely copied without the writer entering the page.

[Severity: Low]
Does annotating only the memcpy() leave the preceding rb_event_length(event)
and following iter_head_page->page->time_stamp loads in the same speculative
read section subject to the same data race?

Just above the memcpy(), rb_iter_head_event() calls rb_event_length(event)
on the same shared event pointer while a concurrent writer can update
event->type_len, event->time_delta and event->array[0] in rb_update_event():

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

Because rb_event_length() and its helpers read the event header with plain
loads, the race detector will still report the race there, and the compiler
can reload the event->type_len bitfield across the switch and length
calculation before the barrier().

Similarly, the plain loads of iter_head_page->page->time_stamp after the
memcpy() race with __rb_reserve_next() updating tail_page->page->time_stamp.

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