Counting text lines in a record simply involves counting the number
of newline characters (+1). However, it is searching the full data
block for newline characters, even though the text data can be (and
often is) a subset of that area. Since the extra area in the data
block was never initialized, the result is that extra newlines may
be seen and counted.
Restrict newline searching to the text data length.
Fixes: b6cf8b3f3312 ("printk: add lockless ringbuffer")
Signed-off-by: John Ogness <[email protected]>
---
kernel/printk/printk_ringbuffer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c
index 6704f06e0417..8a7b7362c0dd 100644
--- a/kernel/printk/printk_ringbuffer.c
+++ b/kernel/printk/printk_ringbuffer.c
@@ -1718,7 +1718,7 @@ static bool copy_data(struct prb_data_ring *data_ring,
/* Caller interested in the line count? */
if (line_count)
- *line_count = count_lines(data, data_size);
+ *line_count = count_lines(data, len);
/* Caller interested in the data content? */
if (!buf || !buf_size)
--
2.20.1
On Wed 2021-01-13 15:48:34, John Ogness wrote:
> Counting text lines in a record simply involves counting the number
> of newline characters (+1). However, it is searching the full data
> block for newline characters, even though the text data can be (and
> often is) a subset of that area. Since the extra area in the data
> block was never initialized, the result is that extra newlines may
> be seen and counted.
Great catch!
> Restrict newline searching to the text data length.
>
> Fixes: b6cf8b3f3312 ("printk: add lockless ringbuffer")
> Signed-off-by: John Ogness <[email protected]>
Reviewed-by: Petr Mladek <[email protected]>
There is a note below.
> ---
> kernel/printk/printk_ringbuffer.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c
> index 6704f06e0417..8a7b7362c0dd 100644
> --- a/kernel/printk/printk_ringbuffer.c
> +++ b/kernel/printk/printk_ringbuffer.c
> @@ -1718,7 +1718,7 @@ static bool copy_data(struct prb_data_ring *data_ring,
>
> /* Caller interested in the line count? */
> if (line_count)
> - *line_count = count_lines(data, data_size);
> + *line_count = count_lines(data, len);
>
> /* Caller interested in the data content? */
> if (!buf || !buf_size)
Another question is what line count should be returned when
the data are copied into the buffer. In this case, the text
might get shrunken even more.
Well, this case is not supported by the API at the moment.
@line_count is defined only in prb_read_valid_info() where
the buffer is always NULL.
But we might add a WARN_ONCE() or a comment there to prevent
similar mistakes in the future.
Best Regards,
Petr
On 2021-01-14, Petr Mladek <[email protected]> wrote:
>> --- a/kernel/printk/printk_ringbuffer.c
>> +++ b/kernel/printk/printk_ringbuffer.c
>> @@ -1718,7 +1718,7 @@ static bool copy_data(struct prb_data_ring *data_ring,
>>
>> /* Caller interested in the line count? */
>> if (line_count)
>> - *line_count = count_lines(data, data_size);
>> + *line_count = count_lines(data, len);
>>
>> /* Caller interested in the data content? */
>> if (!buf || !buf_size)
>
> Another question is what line count should be returned when
> the data are copied into the buffer. In this case, the text
> might get shrunken even more.
Good point. The code could look like this:
if (!buf || !buf_size) {
data_size = len;
} else {
data_size = min_t(u16, buf_size, len);
memcpy(&buf[0], data, data_size);
}
if (line_count)
*line_count = count_lines(data, data_size);
return true;
John Ogness
On (21/01/13 15:48), John Ogness wrote:
>
> Counting text lines in a record simply involves counting the number
> of newline characters (+1). However, it is searching the full data
> block for newline characters, even though the text data can be (and
> often is) a subset of that area. Since the extra area in the data
> block was never initialized, the result is that extra newlines may
> be seen and counted.
>
> Restrict newline searching to the text data length.
>
> Fixes: b6cf8b3f3312 ("printk: add lockless ringbuffer")
> Signed-off-by: John Ogness <[email protected]>
Acked-by: Sergey Senozhatsky <[email protected]>
-ss
On Wed 2021-01-13 15:48:34, John Ogness wrote:
> Counting text lines in a record simply involves counting the number
> of newline characters (+1). However, it is searching the full data
> block for newline characters, even though the text data can be (and
> often is) a subset of that area. Since the extra area in the data
> block was never initialized, the result is that extra newlines may
> be seen and counted.
>
> Restrict newline searching to the text data length.
>
> Fixes: b6cf8b3f3312 ("printk: add lockless ringbuffer")
> Signed-off-by: John Ogness <[email protected]>
The patch is committed in printk/linux.git, branch printk-rework.
I plan to send it for-5.11 the following week after it spends
few days in linux-next.
Best Regards,
Petr