[PATCH printk v2 1/2] printk_ringbuffer: Fix get_data() size sanity check

[PATCH printk v2 1/2] printk_ringbuffer: Fix get_data() size sanity check

[John Ogness]

Commit cc3bad11de6e ("printk_ringbuffer: Fix check of valid data
size when blk_lpos overflows") added sanity checking to get_data()
to avoid returning data of illegal sizes (too large or too small).
It uses the helper function data_check_size() for the check.
However, data_check_size() expects the size of the data, not the
size of the data block. get_data() is providing the size of the
data block.  This means that if the data size (text_buf_size) is
at or near the maximum legal size:

sizeof(prb_data_block) + text_buf_size == DATA_SIZE(data_ring) / 2

data_check_size() will report failure because it adds
sizeof(prb_data_block) to the provided size. The sanity check in
get_data() is counting the data block header twice. The result is
that the reader fails to read the legal record.

Since get_data() subtracts the data block header size before returning,
move the sanity check to after the subtraction.

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
reader fails and the WARN_ON is triggered.

Fixes: cc3bad11de6e ("printk_ringbuffer: Fix check of valid data size when blk_lpos overflows")
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 kernel/printk/printk_ringbuffer.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c
index 56c8e3d031f49..a3526bdd4e10d 100644
--- a/kernel/printk/printk_ringbuffer.c
+++ b/kernel/printk/printk_ringbuffer.c
@@ -1302,10 +1302,6 @@ 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)))) {
@@ -1319,6 +1315,10 @@ static const char *get_data(struct prb_data_ring *data_ring,
 	/* 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];
 }
 

base-commit: 9095f233c0258e9a05e958c7d822eb38681e7a5a
-- 
2.47.3

[PATCH printk v2 2/2] printk_ringbuffer: Add sanity check for 0-size data

[John Ogness]

get_data() has a sanity check for regular data blocks to ensure at
least space for the ID exists. But a regular block should also have
at least 1 byte of data (otherwise it would be data-less instead of
regular).

Expand the get_data() block size sanity check to additionally expect
at least 1 byte of data.

Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 kernel/printk/printk_ringbuffer.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c
index a3526bdd4e10d..aa4b39e94cfa2 100644
--- a/kernel/printk/printk_ringbuffer.c
+++ b/kernel/printk/printk_ringbuffer.c
@@ -1308,8 +1308,11 @@ static const char *get_data(struct prb_data_ring *data_ring,
 		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. */
-- 
2.47.3

Re: [PATCH printk v2 1/2] printk_ringbuffer: Fix get_data() size sanity check

[Petr Mladek]

On Thu 2026-03-26 14:44:01, John Ogness wrote:
> Commit cc3bad11de6e ("printk_ringbuffer: Fix check of valid data
> size when blk_lpos overflows") added sanity checking to get_data()
> to avoid returning data of illegal sizes (too large or too small).
> It uses the helper function data_check_size() for the check.
> However, data_check_size() expects the size of the data, not the
> size of the data block. get_data() is providing the size of the
> data block.  This means that if the data size (text_buf_size) is
> at or near the maximum legal size:
> 
> sizeof(prb_data_block) + text_buf_size == DATA_SIZE(data_ring) / 2
> 
> data_check_size() will report failure because it adds
> sizeof(prb_data_block) to the provided size. The sanity check in
> get_data() is counting the data block header twice. The result is
> that the reader fails to read the legal record.
> 
> Since get_data() subtracts the data block header size before returning,
> move the sanity check to after the subtraction.
> 
> 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
> reader fails and the WARN_ON is triggered.
> 
> Fixes: cc3bad11de6e ("printk_ringbuffer: Fix check of valid data size when blk_lpos overflows")
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

Great catch. Looks good to me.

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

Best Regards,
Petr

Re: [PATCH printk v2 2/2] printk_ringbuffer: Add sanity check for 0-size data

[Petr Mladek]

On Thu 2026-03-26 14:44:02, John Ogness wrote:
> get_data() has a sanity check for regular data blocks to ensure at
> least space for the ID exists. But a regular block should also have
> at least 1 byte of data (otherwise it would be data-less instead of
> regular).
> 
> Expand the get_data() block size sanity check to additionally expect
> at least 1 byte of data.
> 
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

Looks good to me. It complements the 1st patch.

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

Best Regards,
Petr

Re: [PATCH printk v2 1/2] printk_ringbuffer: Fix get_data() size sanity check

[Petr Mladek]

On Thu 2026-03-26 14:44:01, John Ogness wrote:
> Commit cc3bad11de6e ("printk_ringbuffer: Fix check of valid data
> size when blk_lpos overflows") added sanity checking to get_data()
> to avoid returning data of illegal sizes (too large or too small).
> It uses the helper function data_check_size() for the check.
> However, data_check_size() expects the size of the data, not the
> size of the data block. get_data() is providing the size of the
> data block.  This means that if the data size (text_buf_size) is
> at or near the maximum legal size:
> 
> sizeof(prb_data_block) + text_buf_size == DATA_SIZE(data_ring) / 2
> 
> data_check_size() will report failure because it adds
> sizeof(prb_data_block) to the provided size. The sanity check in
> get_data() is counting the data block header twice. The result is
> that the reader fails to read the legal record.
> 
> Since get_data() subtracts the data block header size before returning,
> move the sanity check to after the subtraction.
> 
> 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
> reader fails and the WARN_ON is triggered.
> 
> Fixes: cc3bad11de6e ("printk_ringbuffer: Fix check of valid data size when blk_lpos overflows")
> Signed-off-by: John Ogness <john.ogness@linutronix.de>

JFYI, both patches have been comitted into printk/linux.git,
branch rework/prb-fixes.

They are queued for the next merge window (7.1).

Best Regards,
Petr