Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1612221 > unrolled thread

[RFC v2] Introduce rare_write() infrastructure

Back to article view | Back to linux.kernel

Contents

  [RFC v2] Introduce rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
    [RFC v2][PATCH 10/11] gcc-plugins: Add constify plugin Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
    [RFC v2][PATCH 03/11] net: switch sock_diag handlers to rare_write() Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
    [RFC v2][PATCH 07/11] ARM: mm: set DOMAIN_WR_RARE for rodata Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
    [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
      Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-03-29 20:30 +0200
        Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure Ho-Eun Ryu <hoeun.ryu@gmail.com> - 2017-03-30 09:50 +0200
          Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-03-30 19:10 +0200
      Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure Ho-Eun Ryu <hoeun.ryu@gmail.com> - 2017-04-07 10:20 +0200
        Re: [RFC v2][PATCH 01/11] Introduce rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-04-07 22:40 +0200
    [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
      Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@amacapital.net> - 2017-03-30 00:40 +0200
        Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Kees Cook <keescook@chromium.org> - 2017-03-30 03:50 +0200
          Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@kernel.org> - 2017-04-06 02:00 +0200
            Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Kees Cook <keescook@chromium.org> - 2017-04-06 02:20 +0200
              Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@amacapital.net> - 2017-04-06 18:00 +0200
                Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Mathias Krause <minipli@googlemail.com> - 2017-04-07 10:40 +0200
                  Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Thomas Gleixner <tglx@linutronix.de> - 2017-04-07 11:50 +0200
                    Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Mathias Krause <minipli@googlemail.com> - 2017-04-07 13:00 +0200
                      Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Thomas Gleixner <tglx@linutronix.de> - 2017-04-07 15:20 +0200
                        Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Mathias Krause <minipli@googlemail.com> - 2017-04-07 15:40 +0200
                          Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@kernel.org> - 2017-04-07 18:20 +0200
                            Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Mark Rutland <mark.rutland@arm.com> - 2017-04-07 18:30 +0200
                            Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() "PaX Team" <pageexec@freemail.hu> - 2017-04-07 22:10 +0200
                              Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@kernel.org> - 2017-04-08 07:00 +0200
                                Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() "PaX Team" <pageexec@freemail.hu> - 2017-04-09 14:50 +0200
                                  Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@kernel.org> - 2017-04-10 02:20 +0200
                                    Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() "PaX Team" <pageexec@freemail.hu> - 2017-04-10 12:50 +0200
                            Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Thomas Gleixner <tglx@linutronix.de> - 2017-04-07 22:50 +0200
                              Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Kees Cook <keescook@chromium.org> - 2017-04-07 23:30 +0200
                                Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Daniel Micay <danielmicay@gmail.com> - 2017-04-08 06:20 +0200
                                  Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Daniel Micay <danielmicay@gmail.com> - 2017-04-08 06:20 +0200
                            Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Daniel Micay <danielmicay@gmail.com> - 2017-04-08 06:30 +0200
                              Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@kernel.org> - 2017-04-08 07:10 +0200
                                Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Daniel Micay <danielmicay@gmail.com> - 2017-04-08 09:40 +0200
                                  Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@kernel.org> - 2017-04-08 17:30 +0200
                                    Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Ingo Molnar <mingo@kernel.org> - 2017-04-09 13:00 +0200
                                    Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Mark Rutland <mark.rutland@arm.com> - 2017-04-10 12:30 +0200
                                Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() "PaX Team" <pageexec@freemail.hu> - 2017-04-09 22:30 +0200
                                  Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Andy Lutomirski <luto@kernel.org> - 2017-04-10 02:40 +0200
                          Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Thomas Gleixner <tglx@linutronix.de> - 2017-04-07 21:30 +0200
                      Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Peter Zijlstra <peterz@infradead.org> - 2017-04-07 16:50 +0200
                        Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Mark Rutland <mark.rutland@arm.com> - 2017-04-10 12:40 +0200
                    Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() "PaX Team" <pageexec@freemail.hu> - 2017-04-07 22:40 +0200
                      Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Thomas Gleixner <tglx@linutronix.de> - 2017-04-10 10:30 +0200
      Re: [RFC v2][PATCH 04/11] x86: Implement  __arch_rare_write_begin/unmap() Peter Zijlstra <peterz@infradead.org> - 2017-04-07 11:40 +0200
    [RFC v2][PATCH 09/11] list: add rare_write() list helpers Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
    [RFC v2][PATCH 05/11] ARM: mm: dump: Add domain to output Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
    [RFC v2][PATCH 11/11] cgroups: force all struct cftype const Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
    [RFC v2][PATCH 08/11] ARM: Implement __arch_rare_write_begin/end() Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
      Re: [RFC v2][PATCH 08/11] ARM: Implement  __arch_rare_write_begin/end() Peter Zijlstra <peterz@infradead.org> - 2017-04-07 11:40 +0200
    [RFC v2][PATCH 02/11] lkdtm: add test for rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-03-29 20:20 +0200
      Re: [kernel-hardening] [RFC v2][PATCH 02/11] lkdtm: add test for  rare_write() infrastructure Ian Campbell <ijc@hellion.org.uk> - 2017-03-30 12:20 +0200
        Re: [kernel-hardening] [RFC v2][PATCH 02/11] lkdtm: add test for  rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-03-30 18:20 +0200
    Re: [RFC v2] Introduce rare_write() infrastructure Russell King - ARM Linux <linux@armlinux.org.uk> - 2017-03-29 21:10 +0200
      Re: [RFC v2] Introduce rare_write() infrastructure Kees Cook <keescook@chromium.org> - 2017-03-29 21:20 +0200

Page 3 of 3 — ← Prev page 1 2 [3]

#1619057 — Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

On Fri, 7 Apr 2017, Mathias Krause wrote:
> On 7 April 2017 at 15:14, Thomas Gleixner <tglx@linutronix.de> wrote:
> > On Fri, 7 Apr 2017, Mathias Krause wrote:
> >> On 7 April 2017 at 11:46, Thomas Gleixner <tglx@linutronix.de> wrote:
> >> > Whether protected by preempt_disable or local_irq_disable, to make that
> >> > work it needs CR0 handling in the exception entry/exit at the lowest
> >> > level. And that's just a nightmare maintainence wise as it's prone to be
> >> > broken over time.
> >>
> >> It seems to be working fine for more than a decade now in PaX. So it
> >> can't be such a big maintenance nightmare ;)
> >
> > I really do not care whether PaX wants to chase and verify that over and
> > over. I certainly don't want to take the chance to leak CR0.WP ever and I
> > very much care about extra stuff to check in the entry/exit path.
> 
> Fair enough. However, placing a BUG_ON(!(read_cr0() & X86_CR0_WP))
> somewhere sensible should make those "leaks" visible fast -- and their
> exploitation impossible, i.e. fail hard.

Sure, you trade leaking WP with an potentially exploitable BUG().

> >> The "proper solution" seems to be much slower compared to just
> >> toggling CR0.WP (which is costly in itself, already) because of the
> >> TLB invalidation / synchronisation involved.
> >
> > Why the heck should we care about rare writes being performant?
> 
> As soon as they stop being rare and people start extending the r/o
> protection to critical data structures accessed often. Then
> performance matters.

Emphasis on "Then". I'm not seeing it, because no matter what you do it's
going to be slow. Aside of that increasing the usage will also increase the
chance to leak stuff. In that case I rather leak a single page mapping
temporarily than taking the chance to leak WP.

> >> > It's valid (at least on x86) to have a shadow map with the same page
> >> > attributes but write enabled. That does not require any fixups of CR0 and
> >> > just works.
> >>
> >> "Just works", sure -- but it's not as tightly focused as the PaX
> >> solution which is CPU local, while your proposed solution is globally
> >> visible.
> >
> > Making the world and some more writeable hardly qualifies as tightly
> > focussed. Making the mapping concept CPU local is not rocket science
> > either. The question is whethers it's worth the trouble.
> 
> No, the question is if the value of the concept is well understood and
> if people can see what could be done with such a strong primitive.
> Apparently not...

Oh, well. We can stop that discussion right here, if all you can provide
is a killer phrase.

I'm well aware what can be done with a strong primitive and I certainly
understand the concept, but I'm not naive enough to believe that lifting
one of the strong protections the kernel has by globaly disabling WP is
anything which should be even considered. That bit is a horrible
misconception and should be fused to 1.

Aside of that, if you had taken the time to figure out how kmap_atomic
stuff works then you would have noticed that it does not require cross CPU
pagetable syncs and that the mapping place can be randomized to a certain
degree. So this has neither global impact, nor does it become immediately
globally visible.

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1618862 — Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

On Fri, Apr 07, 2017 at 12:51:15PM +0200, Mathias Krause wrote:
> Why that? It allows fast and CPU local modifications of r/o memory.
> OTOH, an approach that needs to fiddle with page table entries
> requires global synchronization to keep the individual TLB states in
> sync. Hmm.. Not that fast, I'd say.

The fixmaps used for kmap_atomic are per-cpu, no global sync required.

[toc] | [prev] | [next] | [standalone]

#1619793 — Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

On Fri, Apr 07, 2017 at 04:45:26PM +0200, Peter Zijlstra wrote:
> On Fri, Apr 07, 2017 at 12:51:15PM +0200, Mathias Krause wrote:
> > Why that? It allows fast and CPU local modifications of r/o memory.
> > OTOH, an approach that needs to fiddle with page table entries
> > requires global synchronization to keep the individual TLB states in
> > sync. Hmm.. Not that fast, I'd say.
> 
> The fixmaps used for kmap_atomic are per-cpu, no global sync required.

That might be fine for x86, but for some architectures fixmap slots and
kmap_atomic mappings happen to be visible to other CPUs even if they're
not required to be.

Using an mm solves that for all, though.

Thanks,
Mark.

[toc] | [prev] | [next] | [standalone]

#1619087 — Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

On 7 Apr 2017 at 11:46, Thomas Gleixner wrote:

> On Fri, 7 Apr 2017, Mathias Krause wrote:
> > Well, doesn't look good to me. NMIs will still be able to interrupt
> > this code and will run with CR0.WP = 0.
> > 
> > Shouldn't you instead question yourself why PaX can do it "just" with
> > preempt_disable() instead?!
> 
> That's silly. Just because PaX does it, doesn't mean it's correct.

is that FUD or do you have actionable information to share?

> To be honest, playing games with the CR0.WP bit is outright stupid to begin with.

why is that? cr0.wp exists since the i486 and its behaviour fits my
purposes quite well, it's the best security/performance i know of.

> Whether protected by preempt_disable or local_irq_disable, to make that
> work it needs CR0 handling in the exception entry/exit at the lowest
> level.

correct.

> And that's just a nightmare maintainence wise as it's prone to be
> broken over time.

i've got 14 years of experience of maintaining it and i never saw it break.

> Aside of that it's pointless overhead for the normal case.

unless it's optional code as the whole feature already is.

> The proper solution is:
> 
> write_rare(ptr, val)
> {
>  mp = map_shadow_rw(ptr);
>  *mp = val;
>  unmap_shadow_rw(mp);
> }

this is not *the* proper solution, but only a naive one that suffers from
the exact same need that the cr0.wp approach does and has worse performance
impact. not exactly a win...

[continuing from your next mail in order to save round-trip time]

> I really do not care whether PaX wants to chase and verify that over and
> over.

verifying it is no different than verifying, say, swapgs use.

> I certainly don't want to take the chance to leak CR0.WP ever

why and where would cr0.wp leak?

> and I very much care about extra stuff to check in the entry/exit path.

your 'proper' solution needs (a lot more) extra stuff too.

> Why the heck should we care about rare writes being performant?

because you've been misled by the NIH crowd here that the PaX feature they
tried to (badly) extract from has anything to do with frequency of writes.
it does not. what it does do is provide an environment for variables that
are conceptually writable but for security reasons should be read-only most
of the time by most of the code (ditto for the grossly misunderstood and thus
misnamed ro-after-shit). now imagine locking down the page table hierarchy
with it...

> Making the world and some more writeable hardly qualifies as tightly
> focused.

you forgot to add 'for a window of a few insns' and that the map/unmap
approach does the same under an attacker controlled ptr.

> Making the mapping concept CPU local is not rocket science
> either. The question is whether it's worth the trouble.

it is for people who care about the integrity of the kernel, and all this
read-onlyness stuff implies that some do.

[toc] | [prev] | [next] | [standalone]

#1619715 — Re: [kernel-hardening] Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

On Fri, 7 Apr 2017, PaX Team wrote:
> On 7 Apr 2017 at 11:46, Thomas Gleixner wrote:
> 
> > On Fri, 7 Apr 2017, Mathias Krause wrote:
> > > Well, doesn't look good to me. NMIs will still be able to interrupt
> > > this code and will run with CR0.WP = 0.
> > > 
> > > Shouldn't you instead question yourself why PaX can do it "just" with
> > > preempt_disable() instead?!
> > 
> > That's silly. Just because PaX does it, doesn't mean it's correct.
> 
> is that FUD or do you have actionable information to share?

That has absolutely nothing to do with FUD. I'm merily not accepting
argumentations which say: PaX can do it "just"....

That has exactly zero technical merit and it's not asked too much to
provide precise technical arguments why one implementation is better than
some other.

> > To be honest, playing games with the CR0.WP bit is outright stupid to begin with.
> 
> why is that? cr0.wp exists since the i486 and its behaviour fits my
> purposes quite well, it's the best security/performance i know of.

Works for me has never be a good engineering principle.

> > Whether protected by preempt_disable or local_irq_disable, to make that
> > work it needs CR0 handling in the exception entry/exit at the lowest
> > level.
> 
> correct.
> 
> > And that's just a nightmare maintainence wise as it's prone to be
> > broken over time.
> 
> i've got 14 years of experience of maintaining it and i never saw it break.

It's a difference whether you maintain a special purpose patch set out of
tree for a subset of architectures - I certainly know what I'm talking
about - or keeping stuff sane in the upstream kernel.

> > I certainly don't want to take the chance to leak CR0.WP ever
>
> why and where would cr0.wp leak?

It's bound to happen due to some subtle mistake and up to the point where
you catch it (in the scheduler or entry/exit path) the world is
writeable. And that will be some almost never executed error path which can
be triggered by a carefully crafted attack. A very restricted writeable
region is definitely preferred over full world writeable then, right?

> > Why the heck should we care about rare writes being performant?
> 
> because you've been misled by the NIH crowd here that the PaX feature they
> tried to (badly) extract from has anything to do with frequency of writes.

It would be apprectiated if you could keep your feud out of this. It's
enough to tell me that 'rare write' is a misleading term and why.

> > Making the world and some more writeable hardly qualifies as tightly
> > focused.
> 
> you forgot to add 'for a window of a few insns' and that the map/unmap

If it'd be guaranteed to be a few instructions, then I wouldn't be that
worried. The availability of make_world_writeable() as an unrestricted
usable function makes me nervous as hell. We've had long standing issues
where kmap_atomic() got leaked through a hard to spot almost never executed
error handling path. And the same is bound to happen with this, just with a
way worse outcome.

> approach does the same under an attacker controlled ptr.

Which attacker controlled pointer?

Thanks,

	tglx

[toc] | [prev] | [next] | [standalone]

#1618625 — Re: [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap()

On Wed, Mar 29, 2017 at 11:15:56AM -0700, Kees Cook wrote:
> +static __always_inline unsigned long __arch_rare_write_end(void)
> +{
> +	unsigned long cr0;
> +
> +	barrier();
> +	cr0 = read_cr0() ^ X86_CR0_WP;
> +	BUG_ON(!(cr0 & X86_CR0_WP));
> +	write_cr0(cr0);
> +	barrier();
> +	preempt_enable_no_resched();

NAK

> +	return cr0 ^ X86_CR0_WP;
> +}

[toc] | [prev] | [next] | [standalone]

#1612231 — [RFC v2][PATCH 09/11] list: add rare_write() list helpers

Some structures that are intended to be made write-rarely are designed to
be linked by lists. As a result, there need to be rare_write()-supported
linked list primitives.

As found in PaX, this adds list management helpers for doing updates to
rarely-changed lists.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/list.h | 17 +++++++++++++++++
 lib/Makefile         |  2 +-
 lib/list_debug.c     | 37 +++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/include/linux/list.h b/include/linux/list.h
index ae537fa46216..50fdd5b737aa 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -126,6 +126,23 @@ static inline void list_del(struct list_head *entry)
 	entry->prev = LIST_POISON2;
 }
 
+extern void __rare_list_add(struct list_head *new,
+			    struct list_head *prev,
+			    struct list_head *next);
+
+static inline void
+rare_list_add(__wr_rare_type struct list_head *new, struct list_head *head)
+{
+	__rare_list_add((struct list_head *)new, head, head->next);
+}
+static inline void
+rare_list_add_tail(__wr_rare_type struct list_head *new, struct list_head *head)
+{
+	__rare_list_add((struct list_head *)new, head->prev, head);
+}
+
+extern void rare_list_del(__wr_rare_type struct list_head *entry);
+
 /**
  * list_replace - replace old entry by new one
  * @old : the element to be replaced
diff --git a/lib/Makefile b/lib/Makefile
index 320ac46a8725..cd64fd8f7a21 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -83,7 +83,7 @@ obj-$(CONFIG_BTREE) += btree.o
 obj-$(CONFIG_INTERVAL_TREE) += interval_tree.o
 obj-$(CONFIG_ASSOCIATIVE_ARRAY) += assoc_array.o
 obj-$(CONFIG_DEBUG_PREEMPT) += smp_processor_id.o
-obj-$(CONFIG_DEBUG_LIST) += list_debug.o
+obj-y += list_debug.o
 obj-$(CONFIG_DEBUG_OBJECTS) += debugobjects.o
 
 ifneq ($(CONFIG_HAVE_DEC_LOCK),y)
diff --git a/lib/list_debug.c b/lib/list_debug.c
index a34db8d27667..1add73f9479a 100644
--- a/lib/list_debug.c
+++ b/lib/list_debug.c
@@ -10,7 +10,9 @@
 #include <linux/bug.h>
 #include <linux/kernel.h>
 #include <linux/rculist.h>
+#include <linux/mm.h>
 
+#ifdef CONFIG_DEBUG_LIST
 /*
  * Check that the data structures for the list manipulations are reasonably
  * valid. Failures here indicate memory corruption (and possibly an exploit
@@ -60,3 +62,38 @@ bool __list_del_entry_valid(struct list_head *entry)
 
 }
 EXPORT_SYMBOL(__list_del_entry_valid);
+
+#endif /* CONFIG_DEBUG_LIST */
+
+void __rare_list_add(struct list_head *new, struct list_head *prev,
+		     struct list_head *next)
+{
+	if (!__list_add_valid(new, prev, next))
+		return;
+
+	rare_write_begin();
+	__rare_write(next->prev, new);
+	__rare_write(new->next, next);
+	__rare_write(new->prev, prev);
+	__rare_write(prev->next, new);
+	rare_write_end();
+}
+EXPORT_SYMBOL(__rare_list_add);
+
+void rare_list_del(__wr_rare_type struct list_head *entry_const)
+{
+	struct list_head *entry = (struct list_head *)entry_const;
+	struct list_head *prev = entry->prev;
+	struct list_head *next = entry->next;
+
+	if (!__list_del_entry_valid(entry))
+		return;
+
+	rare_write_begin();
+	__rare_write(next->prev, prev);
+	__rare_write(prev->next, next);
+	__rare_write(entry->next, LIST_POISON1);
+	__rare_write(entry->prev, LIST_POISON2);
+	rare_write_end();
+}
+EXPORT_SYMBOL(rare_list_del);
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1612233 — [RFC v2][PATCH 05/11] ARM: mm: dump: Add domain to output

This adds the memory domain (on non-LPAE) to the PMD and PTE dumps. This
isn't in the regular PMD bits because I couldn't find a clean way to
fall back to retain some of the PMD bits when reporting PTE. So this is
special-cased currently.

New output example:

  ---[ Modules ]---
  0x7f000000-0x7f001000       4K KERNEL      ro x  SHD MEM/CACHED/WBWA
  0x7f001000-0x7f002000       4K KERNEL      ro NX SHD MEM/CACHED/WBWA
  0x7f002000-0x7f004000       8K KERNEL      RW NX SHD MEM/CACHED/WBWA
  ---[ Kernel Mapping ]---
  0x80000000-0x80100000       1M KERNEL      RW NX SHD
  0x80100000-0x80800000       7M KERNEL      ro x  SHD
  0x80800000-0x80b00000       3M KERNEL      ro NX SHD
  0x80b00000-0xa0000000     501M KERNEL      RW NX SHD
  ...
  ---[ Vectors ]---
  0xffff0000-0xffff1000       4K VECTORS USR ro x  SHD MEM/CACHED/WBWA
  0xffff1000-0xffff2000       4K VECTORS     ro x  SHD MEM/CACHED/WBWA

Signed-off-by: Kees Cook <keescook@chromium.org>
---
This patch is already queued in the ARM tree, but I'm including it here too
since a following patch updates the list of domain names from this patch...
---
 arch/arm/mm/dump.c | 54 ++++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 44 insertions(+), 10 deletions(-)

diff --git a/arch/arm/mm/dump.c b/arch/arm/mm/dump.c
index 21192d6eda40..35ff45470dbf 100644
--- a/arch/arm/mm/dump.c
+++ b/arch/arm/mm/dump.c
@@ -17,6 +17,7 @@
 #include <linux/mm.h>
 #include <linux/seq_file.h>
 
+#include <asm/domain.h>
 #include <asm/fixmap.h>
 #include <asm/memory.h>
 #include <asm/pgtable.h>
@@ -43,6 +44,7 @@ struct pg_state {
 	unsigned long start_address;
 	unsigned level;
 	u64 current_prot;
+	const char *current_domain;
 };
 
 struct prot_bits {
@@ -216,7 +218,8 @@ static void dump_prot(struct pg_state *st, const struct prot_bits *bits, size_t
 	}
 }
 
-static void note_page(struct pg_state *st, unsigned long addr, unsigned level, u64 val)
+static void note_page(struct pg_state *st, unsigned long addr,
+		      unsigned int level, u64 val, const char *domain)
 {
 	static const char units[] = "KMGTPE";
 	u64 prot = val & pg_level[level].mask;
@@ -224,8 +227,10 @@ static void note_page(struct pg_state *st, unsigned long addr, unsigned level, u
 	if (!st->level) {
 		st->level = level;
 		st->current_prot = prot;
+		st->current_domain = domain;
 		seq_printf(st->seq, "---[ %s ]---\n", st->marker->name);
 	} else if (prot != st->current_prot || level != st->level ||
+		   domain != st->current_domain ||
 		   addr >= st->marker[1].start_address) {
 		const char *unit = units;
 		unsigned long delta;
@@ -240,6 +245,8 @@ static void note_page(struct pg_state *st, unsigned long addr, unsigned level, u
 				unit++;
 			}
 			seq_printf(st->seq, "%9lu%c", delta, *unit);
+			if (st->current_domain)
+				seq_printf(st->seq, " %s", st->current_domain);
 			if (pg_level[st->level].bits)
 				dump_prot(st, pg_level[st->level].bits, pg_level[st->level].num);
 			seq_printf(st->seq, "\n");
@@ -251,11 +258,13 @@ static void note_page(struct pg_state *st, unsigned long addr, unsigned level, u
 		}
 		st->start_address = addr;
 		st->current_prot = prot;
+		st->current_domain = domain;
 		st->level = level;
 	}
 }
 
-static void walk_pte(struct pg_state *st, pmd_t *pmd, unsigned long start)
+static void walk_pte(struct pg_state *st, pmd_t *pmd, unsigned long start,
+		     const char *domain)
 {
 	pte_t *pte = pte_offset_kernel(pmd, 0);
 	unsigned long addr;
@@ -263,25 +272,50 @@ static void walk_pte(struct pg_state *st, pmd_t *pmd, unsigned long start)
 
 	for (i = 0; i < PTRS_PER_PTE; i++, pte++) {
 		addr = start + i * PAGE_SIZE;
-		note_page(st, addr, 4, pte_val(*pte));
+		note_page(st, addr, 4, pte_val(*pte), domain);
 	}
 }
 
+static const char *get_domain_name(pmd_t *pmd)
+{
+#ifndef CONFIG_ARM_LPAE
+	switch (pmd_val(*pmd) & PMD_DOMAIN_MASK) {
+	case PMD_DOMAIN(DOMAIN_KERNEL):
+		return "KERNEL ";
+	case PMD_DOMAIN(DOMAIN_USER):
+		return "USER   ";
+	case PMD_DOMAIN(DOMAIN_IO):
+		return "IO     ";
+	case PMD_DOMAIN(DOMAIN_VECTORS):
+		return "VECTORS";
+	default:
+		return "unknown";
+	}
+#endif
+	return NULL;
+}
+
 static void walk_pmd(struct pg_state *st, pud_t *pud, unsigned long start)
 {
 	pmd_t *pmd = pmd_offset(pud, 0);
 	unsigned long addr;
 	unsigned i;
+	const char *domain;
 
 	for (i = 0; i < PTRS_PER_PMD; i++, pmd++) {
 		addr = start + i * PMD_SIZE;
+		domain = get_domain_name(pmd);
 		if (pmd_none(*pmd) || pmd_large(*pmd) || !pmd_present(*pmd))
-			note_page(st, addr, 3, pmd_val(*pmd));
+			note_page(st, addr, 3, pmd_val(*pmd), domain);
 		else
-			walk_pte(st, pmd, addr);
+			walk_pte(st, pmd, addr, domain);
 
-		if (SECTION_SIZE < PMD_SIZE && pmd_large(pmd[1]))
-			note_page(st, addr + SECTION_SIZE, 3, pmd_val(pmd[1]));
+		if (SECTION_SIZE < PMD_SIZE && pmd_large(pmd[1])) {
+			addr += SECTION_SIZE;
+			pmd++;
+			domain = get_domain_name(pmd);
+			note_page(st, addr, 3, pmd_val(*pmd), domain);
+		}
 	}
 }
 
@@ -296,7 +330,7 @@ static void walk_pud(struct pg_state *st, pgd_t *pgd, unsigned long start)
 		if (!pud_none(*pud)) {
 			walk_pmd(st, pud, addr);
 		} else {
-			note_page(st, addr, 2, pud_val(*pud));
+			note_page(st, addr, 2, pud_val(*pud), NULL);
 		}
 	}
 }
