From: Andy Lutomirski <luto@amacapital.net>
To: PaX Team <pageexec@freemail.hu>
Cc: Ingo Molnar <mingo@kernel.org>, Kees Cook <keescook@chromium.org>,
Christoph Lameter <cl@linux.com>,
Andrew Morton <akpm@linux-foundation.org>,
Brad Spengler <spender@grsecurity.net>,
Pekka Enberg <penberg@kernel.org>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>,
Casey Schaufler <casey@schaufler-ca.com>,
Will Deacon <will.deacon@arm.com>, Rik van Riel <riel@redhat.com>,
Benjamin Herrenschmidt <benh@kernel.crashing.org>,
Dmitry Vyukov <dvyukov@google.com>,
"linux-ia64@vger.kernel.org" <linux-ia64@vger.kernel.org>,
"linux-arm-kernel@lists.infradead.org"
<linux-arm-kernel@lists.infradead.org>, X86 ML <x86@kernel.org>,
Catalin Marinas <catalin.marinas@arm.com>,
linux-arch <linux-arch@vger.kernel.org>,
David Rientjes <rientjes@google.com>,
Mathias Krause <minipli@googlemail.com>,
"kernel-hardening@lists.openwall.com"
<kernel-hardening@lists.openwall.com>,
"David S. Miller" <davem@davemloft.net>,
Laura Abbott <labbott@fedoraproject.org>,
"linux-mm@kvack.org" <linux-mm@kvack.org>,
Jan Kara <jack@suse.cz>, Russell King <linux@armlinux.org.uk>,
Michael Ellerman <mpe@ellerman.id.au>,
Andrea Arcangeli <aarcange@redhat.com>,
Fenghua Yu <fenghua.yu@intel.com>,
linuxppc-dev@lists.ozlabs.org, Vitaly Wool <vitalywool@gmail.com>,
"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
Borislav Petkov <bp@suse.de>, Tony Luck <tony.luck@intel.com>,
Joonsoo Kim <iamjoonsoo.kim@lge.com>,
sparclinux@vger.kernel.org,
Linus Torvalds <torvalds@linux-foundation.org>,
Thomas Gleixner <tglx@linutronix.de>,
Peter Zijlstra <a.p.zijlstra@chello.nl>,
"H. Peter Anvin" <hpa@zytor.com>
Subject: Re: [PATCH 0/9] mm: Hardened usercopy
Date: Sun, 10 Jul 2016 05:38:31 -0700 [thread overview]
Message-ID: <CALCETrXfdEbmSTs6XkZjHkAc3W_380bpde4bWQgRA5CQM0PtLA@mail.gmail.com> (raw)
In-Reply-To: <5782398B.32731.26E46C3D@pageexec.freemail.hu>
On Sun, Jul 10, 2016 at 5:03 AM, PaX Team <pageexec@freemail.hu> wrote:
> On 10 Jul 2016 at 11:16, Ingo Molnar wrote:
>
>> * PaX Team <pageexec@freemail.hu> wrote:
>>
>> > On 9 Jul 2016 at 14:27, Andy Lutomirski wrote:
>> >
>> > > I like the series, but I have one minor nit to pick. The effect of this
>> > > series is to harden usercopy, but most of the code is really about
>> > > infrastructure to validate that a pointed-to object is valid.
>> >
>> > actually USERCOPY has never been about validating pointers. its sole purpose is
>> > to validate the *size* argument of copy*user calls, a very specific form of
>> > runtime bounds checking.
>>
>> What this code has been about originally is largely immaterial, unless you can
>> formulate it into a technical argument.
>
> we design defense mechanisms for specific and clear purposes, starting with
> a threat model, evaluating defense options based on various criteria, etc.
> USERCOPY underwent this same process and taking it out of its original context
> means that all you get in the end is cargo cult security (wouldn't be the first
> time it has happened (ExecShield, ASLR, etc)).
>
> that said, i actually started that discussion but for some reason you chose
> not to respond to that one part of my mail so let me ask it again:
>
> what kind of checks are you thinking of here? and more fundamentally, against
> what kind of threats?
>
> as far as i'm concerned, a defense mechanism is only as good as its underlying
> threat model. by validating pointers (for yet to be stated security related
> properties) you're presumably assuming some kind of threat and unless stated
> clearly what that threat is (unintended pointer modification through memory
> corruption and/or other bugs?) noone can tell whether the proposed defense
> mechanism will actually be effective in preventing exploitation. it is the
> worst kind of defense that doesn't actually achieve its stated goals, that
> way lies false sense of security and i hope noone here is in that business.
I'm imaging security bugs that involve buffer length corruption but
that don't call copy_to/from_user. Hardened usercopy shuts
expoitation down if the first use of the corrupt size is
copy_to/from_user or similar. I bet that a bit better coverage could
be achieved by instrumenting more functions.
To be clear: I'm not objecting to calling the overall feature hardened
usercopy or similar. I object to
CONFIG_HAVE_HARDENED_USERCOPY_ALLOCATOR. That feature is *used* for
hardened usercopy but is not, in and of itself, a usercopy thing.
It's an object / memory range validation thing. So we'll feel silly
down the road if we use it for something else and the config option
name has nothing to do with the feature.
>> > [...] like the renaming of .data..read_only to .data..ro_after_init which also
>> > had nothing to do with init but everything to do with objects being conceptually
>> > read-only...
>>
>> .data..ro_after_init objects get written to during bootup so it's conceptually
>> quite confusing to name it "read-only" without any clear qualifiers.
>>
>> That it's named consistently with its role of "read-write before init and read
>> only after init" on the other hand is not confusing at all. Not sure what your
>> problem is with the new name.
>
> the new name reflects a complete misunderstanding of the PaX feature it was based
> on (typical case of cargo cult security). in particular, the __read_only facility
> in PaX is part of a defense mechanism that attempts to solve a specific problem
> (like everything else) and that problem has nothing whatsoever to do with what
> happens before/after the kernel init process. enforcing read-ony kernel memory at
> the end of kernel initialization is an implementation detail only and wasn't even
> true always (and still isn't true for kernel modules for example): in the linux 2.4
> days PaX actually enforced read-only kernel memory properties in startup_32 already
> but i relaxed that for the 2.6+ port as the maintenance cost (finding out and
> handling new exceptional cases) wasn't worth it.
>
> also naming things after their implementation is poor taste and can result in
> even bigger problems down the line since as soon as the implementation changes,
> you will have a flag day or have to keep a bad name. this is a lesson that the
> REFCOUNT submission will learn too since the kernel's atomic*_t types (an
> implementation detail) are used extensively for different purposes, instead of
> using specialized types (kref is a good example of that). for .data..ro_after_init
> the lesson will happen when you try to add back the remaining pieces from PaX,
> such as module handling and not-always-const-in-the-C-sense objects and associated
> accessors.
The name is related to how the thing works. If I understand
correctly, in PaX, the idea is to make some things readonly and use
pax_open_kernel(), etc to write it as needed. This is a nifty
mechanism, but it's *not* what .data..ro_after_init does upstream. If
I mark something __ro_after_init, then I can write it freely during
boot, but I can't write it thereafter. In contrast, if I put
something in .rodata (using 'const', for example), then I must not
write it *at all* unless I use special helpers (kmap, pax_open_kernel,
etc). So the practical effect from a programer's perspective of
__ro_after_init is quite different from .rodata, and I think the names
should reflect that.
(And yes, the upstream kernel should soon have __ro_after_init working
in modules. And the not-always-const-in-the-C-sense objects using
accessors will need changes to add those accessors, and we can and
should change the annotation on the object itself at the same time.
But if I mark something __ro_after_init, I can write it using normal C
during init, and there's nothing wrong with that.)
--Andy
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
next prev parent reply other threads:[~2016-07-10 12:38 UTC|newest]
Thread overview: 56+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-06 22:25 [PATCH 0/9] mm: Hardened usercopy Kees Cook
2016-07-06 22:25 ` [PATCH 1/9] " Kees Cook
2016-07-07 5:37 ` Baruch Siach
2016-07-07 17:25 ` Kees Cook
2016-07-07 18:35 ` Baruch Siach
2016-07-07 7:42 ` Thomas Gleixner
2016-07-07 17:29 ` Kees Cook
2016-07-07 19:34 ` Thomas Gleixner
2016-07-07 8:01 ` Arnd Bergmann
2016-07-07 17:37 ` Kees Cook
2016-07-08 5:34 ` Michael Ellerman
2016-07-08 5:34 ` Michael Ellerman
2016-07-08 5:34 ` Michael Ellerman
2016-07-08 5:34 ` Michael Ellerman
2016-07-08 9:22 ` Arnd Bergmann
2016-07-07 16:19 ` Rik van Riel
2016-07-07 16:35 ` Rik van Riel
2016-07-07 17:41 ` Kees Cook
2016-07-06 22:25 ` [PATCH 2/9] x86/uaccess: Enable hardened usercopy Kees Cook
2016-07-06 22:25 ` [PATCH 3/9] ARM: uaccess: " Kees Cook
2016-07-06 22:25 ` [PATCH 4/9] arm64/uaccess: " Kees Cook
2016-07-07 10:07 ` Mark Rutland
2016-07-07 17:19 ` Kees Cook
2016-07-06 22:25 ` [PATCH 5/9] ia64/uaccess: " Kees Cook
2016-07-06 22:25 ` [PATCH 6/9] powerpc/uaccess: " Kees Cook
2016-07-06 22:25 ` [PATCH 7/9] sparc/uaccess: " Kees Cook
2016-07-06 22:25 ` [PATCH 8/9] mm: SLAB hardened usercopy support Kees Cook
2016-07-06 22:25 ` [PATCH 9/9] mm: SLUB " Kees Cook
2016-07-07 4:35 ` Michael Ellerman
2016-07-07 4:35 ` Michael Ellerman
2016-07-07 4:35 ` Michael Ellerman
2016-07-07 4:35 ` Michael Ellerman
[not found] ` <577ddc18.d351190a.1fa54.ffffbe79SMTPIN_ADDED_BROKEN@mx.google.com>
2016-07-07 18:56 ` [kernel-hardening] " Kees Cook
2016-07-08 10:19 ` Michael Ellerman
2016-07-08 10:19 ` Michael Ellerman
2016-07-08 10:19 ` Michael Ellerman
2016-07-08 10:19 ` [kernel-hardening] " Michael Ellerman
2016-07-07 7:30 ` [PATCH 0/9] mm: Hardened usercopy Christian Borntraeger
2016-07-07 17:27 ` Kees Cook
2016-07-08 8:46 ` Ingo Molnar
2016-07-08 16:19 ` Linus Torvalds
2016-07-08 18:23 ` Ingo Molnar
2016-07-09 2:22 ` Laura Abbott
2016-07-09 2:44 ` Rik van Riel
2016-07-09 7:55 ` Ingo Molnar
2016-07-09 8:25 ` Ard Biesheuvel
2016-07-09 12:58 ` Laura Abbott
2016-07-09 17:03 ` Kees Cook
2016-07-09 17:01 ` Kees Cook
2016-07-09 21:27 ` Andy Lutomirski
2016-07-09 23:16 ` PaX Team
2016-07-10 9:16 ` Ingo Molnar
2016-07-10 12:03 ` PaX Team
2016-07-10 12:38 ` Andy Lutomirski [this message]
2016-07-11 18:40 ` Kees Cook
2016-07-11 18:34 ` Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CALCETrXfdEbmSTs6XkZjHkAc3W_380bpde4bWQgRA5CQM0PtLA@mail.gmail.com \
--to=luto@amacapital.net \
--cc=a.p.zijlstra@chello.nl \
--cc=aarcange@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=ard.biesheuvel@linaro.org \
--cc=benh@kernel.crashing.org \
--cc=bp@suse.de \
--cc=casey@schaufler-ca.com \
--cc=catalin.marinas@arm.com \
--cc=cl@linux.com \
--cc=davem@davemloft.net \
--cc=dvyukov@google.com \
--cc=fenghua.yu@intel.com \
--cc=hpa@zytor.com \
--cc=iamjoonsoo.kim@lge.com \
--cc=jack@suse.cz \
--cc=keescook@chromium.org \
--cc=kernel-hardening@lists.openwall.com \
--cc=labbott@fedoraproject.org \
--cc=linux-arch@vger.kernel.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-ia64@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=linux@armlinux.org.uk \
--cc=linuxppc-dev@lists.ozlabs.org \
--cc=mingo@kernel.org \
--cc=minipli@googlemail.com \
--cc=mpe@ellerman.id.au \
--cc=pageexec@freemail.hu \
--cc=penberg@kernel.org \
--cc=riel@redhat.com \
--cc=rientjes@google.com \
--cc=sparclinux@vger.kernel.org \
--cc=spender@grsecurity.net \
--cc=tglx@linutronix.de \
--cc=tony.luck@intel.com \
--cc=torvalds@linux-foundation.org \
--cc=vitalywool@gmail.com \
--cc=will.deacon@arm.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).