seq_buf: Add seq_buf_strlen()

seq_buf_do_printk() prints a buffer line by line, then prints whatever follows the last newline. When a string has overflowed at exactly a newline, an empty line is printed since the pointer hasn't reached the overflow mark of the seq_buf. Switch to just check if the string is already empty and only print if not.

The only caller is the memory cgroup OOM report, so this could only ever add a blank line to a report whose statistics already did not fit.

Add a test that registers a console to count the records that seq_buf_do_printk() emits. It counts the records carrying the test's marker, and the records holding nothing but a line feed that arrive after one, so that unrelated kernel messages do not disturb it. Both states that reach the flaw are covered: exactly full, and overflowed.

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.

Fixes 96928d9032a7c ("seq_buf: Add seq_buf_do_printk() helper")

Assisted-by LLM

Signed-off-by Kees Cook <[email protected]>

---

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 <[email protected]>

Signed-off-by Kees Cook <[email protected]>

---

lpfc_rx_monitor_report() formats each record into a stack buffer, then appends it with seq_buf_puts(), relying on seq_buf_puts() appending nothing when a string does not fit: the debugfs output then ends at the last whole record, and the record left out is read in full next time. The next patch makes seq_buf_puts() copy as much as fits instead, as seq_buf_printf() and strlcat() do.

Print each record with seq_buf_printf(), and when it does not fit, end the string where the record began, since the reader takes the buffer up to its NUL. This also takes the DBG_LOG_STR_SZ buffer off the stack.

Build tested ARCH=x86_64 with GCC 16.2.0, CONFIG_SCSI_LPFC=m and W=1.

Assisted-by LLM

Signed-off-by Kees Cook <[email protected]>

---
 drivers/scsi/lpfc/lpfc_sli.c | 45 +++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 18 deletions(-)

drivers/scsi/lpfc/lpfc_sli.c +24 -10