@@ -317,11 +351,11 @@ static void walk_pgd(struct seq_file *m)
 		if (!pgd_none(*pgd)) {
 			walk_pud(&st, pgd, addr);
 		} else {
-			note_page(&st, addr, 1, pgd_val(*pgd));
+			note_page(&st, addr, 1, pgd_val(*pgd), NULL);
 		}
 	}
 
-	note_page(&st, 0, 0, 0);
+	note_page(&st, 0, 0, 0, NULL);
 }
 
 static int ptdump_show(struct seq_file *m, void *v)
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1612234 — [RFC v2][PATCH 11/11] cgroups: force all struct cftype const

As found in PaX, mark struct cftype with __do_const and add helpers
to deal with rare writes. This is a more complex example of a write-rarely
structure, which needs to use list helpers and blocks of begin/end pairs
to perform the needed updates.

With this change and the constify plugin enabled, the before/after
section byte sizes show:

before:
    rodata: 0x2cc2f0
    data:   0x130d00

after:
    rodata: 0x2cf2f0 (+74478)
    data:   0x12e5c0 (-65710)

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 include/linux/cgroup-defs.h |  2 +-
 kernel/cgroup/cgroup.c      | 35 +++++++++++++++++++++++------------
 2 files changed, 24 insertions(+), 13 deletions(-)

