[PATCH v2] printk: ringbuffer: Annotate speculative record reads

[PATCH v2] printk: ringbuffer: Annotate speculative record reads

[Zhu Jun]

Readers may copy a finalized record while a writer recycles its storage.
prb_read() checks the descriptor again after copying and rejects records
that have changed, but the plain metadata and text accesses still trigger
KCSAN reports.

Annotate the speculative copies, line counting and length read with
data_race(). Retain READ_ONCE() for text_len so that the size check and
copy use one snapshot. The descriptor validation continues to determine
whether the copied record can be used.

OpenAI Codex assisted with analysis and patch preparation.

Fixes: b6cf8b3f3312 ("printk: add lockless ringbuffer")
Signed-off-by: Zhu Jun <zhujun2@cmss.chinamobile.com>
---
Changes in v2:
- Mark the speculative text_len snapshot with data_race(), retaining
  READ_ONCE() and documenting the reader-side validation.

 kernel/printk/printk_ringbuffer.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c
index 46a79ec0940a..8a1075d00d0a 100644
--- a/kernel/printk/printk_ringbuffer.c
+++ b/kernel/printk/printk_ringbuffer.c
@@ -1894,9 +1894,10 @@ static bool copy_data(struct prb_data_ring *data_ring,
 	if (data_size < (unsigned int)len)
 		return false;
 
-	/* Caller interested in the line count? */
-	if (line_count)
-		*line_count = count_lines(data, len);
+	if (line_count) {
+		/* Racing text is rejected by the caller's descriptor recheck. */
+		*line_count = data_race(count_lines(data, len));
+	}
 
 	/* Caller interested in the data content? */
 	if (!buf || !buf_size)
@@ -1904,7 +1905,7 @@ static bool copy_data(struct prb_data_ring *data_ring,
 
 	data_size = min_t(unsigned int, buf_size, len);
 
-	memcpy(&buf[0], data, data_size); /* LMM(copy_data:A) */
+	data_race(memcpy(&buf[0], data, data_size)); /* LMM(copy_data:A) */
 	return true;
 }
 
@@ -1969,6 +1970,7 @@ static int prb_read(struct printk_ringbuffer *rb, u64 seq,
 	atomic_long_t *state_var = &rdesc->state_var;
 	struct prb_desc desc;
 	unsigned long id;
+	u16 text_len;
 	int err;
 
 	/* Extract the ID, used to specify the descriptor to read. */
@@ -1984,12 +1986,16 @@ static int prb_read(struct printk_ringbuffer *rb, u64 seq,
 	if (err || !r)
 		return err;
 
-	/* If requested, copy meta data. */
-	if (r->info)
-		memcpy(r->info, info, sizeof(*(r->info)));
+	if (r->info) {
+		/* The record may be recycled; the descriptor is rechecked below. */
+		data_race(memcpy(r->info, info, sizeof(*r->info)));
+	}
+
+	/* The descriptor recheck below also rejects a raced length. */
+	text_len = data_race(READ_ONCE(info->text_len));
 
 	/* Copy text data. If it fails, this is a data-less record. */
-	if (!copy_data(&rb->text_data_ring, &desc.text_blk_lpos, info->text_len,
+	if (!copy_data(&rb->text_data_ring, &desc.text_blk_lpos, text_len,
 		       r->text_buf, r->text_buf_size, line_count)) {
 		return -ENOENT;
 	}
-- 
2.51.0

Re: [PATCH v2] printk: ringbuffer: Annotate speculative record reads

[John Ogness]

Hi Zhu,

On 2026-11-12, Zhu Jun <zhujun2@cmss.chinamobile.com> wrote:
> diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c
> index 46a79ec0940a..8a1075d00d0a 100644
> --- a/kernel/printk/printk_ringbuffer.c
> +++ b/kernel/printk/printk_ringbuffer.c
> @@ -1894,9 +1894,10 @@ static bool copy_data(struct prb_data_ring *data_ring,
>  	if (data_size < (unsigned int)len)
>  		return false;
>  
> -	/* Caller interested in the line count? */

Please do not drop the above comment.

> -	if (line_count)
> -		*line_count = count_lines(data, len);
> +	if (line_count) {
> +		/* Racing text is rejected by the caller's descriptor recheck. */
> +		*line_count = data_race(count_lines(data, len));
> +	}
>  
>  	/* Caller interested in the data content? */
>  	if (!buf || !buf_size)
> @@ -1969,6 +1970,7 @@ static int prb_read(struct printk_ringbuffer *rb, u64 seq,
>  	atomic_long_t *state_var = &rdesc->state_var;
>  	struct prb_desc desc;
>  	unsigned long id;
> +	u16 text_len;
>  	int err;
>  
>  	/* Extract the ID, used to specify the descriptor to read. */
> @@ -1984,12 +1986,16 @@ static int prb_read(struct printk_ringbuffer *rb, u64 seq,
>  	if (err || !r)
>  		return err;
>  
> -	/* If requested, copy meta data. */

Please do not drop the above comment.

> -	if (r->info)
> -		memcpy(r->info, info, sizeof(*(r->info)));
> +	if (r->info) {
> +		/* The record may be recycled; the descriptor is rechecked below. */
> +		data_race(memcpy(r->info, info, sizeof(*r->info)));
> +	}
> +
> +	/* The descriptor recheck below also rejects a raced length. */
> +	text_len = data_race(READ_ONCE(info->text_len));

There is no need to prevent compiler optimization with
READ_ONCE(). Using data_race() is enough.

>  
>  	/* Copy text data. If it fails, this is a data-less record. */
> -	if (!copy_data(&rb->text_data_ring, &desc.text_blk_lpos, info->text_len,
> +	if (!copy_data(&rb->text_data_ring, &desc.text_blk_lpos, text_len,
>  		       r->text_buf, r->text_buf_size, line_count)) {
>  		return -ENOENT;
>  	}

John Ogness