From patchwork Thu Nov 30 03:58:26 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Joe Perches X-Patchwork-Id: 10084231 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 B3CB160311 for ; Thu, 30 Nov 2017 03:58:49 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A7BA829C06 for ; Thu, 30 Nov 2017 03:58:49 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9ADA129D33; Thu, 30 Nov 2017 03:58:49 +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.2 required=2.0 tests=BAYES_00, 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 0C63429C06 for ; Thu, 30 Nov 2017 03:58:47 +0000 (UTC) Received: (qmail 24377 invoked by uid 550); 30 Nov 2017 03:58:45 -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 24359 invoked from network); 30 Nov 2017 03:58:45 -0000 X-Session-Marker: 6A6F6540706572636865732E636F6D X-HE-Tag: bit89_2cd0b7418d854 X-Filterd-Recvd-Size: 6044 Message-ID: <1512014306.19952.80.camel@perches.com> From: Joe Perches To: "Tobin C. Harding" , Andrew Morton Cc: kernel-hardening@lists.openwall.com, 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 , Ian Campbell , Sergey Senozhatsky , Catalin Marinas , Will Deacon , Steven Rostedt , Chris Fries , Dave Weinstein , Daniel Micay , Djalal Harouni , Radim =?UTF-8?Q?Kr=C4=8Dm=C3=A1=C5=99?= , linux-kernel@vger.kernel.org, Network Development , David Miller , Stephen Rothwell , Andrey Ryabinin , Alexander Potapenko , Dmitry Vyukov Date: Wed, 29 Nov 2017 19:58:26 -0800 In-Reply-To: <20171129232603.GO6217@eros> References: <1511921105-3647-1-git-send-email-me@tobin.cc> <1511921105-3647-5-git-send-email-me@tobin.cc> <20171129152058.389361d3576e784f47326cac@linux-foundation.org> <20171129232603.GO6217@eros> X-Mailer: Evolution 3.26.1-1 Mime-Version: 1.0 Subject: [kernel-hardening] Re: [PATCH V11 4/5] vsprintf: add printk specifier %px X-Virus-Scanned: ClamAV using ClamSMTP On Thu, 2017-11-30 at 10:26 +1100, Tobin C. Harding wrote: > On Wed, Nov 29, 2017 at 03:20:58PM -0800, Andrew Morton wrote: > > On Wed, 29 Nov 2017 13:05:04 +1100 "Tobin C. Harding" wrote: > > > > > printk specifier %p now hashes all addresses before printing. Sometimes > > > we need to see the actual unmodified address. This can be achieved using > > > %lx but then we face the risk that if in future we want to change the > > > way the Kernel handles printing of pointers we will have to grep through > > > the already existent 50 000 %lx call sites. Let's add specifier %px as a > > > clear, opt-in, way to print a pointer and maintain some level of > > > isolation from all the other hex integer output within the Kernel. > > > > > > Add printk specifier %px to print the actual unmodified address. > > > > > > ... > > > > > > +Unmodified Addresses > > > +==================== > > > + > > > +:: > > > + > > > + %px 01234567 or 0123456789abcdef > > > + > > > +For printing pointers when you _really_ want to print the address. Please > > > +consider whether or not you are leaking sensitive information about the > > > +Kernel layout in memory before printing pointers with %px. %px is > > > +functionally equivalent to %lx. %px is preferred to %lx because it is more > > > +uniquely grep'able. If, in the future, we need to modify the way the Kernel > > > +handles printing pointers it will be nice to be able to find the call > > > +sites. > > > + > > > > You might want to add a checkpatch rule which emits a stern > > do-you-really-want-to-do-this warning when someone uses %px. > > > > Oh, nice idea. It has to be a CHECK but right? No, it has to be something that's not --strict so a WARN would probably be best. > By stern, you mean use stern language? I hope he doesn't mean tweet. Something like: --- scripts/checkpatch.pl | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-) diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl index 0ce249f157a1..9d789cbe7df5 100755 --- a/scripts/checkpatch.pl +++ b/scripts/checkpatch.pl @@ -5758,21 +5758,40 @@ sub process { defined $stat && $stat =~ /^\+(?![^\{]*\{\s*).*\b(\w+)\s*\(.*$String\s*,/s && $1 !~ /^_*volatile_*$/) { + my $complete_extension = ""; + my $extension = ""; my $bad_extension = ""; my $lc = $stat =~ tr@\n@@; $lc = $lc + $linenr; + my $stat_real; for (my $count = $linenr; $count <= $lc; $count++) { my $fmt = get_quoted_string($lines[$count - 1], raw_line($count, 0)); $fmt =~ s/%%//g; - if ($fmt =~ /(\%[\*\d\.]*p(?![\WFfSsBKRraEhMmIiUDdgVCbGNO]).)/) { - $bad_extension = $1; - last; + while ($fmt =~ /(\%[\*\d\.]*p(\w))/g) { + $complete_extension = $1; + $extension = $2; + if ($extension !~ /[FfSsBKRraEhMmIiUDdgVCbGNOx]/) { + $bad_extension = $complete_extension; + last; + } + if ($extension eq "x") { + if (!defined($stat_real)) { + $stat_real = raw_line($linenr, 0); + for (my $count = $linenr + 1; $count <= $lc; $count++) { + $stat_real = $stat_real . "\n" . raw_line($count, 0); + } + } + WARN("VSPRINTF_POINTER_PX", + "Using vsprintf pointer extension '$complete_extension' exposes kernel address for possible hacking\n" . "$here\n$stat_real\n"); + } } } if ($bad_extension ne "") { - my $stat_real = raw_line($linenr, 0); - for (my $count = $linenr + 1; $count <= $lc; $count++) { - $stat_real = $stat_real . "\n" . raw_line($count, 0); + if (!defined($stat_real)) { + $stat_real = raw_line($linenr, 0); + for (my $count = $linenr + 1; $count <= $lc; $count++) { + $stat_real = $stat_real . "\n" . raw_line($count, 0); + } } WARN("VSPRINTF_POINTER_EXTENSION", "Invalid vsprintf pointer extension '$bad_extension'\n" . "$here\n$stat_real\n");