diff --git a/include/linux/cgroup-defs.h b/include/linux/cgroup-defs.h
index 6a3f850cabab..67563a80d01f 100644
--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -434,7 +434,7 @@ struct cftype {
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
 	struct lock_class_key	lockdep_key;
 #endif
-};
+} __do_const;
 
 /*
  * Control Group subsystem type.
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 48851327a15e..94188df45f96 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -3058,11 +3058,11 @@ static int cgroup_add_file(struct cgroup_subsys_state *css, struct cgroup *cgrp,
 	int ret;
 
 #ifdef CONFIG_DEBUG_LOCK_ALLOC
-	key = &cft->lockdep_key;
+	key = (struct lock_class_key *)&cft->lockdep_key;
 #endif
 	kn = __kernfs_create_file(cgrp->kn, cgroup_file_name(cgrp, cft, name),
-				  cgroup_file_mode(cft), 0, cft->kf_ops, cft,
-				  NULL, key);
+				  cgroup_file_mode(cft), 0, cft->kf_ops,
+				  (void *)cft, NULL, key);
 	if (IS_ERR(kn))
 		return PTR_ERR(kn);
 
@@ -3165,11 +3165,16 @@ static void cgroup_exit_cftypes(struct cftype *cfts)
 		/* free copy for custom atomic_write_len, see init_cftypes() */
 		if (cft->max_write_len && cft->max_write_len != PAGE_SIZE)
 			kfree(cft->kf_ops);
