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 direction after terminating it
in refactored seq_buf_terminate(), now known as seq_buf_cstr():

        do_soemthing(seq_buf_cstr(s));

Cc: Steven Rostedt <rost...@goodmis.org>
Cc: "Matthew Wilcox (Oracle)" <wi...@infradead.org>
Cc: Christoph Hellwig <h...@lst.de>
Cc: Justin Stitt <justinst...@google.com>
Cc: Kent Overstreet <kent.overstr...@linux.dev>
Cc: Petr Mladek <pmla...@suse.com>
Cc: Andy Shevchenko <andriy.shevche...@linux.intel.com>
Cc: Rasmus Villemoes <li...@rasmusvillemoes.dk>
Cc: Sergey Senozhatsky <senozhat...@chromium.org>
Cc: Masami Hiramatsu <mhira...@kernel.org>
Cc: Greg Kroah-Hartman <gre...@linuxfoundation.org>
Cc: Arnd Bergmann <a...@arndb.de>
Cc: Jonathan Corbet <cor...@lwn.net>
Cc: Yun Zhou <yun.z...@windriver.com>
Cc: Jacob Keller <jacob.e.kel...@intel.com>
Cc: Zhen Lei <thunder.leiz...@huawei.com>
Cc: linux-trace-ker...@vger.kernel.org
Signed-off-by: Kees Cook <keesc...@chromium.org>
---
 include/linux/seq_buf.h | 19 +++++++++++++++----
 kernel/trace/trace.c    | 11 +----------
 lib/seq_buf.c           |  4 +---
 3 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
index 8483e4b2d0d2..8896b830eb3d 100644
--- a/include/linux/seq_buf.h
+++ b/include/linux/seq_buf.h
@@ -21,9 +21,16 @@ 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 +76,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_cstr - 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 +88,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 char *seq_buf_cstr(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..d83f36dc4bf8 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_cstr(&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..165caed5a74e 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_cstr(s);
        while ((lf = strchr(start, '\n'))) {
                int len = lf - start + 1;
 
-- 
2.34.1


Reply via email to