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

[John Ogness]

Hi Zhu,

On 2026-11-12, Zhu Jun <zhujun2@xxxxxxxxxxxxxxxxxxxx> 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