-		cft->kf_ops = NULL;
-		cft->ss = NULL;
+
+		rare_write_begin();
+		__rare_write(cft->kf_ops, NULL);
+		__rare_write(cft->ss, NULL);
 
 		/* revert flags set by cgroup core while adding @cfts */
-		cft->flags &= ~(__CFTYPE_ONLY_ON_DFL | __CFTYPE_NOT_ON_DFL);
+		__rare_write(cft->flags,
+			     cft->flags & ~(__CFTYPE_ONLY_ON_DFL |
+					    __CFTYPE_NOT_ON_DFL));
+		rare_write_end();
 	}
 }
 
@@ -3200,8 +3205,10 @@ static int cgroup_init_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
 			kf_ops->atomic_write_len = cft->max_write_len;
 		}
 
-		cft->kf_ops = kf_ops;
-		cft->ss = ss;
+		rare_write_begin();
+		__rare_write(cft->kf_ops, kf_ops);
+		__rare_write(cft->ss, ss);
+		rare_write_end();
 	}
 
 	return 0;
@@ -3214,7 +3221,7 @@ static int cgroup_rm_cftypes_locked(struct cftype *cfts)
 	if (!cfts || !cfts[0].ss)
 		return -ENOENT;
 
-	list_del(&cfts->node);
+	rare_list_del(&cfts->node);
 	cgroup_apply_cftypes(cfts, false);
 	cgroup_exit_cftypes(cfts);
 	return 0;
