From: Kees Cook <keescook@chromium.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Kees Cook <keescook@chromium.org>,
"Matthew Wilcox (Oracle)" <willy@infradead.org>,
Christoph Hellwig <hch@lst.de>,
Justin Stitt <justinstitt@google.com>,
Kent Overstreet <kent.overstreet@linux.dev>,
Petr Mladek <pmladek@suse.com>,
Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
Rasmus Villemoes <linux@rasmusvillemoes.dk>,
Sergey Senozhatsky <senozhatsky@chromium.org>,
Masami Hiramatsu <mhiramat@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Arnd Bergmann <arnd@arndb.de>, Jonathan Corbet <corbet@lwn.net>,
Yun Zhou <yun.zhou@windriver.com>,
Jacob Keller <jacob.e.keller@intel.com>,
Zhen Lei <thunder.leizhen@huawei.com>,
linux-trace-kernel@vger.kernel.org,
Yosry Ahmed <yosryahmed@google.com>,
linux-kernel@vger.kernel.org, linux-hardening@vger.kernel.org
Subject: [PATCH v3] seq_buf: Introduce DECLARE_SEQ_BUF and seq_buf_str()
Date: Fri, 27 Oct 2023 08:56:38 -0700 [thread overview]
Message-ID: <20231027155634.make.260-kees@kernel.org> (raw)
Solve two ergonomic issues with struct seq_buf;
1) Too much boilerplate is required to initialize:
struct seq_buf s;
char buf[32];
seq_buf_init(s, buf, sizeof(buf));
Instead, we can build this directly on the stack. Provide
DECLARE_SEQ_BUF() macro to do this:
DECLARE_SEQ_BUF(s, 32);
2) %NUL termination is fragile and requires 2 steps to get a valid
C String (and is a layering violation exposing the "internals" of
seq_buf):
seq_buf_terminate(s);
do_something(s->buffer);
Instead, we can just return s->buffer directly after terminating it in
the refactored seq_buf_terminate(), now known as seq_buf_str():
do_something(seq_buf_str(s));
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org>
Cc: Christoph Hellwig <hch@lst.de>
Cc: Justin Stitt <justinstitt@google.com>
Cc: Kent Overstreet <kent.overstreet@linux.dev>
Cc: Petr Mladek <pmladek@suse.com>
Cc: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Cc: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Cc: Sergey Senozhatsky <senozhatsky@chromium.org>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Jonathan Corbet <corbet@lwn.net>
Cc: Yun Zhou <yun.zhou@windriver.com>
Cc: Jacob Keller <jacob.e.keller@intel.com>
Cc: Zhen Lei <thunder.leizhen@huawei.com>
Cc: linux-trace-kernel@vger.kernel.org
Link: https://lore.kernel.org/r/20231026194033.it.702-kees@kernel.org
Signed-off-by: Kees Cook <keescook@chromium.org>
---
v3
- fix commit log typos
- improve code style for DECLARE_SEQ_BUF (shevchenko)
- const-ify seq_bug_str() return (rostedt)
v2 - https://lore.kernel.org/lkml/20231026194033.it.702-kees@kernel.org
v1 - https://lore.kernel.org/lkml/20231026170722.work.638-kees@kernel.org
---
include/linux/seq_buf.h | 21 +++++++++++++++++----
kernel/trace/trace.c | 11 +----------
lib/seq_buf.c | 4 +---
3 files changed, 19 insertions(+), 17 deletions(-)
diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
index 8483e4b2d0d2..5fb1f12c33f9 100644
--- a/include/linux/seq_buf.h
+++ b/include/linux/seq_buf.h
@@ -21,9 +21,18 @@ struct seq_buf {
size_t len;
};
+#define DECLARE_SEQ_BUF(NAME, SIZE) \
+ char __ ## NAME ## _buffer[SIZE] = ""; \
+ struct seq_buf NAME = { \
+ .buffer = &__ ## NAME ## _buffer, \
+ .size = SIZE, \
+ }
+
static inline void seq_buf_clear(struct seq_buf *s)
{
s->len = 0;
+ if (s->size)
+ s->buffer[0] = '\0';
}
static inline void
@@ -69,8 +78,8 @@ static inline unsigned int seq_buf_used(struct seq_buf *s)
}
/**
- * seq_buf_terminate - Make sure buffer is nul terminated
- * @s: the seq_buf descriptor to terminate.
+ * seq_buf_str - get %NUL-terminated C string from seq_buf
+ * @s: the seq_buf handle
*
* This makes sure that the buffer in @s is nul terminated and
* safe to read as a string.
@@ -81,16 +90,20 @@ static inline unsigned int seq_buf_used(struct seq_buf *s)
*
* After this function is called, s->buffer is safe to use
* in string operations.
+ *
+ * Returns @s->buf after making sure it is terminated.
*/
-static inline void seq_buf_terminate(struct seq_buf *s)
+static inline const char *seq_buf_str(struct seq_buf *s)
{
if (WARN_ON(s->size == 0))
- return;
+ return "";
if (seq_buf_buffer_left(s))
s->buffer[s->len] = 0;
else
s->buffer[s->size - 1] = 0;
+
+ return s->buffer;
}
/**
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index d629065c2383..2539cfc20a97 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -3828,15 +3828,6 @@ static bool trace_safe_str(struct trace_iterator *iter, const char *str,
return false;
}
-static const char *show_buffer(struct trace_seq *s)
-{
- struct seq_buf *seq = &s->seq;
-
- seq_buf_terminate(seq);
-
- return seq->buffer;
-}
-
static DEFINE_STATIC_KEY_FALSE(trace_no_verify);
static int test_can_verify_check(const char *fmt, ...)
@@ -3976,7 +3967,7 @@ void trace_check_vprintf(struct trace_iterator *iter, const char *fmt,
*/
if (WARN_ONCE(!trace_safe_str(iter, str, star, len),
"fmt: '%s' current_buffer: '%s'",
- fmt, show_buffer(&iter->seq))) {
+ fmt, seq_buf_str(&iter->seq.seq))) {
int ret;
/* Try to safely read the string */
diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index b7477aefff53..23518f77ea9c 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -109,9 +109,7 @@ void seq_buf_do_printk(struct seq_buf *s, const char *lvl)
if (s->size == 0 || s->len == 0)
return;
- seq_buf_terminate(s);
-
- start = s->buffer;
+ start = seq_buf_str(s);
while ((lf = strchr(start, '\n'))) {
int len = lf - start + 1;
--
2.34.1
reply other threads:[~2023-10-27 15:56 UTC|newest]
Thread overview: [no followups] expand[flat|nested] mbox.gz Atom feed
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=20231027155634.make.260-kees@kernel.org \
--to=keescook@chromium.org \
--cc=andriy.shevchenko@linux.intel.com \
--cc=arnd@arndb.de \
--cc=corbet@lwn.net \
--cc=gregkh@linuxfoundation.org \
--cc=hch@lst.de \
--cc=jacob.e.keller@intel.com \
--cc=justinstitt@google.com \
--cc=kent.overstreet@linux.dev \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-trace-kernel@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=mhiramat@kernel.org \
--cc=pmladek@suse.com \
--cc=rostedt@goodmis.org \
--cc=senozhatsky@chromium.org \
--cc=thunder.leizhen@huawei.com \
--cc=willy@infradead.org \
--cc=yosryahmed@google.com \
--cc=yun.zhou@windriver.com \
/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;
as well as URLs for NNTP newsgroup(s).