All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Ogness <john.ogness@linutronix.de>
To: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <sergey.senozhatsky.work@gmail.com>,
	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>,
	Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel@vger.kernel.org
Subject: [PATCH 1/1] printk: fix syslog_print_all() 1024-byte edge case
Date: Wed, 20 Jan 2021 20:47:06 +0106	[thread overview]
Message-ID: <20210120194106.26441-2-john.ogness@linutronix.de> (raw)
In-Reply-To: <20210120194106.26441-1-john.ogness@linutronix.de>

If klogctl(SYSLOG_ACTION_READ_ALL) is called with a buffer size
of 1024 and the message data will exactly fill 1024 bytes and the
last message of that is multi-line, the last line of the last
message will be silently dropped.

This is because syslog_print_all() is assuming record_print_text()
will fill @size bytes and chooses the first record on this basis.
But since record_print_text() only fills @size-1 bytes, it will
truncate the last message. This behavior exists since the
introduction of msg_print_text() in commit 3ce9a7c0ac28 ("printk() -
restore prefix/timestamp printing for multi-newline strings").

SYSLOG_ACTION_READ_ALL is only supposed to print full messages, so
the expected behavior would be to drop the full multi-line message.
Fix this edge case by changing syslog_print_all() to correctly
choose the first message with the knowledge that
record_print_text() will only fill up to @size-1 bytes.

To test the syslog interface, a simple "kmsg" tool was written to
call klogctl() based on provided parameters. The tool prints the
syslog data it read and the size returned by klogctl().

A wrapper script was used to generate a single multi-line test
message and run kmsg.

----- BEGIN syslog-1024-test.sh -----
 #!/bin/sh

 msg=""
 for i in `seq $1`; do msg="$msg."; done
 msg="$msg\nhello1"
 msg="$msg\nhello2"

 dmesg -c > /dev/null
 echo -e "$msg" > /dev/kmsg
 ./kmsg SYSLOG_ACTION_READ_ALL 1024
----- END syslog-1024-test.sh -----

When $1 is 928, the syslog data will be 1024 bytes and trigger the
edge case (assuming early timestamps and CALLER_ID enabled).

BEFORE this commit:

 # ./syslog-1024-test.sh 927
 kmsg: klogctl(SYSLOG_ACTION_READ_ALL) with buffer of 1024 bytes
 <12>[  115.933677][ T1138] ................
 <12>[  115.933677][ T1138] hello1
 <12>[  115.933677][ T1138] hello2
 read 1023 bytes from klogctl

 # ./syslog-1024-test.sh 928
 kmsg: klogctl(SYSLOG_ACTION_READ_ALL) with buffer of 1024 bytes
 <12>[  124.834804][ T1143] ................
 <12>[  124.834804][ T1143] hello1
 read 990 bytes from klogctl

 # ./syslog-1024-test.sh 929
 kmsg: klogctl(SYSLOG_ACTION_READ_ALL) with buffer of 1024 bytes
 read 0 bytes from klogctl

AFTER this commit:

 # ./syslog-1024-test.sh 927
 kmsg: klogctl(SYSLOG_ACTION_READ_ALL) with buffer of 1024 bytes
 <12>[   43.079094][ T1096] ................
 <12>[   43.079094][ T1096] hello1
 <12>[   43.079094][ T1096] hello2
 read 1023 bytes from klogctl

 # ./syslog-1024-test.sh 928
 kmsg: klogctl(SYSLOG_ACTION_READ_ALL) with buffer of 1024 bytes
 read 0 bytes from klogctl

 # ./syslog-1024-test.sh 929
 kmsg: klogctl(SYSLOG_ACTION_READ_ALL) with buffer of 1024 bytes
 read 0 bytes from klogctl

Fixes: 3ce9a7c0ac28 ("printk() - restore prefix/timestamp printing for multi-newline strings")
Signed-off-by: John Ogness <john.ogness@linutronix.de>
---
 kernel/printk/printk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/kernel/printk/printk.c b/kernel/printk/printk.c
index 6639a0cfe0ac..b640d34e0351 100644
--- a/kernel/printk/printk.c
+++ b/kernel/printk/printk.c
@@ -1513,7 +1513,7 @@ static int syslog_print_all(char __user *buf, int size, bool clear)
 
 	/* move first record forward until length fits into the buffer */
 	prb_for_each_info(clear_seq, prb, seq, &info, &line_count) {
-		if (len <= size)
+		if (len < size)
 			break;
 		len -= get_record_print_text_size(&info, line_count, true, time);
 	}
-- 
2.20.1


  reply	other threads:[~2021-01-20 19:48 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-20 19:41 [PATCH 0/1] another size-1 fixup from msg_print_text() John Ogness
2021-01-20 19:41 ` John Ogness [this message]
2021-01-21  5:43   ` [PATCH 1/1] printk: fix syslog_print_all() 1024-byte edge case John Ogness

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210120194106.26441-2-john.ogness@linutronix.de \
    --to=john.ogness@linutronix.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=pmladek@suse.com \
    --cc=rostedt@goodmis.org \
    --cc=sergey.senozhatsky.work@gmail.com \
    --cc=sergey.senozhatsky@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.