From patchwork Wed Oct 17 12:10:00 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Michael Ellerman X-Patchwork-Id: 10645391 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork-2.web.codeaurora.org (Postfix) with ESMTP id 627423C13 for ; Wed, 17 Oct 2018 12:10:24 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4FEF52AE93 for ; Wed, 17 Oct 2018 12:10:24 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 4E24A2AED5; Wed, 17 Oct 2018 12:10:24 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=2.0 tests=BAYES_00,MAILING_LIST_MULTI, RCVD_IN_DNSWL_MED autolearn=ham version=3.3.1 Received: from mother.openwall.net (mother.openwall.net [195.42.179.200]) by mail.wl.linuxfoundation.org (Postfix) with SMTP id 301B22AEA2 for ; Wed, 17 Oct 2018 12:10:22 +0000 (UTC) Received: (qmail 17773 invoked by uid 550); 17 Oct 2018 12:10:20 -0000 Mailing-List: contact kernel-hardening-help@lists.openwall.com; run by ezmlm Precedence: bulk List-Post: List-Help: List-Unsubscribe: List-Subscribe: List-ID: Delivered-To: mailing list kernel-hardening@lists.openwall.com Received: (qmail 17735 invoked from network); 17 Oct 2018 12:10:19 -0000 From: Michael Ellerman To: rostedt@goodmis.org Cc: linux-kernel@vger.kernel.org, kernel-hardening@lists.openwall.com Subject: [PATCH] seq_buf: Make seq_buf_puts() NULL terminate the buffer Date: Wed, 17 Oct 2018 23:10:00 +1100 Message-Id: <20181017121000.30240-1-mpe@ellerman.id.au> X-Mailer: git-send-email 2.17.1 MIME-Version: 1.0 X-Virus-Scanned: ClamAV using ClamSMTP 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 Acked-by: Kees Cook --- lib/seq_buf.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) I recently merged a patch which actually hit this behaviour. I worked around it by using seq_buf_printf(), but it would be good to fix the problem at the source. diff --git a/lib/seq_buf.c b/lib/seq_buf.c index 11f2ae0f9099..b1570204cde3 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 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 against the capacity */ + s->len += len - 1; return 0; } seq_buf_set_overflow(s);