@@ -3271,7 +3278,7 @@ static int cgroup_add_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
 
 	mutex_lock(&cgroup_mutex);
 
-	list_add_tail(&cfts->node, &ss->cfts);
+	rare_list_add_tail(&cfts->node, &ss->cfts);
 	ret = cgroup_apply_cftypes(cfts, true);
 	if (ret)
 		cgroup_rm_cftypes_locked(cfts);
@@ -3292,8 +3299,10 @@ int cgroup_add_dfl_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
 {
 	struct cftype *cft;
 
+	rare_write_begin();
 	for (cft = cfts; cft && cft->name[0] != '\0'; cft++)
-		cft->flags |= __CFTYPE_ONLY_ON_DFL;
+		__rare_write(cft->flags, cft->flags | __CFTYPE_ONLY_ON_DFL);
+	rare_write_end();
 	return cgroup_add_cftypes(ss, cfts);
 }
 
@@ -3309,8 +3318,10 @@ int cgroup_add_legacy_cftypes(struct cgroup_subsys *ss, struct cftype *cfts)
 {
 	struct cftype *cft;
 
+	rare_write_begin();
 	for (cft = cfts; cft && cft->name[0] != '\0'; cft++)
-		cft->flags |= __CFTYPE_NOT_ON_DFL;
+		__rare_write(cft->flags, cft->flags | __CFTYPE_NOT_ON_DFL);
+	rare_write_end();
 	return cgroup_add_cftypes(ss, cfts);
 }
 
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1612235 — [RFC v2][PATCH 08/11] ARM: Implement __arch_rare_write_begin/end()