index cfa4371169f1..a7b1ea67ed98 100644
@@ -8108,7 +8108,6 @@ u32 lpfc_rx_monitor_report(struct lpfc_hba *phba,
 	spinlock_t *ring_lock = &rx_monitor->lock;
 	u32 ring_size = rx_monitor->entries;
 	u32 cnt = 0;
-	char tmp[DBG_LOG_STR_SZ] = {0};
 	bool log_to_kmsg = (!buf || !buf_len) ? true : false;
 	struct seq_buf s;
 

@@ -8133,27 +8132,37 @@ u32 lpfc_rx_monitor_report(struct lpfc_hba *phba,
 
 		/* Read out this entry's data. */
 		if (!log_to_kmsg) {
+			unsigned int len;
+
+			/* A header that did not fit leaves no room. */
+			if (seq_buf_has_overflowed(&s))
+				break;
+			len = seq_buf_used(&s);
+
+			seq_buf_printf(&s,
+				       "%03d:\t%-16llu%-16llu%-16llu%-16llu%-8llu%-8llu%-8llu%-8u%-8u%-8u%u(%u)\n",
+				       *head_idx,
+				       entry->max_bytes_per_interval,
+				       entry->cmf_bytes,
+				       entry->total_bytes,
+				       entry->rcv_bytes,
+				       entry->avg_io_latency,
+				       entry->avg_io_size,
+				       entry->max_read_cnt,
+				       entry->cmf_busy, entry->io_cnt,
+				       entry->cmf_info,
+				       entry->timer_utilization,
+				       entry->timer_interval);
+
 			/*
 			 * Drop a record whole if it does not fit, without
-			 * consuming its ring entry.
+			 * consuming its ring entry: the reader takes the
+			 * string up to its NUL.
 			 */
-			scnprintf(tmp, sizeof(tmp),
-				  "%03d:\t%-16llu%-16llu%-16llu%-16llu%-8llu%-8llu%-8llu%-8u%-8u%-8u%u(%u)\n",
-				  *head_idx,
-				  entry->max_bytes_per_interval,
-				  entry->cmf_bytes,
-				  entry->total_bytes,
-				  entry->rcv_bytes,
-				  entry->avg_io_latency,

drivers/scsi/lpfc/lpfc_sli.c +1 -0

 				break;
+			}
 		} else {
 			lpfc_printf_log(phba, KERN_INFO, LOG_CGN_MGMT,
 					"4410 %02u: MBPI %llu Xmit %llu "

Quoted 76 lines · On Mon, 5 Oct 2026 08:56:53 -0700 Kees Cook <[email protected]> wrote:

lpfc_rx_monitor_report() formats each record into a stack buffer, then appends it with seq_buf_puts(), relying on seq_buf_puts() appending nothing when a string does not fit: the debugfs output then ends at the last whole record, and the record left out is read in full next time. The next patch makes seq_buf_puts() copy as much as fits instead, as seq_buf_printf() and strlcat() do.

Print each record with seq_buf_printf(), and when it does not fit, end the string where the record began, since the reader takes the buffer up to its NUL. This also takes the DBG_LOG_STR_SZ buffer off the stack.

Build tested ARCH=x86_64 with GCC 16.2.0, CONFIG_SCSI_LPFC=m and W=1.

Assisted-by LLM

Signed-off-by Kees Cook <[email protected]>

---
 drivers/scsi/lpfc/lpfc_sli.c | 45 +++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 18 deletions(-)

drivers/scsi/lpfc/lpfc_sli.c +24 -15

index cfa4371169f1..a7b1ea67ed98 100644
@@ -8108,7 +8108,6 @@ u32 lpfc_rx_monitor_report(struct lpfc_hba *phba,
 	spinlock_t *ring_lock = &rx_monitor->lock;
 	u32 ring_size = rx_monitor->entries;
 	u32 cnt = 0;
-	char tmp[DBG_LOG_STR_SZ] = {0};
 	bool log_to_kmsg = (!buf || !buf_len) ? true : false;
 	struct seq_buf s;
 

@@ -8133,27 +8132,37 @@ u32 lpfc_rx_monitor_report(struct lpfc_hba *phba,
 
 		/* Read out this entry's data. */
 		if (!log_to_kmsg) {
+			unsigned int len;
+
+			/* A header that did not fit leaves no room. */
+			if (seq_buf_has_overflowed(&s))
+				break;
+			len = seq_buf_used(&s);
+
+			seq_buf_printf(&s,
+				       "%03d:\t%-16llu%-16llu%-16llu%-16llu%-8llu%-8llu%-8llu%-8u%-8u%-8u%u(%u)\n",
+				       *head_idx,
+				       entry->max_bytes_per_interval,
+				       entry->cmf_bytes,
+				       entry->total_bytes,
+				       entry->rcv_bytes,
+				       entry->avg_io_latency,
+				       entry->avg_io_size,
+				       entry->max_read_cnt,
+				       entry->cmf_busy, entry->io_cnt,
+				       entry->cmf_info,
+				       entry->timer_utilization,
+				       entry->timer_interval);
+
 			/*
 			 * Drop a record whole if it does not fit, without
-			 * consuming its ring entry.
+			 * consuming its ring entry: the reader takes the
+			 * string up to its NUL.
 			 */
-			scnprintf(tmp, sizeof(tmp),
-				  "%03d:\t%-16llu%-16llu%-16llu%-16llu%-8llu%-8llu%-8llu%-8u%-8u%-8u%u(%u)\n",
-				  *head_idx,
-				  entry->max_bytes_per_interval,
-				  entry->cmf_bytes,
-				  entry->total_bytes,
-				  entry->rcv_bytes,
-				  entry->avg_io_latency,
-				  entry->avg_io_size,
-				  entry->max_read_cnt,
-				  entry->cmf_busy, entry->io_cnt,
-				  entry->cmf_info,
-				  entry->timer_utilization,

drivers/scsi/lpfc/lpfc_sli.c u32 lpfc_rx_monitor_report(struct lpfc_hba *phba, · line 8163

Quoted 5 lines

drivers/scsi/lpfc/lpfc_sli.c +2 -3

-				  entry->timer_interval);
-
-			if (seq_buf_puts(&s, tmp) < 0)
+			if (seq_buf_has_overflowed(&s)) {
+				buf[len] = '\0';

There should probably be a seq_buf_truncate() to do that. Then a request for the length will be correct.

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 <[email protected]>

Signed-off-by Kees Cook <[email protected]>

---

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 <[email protected]>

Signed-off-by Kees Cook <[email protected]>

---
 include/linux/seq_buf.h   |  8 +++++--
 lib/seq_buf.c             |  5 +++++
 lib/tests/seq_buf_kunit.c | 44 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 2 deletions(-)

include/linux/seq_buf.h +1 -0

index 77e76e283370..0c0a0db04b09 100644
@@ -5,6 +5,7 @@
 #include <linux/bug.h>
 #include <linux/minmax.h>
 #include <linux/seq_file.h>
+#include <linux/string.h>
 #include <linux/types.h>
 
 /*

@@ -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

include/linux/seq_buf.h +1 -0

+
 	s->len = s->size + 1;
 }
 

lib/seq_buf.c +5 -0

index 60e9eadb3ef7..8da2e9447adf 100644
@@ -76,6 +76,8 @@ int seq_buf_vprintf(struct seq_buf *s, const char *fmt, va_list args)
 			s->len += len;
 			return 0;
 		}
+		/* vsnprintf() wrote as much as fits, so none of it is stale */
+		s->len = s->size;
 	}
 	seq_buf_set_overflow(s);
 	return -1;

@@ -164,6 +166,8 @@ int seq_buf_bprintf(struct seq_buf *s, const char *fmt, const u32 *binary)
 			s->len += ret;
 			return 0;
 		}
+		/* bstr_printf() wrote as much as fits, so none of it is stale */
+		s->len = s->size;
 	}
 	seq_buf_set_overflow(s);
 	return -1;

@@ -343,6 +347,7 @@ int seq_buf_path(struct seq_buf *s, const struct path *path, const char *esc)
 
 	return res;
 }
+EXPORT_SYMBOL_GPL(seq_buf_path);
 
 /**
  * seq_buf_to_user - copy the sequence buffer to user space

lib/tests/seq_buf_kunit.c +44 -0

index 046614100c77..5eaf024fa3a5 100644
@@ -7,7 +7,9 @@
 
 #include <kunit/test.h>
 #include <linux/console.h>
+#include <linux/fs.h>
 #include <linux/seq_buf.h>
+#include <linux/shmem_fs.h>
 #include <linux/string.h>
 
 static void seq_buf_init_test(struct kunit *test)

@@ -466,6 +468,47 @@ static void seq_buf_do_printk_test(struct kunit *test)
 	KUNIT_EXPECT_EQ(test, seq_buf_printk_empty, 0);
 }
 
+/* Long enough that it cannot fit in the room the test leaves for it. */
+#define SEQ_BUF_TEST_PATH	"/seq_buf_kunit_path_name"
+
+static void seq_buf_path_overflow_test(struct kunit *test)
+{
+	DECLARE_SEQ_BUF(s, 32);
+	const char *expected = "keep:xxxxxxxxxxxxxxxxxxxxxxx";
+	struct file *file;
+	size_t len;
+	int i;
+
+	/*
+	 * The tests run before anything writable is mounted, so take the file
+	 * whose path gets printed from shmem, which needs no mount of its own.
+	 */
+	file = shmem_file_setup(SEQ_BUF_TEST_PATH, 0, EMPTY_VMA_FLAGS);
+	if (IS_ERR(file))
+		kunit_skip(test, "cannot create a file to print the path of");
+
+	/* Leave less room than the path needs, so d_path() cannot fit it. */
+	seq_buf_puts(&s, "keep:");
+	len = seq_buf_used(&s);
+	for (i = len; i < 28; i++)
+		seq_buf_putc(&s, 'x');
+
+	KUNIT_EXPECT_EQ(test, seq_buf_path(&s, &file->f_path, "\n"), -1);
+	fput(file);
+
+	KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s));
+
+	/*
+	 * d_path() keeps as much of the path as fits when it does not fit
+	 * whole, and seq_buf_str() would hand out that fragment, as it ends
+	 * the string at the last byte of an overflowed buffer.
+	 */
+	for (i = 28; i < 32; i++)
+		KUNIT_EXPECT_EQ_MSG(test, s.buffer[i], '\0',
+				    "byte %d past the data is not cleared", i);
+	KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), expected);
+}
+
 static struct kunit_case seq_buf_test_cases[] = {
 	KUNIT_CASE(seq_buf_init_test),
 	KUNIT_CASE(seq_buf_declare_test),

@@ -482,6 +525,7 @@ static struct kunit_case seq_buf_test_cases[] = {
 	KUNIT_CASE(seq_buf_puts_partial_overflow_test),
 	KUNIT_CASE(seq_buf_putmem_partial_overflow_test),
 	KUNIT_CASE(seq_buf_putmem_hex_partial_overflow_test),
+	KUNIT_CASE(seq_buf_path_overflow_test),
 	KUNIT_CASE(seq_buf_do_printk_test),
 	{}
 };

include/linux/seq_buf.h seq_buf_has_overflowed(struct seq_buf *s) · line 69

Quoted 5 lines

static inline void
seq_buf_set_overflow(struct seq_buf *s)
{

include/linux/seq_buf.h +2 -0

+	if (s->len < s->size)
+		memset(s->buffer + s->len, 0, s->size - s->len);

What is the performance impact of zeroing the buffer?

I don't think it would be a good idea to be zeroing the buffer on entry either (I've not looked to see it that happens - but there will be PAGE_SIZE buffers (maybe 64k) that get a a small number of characters written to them).

Writing a single '\0' really ought to be enough.

Quoted 7 lines · David Laight

include/linux/seq_buf.h seq_buf_has_overflowed(struct seq_buf *s) · line 69

Quoted 5 lines

static inline void
seq_buf_set_overflow(struct seq_buf *s)
{

include/linux/seq_buf.h +2 -0

+	if (s->len < s->size)
+		memset(s->buffer + s->len, 0, s->size - s->len);  

What is the performance impact of zeroing the buffer?

Overflow is never a fast path: it happens at most once per seq_buf (after that len > size, so the memset is skipped), and it is already a failure the caller has to handle.

It is also usually zero bytes. printf, bprintf, puts, putmem, and putc all fill to the end before overflowing, so len == size and there is nothing to clear. Only a writer handed the tail by seq_buf_get_buf() that then gives up leaves anything behind (seq_buf_path() with d_path(), or landlock's string_escape_mem()), and then the clear covers only the space that writer was given, which it may have partly filled.

Quoted 4 lines

I don't think it would be a good idea to be zeroing the buffer on entry either (I've not looked to see it that happens - but there will be PAGE_SIZE buffers (maybe 64k) that get a a small number of characters written to them).

Agreed, and it doesn't: seq_buf_init() writes a single NUL.

Quoted 1 line

Writing a single '\0' really ought to be enough.

It would be for seq_buf_str(), but once a seq_buf has overflowed, seq_buf_used() reports the whole buffer, and seq_buf_print_seq() and seq_buf_to_user() copy that many bytes. Whatever followed the NUL (the path fragment d_path() left at the end, say) would still reach the seq_file or userspace.

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 <[email protected]>

Signed-off-by Kees Cook <[email protected]>

---

include/linux/seq_buf.h +7 -0

+/*
+ * 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.
+ */

include/linux/seq_buf.h +13 -3

+
+	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))

include/linux/seq_buf.h +28 -0

+ * @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

Quoted 14 lines · On Mon, 5 Oct 2026 08:56:56 -0700 Kees Cook <[email protected]> wrote:

include/linux/seq_buf.h +14 -0

+/*
+ * 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

include/linux/seq_buf.h static inline unsigned int seq_buf_used(struct seq_buf *s) · line 108

Quoted 3 lines

include/linux/seq_buf.h +3 -0

+		end = s->size - 1;
+
+	s->buffer[end] = 0;

Hmm, I just realized that this could inadvertently overflow the buffer. If len == size, it may not be overflowed. If using seq_buf_putmem() and filling the entire buffer, calling this would force an overflow as we just truncated the last byte.

Quoted 33 lines

include/linux/seq_buf.h +13 -3

+
+	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))

include/linux/seq_buf.h static inline unsigned int seq_buf_used(struct seq_buf *s) · line 147

Quoted 10 lines

include/linux/seq_buf.h +3 -3

-		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

Hmm,

I don't really like the way this modifies the string, as the request is just to return the string length.

Just like the above, this could cause an inadvertent overflow.

I could envision a use case where we want to use the entire buffer and do not expect it to be nul terminated. And this function could be used to get the length to print. That is, it may want to use the entire length.

Having "strlen()" modify the string just seems wrong to me.

Is it really needed? If the user wants to make sure the string is terminated, it should use the next patch's "seq_buf_terminate()".

And we need to update it so that if seq_buf_terminate() causes a truncation, it needs to set that it overflowed.

Date: 2026-10-22 11:06:32-0700
From: Kees Cook <[email protected]>

Quoted 17 lines · Steven Rostedt

Quoted 14 lines · On Mon, 5 Oct 2026 08:56:56 -0700 Kees Cook <[email protected]> wrote:

include/linux/seq_buf.h +14 -0

+/*
+ * 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

Quoted 8 lines

include/linux/seq_buf.h static inline unsigned int seq_buf_used(struct seq_buf *s) · line 108

Quoted 3 lines

include/linux/seq_buf.h +3 -0

+		end = s->size - 1;
+
+	s->buffer[end] = 0;

Hmm, I just realized that this could inadvertently overflow the buffer. If len == size, it may not be overflowed. If using seq_buf_putmem() and filling the entire buffer, calling this would force an overflow as we just truncated the last byte.

Hm, yes. So the "else" branch needs to include:

	s->len = s->size + 1;

Quoted 5 lines

Quoted 2 lines

+/**
+ * seq_buf_strlen - get the length of the NUL-terminated C string in seq_buf

Hmm,

Quoted 8 lines

I don't really like the way this modifies the string, as the request is just to return the string length.

Just like the above, this could cause an inadvertent overflow.

I could envision a use case where we want to use the entire buffer and do not expect it to be nul terminated. And this function could be used to get the length to print. That is, it may want to use the entire length.

Non-C String users would be using seq_buf_used(), not seq_buf_strlen().

Quoted 2 lines

Having "strlen()" modify the string just seems wrong to me.

No, this is absolutely required. It's nonsensical to work on a C String without termination. (And it's the entire purpose of this series...)

Quoted 2 lines

Is it really needed? If the user wants to make sure the string is terminated, it should use the next patch's "seq_buf_terminate()".

The ergonomics on this are very bad. I explicitly don't want to leave a foot-gun around for this. :) (i.e. require all C String users also remember to terminate it first; the whole point of the API is to have the string always be safe.)

Quoted 2 lines

And we need to update it so that if seq_buf_terminate() causes a truncation, it needs to set that it overflowed.

That I agree with. I will send a patch on top of the series since this has been in -next for a while and the merge window is here now. :P

Signature · 1 line

Kees Cook

Quoted 8 lines

Quoted 4 lines

Hmm, I just realized that this could inadvertently overflow the buffer. If len == size, it may not be overflowed. If using seq_buf_putmem() and filling the entire buffer, calling this would force an overflow as we just truncated the last byte.

Hm, yes. So the "else" branch needs to include:

	s->len = s->size + 1;  

Please think again about the possibility of using pointers instead of lengths. This is a source of off-by-one bugs that's only caused by the choice of data, I believe. If using pointers, truncation would be stored as a NULL pointer, and off-by-ones would be less likely.

Consider this off-by-one to be just a sympthom of the underlying problematic design. Whenever a design forces you to write +1 or -1, there are high chances that the design is sub-optimal regarding safety.

It's not an off-by-one error here. The s->len = size - 1 is correct, but it also has a side effect of removing a character without marking it overflowed. The seq_buf defines being "overflowed" as s->len == s->size + 1. What the else statement should have is:

	else
		seq_buf_set_overflow(s);

Which would be less confusing to reviewers as that is defined as:

static inline void
seq_buf_set_overflow(struct seq_buf *s)

{
	s->len = s->size + 1;
}

Date: 2026-10-22 15:57:51-0400
From: Steven Rostedt <[email protected]>

Quoted 14 lines · On Thu, 22 Oct 2026 21:41:53 +0200 Alejandro Colomar…

Quoted 8 lines

Quoted 4 lines

Hmm, I just realized that this could inadvertently overflow the buffer. If len == size, it may not be overflowed. If using seq_buf_putmem() and filling the entire buffer, calling this would force an overflow as we just truncated the last byte.

Hm, yes. So the "else" branch needs to include:

	s->len = s->size + 1;  

Please think again about the possibility of using pointers instead of lengths. This is a source of off-by-one bugs that's only caused by the choice of data, I believe. If using pointers, truncation would be stored as a NULL pointer, and off-by-ones would be less likely.

Quoted 3 lines

Consider this off-by-one to be just a sympthom of the underlying problematic design. Whenever a design forces you to write +1 or -1, there are high chances that the design is sub-optimal regarding safety.

It's not an off-by-one error here. The s->len = size - 1 is correct, but it also has a side effect of removing a character without marking it overflowed.

The seq_buf defines being "overflowed" as s->len == s->size + 1. What the else statement should have is:

	else
		seq_buf_set_overflow(s);

Which would be less confusing to reviewers as that is defined as:

static inline void
seq_buf_set_overflow(struct seq_buf *s)

{
	s->len = s->size + 1;
}

-- Steve

Agree.

But still, a better design would do:

	static inline void
	seq_buf_set_overflow(struct seq_buf *s)
	{
		s->p = NULL;
	}

This is way more robust, and also easier to understand. My head is boiling when trying to review this patch set for correctness. That's not a good sign.

Date: 2026-10-22 16:20:00-0400
From: Steven Rostedt <[email protected]>

Quoted 10 lines · On Thu, 22 Oct 2026 22:11:27 +0200 Alejandro Colomar…

Agree.

But still, a better design would do:

	static inline void
	seq_buf_set_overflow(struct seq_buf *s)
	{
		s->p = NULL;
	}

Quoted 3 lines

This is way more robust, and also easier to understand. My head is boiling when trying to review this patch set for correctness. That's not a good sign.

But that loses all data. We still want access to what was written.

-- Steve

Date: 2026-10-22 22:38:12+0200
From: Alejandro Colomar <[email protected]>

Hi Steven,

Quoted 16 lines

Date: 2026-10-22 16:20:00-0400
From: Steven Rostedt <[email protected]>

Quoted 10 lines · On Thu, 22 Oct 2026 22:11:27 +0200 Alejandro Colomar…

Agree.

But still, a better design would do:

	static inline void
	seq_buf_set_overflow(struct seq_buf *s)
	{
		s->p = NULL;
	}

Quoted 5 lines

Quoted 3 lines

This is way more robust, and also easier to understand. My head is boiling when trying to review this patch set for correctness. That's not a good sign.

But that loses all data. We still want access to what was written.

No, it doesn't lose data. My proposal is to have this:

	struct my_seq_buf {
		// .buffer points to the start of the buffer
		char  *buffer;

		/* .p points to the current position.  This is the last
		 * '\0' that has been written to the buffer.  On
		 * truncation, it is a NULL pointer.
		 */
		char  *p;

		/* .end always holds ARRAY_END(buffer).  It is a pointer
		 * one-past-the-last character in the buffer.
		 */
		char  *end;
	};

