[PATCH 5/7] seq_buf: Use seq_buf_strlen() for the string end in seq_buf_do_printk()

[Kees Cook <kees@kernel.org>]

seq_buf_do_printk() prints whatever follows the last line feed when
"start" is still inside the buffer, and for an overflowed seq_buf,
this may end up pointing at the NUL, so an extra blank line would be
emitted.

Take the end from seq_buf_strlen() instead, which is where the string
ends whether the buffer overflowed, is exactly full, or has room left.

The only caller is the memory cgroup OOM report, so this only ever
added a blank line to a report whose stats did not fit.

Add a test that registers a console to count the records that
seq_buf_do_printk() emits, with a seq_buf filled so that its string
ends in a line feed. It counts only the records carrying the test's
marker and the records holding nothing but a line feed, so that
unrelated kernel messages do not disturb it.

Tests passed under qemu on ARCH=x86_64 with GCC 16.2.0 and CONFIG_KASAN=y,
and on big-endian ARCH=s390 with GCC s390x-linux-gnu 16.1.0.

Assisted-by: LLM
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Shuvam Pandey <shuvampandey1@gmail.com>
Cc: David Gow <david@davidgow.net>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
---
 lib/seq_buf.c             |  5 ++--
 lib/tests/seq_buf_kunit.c | 63 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 66 insertions(+), 2 deletions(-)

diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index 65806d5e4bb4..9977c2a6a315 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -116,12 +116,13 @@ EXPORT_SYMBOL_GPL(seq_buf_printf);
  */
 void seq_buf_do_printk(struct seq_buf *s, const char *lvl)
 {
-	const char *start, *lf;
+	const char *start, *lf, *end;
 
 	if (s->size == 0 || s->len == 0)
 		return;
 
 	start = seq_buf_str(s);
+	end = s->buffer + seq_buf_strlen(s);
 	while ((lf = strchr(start, '\n'))) {
 		int len = lf - start + 1;
 
@@ -130,7 +131,7 @@ void seq_buf_do_printk(struct seq_buf *s, const char *lvl)
 	}
 
 	/* No trailing LF */
-	if (start < s->buffer + s->len)
+	if (start < end)
 		printk("%s%s\n", lvl, start);
 }
 EXPORT_SYMBOL_GPL(seq_buf_do_printk);
diff --git a/lib/tests/seq_buf_kunit.c b/lib/tests/seq_buf_kunit.c
index 2b8cacf000cb..43ca47ffdb7d 100644
--- a/lib/tests/seq_buf_kunit.c
+++ b/lib/tests/seq_buf_kunit.c
@@ -6,6 +6,7 @@
  */
 
 #include <kunit/test.h>
+#include <linux/console.h>
 #include <linux/fs.h>
 #include <linux/seq_buf.h>
 #include <linux/shmem_fs.h>
@@ -431,6 +432,67 @@ static void seq_buf_strlen_puts_overflow_test(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, seq_buf_strlen(&s), strlen(seq_buf_str(&s)));
 }
 
+/*
+ * Counters for the console that seq_buf_do_printk_test() registers while it
+ * runs. Only records carrying the marker, and records holding nothing but a
+ * line feed, are counted, so unrelated kernel messages do not disturb them.
+ */
+#define SEQ_BUF_PRINTK_MARKER	"sbdpkx"
+
+static unsigned int seq_buf_printk_marked;
+static unsigned int seq_buf_printk_empty;
+
+static void seq_buf_printk_capture(struct console *con, const char *s,
+				   unsigned int count)
+{
+	const char *text = s;
+	const char *prefix;
+
+	/* Skip the "[   12.345678] " timestamp, when there is one. */
+	prefix = memchr(s, ']', count);
+	if (prefix && prefix + 2 <= s + count && prefix[1] == ' ')
+		text = prefix + 2;
+	count -= text - s;
+
+	if (count == 0 || (count == 1 && text[0] == '\n'))
+		seq_buf_printk_empty++;
+	else if (strnstr(text, SEQ_BUF_PRINTK_MARKER, count))
+		seq_buf_printk_marked++;
+}
+
+static void seq_buf_do_printk_test(struct kunit *test)
+{
+	static struct console capture = {
+		.name = "sbufcap",
+		.write = seq_buf_printk_capture,
+		.flags = CON_ENABLED,
+		.index = -1,
+	};
+	DECLARE_SEQ_BUF(s, 8);
+
+	/*
+	 * Fill the buffer exactly, so that the NUL takes the place of the
+	 * last byte and the string ends with the line feed before it.
+	 */
+	seq_buf_puts(&s, SEQ_BUF_PRINTK_MARKER);
+	seq_buf_putc(&s, '\n');
+	seq_buf_putc(&s, '!');
+	KUNIT_ASSERT_FALSE(test, seq_buf_has_overflowed(&s));
+	KUNIT_ASSERT_EQ(test, seq_buf_used(&s), 8);
+	KUNIT_ASSERT_EQ(test, seq_buf_strlen(&s), 7);
+
+	seq_buf_printk_marked = 0;
+	seq_buf_printk_empty = 0;
+
+	register_console(&capture);
+	seq_buf_do_printk(&s, KERN_INFO);
+	unregister_console(&capture);
+
+	/* The one line that was written, and nothing after it. */
+	KUNIT_EXPECT_EQ(test, seq_buf_printk_marked, 1);
+	KUNIT_EXPECT_EQ(test, seq_buf_printk_empty, 0);
+}
+
 static struct kunit_case seq_buf_test_cases[] = {
 	KUNIT_CASE(seq_buf_init_test),
 	KUNIT_CASE(seq_buf_declare_test),
@@ -451,6 +513,7 @@ static struct kunit_case seq_buf_test_cases[] = {
 	KUNIT_CASE(seq_buf_strlen_printf_overflow_test),
 	KUNIT_CASE(seq_buf_strlen_full_test),
 	KUNIT_CASE(seq_buf_strlen_puts_overflow_test),
+	KUNIT_CASE(seq_buf_do_printk_test),
 	{}
 };
 
-- 
2.34.1