Based on grsecurity's ARM pax_{open,close}_kernel() implementation, this
allows HAVE_ARCH_RARE_WRITE to work on ARM.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/arm/Kconfig               |  1 +
 arch/arm/include/asm/domain.h  |  3 ++-
 arch/arm/include/asm/pgtable.h | 27 +++++++++++++++++++++++++++
 3 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 0d4e71b42c77..57b8aeaf501c 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -45,6 +45,7 @@ config ARM
 	select HAVE_ARCH_JUMP_LABEL if !XIP_KERNEL && !CPU_ENDIAN_BE32 && MMU
 	select HAVE_ARCH_KGDB if !CPU_ENDIAN_BE32 && MMU
 	select HAVE_ARCH_MMAP_RND_BITS if MMU
+	select HAVE_ARCH_RARE_WRITE if MMU && !ARM_LPAE && !CPU_USE_DOMAINS
 	select HAVE_ARCH_SECCOMP_FILTER if (AEABI && !OABI_COMPAT)
 	select HAVE_ARCH_TRACEHOOK
 	select HAVE_ARM_SMCCC if CPU_V7
diff --git a/arch/arm/include/asm/domain.h b/arch/arm/include/asm/domain.h
index b5ca80ac823c..b3fb5c0a2efd 100644
--- a/arch/arm/include/asm/domain.h
+++ b/arch/arm/include/asm/domain.h
@@ -54,6 +54,7 @@
 #define DOMAIN_MANAGER	3
 #else
 #define DOMAIN_MANAGER	1
+#define DOMAIN_FORCE_MANAGER 3
 #endif
 
 #define domain_mask(dom)	((3) << (2 * (dom)))
@@ -118,7 +119,7 @@ static inline void set_domain(unsigned val)
 }
 #endif
 
