[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20181019042109.8064-1-mpe@ellerman.id.au>
Date: Fri, 19 Oct 2018 15:21:08 +1100
From: Michael Ellerman <mpe@...erman.id.au>
To: rostedt@...dmis.org
Cc: linux-kernel@...r.kernel.org,
kernel-hardening@...ts.openwall.com,
jannh@...gle.com,
keescook@...omium.org
Subject: [PATCH v2 1/2] seq_buf: Make seq_buf_puts() null-terminate the buffer
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.
Acked-by: Kees Cook <keescook@...omium.org>
Signed-off-by: Michael Ellerman <mpe@...erman.id.au>
---
lib/seq_buf.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
v2: Fix NULL/null terminology.
diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index 11f2ae0f9099..6aabb609dd87 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -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);
--
2.17.2
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
