From b7e688b5635a0888613f4d620eeb7440e2f23a7e Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 5 Oct 2026 08:56:52 -0700 Subject: seq_buf: Do not pop from an overflowed seq_buf 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 Link: https://patch.msgid.link/20261005155708.1471260-2-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/seq_buf.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'include/linux/seq_buf.h') 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--; -- cgit v1.2.3 From 505a4aeea2c98ab9b09bc157e81e19cc0d4bf9cb Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 5 Oct 2026 08:56:54 -0700 Subject: seq_buf: Copy what fits when seq_buf_puts() and seq_buf_putmem() overflow When seq_buf_puts() or seq_buf_putmem() is given more than fits, it copies nothing and only marks the seq_buf as overflowed. If seq_buf_str() is used, it will terminate the buffer in its last byte, so every byte between the end of the data and the end of the buffer becomes part of the string, though the seq_buf never wrote them. seq_buf_printf() does not have this problem, because vsnprintf() writes as much of the output as fits, followed by a NUL. Repeat this behavior in seq_buf_puts(), using strscpy(), and in seq_buf_putmem(), which also covers seq_buf_putmem_hex(). seq_buf_putc() needs no change, as it can only overflow when the buffer is already full. Each writer now records the buffer as full once it has copied what fits, so that what it wrote can be told apart from bytes nothing touched. Of the callers that check the return value or test for overflow afterward, only lpfc_rx_monitor_report() relied on a string that did not fit being left out, and the previous patch has it drop such a record itself. The others treat overflow as an error or report it. The callers that ignore overflow use whatever the buffer holds, which now ends with the part of the string that fit. Update seq_buf_putmem_hex_overflow_test, which expected a hex group that did not fit whole to be left out entirely, and add tests that overflow seq_buf_puts(), seq_buf_putmem() and seq_buf_putmem_hex() with stale bytes in the buffer. The three partial-overflow tests check the buffer itself rather than seq_buf_str(). Neither writer leaves the last byte of the buffer alone: seq_buf_putmem() copies raw bytes and writes no NUL, and seq_buf_puts() relies on strscpy() to write one. That byte is exactly where seq_buf_str() writes its terminator, so asserting only on the string would pass whether the copy stopped a byte early or dropped the NUL entirely. seq_buf_putmem() was not exported, so the test failed to link as a module. Export 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.2.0, and the test builds as a module (CONFIG_SEQ_BUF_KUNIT_TEST=m). Assisted-by: LLM Reviewed-by: Andy Shevchenko Link: https://patch.msgid.link/20261005155708.1471260-4-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/seq_buf.h | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'include/linux/seq_buf.h') diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h index f5a350347bc5..77e76e283370 100644 --- a/include/linux/seq_buf.h +++ b/include/linux/seq_buf.h @@ -55,6 +55,12 @@ seq_buf_has_overflowed(struct seq_buf *s) return s->len > s->size; } +/* + * Mark @s as overflowed, which discards the length of what it holds. The + * bytes up to its last one are the string from then on, as that is where + * seq_buf_str() terminates it, so a caller that could not fill the buffer + * has to NUL them itself before calling this. + */ static inline void seq_buf_set_overflow(struct seq_buf *s) { -- cgit v1.2.3 From ea5666e56723e9ac7094ff9849cf154dcb0e436e Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 5 Oct 2026 08:56:55 -0700 Subject: seq_buf: Clear what a writer did not claim when a seq_buf overflows Using seq_buf_set_overflow() would leave the bytes between "len" and "size" untouched, so if seq_buf_str() is used on an overflowed seq_buf, those bytes may be exposed. For any paths that don't claim partially written bytes, by setting "len = size" before calling seq_buf_set_overflow(), wipe the unclaimed bytes. The seq_buf_puts() and related APIs already claim those bytes now, so only the unclaimed cases remain. A specific example of this was seq_buf_path() which uses d_path() and would write to the tail before discovering it was out of space, and would correctly mark a seq_buf as overflowed, but the path fragment would be left over. Clear from len to the end of the buffer in seq_buf_set_overflow(), which every overflow goes through, including seq_buf_commit() with a negative count. Add a test that fills a seq_buf, leaves it too little room for a path, and checks that nothing of the path is left in the buffer. The tests run before anything writable is mounted, so it takes its file from shmem. seq_buf_path() was not exported, so the test failed to link as a module. Export 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.2.0, and the test builds as a module (CONFIG_SEQ_BUF_KUNIT_TEST=m). Assisted-by: LLM Reviewed-by: Andy Shevchenko Link: https://patch.msgid.link/20261005155708.1471260-5-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/seq_buf.h | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'include/linux/seq_buf.h') diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h index 77e76e283370..0c0a0db04b09 100644 --- a/include/linux/seq_buf.h +++ b/include/linux/seq_buf.h @@ -5,6 +5,7 @@ #include #include #include +#include #include /* @@ -58,12 +59,15 @@ seq_buf_has_overflowed(struct seq_buf *s) /* * Mark @s as overflowed, which discards the length of what it holds. The * bytes up to its last one are the string from then on, as that is where - * seq_buf_str() terminates it, so a caller that could not fill the buffer - * has to NUL them itself before calling this. + * seq_buf_str() terminates it, so clear whatever was not written: a writer + * sets len to how much it filled, and anything past that was never adopted. */ static inline void seq_buf_set_overflow(struct seq_buf *s) { + if (s->len < s->size) + memset(s->buffer + s->len, 0, s->size - s->len); + s->len = s->size + 1; } -- cgit v1.2.3 From 8d1478bfd63be0f065defd1066d2fa294f419ae2 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 5 Oct 2026 08:56:56 -0700 Subject: seq_buf: Add seq_buf_strlen() Several strlcat() call sites being converted to seq_buf need behavior seq_buf doesn't currently provide. The return from seq_buf_used() is not the length of the string in a seq_buf. Once the buffer is full or has overflowed it returns the buffer size, which counts the byte that seq_buf_str() replaces with the NUL, so a caller that needs the string and its length has to call seq_buf_str() and then walk the string with strlen(). Move the termination out of seq_buf_str() into a helper that returns where it put the NUL, and add seq_buf_strlen(), which terminates the buffer in the same way and returns that offset. As discussed in review, don't add WARN_ON() for seq_buf_strlen() and drop it from seq_buf_str(). Add tests comparing seq_buf_strlen() against strlen() of seq_buf_str() for empty, appended, truncated, exactly full, and overflowed buffers, checking that seq_buf_strlen() alone terminates a full buffer, and checking that a zero-sized seq_buf reports an empty string from both accessors without touching the buffer. 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.2.0. Assisted-by: LLM Reviewed-by: Andy Shevchenko Link: https://patch.msgid.link/20261005155708.1471260-6-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/seq_buf.h | 67 ++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 61 insertions(+), 6 deletions(-) (limited to 'include/linux/seq_buf.h') diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h index 0c0a0db04b09..7abeca1c166b 100644 --- a/include/linux/seq_buf.h +++ b/include/linux/seq_buf.h @@ -89,6 +89,27 @@ static inline unsigned int seq_buf_used(struct seq_buf *s) return min(s->len, s->size); } +/* + * NUL-terminate the buffer in @s: directly after the data when there is + * room for it, otherwise in the last byte of the buffer. @s->size must not + * be zero. + * + * Returns: the offset of the NUL. + */ +static inline size_t __seq_buf_terminate(struct seq_buf *s) +{ + size_t end; + + if (seq_buf_buffer_left(s)) + end = s->len; + else + end = s->size - 1; + + s->buffer[end] = 0; + + return end; +} + /** * seq_buf_str - get NUL-terminated C string from seq_buf * @s: the seq_buf handle @@ -98,7 +119,14 @@ static inline unsigned int seq_buf_used(struct seq_buf *s) * * Note, if this is called when the buffer has overflowed, then * the last byte of the buffer is zeroed, and the len will still - * point passed it. + * point passed it. The same happens when the buffer is exactly + * full: the NUL takes the place of the last byte written, which is + * lost, though seq_buf_used() still counts it. + * + * A zero-sized seq_buf has nowhere to put a NUL, so the empty string + * is returned instead of writing to @s->buffer. Any other seq_buf + * returns @s->buffer, even when it holds an empty string, so callers + * always get their own buffer back. * * After this function is called, s->buffer is safe to use * in string operations. @@ -107,17 +135,44 @@ static inline unsigned int seq_buf_used(struct seq_buf *s) */ static inline const char *seq_buf_str(struct seq_buf *s) { - if (WARN_ON(s->size == 0)) + if (s->size == 0) return ""; - if (seq_buf_buffer_left(s)) - s->buffer[s->len] = 0; - else - s->buffer[s->size - 1] = 0; + __seq_buf_terminate(s); return s->buffer; } +/** + * seq_buf_strlen - get the length of the NUL-terminated C string in seq_buf + * @s: the seq_buf handle + * + * This makes sure that the buffer in @s is NUL-terminated, exactly as + * seq_buf_str() does, and returns the length of the resulting string + * without walking it. Unlike seq_buf_used(), this does not count the byte + * given up to the NUL when the buffer is full or has overflowed. When the + * buffer is exactly full, that byte is the last one written, and calling + * either function loses it. + * + * A zero-sized seq_buf holds no string, so 0 is returned without writing + * to @s->buffer, matching what seq_buf_str() returns for one. + * + * After this function is called, s->buffer is safe to use + * in string operations. + * + * Returns: the offset of the NUL that terminates @s->buffer. That is the + * length of the string unless an earlier NUL is in the way, either one the + * data written to @s carried itself, or one seq_buf_set_overflow() left + * behind when it cleared what no writer had claimed. + */ +static inline size_t seq_buf_strlen(struct seq_buf *s) +{ + if (s->size == 0) + return 0; + + return __seq_buf_terminate(s); +} + /** * seq_buf_get_buf - get buffer to write arbitrary data to * @s: the seq_buf handle -- cgit v1.2.3 From 88c652df02ccf9e7644c012f8375fec24d848be3 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 5 Oct 2026 08:56:57 -0700 Subject: seq_buf: Add seq_buf_terminate() Seven callers call seq_buf_str() only to NUL-terminate the buffer, discarding the returned pointer. Two of them need a comment to say so. Add seq_buf_terminate(), wrapping the __seq_buf_terminate() helper that seq_buf_str() and seq_buf_strlen() already use, and convert those callers. It returns void: returning the offset would just be seq_buf_strlen() under another name. A zero-sized seq_buf is left untouched, as in the other accessors. Add tests for the three cases: room for the NUL after the data, an overflowed buffer where it lands in the last byte, and a zero-sized buffer that must not be written to. Build tested ARCH=x86_64 defconfig with GCC 16.2.0, plus CONFIG_HIST_TRIGGERS=y and CONFIG_BPF_SYSCALL=y to reach the converted call sites in kernel/trace/trace_events_hist.c and kernel/bpf/diagnostics.c. Tests run 24/24 passing on ARCH=um. Assisted-by: LLM Acked-by: Alexei Starovoitov Link: https://patch.msgid.link/20261005155708.1471260-7-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/seq_buf.h | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) (limited to 'include/linux/seq_buf.h') diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h index 7abeca1c166b..9830f905f0ee 100644 --- a/include/linux/seq_buf.h +++ b/include/linux/seq_buf.h @@ -173,6 +173,29 @@ static inline size_t seq_buf_strlen(struct seq_buf *s) return __seq_buf_terminate(s); } +/** + * seq_buf_terminate - NUL-terminate the string in a seq_buf + * @s: the seq_buf handle + * + * Terminate @s->buffer exactly as seq_buf_str() and seq_buf_strlen() do, + * for callers that want neither the pointer nor the length and only need + * the buffer to be safe to read as a C string. A zero-sized seq_buf has + * nowhere to put a NUL and is left untouched. + * + * Nothing is returned on purpose: a caller that wants the length should + * use seq_buf_strlen(), which says so. + * + * After this function is called, s->buffer is safe to use + * in string operations. + */ +static inline void seq_buf_terminate(struct seq_buf *s) +{ + if (s->size == 0) + return; + + __seq_buf_terminate(s); +} + /** * seq_buf_get_buf - get buffer to write arbitrary data to * @s: the seq_buf handle -- cgit v1.2.3 From 61ef3f1490f879ddffe39a8f1a1bf1ae1e60fc23 Mon Sep 17 00:00:00 2001 From: Bill Wendling Date: Mon, 5 Oct 2026 08:56:59 -0700 Subject: seq_buf: Add seq_buf_init_append() Several strlcat() call sites being converted to seq_buf need behavior seq_buf doesn't currently provide. The normal seq_buf_init() always empties the buffer (sets its "len" to 0) via seq_buf_clear(). Code migrating from strlcat(buf, ...), which appends to whatever buf already contains, can't use seq_buf_init() without discarding that existing content. Add seq_buf_init_append(), which preserves the existing contents and positions the seq_buf to append after it. A buffer with no NUL within its size starts out overflowed, as strlcat() treats it as already truncated. Add KUnit tests for behavior coverage. 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.2.0. Assisted-by: LLM Signed-off-by: Bill Wendling Reviewed-by: Andy Shevchenko Co-developed-by: Kees Cook Link: https://patch.msgid.link/20261005155708.1471260-9-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/seq_buf.h | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 'include/linux/seq_buf.h') diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h index 9830f905f0ee..be979089228f 100644 --- a/include/linux/seq_buf.h +++ b/include/linux/seq_buf.h @@ -71,6 +71,31 @@ seq_buf_set_overflow(struct seq_buf *s) s->len = s->size + 1; } +/** + * seq_buf_init_append - initialize a seq_buf over a buffer that may + * already hold NUL-terminated content + * @s: the seq_buf handle + * @buf: pointer to the (possibly non-empty) buffer + * @size: total size of @buf + * + * Unlike seq_buf_init(), which always starts @buf at len=0, this + * preserves whatever NUL-terminated content @buf already holds and + * positions @s to append after it. Useful for converting code that used + * to append to an existing buffer with strlcat()/scnprintf() and friends. + * + * If @buf holds no NUL within @size, @s starts out overflowed, as + * strlcat() treats such a buffer as already truncated. + */ +static inline void +seq_buf_init_append(struct seq_buf *s, char *buf, unsigned int size) +{ + s->buffer = buf; + s->size = size; + s->len = strnlen(buf, size); + if (s->len == size) + seq_buf_set_overflow(s); +} + /* * How much buffer is left on the seq_buf? */ -- cgit v1.2.3 From e3f8dfafd7d6ef905afd6603b0b57984abd0677b Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 5 Oct 2026 08:57:02 -0700 Subject: docs: core-api: Document the seq_buf API The kernel-doc in include/linux/seq_buf.h and lib/seq_buf.c documents the seq_buf interface, but no .rst file pulls either of them in, so none of it reaches the generated documentation. Add the missing kernel-doc for seq_buf_clear() and seq_buf_init(), and a Sequence Buffers section to the kernel API documentation. The static internal helper seq_buf_can_fit() is left out. Additionally fix seq_buf_hex_dump() indentation to avoid the reported Sphinx error: ERROR: Unexpected indentation. WARNING: Block quote ends without a blank line; unexpected unindent. Verified with "make SPHINXDIRS=core-api htmldocs", which rendered happily into core-api/kernel-api.html. Assisted-by: LLM Co-developed-by: Bill Wendling Signed-off-by: Bill Wendling Tested-by: Randy Dunlap Reviewed-by: Randy Dunlap Link: https://patch.msgid.link/20261005155708.1471260-12-kees@kernel.org Signed-off-by: Kees Cook --- include/linux/seq_buf.h | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'include/linux/seq_buf.h') diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h index be979089228f..f006f2654e4c 100644 --- a/include/linux/seq_buf.h +++ b/include/linux/seq_buf.h @@ -31,6 +31,10 @@ struct seq_buf { .size = SIZE, \ } +/** + * seq_buf_clear - reset the seq_buf to be read / appended from the beginning + * @s: the seq_buf handle + */ static inline void seq_buf_clear(struct seq_buf *s) { s->len = 0; @@ -38,6 +42,14 @@ static inline void seq_buf_clear(struct seq_buf *s) s->buffer[0] = '\0'; } +/** + * seq_buf_init - initialize a seq_buf + * @s: the seq_buf handle + * @buf: pointer to the buffer + * @size: total size of @buf + * + * The contents of the buffer are ignored. + */ static inline void seq_buf_init(struct seq_buf *s, char *buf, unsigned int size) { -- cgit v1.2.3