Re: [PATCH printk v3 02/14] printk: Adjust mapping for 32bit seq macros

From: John Ogness
Date: Mon Jan 15 2024 - 12:09:09 EST


On 2024-01-15, Petr Mladek <pmladek@xxxxxxxx> wrote:
> You know, the code around reading the messages is getting more
> and more complex. Also the u32 -> u64 transition is not 100% safe.
> So some sanity checks might be useful.

This complexity only exists for 32bit systems, but yeah, it is still
important.

> That said, I do not see any obvious trivial one. Just the following
> came to my mind. prb_first_seq() is reliable and we could do:
>
> void next_seq_sanity_check(struct printk_ringbuffer *rb, u64 next_seq)
> {
> struct prb_desc_ring *desc_ring = &rb->desc_ring;
> u64 first_seq = prb_first_seq(rb);
> u64 desc_count = DESC_COUNT(&rb->desc_ring)).
>
> WARN_ONCE(seq > first_seq + DESC_COUNT(desc_ring));
> }

@seq is allowed to be 2^31 before or after @first_seq. The check would
look more like this:

WARN_ONCE((rb_first_seq < 0xffffffff80000000 &&
seq > rb_first_seq + 0x80000000) ||
(rb_first_seq > 0x80000000 &&
seq < rb_first_seq - 0x80000000));

> Well, I am not sure if it is worth it. Also the WARN() won't be
> printed on consoles when the reading is broken.

Broken printing is irrelevant. There are plenty of debug methods to get
at the ringbuffer. I am OK with adding this sanity check (again, only
for 32bit, within __ulseq_to_u64seq()).

John