All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bill Wendling <morbo@google.com>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Kees Cook <kees@kernel.org>, Steven Rostedt <rostedt@goodmis.org>,
	linux-kernel@vger.kernel.org,  linux-hardening@vger.kernel.org,
	Bill Wendling <morbo@google.com>
Subject: [PATCH v3] seq_buf: add seq_buf_strlen(), seq_buf_init_append(), seq_buf_puts_trunc()
Date: Thu, 17 Sep 2026 01:13:59 +0000	[thread overview]
Message-ID: <20260917011359.1585961-1-morbo@google.com> (raw)
In-Reply-To: <20260916222125.1259631-1-morbo@google.com>

Converting some strlcat() call sites seq_buf need behavior seq_buf
doesn't currently provide, either directly or without introducing subtle
bugs:

 - seq_buf_strlen(): seq_buf_used() reports the full buffer size when
   the buffer is completely filled, even though seq_buf_str() then
   overwrites the final byte with a NUL terminator, leaving only size -
   1 bytes of actual content. Callers that need the true length of the
   NUL-terminated string have to fall back to strlen(seq_buf_str(s)).
   seq_buf_strlen() mirrors seq_buf_str()'s NUL-termination logic but
   returns the resulting string's length directly.

 - seq_buf_init_append(): seq_buf_init() always clears the buffer it's
   given via seq_buf_clear(). Code migrating from strlcat(buf, ...),
   which appends to whatever @buf already contains, can't use
   seq_buf_init() without silently discarding that existing content.
   seq_buf_init_append() preserves it and positions the seq_buf to
   append after it.

 - seq_buf_puts_trunc(): seq_buf_puts() (like seq_buf_printf() and
   friends) writes nothing at all if the string doesn't fully fit,
   whereas strlcat() always copies as much of the source as there is
   room for. Converting a strlcat() call site that relied on that
   partial-copy behavior to plain seq_buf_puts() can silently drop
   content that used to survive truncated. seq_buf_puts_trunc() keeps
   the leading bytes of the string that fit.

Assisted-by: LLM
Suggested-by: Kees Cook <kees@kernel.org>
Signed-off-by: Bill Wendling <morbo@google.com>
---
v2: Reword the commit message to be clearer and not refer to a series of
    patches
v3: Add EXPORT_SYMBOL_GPL, make sure to overflow when truncating, correct
    some logic discovered by the KUnit tests, and add KUnit tests.
---
 include/linux/seq_buf.h   | 60 +++++++++++++++++++++++++++++++++++++++
 lib/seq_buf.c             | 33 +++++++++++++++++++++
 lib/tests/seq_buf_kunit.c | 48 +++++++++++++++++++++++++++++++
 3 files changed, 141 insertions(+)

diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
index 9f2839e73f8a..a552bcaab07f 100644
--- a/include/linux/seq_buf.h
+++ b/include/linux/seq_buf.h
@@ -30,6 +30,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;
@@ -37,6 +41,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)
 {
@@ -45,6 +57,26 @@ seq_buf_init(struct seq_buf *s, char *buf, unsigned int size)
 	seq_buf_clear(s);
 }
 
