From patchwork Mon Nov 27 23:40:55 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Tobin Harding X-Patchwork-Id: 10078375 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2098D6028E for ; Mon, 27 Nov 2017 23:41:45 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1424C28F8B for ; Mon, 27 Nov 2017 23:41:45 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 08B3F2908E; Mon, 27 Nov 2017 23:41:45 +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=-4.1 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_MED,T_DKIM_INVALID 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 37FB228F8B for ; Mon, 27 Nov 2017 23:41:43 +0000 (UTC) Received: (qmail 5829 invoked by uid 550); 27 Nov 2017 23:41:35 -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 5680 invoked from network); 27 Nov 2017 23:41:33 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tobin.cc; h=cc :date:from:in-reply-to:message-id:references:subject:to :x-me-sender:x-me-sender:x-sasl-enc; s=fm1; bh=PCjuXVOw9J+ezPjXM GF5n0PQ644Fi669JE4ckawl0TI=; b=IE7iAnwNeySGykxUKI4S4ECOl1nlg0EKE Yw9ZOf7DSRZIMuv5dXtVhgGcsjjDeuGFrZXGcRw7eSMi9EO1dBcbRWM/HAydxSib PXVyVbbJnfKE4z+VTBOtHLfPra/wBkgUmpg1rBKhlkeKpGz8RyJ39q7dxraU1f0C iPNaX0M9dEI6P0wB5Cqb57epUaHiQ/dVj0pkyUD/shA1i8yen0mCCyQeDFPRKXT/ HtCh9bMWHOXNXONMJBOoru9abIDjicnoqtYoo9a1nFvYei/l78lTQ+XkNnhq3xCZ Acg1U0fC01GcZ2K1xi/4zxKB+c20ACZ3wTolQlkpBObGFQrn9nhQA== DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:date:from:in-reply-to:message-id :references:subject:to:x-me-sender:x-me-sender:x-sasl-enc; s= fm1; bh=PCjuXVOw9J+ezPjXMGF5n0PQ644Fi669JE4ckawl0TI=; b=DZ3ACfjB S9z3ja5qHlmPkYnnlU5yaYottYzCxwCtmqj8ArQ601tgjA8RhLAyBdcUnH9CdlIC xaWuPE87jy/GZz+QWD5GbKGg67F2Qj9LtJH0EGtgcBVPtYYKszVcdAE1bUSve/Wk kDjkjIeDjVdYnk4Jc0i+gDDN+Y5CNmv43y4MXb5ftOOYs0XaJXukLwWYouPlXE2D geACI48XwKlQrXBjRgRNLspo87gj0/8mledCgImVzogw7ELS+pDr+TBbOXU3ItSb PcJJO6oGCQXy3QetfAIg+DmJQvXJmNLqeDB9SD3KvU0Z1dNPBOB12CmdlqORGJww 0PZEEtWucSyGHA== X-ME-Sender: From: "Tobin C. Harding" To: Linus Torvalds Cc: "Tobin C. Harding" , "Jason A. Donenfeld" , Theodore Ts'o , Kees Cook , Paolo Bonzini , Tycho Andersen , "Roberts, William C" , Tejun Heo , Jordan Glover , Greg KH , Petr Mladek , Joe Perches , Ian Campbell , Sergey Senozhatsky , Catalin Marinas , Will Deacon , Steven Rostedt , Chris Fries , Dave Weinstein , Daniel Micay , Djalal Harouni , =?UTF-8?q?Radim=20Kr=C4=8Dm=C3=A1=C5=99?= , linux-kernel@vger.kernel.org, kvm@vger.kernel.org, kernel-hardening@lists.openwall.com Date: Tue, 28 Nov 2017 10:40:55 +1100 Message-Id: <1511826058-2563-3-git-send-email-me@tobin.cc> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1511826058-2563-1-git-send-email-me@tobin.cc> References: <1511826058-2563-1-git-send-email-me@tobin.cc> Subject: [kernel-hardening] [PATCH 2/5] vsprintf: refactor pK code out of pointer() X-Virus-Scanned: ClamAV using ClamSMTP Currently code to handle %pK is all within the switch statement in pointer(). This is the wrong level of abstraction. Each of the other switch clauses call a helper function, pK should do the same. Refactor code out of pointer() to new function kernel_pointer(). Signed-off-by: Tobin C. Harding --- lib/vsprintf.c | 97 ++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 54 insertions(+), 43 deletions(-) diff --git a/lib/vsprintf.c b/lib/vsprintf.c index 1746bae94d41..360731f81dd3 100644 --- a/lib/vsprintf.c +++ b/lib/vsprintf.c @@ -1343,6 +1343,59 @@ char *uuid_string(char *buf, char *end, const u8 *addr, return string(buf, end, uuid, spec); } +int kptr_restrict __read_mostly; + +static noinline_for_stack +char *kernel_pointer(char *buf, char *end, const void *ptr, + struct printf_spec spec) +{ + spec.base = 16; + spec.flags |= SMALL; + if (spec.field_width == -1) { + spec.field_width = 2 * sizeof(ptr); + spec.flags |= ZEROPAD; + } + + switch (kptr_restrict) { + case 0: + /* Always print %pK values */ + break; + case 1: { + const struct cred *cred; + + /* + * kptr_restrict==1 cannot be used in IRQ context + * because its test for CAP_SYSLOG would be meaningless. + */ + if (in_irq() || in_serving_softirq() || in_nmi()) + return string(buf, end, "pK-error", spec); + + /* + * Only print the real pointer value if the current + * process has CAP_SYSLOG and is running with the + * same credentials it started with. This is because + * access to files is checked at open() time, but %pK + * checks permission at read() time. We don't want to + * leak pointer values if a binary opens a file using + * %pK and then elevates privileges before reading it. + */ + cred = current_cred(); + if (!has_capability_noaudit(current, CAP_SYSLOG) || + !uid_eq(cred->euid, cred->uid) || + !gid_eq(cred->egid, cred->gid)) + ptr = NULL; + break; + } + case 2: + default: + /* Always print 0's for %pK */ + ptr = NULL; + break; + } + + return number(buf, end, (unsigned long)ptr, spec); +} + static noinline_for_stack char *netdev_bits(char *buf, char *end, const void *addr, const char *fmt) { @@ -1591,8 +1644,6 @@ char *device_node_string(char *buf, char *end, struct device_node *dn, return widen_string(buf, buf - buf_start, end, spec); } -int kptr_restrict __read_mostly; - /* * Show a '%p' thing. A kernel extension is that the '%p' is followed * by an extra set of alphanumeric characters that are extended format @@ -1792,47 +1843,7 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, return buf; } case 'K': - switch (kptr_restrict) { - case 0: - /* Always print %pK values */ - break; - case 1: { - const struct cred *cred; - - /* - * kptr_restrict==1 cannot be used in IRQ context - * because its test for CAP_SYSLOG would be meaningless. - */ - if (in_irq() || in_serving_softirq() || in_nmi()) { - if (spec.field_width == -1) - spec.field_width = default_width; - return string(buf, end, "pK-error", spec); - } - - /* - * Only print the real pointer value if the current - * process has CAP_SYSLOG and is running with the - * same credentials it started with. This is because - * access to files is checked at open() time, but %pK - * checks permission at read() time. We don't want to - * leak pointer values if a binary opens a file using - * %pK and then elevates privileges before reading it. - */ - cred = current_cred(); - if (!has_capability_noaudit(current, CAP_SYSLOG) || - !uid_eq(cred->euid, cred->uid) || - !gid_eq(cred->egid, cred->gid)) - ptr = NULL; - break; - } - case 2: - default: - /* Always print 0's for %pK */ - ptr = NULL; - break; - } - break; - + return kernel_pointer(buf, end, ptr, spec); case 'N': return netdev_bits(buf, end, ptr, fmt); case 'a':