This would be initialized as (of course, wrapper in static inline functions as needed) this:

	char            buf[100];
	struct seq_buf  b;

	b.buffer = buf;
	b.p = buf;
	b.end = ARRAY_END(buf);

Then, you would write to it as:

	b.p = stpecpy(b.p, b.end, "foo");
	b.p = stpecpy(b.p, b.end, "bar");
	...
	b.p = stpecpy(b.p, b.end, "baz");
	if (b.p == NULL)
		goto fail;

Then, at some point, you might want to initialize a new buffer out of the remainings of it:

	struct seq_buf  b2;

	if (b.p == NULL)
		goto fail;
	b2.start = b.p;

	b2.p = b.p;
	b2.end = b.end;

See how simple this becomes? (So much that you may doubt the necessity of the set of APIs at all, but they don't hurt much either.)

Cheers,
Alex

Quoted 2 lines

-- Steve

Signature · 1 line

<https://www.alejandro-colomar.es>

No, it doesn't lose data. My proposal is to have this:

	struct my_seq_buf {
		// .buffer points to the start of the buffer
		char  *buffer;

		/* .p points to the current position.  This is the last
		 * '\0' that has been written to the buffer.  On
		 * truncation, it is a NULL pointer.
		 */
		char  *p;

		/* .end always holds ARRAY_END(buffer).  It is a pointer
		 * one-past-the-last character in the buffer.
		 */
		char  *end;
	};

This would be initialized as (of course, wrapper in static inline functions as needed) this:

	char            buf[100];
	struct seq_buf  b;

	b.buffer = buf;
	b.p = buf;
	b.end = ARRAY_END(buf);

Then, you would write to it as:

	b.p = stpecpy(b.p, b.end, "foo");
	b.p = stpecpy(b.p, b.end, "bar");
	...
	b.p = stpecpy(b.p, b.end, "baz");
	if (b.p == NULL)
		goto fail;

Then, at some point, you might want to initialize a new buffer out of the remainings of it:

	struct seq_buf  b2;

	if (b.p == NULL)
		goto fail;
	b2.start = b.p;
	b2.p = b.p;
	b2.end = b.end;

See how simple this becomes? (So much that you may doubt the necessity of the set of APIs at all, but they don't hurt much either.)

Date: 2026-10-22 16:53:36-0400
From: Steven Rostedt <[email protected]>

Quoted 16 lines

	struct my_seq_buf {
		// .buffer points to the start of the buffer
		char  *buffer;

		/* .p points to the current position.  This is the last
		 * '\0' that has been written to the buffer.  On
		 * truncation, it is a NULL pointer.
		 */
		char  *p;

		/* .end always holds ARRAY_END(buffer).  It is a pointer
		 * one-past-the-last character in the buffer.
		 */
		char  *end;
	};

I rather not increase its size. It is embedded in the trace_seq.

Yes the seq_buf code can be complex, but it's in one location where it can be highly tested and verified. The point of seq_buf is to do the complexity so that other places in the kernel doesn't need to.

-- Steve

Quoted 2 lines

I rather not increase its size. It is embedded in the trace_seq.

The size shouldn't change:

	alx@debian:~/tmp$ cat seq_buf_size_comparison.c 
	#include <stddef.h>
	#include <stdio.h>

	struct seq_buf {
		char    *buffer;
		size_t  size;
		size_t  len;
	};

	struct alx_seq_buf {
		char  *buffer;
		char  *p;
		char  *end;
	};

	int
	main(void)
	{
		printf("%zu\n", sizeof(struct seq_buf));
		printf("%zu\n", sizeof(struct alx_seq_buf));
	}
	alx@debian:~/tmp$ gcc -Wall -Wextra seq_buf_size_comparison.c 
	alx@debian:~/tmp$ ./a.out 
	24
	24

size_t and pointers should always have the same size, so this doesn't change the size.

Am I missing something?

Cheers,
Alex

Quoted 3 lines

Yes the seq_buf code can be complex, but it's in one location where it can be highly tested and verified. The point of seq_buf is to do the complexity so that other places in the kernel doesn't need to.

Date: 2026-10-22 20:37:13-0400
From: Steven Rostedt <[email protected]>

Quoted 11 lines

	struct seq_buf {
		char    *buffer;
		size_t  size;
		size_t  len;
	};

	struct alx_seq_buf {
		char  *buffer;
		char  *p;
		char  *end;
	};

OK, I was thinking you were adding another pointer.

Quoted 21 lines

	int
	main(void)
	{
		printf("%zu\n", sizeof(struct seq_buf));
		printf("%zu\n", sizeof(struct alx_seq_buf));
	}
	alx@debian:~/tmp$ gcc -Wall -Wextra seq_buf_size_comparison.c 
	alx@debian:~/tmp$ ./a.out 
	24
	24

size_t and pointers should always have the same size, so this doesn't change the size.

Am I missing something?

Cheers,
Alex

Quoted 3 lines

Quoted 3 lines

Yes the seq_buf code can be complex, but it's in one location where it can be highly tested and verified. The point of seq_buf is to do the complexity so that other places in the kernel doesn't need to.

We could look into updating it to pointers later, but right now I rather keep what we have. Let's focus first on the new stuff. It's not that hard to validate.

-- Steve

Date: 2026-10-22 22:38:12+0200
From: Alejandro Colomar <[email protected]>

No, it doesn't lose data. My proposal is to have this:

	struct my_seq_buf {
		// .buffer points to the start of the buffer
		char  *buffer;

		/* .p points to the current position.  This is the last
		 * '\0' that has been written to the buffer.  On
		 * truncation, it is a NULL pointer.
		 */
		char  *p;

		/* .end always holds ARRAY_END(buffer).  It is a pointer
		 * one-past-the-last character in the buffer.
		 */
		char  *end;
	};

This would be initialized as (of course, wrapper in static inline functions as needed) this:

	char            buf[100];
	struct seq_buf  b;

	b.buffer = buf;
	b.p = buf;
	b.end = ARRAY_END(buf);

Then, you would write to it as:

	b.p = stpecpy(b.p, b.end, "foo");
	b.p = stpecpy(b.p, b.end, "bar");
	...
	b.p = stpecpy(b.p, b.end, "baz");
	if (b.p == NULL)
		goto fail;

Then, at some point, you might want to initialize a new buffer out of the remainings of it:

	struct seq_buf  b2;

	if (b.p == NULL)
		goto fail;
	b2.start = b.p;

	b2.p = b.p;
	b2.end = b.end;

See how simple this becomes? (So much that you may doubt the necessity of the set of APIs at all, but they don't hurt much either.)

Cheers,
Alex

Quoted 2 lines

-- Steve

Signature · 1 line

<https://www.alejandro-colomar.es>

From: Bill Wendling <[email protected]>

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 <[email protected]>

Reviewed-by Andy Shevchenko <[email protected]>

Co-developed-by Kees Cook <[email protected]>

Signed-off-by Kees Cook <[email protected]>

---

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 <[email protected]>

Signed-off-by Bill Wendling <[email protected]>

Tested-by Randy Dunlap <[email protected]>

Reviewed-by Randy Dunlap <[email protected]>

Signed-off-by Kees Cook <[email protected]>

---

While working on seq_buf conversions[1], we found there was a need for seq_buf_strlen() (since calling strlen(seq_buf_str()) would be a waste of time: seq_buf already knows the length), seq_buf_init_append() (to start a seq_buf from an existing string buffer), and seq_buf_terminate() (as there are many callers using seq_buf_str() just for its termination side-effect). And while implementing that, we found a bunch of other related things that needed fixing. This is that ever-growing series, with tests for each fix.

[...]