Before commit b6cf8b3f3312 ("printk: add 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: b6cf8b3f3312 ("printk: add lockless ringbuffer")
Signed-off-by: John Ogness <john.ogn...@linutronix.de>
---
 kernel/printk/printk.c | 35 +++++++++++++++++++++++++++--------
 1 file changed, 27 insertions(+), 8 deletions(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index ffdd0dc7ec6d..73f9eae19f05 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1293,9 +1293,15 @@ static size_t info_print_prefix(const struct printk_info 
 *info, bool syslog,
  *   - Add prefix for each line.
  *   - Add the trailing newline that has been removed in vprintk_store().
  *   - Drop truncated lines that do not longer fit into the buffer.
+ *   - Add a trailing newline.
+ *   - 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)
@@ -1338,26 +1344,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;
@@ -1382,6 +1393,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;
 }
 
-- 
2.20.1

Reply via email to