From patchwork Fri Oct 19 04:21:08 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Michael Ellerman X-Patchwork-Id: 10648647 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 ACAE013B0 for ; Fri, 19 Oct 2018 04:21:28 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9C5B628BBA for ; Fri, 19 Oct 2018 04:21:28 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8E32728BE1; Fri, 19 Oct 2018 04:21:28 +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 BDAC628BBA for ; Fri, 19 Oct 2018 04:21:27 +0000 (UTC) Received: (qmail 26320 invoked by uid 550); 19 Oct 2018 04:21:26 -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 26283 invoked from network); 19 Oct 2018 04:21:25 -0000 From: Michael Ellerman To: rostedt@goodmis.org Cc: linux-kernel@vger.kernel.org, kernel-hardening@lists.openwall.com, jannh@google.com, keescook@chromium.org Subject: [PATCH v2 1/2] seq_buf: Make seq_buf_puts() null-terminate the buffer Date: Fri, 19 Oct 2018 15:21:08 +1100 Message-Id: <20181019042109.8064-1-mpe@ellerman.id.au> X-Mailer: git-send-email 2.17.2 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 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 Signed-off-by: Michael Ellerman --- 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); From patchwork Fri Oct 19 04:21:09 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Michael Ellerman X-Patchwork-Id: 10648649 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 A4EB7109C for ; Fri, 19 Oct 2018 04:21:33 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 9259128BBA for ; Fri, 19 Oct 2018 04:21:33 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 8322528BE0; Fri, 19 Oct 2018 04:21:33 +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 BC0EF28BBA for ; Fri, 19 Oct 2018 04:21:32 +0000 (UTC) Received: (qmail 26585 invoked by uid 550); 19 Oct 2018 04:21:28 -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 26288 invoked from network); 19 Oct 2018 04:21:26 -0000 From: Michael Ellerman To: rostedt@goodmis.org Cc: linux-kernel@vger.kernel.org, kernel-hardening@lists.openwall.com, jannh@google.com, keescook@chromium.org Subject: [PATCH v2 2/2] seq_buf: Use size_t for len in seq_buf_puts() Date: Fri, 19 Oct 2018 15:21:09 +1100 Message-Id: <20181019042109.8064-2-mpe@ellerman.id.au> X-Mailer: git-send-email 2.17.2 In-Reply-To: <20181019042109.8064-1-mpe@ellerman.id.au> References: <20181019042109.8064-1-mpe@ellerman.id.au> X-Virus-Scanned: ClamAV using ClamSMTP Jann Horn points out that we're using unsigned int for len in seq_buf_puts(), which could potentially overflow if we're passed a UINT_MAX sized string. The rest of the code already uses size_t, so we should also use that in seq_buf_puts() to avoid any issues. Suggested-by: Jann Horn Signed-off-by: Michael Ellerman --- lib/seq_buf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) v2: New in v2. diff --git a/lib/seq_buf.c b/lib/seq_buf.c index 6aabb609dd87..bd807f545a9d 100644 --- a/lib/seq_buf.c +++ b/lib/seq_buf.c @@ -140,7 +140,7 @@ int seq_buf_bprintf(struct seq_buf *s, const char *fmt, const u32 *binary) */ int seq_buf_puts(struct seq_buf *s, const char *str) { - unsigned int len = strlen(str); + size_t len = strlen(str); WARN_ON(s->size == 0);