-#ifdef CONFIG_CPU_USE_DOMAINS
+#if defined(CONFIG_CPU_USE_DOMAINS) || defined(CONFIG_HAVE_ARCH_RARE_WRITE)
 #define modify_domain(dom,type)					\
 	do {							\
 		unsigned int domain = get_domain();		\
diff --git a/arch/arm/include/asm/pgtable.h b/arch/arm/include/asm/pgtable.h
index 1c462381c225..104923ea9eb5 100644
--- a/arch/arm/include/asm/pgtable.h
+++ b/arch/arm/include/asm/pgtable.h
@@ -57,6 +57,33 @@ extern void __pgd_error(const char *file, int line, pgd_t);
 #define pmd_ERROR(pmd)		__pmd_error(__FILE__, __LINE__, pmd)
 #define pgd_ERROR(pgd)		__pgd_error(__FILE__, __LINE__, pgd)
 
+#ifdef CONFIG_HAVE_ARCH_RARE_WRITE
+#include <asm/domain.h>
+#include <linux/preempt.h>
+
+static inline int test_domain(int domain, int domaintype)
+{
+	return (get_domain() & domain_val(domain, 3)) ==
+		domain_val(domain, domaintype);
+}
+
+static inline unsigned long __arch_rare_write_begin(void)
+{
+	preempt_disable();
+	BUG_ON(test_domain(DOMAIN_WR_RARE, DOMAIN_FORCE_MANAGER));
+	modify_domain(DOMAIN_WR_RARE, DOMAIN_FORCE_MANAGER);
+	return 0;
+}
+
+static inline unsigned long __arch_rare_write_end(void)
+{
+	BUG_ON(test_domain(DOMAIN_WR_RARE, DOMAIN_CLIENT));
+	modify_domain(DOMAIN_WR_RARE, DOMAIN_CLIENT);
+	preempt_enable_no_resched();
+	return 0;
+}
+#endif
+
 /*
  * This is the lowest virtual address we can permit any user space
  * mapping to be mapped at.  This is particularly important for
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1618629 — Re: [RFC v2][PATCH 08/11] ARM: Implement __arch_rare_write_begin/end()

On Wed, Mar 29, 2017 at 11:16:00AM -0700, Kees Cook wrote:
> +static inline unsigned long __arch_rare_write_end(void)
> +{
> +	BUG_ON(test_domain(DOMAIN_WR_RARE, DOMAIN_CLIENT));
> +	modify_domain(DOMAIN_WR_RARE, DOMAIN_CLIENT);
> +	preempt_enable_no_resched();

NAK

> +	return 0;
> +}

[toc] | [prev] | [next] | [standalone]

#1612236 — [RFC v2][PATCH 02/11] lkdtm: add test for rare_write() infrastructure

This adds the WRITE_RARE_WRITE test to validate variables marked with
__wr_rare.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 drivers/misc/lkdtm.h       |  1 +
 drivers/misc/lkdtm_core.c  |  1 +
 drivers/misc/lkdtm_perms.c | 19 ++++++++++++++++++-
 3 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/drivers/misc/lkdtm.h b/drivers/misc/lkdtm.h
index 67d27be60405..d1fd5aefa235 100644
--- a/drivers/misc/lkdtm.h
+++ b/drivers/misc/lkdtm.h
@@ -39,6 +39,7 @@ void lkdtm_READ_BUDDY_AFTER_FREE(void);
 void __init lkdtm_perms_init(void);
 void lkdtm_WRITE_RO(void);
 void lkdtm_WRITE_RO_AFTER_INIT(void);
+void lkdtm_WRITE_RARE_WRITE(void);
 void lkdtm_WRITE_KERN(void);
 void lkdtm_EXEC_DATA(void);
 void lkdtm_EXEC_STACK(void);
diff --git a/drivers/misc/lkdtm_core.c b/drivers/misc/lkdtm_core.c
index b9a4cd4a9b68..ac8a55947189 100644
--- a/drivers/misc/lkdtm_core.c
+++ b/drivers/misc/lkdtm_core.c
@@ -219,6 +219,7 @@ struct crashtype crashtypes[] = {
 	CRASHTYPE(ACCESS_USERSPACE),
 	CRASHTYPE(WRITE_RO),
 	CRASHTYPE(WRITE_RO_AFTER_INIT),
+	CRASHTYPE(WRITE_RARE_WRITE),
 	CRASHTYPE(WRITE_KERN),
 	CRASHTYPE(REFCOUNT_SATURATE_INC),
 	CRASHTYPE(REFCOUNT_SATURATE_ADD),
diff --git a/drivers/misc/lkdtm_perms.c b/drivers/misc/lkdtm_perms.c
index c7635a79341f..8fbadfa4cc34 100644
--- a/drivers/misc/lkdtm_perms.c
+++ b/drivers/misc/lkdtm_perms.c
@@ -20,12 +20,15 @@
 /* This is non-const, so it will end up in the .data section. */
 static u8 data_area[EXEC_SIZE];
 
-/* This is cost, so it will end up in the .rodata section. */
+/* This is const, so it will end up in the .rodata section. */
 static const unsigned long rodata = 0xAA55AA55;
 
 /* This is marked __ro_after_init, so it should ultimately be .rodata. */
 static unsigned long ro_after_init __ro_after_init = 0x55AA5500;
 
+/* This is marked __wr_rare, so it should ultimately be .rodata. */
+static unsigned long wr_rare __wr_rare = 0xAA66AA66;
+
 /*
  * This just returns to the caller. It is designed to be copied into
  * non-executable memory regions.
@@ -103,6 +106,20 @@ void lkdtm_WRITE_RO_AFTER_INIT(void)
 	*ptr ^= 0xabcd1234;
 }
 
+void lkdtm_WRITE_RARE_WRITE(void)
+{
+	/* Explicitly cast away "const" for the test. */
+	unsigned long *ptr = (unsigned long *)&wr_rare;
+
+	pr_info("attempting good rare write at %p\n", ptr);
+	rare_write(*ptr, 0x11335577);
+	if (wr_rare != 0x11335577)
+		pr_warn("Yikes: wr_rare did not actually change!\n");
+
+	pr_info("attempting bad rare write at %p\n", ptr);
+	*ptr ^= 0xbcd12345;
+}
+
 void lkdtm_WRITE_KERN(void)
 {
 	size_t size;
-- 
2.7.4

[toc] | [prev] | [next] | [standalone]

#1612869 — Re: [kernel-hardening] [RFC v2][PATCH 02/11] lkdtm: add test for rare_write() infrastructure

On Wed, 2017-03-29 at 11:15 -0700, Kees Cook wrote:
> diff --git a/drivers/misc/lkdtm_perms.c b/drivers/misc/lkdtm_perms.c
> index c7635a79341f..8fbadfa4cc34 100644
> --- a/drivers/misc/lkdtm_perms.c
> +++ b/drivers/misc/lkdtm_perms.c
> [...]
> +/* This is marked __wr_rare, so it should ultimately be .rodata. */
> +static unsigned long wr_rare __wr_rare = 0xAA66AA66;
> [...]
> +void lkdtm_WRITE_RARE_WRITE(void)
> +{
> +	/* Explicitly cast away "const" for the test. */

wr_rare isn't actually declared const above though? I don't think
__wr_rare includes a const, apologies if I missed it.

OOI, if wr_rare _were_ const then can the compiler optimise the a pair
of reads spanning the rare_write? i.e. adding const to the declaration
above to get:

    static const unsigned long wr_rare __wr_rare = 0xAA66AA66;
x = wr_read;
rare_write(x, 0xf000baaa);
y = wr_read;

Is it possible that x == y == 0xaa66aa66 because gcc realises the x and
y came from the same const location? Have I missed a clobber somewhere
(I can't actually find a definition of __arch_rare_write_memcpy in this
series so maybe it's there), or is such code expected to always cast
away the const first?

I suppose such constructs are rare in practice in the sorts of places
where rare_write is appropriate, but with aggressive inlining it could
occur as an unexpected trap for the unwary perhaps.

Ian.

[toc] | [prev] | [next] | [standalone]

#1613262 — Re: [kernel-hardening] [RFC v2][PATCH 02/11] lkdtm: add test for rare_write() infrastructure

On Thu, Mar 30, 2017 at 2:34 AM, Ian Campbell <ijc@hellion.org.uk> wrote:
> On Wed, 2017-03-29 at 11:15 -0700, Kees Cook wrote:
>> diff --git a/drivers/misc/lkdtm_perms.c b/drivers/misc/lkdtm_perms.c
>> index c7635a79341f..8fbadfa4cc34 100644
>> --- a/drivers/misc/lkdtm_perms.c
>> +++ b/drivers/misc/lkdtm_perms.c
>> [...]
>> +/* This is marked __wr_rare, so it should ultimately be .rodata. */
>> +static unsigned long wr_rare __wr_rare = 0xAA66AA66;
>> [...]
>> +void lkdtm_WRITE_RARE_WRITE(void)
>> +{
>> +     /* Explicitly cast away "const" for the test. */
>
> wr_rare isn't actually declared const above though? I don't think
> __wr_rare includes a const, apologies if I missed it.

Yeah, good point. I think this was a left-over from an earlier version
where I'd forgotten about that detail.

> OOI, if wr_rare _were_ const then can the compiler optimise the a pair
> of reads spanning the rare_write? i.e. adding const to the declaration
> above to get:
>
>     static const unsigned long wr_rare __wr_rare = 0xAA66AA66;
> x = wr_read;
> rare_write(x, 0xf000baaa);
> y = wr_read;
>
> Is it possible that x == y == 0xaa66aa66 because gcc realises the x and
> y came from the same const location? Have I missed a clobber somewhere
> (I can't actually find a definition of __arch_rare_write_memcpy in this
> series so maybe it's there), or is such code expected to always cast
> away the const first?
>
> I suppose such constructs are rare in practice in the sorts of places
> where rare_write is appropriate, but with aggressive inlining it could
> occur as an unexpected trap for the unwary perhaps.

Right, __wr_rare is actually marked as .data..ro_after_init, which gcc
effectively ignores (thinking it's part of .data), but the linker
script later movies this section into the read-only portion with
.rodata. As a result, the compiler treats it as writable, but the
storage location is actually read-only.

(And, AIUI, the constify plugin makes things read-only in a similar
way, though I think it's more subtle but still avoids the
const-optimization dangers.)

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [next] | [standalone]

#1612263

On Wed, Mar 29, 2017 at 11:15:52AM -0700, Kees Cook wrote:
> This is take 2 of an RFC series to demonstrate a possible infrastructure
> for the "write rarely" memory storage type in the kernel (patch 1). The
> intent is to further reduce the internal attack surface of the kernel
> by making more variables read-only while "at rest". This is heavily
> based on the "__read_only" portion of the KERNEXEC infrastructure from
> PaX/grsecurity, though I tried to adjust it to be more in line with
> the upstream discussions around the APIs.

How much data are we talking about here?

> As part of the series I've included both x86 support (patch 4), exactly
> as done in PaX, and ARM support (patches 5-7), similar to what is done in
> grsecurity but without support for earlier ARM CPUs. Both are lightly
> tested by me, though they need a bit more work, especially ARM as it is
> missing the correct domain marking for kernel modules.

The implementation as it stands on ARM is going to gobble up
multiples of 1MB of RAM as you need it to be section aligned due to
using domains, so if we're talking about a small amount of data,
this is very inefficient.  That also only works for non-LPAE as LPAE
is unable to use that method.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[toc] | [prev] | [next] | [standalone]

#1612267

On Wed, Mar 29, 2017 at 12:00 PM, Russell King - ARM Linux
<linux@armlinux.org.uk> wrote:
> On Wed, Mar 29, 2017 at 11:15:52AM -0700, Kees Cook wrote:
>> This is take 2 of an RFC series to demonstrate a possible infrastructure
>> for the "write rarely" memory storage type in the kernel (patch 1). The
>> intent is to further reduce the internal attack surface of the kernel
>> by making more variables read-only while "at rest". This is heavily
>> based on the "__read_only" portion of the KERNEXEC infrastructure from
>> PaX/grsecurity, though I tried to adjust it to be more in line with
>> the upstream discussions around the APIs.
>
> How much data are we talking about here?

The goal is to put as much sensitive stuff from .data as possible into
.rodata, which is mostly structures with function pointers, etc. I
haven't measured the "final" results, since there's still a lot of
work to do to get all the other annotations into upstream.

>> As part of the series I've included both x86 support (patch 4), exactly
>> as done in PaX, and ARM support (patches 5-7), similar to what is done in
>> grsecurity but without support for earlier ARM CPUs. Both are lightly
>> tested by me, though they need a bit more work, especially ARM as it is
>> missing the correct domain marking for kernel modules.
>
> The implementation as it stands on ARM is going to gobble up
> multiples of 1MB of RAM as you need it to be section aligned due to
> using domains, so if we're talking about a small amount of data,
> this is very inefficient.  That also only works for non-LPAE as LPAE
> is unable to use that method.

AIUI, we're just flipping toe domain from DOMAIN_KERNEL to
DOMAIN_WR_RARE on the .rodata section (which is already 1MB aligned),
and (in the future if I or someone else can figure out how) on the
kernel module vmap area (which also shouldn't be changing alignment at
all).

-Kees

-- 
Kees Cook
Pixel Security

[toc] | [prev] | [standalone]

Page 3 of 3 — ← Prev page 1 2 [3]

Back to top | Article view | linux.kernel

csiph-web