+/**
+ * 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 clears @buf, 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.
+ */
+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);
+}
+
 /*
  * seq_buf have a buffer that might overflow. When this happens
  * len is set to be greater than size.
@@ -108,6 +140,33 @@ static inline const char *seq_buf_str(struct seq_buf *s)
 	return s->buffer;
 }
 
+/**
+ * seq_buf_strlen - get the length of the NUL-terminated string in seq_buf
+ * @s: the seq_buf handle
+ *
+ * Like seq_buf_str(), this makes sure that the buffer in @s is
+ * NUL-terminated, and returns the length of the resulting string.
+ * Unlike seq_buf_used(), the returned length is always correct, even
+ * when the buffer is completely full: in that case seq_buf_used()
+ * reports @s->size, but the last byte was overwritten with the
+ * trailing NUL, so only @s->size - 1 bytes of content remain.
+ *
+ * Returns: the length of the NUL-terminated string in @s->buffer.
+ */
+static inline size_t seq_buf_strlen(struct seq_buf *s)
+{
+	if (WARN_ON(s->size == 0))
+		return 0;
+
+	if (seq_buf_buffer_left(s)) {
+		s->buffer[s->len] = 0;
+		return s->len;
+	}
+
+	s->buffer[s->size - 1] = 0;
+	return s->size - 1;
+}
+
 /**
  * seq_buf_get_buf - get buffer to write arbitrary data to
  * @s: the seq_buf handle
@@ -179,6 +238,7 @@ extern int seq_buf_putmem(struct seq_buf *s, const void *mem, unsigned int len);
 extern int seq_buf_putmem_hex(struct seq_buf *s, const void *mem,
 			      unsigned int len);
 extern int seq_buf_path(struct seq_buf *s, const struct path *path, const char *esc);
+extern size_t seq_buf_puts_trunc(struct seq_buf *s, const char *str);
 extern int seq_buf_hex_dump(struct seq_buf *s, const char *prefix_str,
 			    int prefix_type, int rowsize, int groupsize,
 			    const void *buf, size_t len, bool ascii);
diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index a92093f346da..831a1542fb78 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -199,6 +199,39 @@ int seq_buf_puts(struct seq_buf *s, const char *str)
 }
 EXPORT_SYMBOL_GPL(seq_buf_puts);
 
+/**
+ * seq_buf_puts_trunc - append as much of a string as fits, keeping any of it
+ * @s: the seq_buf handle
+ * @str: the string to append
+ *
+ * This copies the leading bytes of @str that fit, reserving room for the NUL
+ * terminator later added by seq_buf_str(). This differs from seq_buf_puts(),
+ * which writes nothing at all if @str doesn't fully fit.
+ *
+ * Unlike seq_buf_puts(), this does NOT NUL-terminate @s->buffer as it
+ * goes (it copies raw bytes via seq_buf_putmem(), not @str's own
+ * terminator). Callers MUST call seq_buf_str() or seq_buf_strlen()
+ * before using @s->buffer as a C string.
+ *
+ * Returns: the number of bytes copied from @str.
+ */
+size_t seq_buf_puts_trunc(struct seq_buf *s, const char *str)
+{
+	size_t len = strlen(str);
+
+	WARN_ON(s->size == 0);
+
+	if (seq_buf_can_fit(s, len))
+		return seq_buf_puts(s, str);
+
+	/* Truncate the string to fit the buffer. */
+	len = s->size - s->len;
+	memcpy(s->buffer + s->len, str, len);
+	seq_buf_set_overflow(s);
+	return len;
+}
+EXPORT_SYMBOL_GPL(seq_buf_puts_trunc);
+
 /**
  * seq_buf_putc - sequence printing of simple character
  * @s: seq_buf descriptor
diff --git a/lib/tests/seq_buf_kunit.c b/lib/tests/seq_buf_kunit.c
index eb466386bbef..a16b1365b503 100644
--- a/lib/tests/seq_buf_kunit.c
+++ b/lib/tests/seq_buf_kunit.c
@@ -23,6 +23,21 @@ static void seq_buf_init_test(struct kunit *test)
 	KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "");
 }
 
+static void seq_buf_init_append_test(struct kunit *test)
+{
+	char buf[32] = "hello world";
+	struct seq_buf s;
+
+	seq_buf_init_append(&s, buf, sizeof(buf));
+
+	KUNIT_EXPECT_EQ(test, s.size, 32);
+	KUNIT_EXPECT_EQ(test, s.len, 11);
+	KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
+	KUNIT_EXPECT_EQ(test, seq_buf_buffer_left(&s), 32 - 11);
+	KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 11);
+	KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello world");
+}
+
 static void seq_buf_declare_test(struct kunit *test)
 {
 	DECLARE_SEQ_BUF(s, 24);
@@ -51,6 +66,36 @@ static void seq_buf_clear_test(struct kunit *test)
 	KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "");
 }
 
+static void seq_buf_strlen_test(struct kunit *test)
+{
+	DECLARE_SEQ_BUF(s, 16);
+
+	seq_buf_puts(&s, "hello world!");
+	KUNIT_EXPECT_EQ(test, seq_buf_strlen(&s), 12);
+	KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
+	KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello world!");
+
+	seq_buf_puts(&s, " It's a small world!");
+	KUNIT_EXPECT_EQ(test, seq_buf_strlen(&s), s.size - 1);
+	KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s));
+	KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello world!");
+}
+
+static void seq_buf_puts_trunc_test(struct kunit *test)
+{
+	DECLARE_SEQ_BUF(s, 10);
+
+	seq_buf_puts(&s, "hello");
+	KUNIT_EXPECT_EQ(test, seq_buf_used(&s), 5);
+	KUNIT_EXPECT_FALSE(test, seq_buf_has_overflowed(&s));
+	KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello");
+
+	seq_buf_puts_trunc(&s, " world!");
+	KUNIT_EXPECT_EQ(test, seq_buf_strlen(&s), s.size - 1);
+	KUNIT_EXPECT_TRUE(test, seq_buf_has_overflowed(&s));
+	KUNIT_EXPECT_STREQ(test, seq_buf_str(&s), "hello wor");
+}
+
 static void seq_buf_puts_test(struct kunit *test)
 {
 	DECLARE_SEQ_BUF(s, 16);
@@ -218,8 +263,11 @@ static void seq_buf_putmem_hex_overflow_test(struct kunit *test)
 
 static struct kunit_case seq_buf_test_cases[] = {
 	KUNIT_CASE(seq_buf_init_test),
+	KUNIT_CASE(seq_buf_init_append_test),
 	KUNIT_CASE(seq_buf_declare_test),
 	KUNIT_CASE(seq_buf_clear_test),
+	KUNIT_CASE(seq_buf_strlen_test),
+	KUNIT_CASE(seq_buf_puts_trunc_test),
 	KUNIT_CASE(seq_buf_puts_test),
 	KUNIT_CASE(seq_buf_puts_overflow_test),
 	KUNIT_CASE(seq_buf_putc_test),
-- 
2.55.0.1082.g2b9226bbc0-goog


  parent reply	other threads:[~2026-09-17  1:14 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-16 22:15 [PATCH] seq_buf: add seq_buf_strlen(), seq_buf_init_append(), seq_buf_puts_trunc() Bill Wendling
2026-09-16 22:21 ` [PATCH v2] " Bill Wendling
2026-09-16 23:47   ` Andrew Morton
2026-09-16 23:55     ` Bill Wendling
2026-09-17  1:13   ` Bill Wendling [this message]
2026-09-17 14:24     ` [PATCH v3] " Kees Cook
2026-09-17 21:29       ` Bill Wendling

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=20260917011359.1585961-1-morbo@google.com \
    --to=morbo@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=kees@kernel.org \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    /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.