mirror of https://lore.kernel.org/lkml/
 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] " 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®