Skip to content

Commit

Permalink
seq_buf: Make seq_buf_puts() null-terminate the buffer
Browse files Browse the repository at this point in the history
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 zero 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.

Link: http://lkml.kernel.org/r/[email protected]

Acked-by: Kees Cook <[email protected]>
Signed-off-by: Michael Ellerman <[email protected]>
Signed-off-by: Steven Rostedt (VMware) <[email protected]>
  • Loading branch information
mpe authored and rostedt committed Dec 22, 2018
1 parent a448276 commit 0464ed2
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion lib/seq_buf.c
Original file line number Diff line number Diff line change
Expand Up @@ -144,9 +144,13 @@ int seq_buf_puts(struct seq_buf *s, const char *str)

WARN_ON(s->size == 0);

/* Add 1 to len for the trailing null byte which must be there */
len += 1;

if (seq_buf_can_fit(s, len)) {
memcpy(s->buffer + s->len, str, len);
s->len += len;
/* Don't count the trailing null byte against the capacity */
s->len += len - 1;
return 0;
}
seq_buf_set_overflow(s);
Expand Down

0 comments on commit 0464ed2

Please sign in to comment.