From patchwork Wed Nov 29 02:05:02 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Tobin C. Harding" X-Patchwork-Id: 842400 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=netdev-owner@vger.kernel.org; receiver=) Authentication-Results: ozlabs.org; dkim=pass (2048-bit key; unprotected) header.d=tobin.cc header.i=@tobin.cc header.b="cjvINOji"; dkim=pass (2048-bit key; unprotected) header.d=messagingengine.com header.i=@messagingengine.com header.b="Du/b//yu"; dkim-atps=neutral Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3ymkRz5qDqz9s7g for ; Wed, 29 Nov 2017 13:06:55 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752523AbdK2CGo (ORCPT ); Tue, 28 Nov 2017 21:06:44 -0500 Received: from out4-smtp.messagingengine.com ([66.111.4.28]:51781 "EHLO out4-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752176AbdK2CFc (ORCPT ); Tue, 28 Nov 2017 21:05:32 -0500 Received: from compute5.internal (compute5.nyi.internal [10.202.2.45]) by mailout.nyi.internal (Postfix) with ESMTP id 098C820AE6; Tue, 28 Nov 2017 21:05:31 -0500 (EST) Received: from frontend2 ([10.202.2.161]) by compute5.internal (MEProxy); Tue, 28 Nov 2017 21:05:31 -0500 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=2Wn5NKHohUMYbPrsk pjKf4XrED+feKupArEt+MS7MRc=; b=cjvINOji5Zy4Jx8CJXOmeOO/RWTk3qXbV OhC7QO5tnam7qMUp1h1g7O7BA2s/3P7yeQ6wS7wPRSvfdfB9mZyHvhIqyKv8tadi 7TQGyIIvsb9AWhLsArojwBHvX7C5fdovF4voYyte+9cgykBXPs+jjweL1JeQHs9x n72ElwMWip5ivdtzuxY2PdT5fsamejJrsO/sd1BHXmLQDnnL/7RhXX0427nHJh2X +BR9EGsYPk1b3gWbmMd8FEFGewBTHqDeimFRcsDDalxe+c7IIUNFrp7Eehhsckwq KR36ouD+bpmFUvZpi9WDCGImC9NW/+VS3eYmpSUl1lzg9Hy9MENOQ== 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=2Wn5NKHohUMYbPrskpjKf4XrED+feKupArEt+MS7MRc=; b=Du/b//yu OmJUpxqTGPZz4Seh5AJd60Z5B4OIgq5b1gCE13TwuGK+wenFR19dtEmAWTuMBlz8 oPVS2p5izrP/23ge/VB0h91kiH9lDzF6cEMvAVA0bWgY2hikk1o8pfpXN76eQwuF jbDXq0IjecMrTHpaNvODDXSc/Ngd3RetD4mt//LK1RNkh8iQpgLVKibZjFdg5fT5 m0BP9dV+HL2gpk52KfUyyCP25cCgPDkXqtOftP/Ai+m8OF0niQ+StCIrUW8dipIK XTaGU91a8IXKmQi/F60wQ6ibWyc6JR66lRp1KXP9opvoQ4tW8RK006pxO6atI0t1 qN7KY5pT14bH/w== X-ME-Sender: Received: from localhost (202-159-174-127.dyn.iinet.net.au [202.159.174.127]) by mail.messagingengine.com (Postfix) with ESMTPA id 107B22471E; Tue, 28 Nov 2017 21:05:30 -0500 (EST) From: "Tobin C. Harding" To: kernel-hardening@lists.openwall.com Cc: "Tobin C. Harding" , Linus Torvalds , "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?b?UmFkaW0gS3LEjW3DocWZ?= , linux-kernel@vger.kernel.org, Network Development , David Miller , Stephen Rothwell , Andrey Ryabinin , Alexander Potapenko , Dmitry Vyukov , Andrew Morton Subject: [PATCH V11 2/5] vsprintf: refactor %pK code out of pointer() Date: Wed, 29 Nov 2017 13:05:02 +1100 Message-Id: <1511921105-3647-3-git-send-email-me@tobin.cc> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1511921105-3647-1-git-send-email-me@tobin.cc> References: <1511921105-3647-1-git-send-email-me@tobin.cc> Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org 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 restricted_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..8dc5cf85cef4 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 *restricted_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 restricted_pointer(buf, end, ptr, spec); case 'N': return netdev_bits(buf, end, ptr, fmt); case 'a':