linux-kernel - Re: [kernel-hardening] [RFC V2 4/6] lib: vsprintf: default kptr

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CA+55aFxBrFsN0wm1_SyafNZ6GHxCLBvwmRaAzfKD9xnS7K3XGg@mail.gmail.com>
Date:   Wed, 4 Oct 2017 10:08:02 -0700
From:   Linus Torvalds <torvalds@...ux-foundation.org>
To:     Kees Cook <keescook@...omium.org>
Cc:     "Tobin C. Harding" <me@...in.cc>,
        Greg KH <gregkh@...uxfoundation.org>,
        Petr Mladek <pmladek@...e.com>, Joe Perches <joe@...ches.com>,
        Ian Campbell <ijc@...lion.org.uk>,
        Sergey Senozhatsky <sergey.senozhatsky@...il.com>,
        "kernel-hardening@...ts.openwall.com" 
        <kernel-hardening@...ts.openwall.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Catalin Marinas <catalin.marinas@....com>,
        Will Deacon <will.deacon@....com>,
        Steven Rostedt <rostedt@...dmis.org>,
        William Roberts <william.c.roberts@...el.com>,
        Chris Fries <cfries@...gle.com>,
        Dave Weinstein <olorin@...gle.com>
Subject: Re: [kernel-hardening] [RFC V2 4/6] lib: vsprintf: default
 kptr_restrict to the maximum value

On Wed, Oct 4, 2017 at 9:42 AM, Kees Cook <keescook@...omium.org> wrote:
>
> I'd argue that a default of "1" would be a sensible starting place,
> but that can be a separate patch, IMO.

I agree that '1' is a much saner default for _some_ uses, in that it
still gives root access to /proc file data etc.

However, the sad fact is that kptr_restrict just has bad semantics for
that case too, in that you do want to give root access to /proc files,
but the whole "is the current thread root" is a horrible horrible
test.

Partly it's horrible for the reasons mentioned in the source code (ie
the whole IRQ context thing etc), but that's actually the smallest
reason it's not great: the more fundamental issue is that even for
/proc files, it should use the cred for the file opener, not the
current user.

And for anything *but* /proc files, it's almost always the wrong thing
(ie random printk's aren't generally really associated with any user).

It might almost by mistake do the right thing (ie some kernel printk
that can be triggered by a normal user doing something odd), so it's
not like it always does the wrong thing, but it really is almost
entirely random.

So I seriously dislike kptr_restrict. The whole thing is mis-designed
for very very fundamental reasons.

And no, I also refuse to believe that the fix is "just make it have a
value of 4, and just hide all pointers". Because that will just mean
that people won't be using '%p' at all for debug messages etc, they'll
just use %x or whatever.

So I honestly doubt the value of kptr_restrict. Any *sane* policy
pretty much has to be in the caller, and by thinking about what you
print out. IOW, things like proc_pid_wchan().

                    Linus