Skip to content

Commit 8e81ecb

Browse files
jognesspmladek
authored andcommitted
printk_ringbuffer: Fix get_data() size sanity check
Commit cc3bad1 ("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: cc3bad1 ("printk_ringbuffer: Fix check of valid data size when blk_lpos overflows") Signed-off-by: John Ogness <[email protected]> Reviewed-by: Petr Mladek <[email protected]> Tested-by: Petr Mladek <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Petr Mladek <[email protected]>
1 parent 9095f23 commit 8e81ecb

1 file changed

Lines changed: 4 additions & 4 deletions

File tree

kernel/printk/printk_ringbuffer.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1302,10 +1302,6 @@ static const char *get_data(struct prb_data_ring *data_ring,
13021302
return NULL;
13031303
}
13041304

1305-
/* Sanity check. Data-less blocks were handled earlier. */
1306-
if (WARN_ON_ONCE(!data_check_size(data_ring, *data_size) || !*data_size))
1307-
return NULL;
1308-
13091305
/* A valid data block will always be aligned to the ID size. */
13101306
if (WARN_ON_ONCE(blk_lpos->begin != ALIGN(blk_lpos->begin, sizeof(db->id))) ||
13111307
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,
13191315
/* Subtract block ID space from size to reflect data size. */
13201316
*data_size -= sizeof(db->id);
13211317

1318+
/* Sanity check the max size of the regular data block. */
1319+
if (WARN_ON_ONCE(!data_check_size(data_ring, *data_size)))
1320+
return NULL;
1321+
13221322
return &db->data[0];
13231323
}
13241324

0 commit comments

Comments
 (0)