Re: [PATCH printk v1] printk: ringbuffer: Fix data block max size check

[Petr Mladek <pmladek@suse.com>]

On Fri 2025-09-26 00:55:59, John Ogness wrote:
> Currently data_check_size() limits data blocks to a maximum size of
> the full buffer minus an ID (long integer):
> 
>     max_size <= DATA_SIZE(data_ring) - sizeof(long)
> 
> However, this is not an appropriate limit due to the nature of
> wrapping data blocks. For example, if a data block is larger than
> half the buffer:
> 
>     size = (DATA_SIZE(data_ring) / 2) + 8
> 
> and begins exactly in the middle of the buffer, then:
> 
>     - the data block will wrap
>     - the ID will be stored at exactly half of the buffer
>     - the record data begins at the beginning of the buffer
>     - the record data ends 8 bytes _past_ exactly half of the buffer
> 
> The record overwrites itself, i.e. needs more space than the full
> buffer!
> 
> Luckily printk() is not vulnerable to this problem because
> truncate_msg() limits printk-messages to 1/4 of the ringbuffer.
> Indeed, by adjusting the printk_ringbuffer KUnit test, which does not
> use printk() and its truncate_msg() check, it is easy to see that the
> ringbuffer becomes corrupted for records larger than half the buffer
> size.
> 
> The corruption occurs because data_push_tail() expects it will never
> be requested to push the tail beyond the head.
> 
> Avoid this problem by adjusting data_check_size() to limit record
> sizes to half the buffer size. Also add WARN_ON_ONCE() before
> relevant data_push_tail() calls to validate that there are no such
> illegal requests. WARN_ON_ONCE() is used, rather than just adding
> extra checks to data_push_tail() because it is considered a bug to
> attempt such illegal actions.
> 
> Link: https://lore.kernel.org/lkml/aMLrGCQSyC8odlFZ@pathway.suse.cz
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

Looks good to me:

Reviewed-by: Petr Mladek <pmladek@suse.com>

The merge window for 6.18 is likely going to start the following week.
I thought about it and I am going to push it there even when
it is this close. The fix is pretty conservative. It gets a lot
of testing using the new kunit test. I am pretty confident
about it.

Best Regards,
Petr