Hi Steve, Friendly ping :)
Do you mind picking this one up for 4.21 ? cheers Michael Ellerman <[email protected]> writes: > 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 <[email protected]> > Signed-off-by: Michael Ellerman <[email protected]> > --- > 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
