[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+55aFw-GgR7DmXsD1dmYWbxuVhg+u0oKwuwNEM4RnuVyA0NDw@mail.gmail.com>
Date: Tue, 7 Nov 2017 13:44:01 -0800
From: Linus Torvalds <torvalds@...ux-foundation.org>
To: Kees Cook <keescook@...omium.org>
Cc: "Tobin C. Harding" <me@...in.cc>, "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>,
Andy Lutomirski <luto@...nel.org>, Joe Perches <joe@...ches.com>,
Network Development <netdev@...r.kernel.org>, David Miller <davem@...emloft.net>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>,
"Jason A. Donenfeld" <Jason@...c4.com>, "Theodore Ts'o" <tytso@....edu>, Paolo Bonzini <pbonzini@...hat.com>,
Tycho Andersen <tycho@...ker.com>, "Roberts, William C" <william.c.roberts@...el.com>,
Tejun Heo <tj@...nel.org>, Jordan Glover <Golden_Miller83@...tonmail.ch>,
Greg KH <gregkh@...uxfoundation.org>, Petr Mladek <pmladek@...e.com>,
Ian Campbell <ijc@...lion.org.uk>, Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
Catalin Marinas <catalin.marinas@....com>, Will Deacon <wilal.deacon@....com>,
Steven Rostedt <rostedt@...dmis.org>, Chris Fries <cfries@...gle.com>,
Dave Weinstein <olorin@...gle.com>, Daniel Micay <danielmicay@...il.com>,
Djalal Harouni <tixxdz@...il.com>, Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3] scripts: add leaking_addresses.pl
On Tue, Nov 7, 2017 at 1:22 PM, Kees Cook <keescook@...omium.org> wrote:
>
> Linus, what do you have in mind for the root-only "yes we really need
> the actual address output" exceptions?
I am convinced that absolutely none of them should use '%pK'.
So far we have actually never seen a valid case wher %pK was really
the right thing to do.
> For example, right now /sys/kernel/debug/kernel_page_tables
> (CONFIG_X86_PTDUMP=y) needs actual address and currently uses %x.
So I think it could continue to use %x, and just make sure the whole
file is root-only.
And that is why %pK is so wrong. It's almost never really about root.
Look at /proc/kallsyms, for example. There it's mainly about kernel
profiles (although there certainly have been other uses historically,
and maybe some of them remain) - which we have another flag for
entirely that is very much specifically about kernel profiles.
> Looking other places that stand out, it seems like
> /proc/lockdep_chains and /proc/lockdep (CONFIG_LOCKDEP=y) has a ton of
> %p usage. It's unclear to me if a hash is sufficient for meaningful
> debugging there?
Maybe not, but that is also _so_ esoteric that I suspect the right fix
is to just make it root-only readable.
I've never used it, we should check with people who have. I get the
feeling that this is purely for PeterZ debugging.
The very first commit that introduced that code actually has a
(FIXME: should go into debugfs)
so I suspect it never should have been user-readable to begin with. I
guess it makes some things easier, but it really is *very* different
from things like profiling.
Profiling you often *cannot* do as root - some things you profile
really shouldn't be run as root, and might even refuse to do so. So
requiring you to be root just to get a kernel profile is very bad.
But looking at lockdep stats? Yeah, 'sudo' isn't so big of a deal.
And I really suspect that's true of a _lot_ of these %p things that
really want a pointer. It's not that they really want %pK, it's that
they shouldn't have been visible to regular users in the first place.
Things that *do* want a pointer and should be visible to regular users
are things like oops messages etc, but even there we obviously
generally want to use %pS/%pF when possible (but generally %x when not
- things like register contents etc that *may* contain pointers).
And if they really are visible to users - because you want to
cross-correlate them for things like netstat - I think the hashing is
the right thing to do both for root and for regular users.
> Seems like these three from dmesg could be removed?
>
> [ 0.000000] Base memory trampoline at [ffffa3fc40099000] 99000 size 24576
> arch/x86/realmode/init.c
>
> [ 0.000000] percpu: Embedded 38 pages/cpu @ffffa4007fc00000 s116944
> r8192 d30512 u524288
> mm/percpu.c
>
> [ 0.456395] software IO TLB [mem 0xbbfdf000-0xbffdf000] (64MB)
> mapped at [ffffa3fcfbfdf000-ffffa3fcfffdefff]
> lib/swiotlb.c
Yes, I think the solution for a lot of the random device discovery
messages etc is to just remove them. They were likely useful when the
code was new and untested, and just stayed around afterwards.
Linus
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
