Re: [PATCH printk] printk_ringbuffer: Fix get_data() size sanity check

[John Ogness]

On 2026-03-26, Petr Mladek <pmladek@xxxxxxxx> wrote:
> So, just to be sure that the new code works as expected.
> Note that the moved check:
>
> 	/* Sanity check. Data-less blocks were handled earlier. */
> 	if (WARN_ON_ONCE(!data_check_size(data_ring, *data_size) || !*data_size))
> 		return NULL;
>
> warns only when data_check_size() fails. It just quietly returns NULL
> when *data_size is zero.

??? The above code warns for !*data_size as well.

> If we really want to warn. Then it would make more sense to change "<"
> to "<=" in the previous check before the subtraction.

Yes, actually I would prefer that. Let me send a v2 where I relocate and
reduce the data_check_size()-WARN and extend the data_size-WARN. So it
is something like this:

diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c
index 56c8e3d031f49..aa4b39e94cfa2 100644
--- a/kernel/printk/printk_ringbuffer.c
+++ b/kernel/printk/printk_ringbuffer.c
@@ -1302,23 +1302,26 @@ static const char *get_data(struct prb_data_ring *data_ring,
 		return NULL;
 	}
 
-	/* Sanity check. Data-less blocks were handled earlier. */
-	if (WARN_ON_ONCE(!data_check_size(data_ring, *data_size) || !*data_size))
-		return NULL;
-
 	/* A valid data block will always be aligned to the ID size. */
 	if (WARN_ON_ONCE(blk_lpos->begin != ALIGN(blk_lpos->begin, sizeof(db->id))) ||
 	    WARN_ON_ONCE(blk_lpos->next != ALIGN(blk_lpos->next, sizeof(db->id)))) {
 		return NULL;
 	}
 
-	/* A valid data block will always have at least an ID. */
-	if (WARN_ON_ONCE(*data_size < sizeof(db->id)))
+	/*
+	 * A regular data block will always have an ID and at least
+	 * 1 byte of data. Data-less blocks were handled earlier.
+	 */
+	if (WARN_ON_ONCE(*data_size <= sizeof(db->id)))
 		return NULL;
 
 	/* Subtract block ID space from size to reflect data size. */
 	*data_size -= sizeof(db->id);
 
+	/* Sanity check the max size of the regular data block. */
+	if (WARN_ON_ONCE(!data_check_size(data_ring, *data_size)))
+		return NULL;
+
 	return &db->data[0];
 }
 
John