* [PATCH v2 2/9] seq_buf: Do not pop from an overflowed seq_buf
[not found] <20260919002658.stay.929-kees@kernel.org>
@ 2026-09-19 0:27 ` Kees Cook
2026-09-19 0:34 ` sashiko-bot
0 siblings, 1 reply; 2+ messages in thread
From: Kees Cook @ 2026-09-19 0:27 UTC (permalink / raw)
To: Bill Wendling
Cc: Kees Cook, Andy Shevchenko, Günther Noack,
Matthew Wilcox (Oracle), Mickaël Salaün, bpf,
linux-security-module, linux-trace-kernel, Andrew Morton,
David Gow, Masami Hiramatsu, Mathieu Desnoyers, Petr Mladek,
Shuvam Pandey, Steven Rostedt, nikitash.mariiaw, linux-kernel,
linux-hardening
When a seq_buf has overflowed, its len is size + 1, so seq_buf_pop()
decrements len to size and reads buffer[size], one byte past the end of
the buffer. It also leaves len equal to size, which no longer counts as
overflowed, so a truncated seq_buf then looks like a complete, full one.
An overflowed seq_buf logically has no last character to pop: the
length of what was written has been lost, and the last byte of the
buffer may be the NUL written by vsnprintf() or bytes that were never
committed. Return -1 for an overflowed seq_buf, as for an empty one,
and leave it overflowed, as the rest of the seq_buf API does until
seq_buf_clear() or seq_buf_init().
The current callers do not reach this, e.g. trace_syscalls only calls
trace_seq_pop() when the trace_seq it pops from has not overflowed, and
kernel/bpf/diagnostics.c sets the length from strnlen() before popping.
Add tests for the pop corner cases.
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.
Fixes: 32e0f607ac6a2 ("tracing: Add trace_seq_pop() and seq_buf_pop()")
Assisted-by: LLM
Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: "Günther Noack" <gnoack@google.com>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: "Mickaël Salaün" <mic@digikod.net>
Cc: <bpf@vger.kernel.org>
Cc: <linux-security-module@vger.kernel.org>
Cc: <linux-trace-kernel@vger.kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: David Gow <david@davidgow.net>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Shuvam Pandey <shuvampandey1@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
---
include/linux/seq_buf.h | 4 ++--
include/linux/trace_seq.h | 5 ++++-
lib/tests/seq_buf_kunit.c | 42 +++++++++++++++++++++++++++++++++++++++
3 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
index 9f2839e73f8a..f5a350347bc5 100644
--- a/include/linux/seq_buf.h
+++ b/include/linux/seq_buf.h
@@ -155,11 +155,11 @@ static inline void seq_buf_commit(struct seq_buf *s, int num)
*
* Removes the last written character to the seq_buf @s.
*
- * Returns the last character or -1 if it is empty.
+ * Returns the last character, or -1 if @s is empty or has overflowed.
*/
static inline int seq_buf_pop(struct seq_buf *s)
{
- if (!s->len)
+ if (!s->len || seq_buf_has_overflowed(s))
return -1;
s->len--;
diff --git a/include/linux/trace_seq.h b/include/linux/trace_seq.h
index 697d619aafdc..7174ebf3f015 100644
--- a/include/linux/trace_seq.h
+++ b/include/linux/trace_seq.h
@@ -86,7 +86,10 @@ static inline bool trace_seq_has_overflowed(struct trace_seq *s)
*
* Removes the last written character to the trace_seq @s.
*
- * Returns the last character or -1 if it is empty.
+ * Returns the last character, or -1 if the underlying seq_buf is empty or
+ * has overflowed. Note that only that buffer is consulted: a @s marked
+ * full by a write that did not fit, which trace_seq_has_overflowed()
+ * reports as overflowed, still pops the last character written.
*/
static inline int trace_seq_pop(struct trace_seq *s)
{
diff --git a/lib/tests/seq_buf_kunit.c b/lib/tests/seq_buf_kunit.c
index 9ceccdc3029f..d5a0c618b880 100644
--- a/lib/tests/seq_buf_kunit.c
+++ b/lib/tests/seq_buf_kunit.c
@@ -115,6 +115,47 @@ static void seq_buf_putc_test(struct kunit *test)
KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "");
}
+static void seq_buf_pop_test(struct kunit *test)
+{
+ DECLARE_SEQ_BUF(s, 8);
+ struct seq_buf t;
+ char *buf;
+
+ /* Nothing to pop. */
+ KUNIT_EXPECT_EQ(test, seq_buf_pop(&s), -1);
+ KUNIT_EXPECT_EQ(test, s.len, 0);
+
+ seq_buf_puts(&s, "hello");
+ KUNIT_EXPECT_EQ(test, seq_buf_pop(&s), 'o');
+ KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 4);
+ KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hell");
+
+ /* A 0xff byte must not be mistaken for an empty buffer. */
+ seq_buf_putc(&s, 0xff);
+ KUNIT_EXPECT_EQ(test, seq_buf_pop(&s), 0xff);
+
+ /* A full buffer pops its last byte. */
+ seq_buf_puts(&s, "abc");
+ seq_buf_putc(&s, 'd');
+ KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
+ KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 8);
+ KUNIT_EXPECT_EQ(test, seq_buf_pop(&s), 'd');
+ KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 7);
+ KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hellabc");
+
+ /*
+ * An overflowed buffer has nothing to pop, and stays overflowed. Use
+ * a buffer allocated at its exact size, so that KASAN reports any
+ * read past its end.
+ */
+ buf = kunit_kmalloc(test, 16, GFP_KERNEL);
+ KUNIT_ASSERT_NOT_NULL(test, buf);
+ seq_buf_init(&t, buf, 16);
+ KUNIT_EXPECT_EQ(test, seq_buf_printf(&t, "%s", "longer than sixteen"), -1);
+ KUNIT_EXPECT_EQ(test, seq_buf_pop(&t), -1);
+ KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&t));
+}
+
static void seq_buf_printf_test(struct kunit *test)
{
DECLARE_SEQ_BUF(s, 32);
@@ -355,6 +396,7 @@ static struct kunit_case seq_buf_test_cases[] = {
KUNIT_CASE(seq_buf_puts_test),
KUNIT_CASE(seq_buf_puts_overflow_test),
KUNIT_CASE(seq_buf_putc_test),
+ KUNIT_CASE(seq_buf_pop_test),
KUNIT_CASE(seq_buf_printf_test),
KUNIT_CASE(seq_buf_printf_overflow_test),
KUNIT_CASE(seq_buf_get_buf_commit_test),
--
2.34.1
^ permalink raw reply related [flat|nested] 2+ messages in thread