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.
Showing the changes to kernel (1 of 1 file). Show all
Unified · Split
| @@ -1302,10 +1302,6 @@ static const char *get_data(struct prb_data_ring *data_ring, · 1301 unmodified lines | ||
| 1302 | 1302 | return NULL; |
| 1303 | 1303 | } |
| 1304 | 1304 | |
| 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 | − | |
| 1309 | 1305 | /* A valid data block will always be aligned to the ID size. */ |
| 1310 | 1306 | if (WARN_ON_ONCE(blk_lpos->begin != ALIGN(blk_lpos->begin, sizeof(db->id))) || |
| 1311 | 1307 | 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, · 7 unmodified lines | ||
| 1319 | 1315 | /* Subtract block ID space from size to reflect data size. */ |
| 1320 | 1316 | *data_size -= sizeof(db->id); |
| 1321 | 1317 | |
| 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 | + | |
| 1322 | 1322 | return &db->data[0]; |
| 1323 | 1323 | } |
| 1324 | 1324 | |