Skip to content

Commit

Permalink
printk: fix buffer overflow potential for print_text()
Browse files Browse the repository at this point in the history
commit f0e386e upstream.

Before the commit 896fbe2 ("printk: use the lockless
ringbuffer"), msg_print_text() would only write up to size-1 bytes
into the provided buffer. Some callers expect this behavior and
append a terminator to returned string. In particular:

arch/powerpc/xmon/xmon.c:dump_log_buf()
arch/um/kernel/kmsg_dump.c:kmsg_dumper_stdout()

msg_print_text() has been replaced by record_print_text(), which
currently fills the full size of the buffer. This causes a
buffer overflow for the above callers.

Change record_print_text() so that it will only use size-1 bytes
for text data. Also, for paranoia sakes, add a terminator after
the text data.

And finally, document this behavior so that it is clear that only
size-1 bytes are used and a terminator is added.

Fixes: 896fbe2 ("printk: use the lockless ringbuffer")
Cc: stable@vger.kernel.org # 5.10+
Signed-off-by: John Ogness <john.ogness@linutronix.de>
Reviewed-by: Petr Mladek <pmladek@suse.com>
Acked-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: Petr Mladek <pmladek@suse.com>
Link: https://lore.kernel.org/r/20210114170412.4819-1-john.ogness@linutronix.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
jogness authored and gregkh committed Jan 30, 2021
1 parent cb14bbb commit 861c2e3
Showing 1 changed file with 27 additions and 9 deletions.
36 changes: 27 additions & 9 deletions kernel/printk/printk.c
Expand Up @@ -1338,11 +1338,16 @@ static size_t info_print_prefix(const struct printk_info *info, bool syslog,
* done:
*
* - Add prefix for each line.
* - Drop truncated lines that no longer fit into the buffer.
* - Add the trailing newline that has been removed in vprintk_store().
* - Drop truncated lines that do not longer fit into the buffer.
* - Add a string terminator.
*
* Since the produced string is always terminated, the maximum possible
* return value is @r->text_buf_size - 1;
*
* Return: The length of the updated/prepared text, including the added
* prefixes and the newline. The dropped line(s) are not counted.
* prefixes and the newline. The terminator is not counted. The dropped
* line(s) are not counted.
*/
static size_t record_print_text(struct printk_record *r, bool syslog,
bool time)
Expand Down Expand Up @@ -1385,26 +1390,31 @@ static size_t record_print_text(struct printk_record *r, bool syslog,

/*
* Truncate the text if there is not enough space to add the
* prefix and a trailing newline.
* prefix and a trailing newline and a terminator.
*/
if (len + prefix_len + text_len + 1 > buf_size) {
if (len + prefix_len + text_len + 1 + 1 > buf_size) {
/* Drop even the current line if no space. */
if (len + prefix_len + line_len + 1 > buf_size)
if (len + prefix_len + line_len + 1 + 1 > buf_size)
break;

text_len = buf_size - len - prefix_len - 1;
text_len = buf_size - len - prefix_len - 1 - 1;
truncated = true;
}

memmove(text + prefix_len, text, text_len);
memcpy(text, prefix, prefix_len);

/*
* Increment the prepared length to include the text and
* prefix that were just moved+copied. Also increment for the
* newline at the end of this line. If this is the last line,
* there is no newline, but it will be added immediately below.
*/
len += prefix_len + line_len + 1;

if (text_len == line_len) {
/*
* Add the trailing newline removed in
* vprintk_store().
* This is the last line. Add the trailing newline
* removed in vprintk_store().
*/
text[prefix_len + line_len] = '\n';
break;
Expand All @@ -1429,6 +1439,14 @@ static size_t record_print_text(struct printk_record *r, bool syslog,
text_len -= line_len + 1;
}

/*
* If a buffer was provided, it will be terminated. Space for the
* string terminator is guaranteed to be available. The terminator is
* not counted in the return value.
*/
if (buf_size > 0)
text[len] = 0;

return len;
}

Expand Down

0 comments on commit 861c2e3

Please sign in to comment.