Re: [PATCH] seq_buf: Make seq_buf_puts() NULL terminate the buffer

[Steven Rostedt]

On Wed, 17 Oct 2018 23:10:00 +1100
Michael Ellerman <mpe@xxxxxxxxxxxxxx> wrote:

> Currently seq_buf_puts() will happily create a non NULL terminated
> string for you in the buffer. This is particularly dangerous if the
> buffer is on the stack.
> 
> For example:
> 
>   char buf[8];
>   char secret = "secret";
>   struct seq_buf s;
> 
>   seq_buf_init(&s, buf, sizeof(buf));
>   seq_buf_puts(&s, "foo");
>   printk("Message is %s\n", buf);
> 
> Can result in:
> 
>   Message is fooÂÂÂÂÂsecret
> 
> We could require all users to memset() their buffer to NULL before
> use. But that seems likely to be forgotten and lead to bugs.
> 
> Instead we can change seq_buf_puts() to always leave the buffer in a
> NULL terminated state.
> 
> The only downside is that this makes the buffer 1 character smaller
> for seq_buf_puts(), but that seems like a good trade off.
> 
> Signed-off-by: Michael Ellerman <mpe@xxxxxxxxxxxxxx>
>

It's been on my todo list for some time, and that was to add this
(trace-cmd had this for years). Would this be useful?

Instead of always writing a nul pointer, just terminate it when you are
done.


-- Steve


diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index 11f2ae0f9099..46f2a8b3e733 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -175,6 +175,29 @@ int seq_buf_putc(struct seq_buf *s, unsigned char c)
 }
 
 /**
+ * seq_buf_terminate - force the buffer to end in a nul character
+ * @s: seq_buf descriptor
+ *
+ * Make sure the current buffer terminates with a nul '\0' character.
+ * This does not increment the length of the buffer.
+ *
+ * Returns 0 if there was room to add the nul character.
+ *        -1 if there was not room. But a nul character was
+ *           still added, overwriting the last character in the buffer.
+ */
+int seq_buf_terminate(struct seq_buf *s)
+{
+	WARN_ON(s->size == 0);
+
+	if (seq_buf_can_fit(s, 1)) {
+		s->buffer[s->len] = '\0';
+		return 0;
+	}
+	s->buffer[--s->len] = '\0';
+	return -1;
+}
+
+/**
  * seq_buf_putmem - write raw data into the sequenc buffer
  * @s: seq_buf descriptor
  * @mem: The raw memory to copy into the buffer