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

Groups > linux.kernel > #1276768 > unrolled thread

[PATCH 0/2] introduce post-init read-only memory

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/2] introduce post-init read-only memory Kees Cook <keescook@chromium.org> - 2015-11-24 22:40 +0100
    [PATCH 1/2] x86: introduce post-init read-only memory Kees Cook <keescook@chromium.org> - 2015-11-24 22:40 +0100
      Re: [PATCH 1/2] x86: introduce post-init read-only memory Andy Lutomirski <luto@amacapital.net> - 2015-11-25 01:40 +0100
        Re: [PATCH 1/2] x86: introduce post-init read-only memory Kees Cook <keescook@chromium.org> - 2015-11-25 01:50 +0100
          Re: [kernel-hardening] Re: [PATCH 1/2] x86: introduce post-init  read-only memory Michael Ellerman <mpe@ellerman.id.au> - 2015-11-25 02:00 +0100
            Re: [kernel-hardening] Re: [PATCH 1/2] x86: introduce post-init  read-only memory Kees Cook <keescook@chromium.org> - 2015-11-25 16:10 +0100
              Re: [kernel-hardening] Re: [PATCH 1/2] x86: introduce post-init  read-only memory Michael Ellerman <mpe@ellerman.id.au> - 2015-11-26 00:10 +0100
                Re: [kernel-hardening] Re: [PATCH 1/2] x86: introduce post-init  read-only memory Kees Cook <keescook@chromium.org> - 2015-11-26 00:40 +0100
    [PATCH 2/2] x86, vdso: mark vDSO read-only after init Kees Cook <keescook@chromium.org> - 2015-11-24 22:50 +0100
    Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory Mathias Krause <minipli@googlemail.com> - 2015-11-25 10:20 +0100
      Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only  memory Clemens Ladisch <clemens@ladisch.de> - 2015-11-25 11:10 +0100
        Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory "PaX Team" <pageexec@freemail.hu> - 2015-11-25 12:20 +0100
      Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory "PaX Team" <pageexec@freemail.hu> - 2015-11-25 12:10 +0100
        Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only  memory Ingo Molnar <mingo@kernel.org> - 2015-11-26 10:00 +0100
          Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory "PaX Team" <pageexec@freemail.hu> - 2015-11-26 11:00 +0100
            Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only  memory Ingo Molnar <mingo@kernel.org> - 2015-11-26 11:50 +0100
              Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory "PaX Team" <pageexec@freemail.hu> - 2015-11-26 13:20 +0100
                Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only  memory Ingo Molnar <mingo@kernel.org> - 2015-11-27 09:10 +0100
                  Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory "PaX Team" <pageexec@freemail.hu> - 2015-11-27 16:40 +0100
                    Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory Andy Lutomirski <luto@amacapital.net> - 2015-11-27 17:40 +0100
                    Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only  memory Ingo Molnar <mingo@kernel.org> - 2015-11-29 09:10 +0100
                      Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory "PaX Team" <pageexec@freemail.hu> - 2015-11-29 12:20 +0100
                        Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only  memory Ingo Molnar <mingo@kernel.org> - 2015-11-29 16:40 +0100
                          Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory Mathias Krause <minipli@googlemail.com> - 2015-11-29 19:10 +0100
                            Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only  memory Ingo Molnar <mingo@kernel.org> - 2015-11-30 09:10 +0100
          Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory Andy Lutomirski <luto@amacapital.net> - 2015-11-26 17:20 +0100
            Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only  memory Ingo Molnar <mingo@kernel.org> - 2015-11-27 09:10 +0100
              Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-27 19:10 +0100
              Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory Linus Torvalds <torvalds@linux-foundation.org> - 2015-11-27 19:10 +0100
                Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory Kees Cook <keescook@chromium.org> - 2015-11-27 21:10 +0100
                  Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory Andy Lutomirski <luto@amacapital.net> - 2015-11-27 21:10 +0100
                    Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only  memory Ingo Molnar <mingo@kernel.org> - 2015-11-29 09:10 +0100
      Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory Kees Cook <keescook@chromium.org> - 2015-11-25 18:30 +0100
      Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only  memory "H. Peter Anvin" <hpa@zytor.com> - 2015-11-25 18:40 +0100
        Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory Kees Cook <keescook@chromium.org> - 2015-11-25 20:00 +0100
          Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only  memory "H. Peter Anvin" <hpa@zytor.com> - 2015-11-25 20:10 +0100

Page 1 of 2  [1] 2  Next page →

#1276768 — [PATCH 0/2] introduce post-init read-only memory

One of the easiest ways to protect the kernel from attack is to reduce
the internal attack surface exposed when a "write" flaw is available. By
making as much of the kernel read-only as possible, we reduce the
attack surface.

Many things are written to only during __init, and never changed
again. These cannot be made "const" since the compiler will do the wrong
thing (we do actually need to write to them). Instead, move these items
into a memory region that will be made read-only during mark_rodata_ro()
which happens after all kernel __init code has finished.

This introduces __read_only as a way to mark such memory, and uses it on
the x86 vDSO to kill an extant kernel exploitation method.

-Kees

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1276771 — [PATCH 1/2] x86: introduce post-init read-only memory

One of the easiest ways to protect the kernel from attack is to reduce
the internal attack surface exposed when a "write" flaw is available. By
making as much of the kernel read-only as possible, we reduce the
attack surface.

Many things are written to only during __init, and never changed
again. These cannot be made "const" since the compiler will do the wrong
thing (we do actually need to write to them). Instead, move these items
into a memory region that will be made read-only during mark_rodata_ro()
which happens after all kernel __init code has finished.

This introduces __read_only as a way to mark such memory, and adds some
documentation about the existing __read_mostly marking.

Based on work by PaX Team and Brad Spengler.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/include/asm/cache.h      |  1 +
 include/asm-generic/vmlinux.lds.h |  1 +
 include/linux/cache.h             | 15 +++++++++++++++
 3 files changed, 17 insertions(+)

diff --git a/arch/x86/include/asm/cache.h b/arch/x86/include/asm/cache.h
index 48f99f15452e..982b21c5eb1f 100644
--- a/arch/x86/include/asm/cache.h
+++ b/arch/x86/include/asm/cache.h
@@ -8,6 +8,7 @@
 #define L1_CACHE_BYTES	(1 << L1_CACHE_SHIFT)
 
 #define __read_mostly __attribute__((__section__(".data..read_mostly")))
+#define __read_only __attribute__((__section__(".data..read_only")))
 
 #define INTERNODE_CACHE_SHIFT CONFIG_X86_INTERNODE_CACHE_SHIFT
 #define INTERNODE_CACHE_BYTES (1 << INTERNODE_CACHE_SHIFT)
diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index c4bd0e2c173c..998a09d7731c 100644
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -256,6 +256,7 @@
 	.rodata           : AT(ADDR(.rodata) - LOAD_OFFSET) {		\
 		VMLINUX_SYMBOL(__start_rodata) = .;			\
 		*(.rodata) *(.rodata.*)					\
+		*(.data..read_only)	/* Read only after init */	\
 		*(__vermagic)		/* Kernel version magic */	\
 		. = ALIGN(8);						\
 		VMLINUX_SYMBOL(__start___tracepoints_ptrs) = .;		\
diff --git a/include/linux/cache.h b/include/linux/cache.h
index 17e7e82d2aa7..b2967e711a75 100644
--- a/include/linux/cache.h
+++ b/include/linux/cache.h
@@ -12,10 +12,25 @@
 #define SMP_CACHE_BYTES L1_CACHE_BYTES
 #endif
 
+/*
+ * __read_mostly is used to keep rarely changing variables out of frequently
+ * updated cachelines. If an architecture doesn't support it, ignore the
+ * hint.
+ */
 #ifndef __read_mostly
 #define __read_mostly
 #endif
 
+/*
+ * __read_only is used to mark things that are read-only after init (i.e.
+ * after mark_rodata_ro() has been called). These are effectively read-only,
+ * but may get written to during init, so can't live in .rodata (via "const").
+ * Hint to __read_mostly if the architecture hasn't wired this up.
+ */
+#ifndef __read_only
+#define __read_only __read_mostly
+#endif
+
 #ifndef ____cacheline_aligned
 #define ____cacheline_aligned __attribute__((__aligned__(SMP_CACHE_BYTES)))
 #endif
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1276928 — Re: [PATCH 1/2] x86: introduce post-init read-only memory

On Nov 24, 2015 1:38 PM, "Kees Cook" <keescook@chromium.org> wrote:
>
> One of the easiest ways to protect the kernel from attack is to reduce
> the internal attack surface exposed when a "write" flaw is available. By
> making as much of the kernel read-only as possible, we reduce the
> attack surface.
>
> Many things are written to only during __init, and never changed
> again. These cannot be made "const" since the compiler will do the wrong
> thing (we do actually need to write to them). Instead, move these items
> into a memory region that will be made read-only during mark_rodata_ro()
> which happens after all kernel __init code has finished.
>
> This introduces __read_only as a way to mark such memory, and adds some
> documentation about the existing __read_mostly marking.

Obligatory bikeshed:  __ro_after_init, please.  It's barely longer,
and it directly explains what's going on.  __read_only makes me think
that it's really read-only and could, for example, actually be in ROM.

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1276935 — Re: [PATCH 1/2] x86: introduce post-init read-only memory

On Tue, Nov 24, 2015 at 4:34 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> On Nov 24, 2015 1:38 PM, "Kees Cook" <keescook@chromium.org> wrote:
>>
>> One of the easiest ways to protect the kernel from attack is to reduce
>> the internal attack surface exposed when a "write" flaw is available. By
>> making as much of the kernel read-only as possible, we reduce the
>> attack surface.
>>
>> Many things are written to only during __init, and never changed
>> again. These cannot be made "const" since the compiler will do the wrong
>> thing (we do actually need to write to them). Instead, move these items
>> into a memory region that will be made read-only during mark_rodata_ro()
>> which happens after all kernel __init code has finished.
>>
>> This introduces __read_only as a way to mark such memory, and adds some
>> documentation about the existing __read_mostly marking.
>
> Obligatory bikeshed:  __ro_after_init, please.  It's barely longer,
> and it directly explains what's going on.  __read_only makes me think
> that it's really read-only and could, for example, actually be in ROM.

I'm fine with that. Anyone else want to chime in before I send a v2?

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1276936 — Re: [kernel-hardening] Re: [PATCH 1/2] x86: introduce post-init read-only memory

On Tue, 2015-11-24 at 16:44 -0800, Kees Cook wrote:
> On Tue, Nov 24, 2015 at 4:34 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> > On Nov 24, 2015 1:38 PM, "Kees Cook" <keescook@chromium.org> wrote:
> > > 
> > > One of the easiest ways to protect the kernel from attack is to reduce
> > > the internal attack surface exposed when a "write" flaw is available. By
> > > making as much of the kernel read-only as possible, we reduce the
> > > attack surface.
> > > 
> > > Many things are written to only during __init, and never changed
> > > again. These cannot be made "const" since the compiler will do the wrong
> > > thing (we do actually need to write to them). Instead, move these items
> > > into a memory region that will be made read-only during mark_rodata_ro()
> > > which happens after all kernel __init code has finished.
> > > 
> > > This introduces __read_only as a way to mark such memory, and adds some
> > > documentation about the existing __read_mostly marking.
> > 
> > Obligatory bikeshed:  __ro_after_init, please.  It's barely longer,
> > and it directly explains what's going on.  __read_only makes me think
> > that it's really read-only and could, for example, actually be in ROM.
> 
> I'm fine with that. Anyone else want to chime in before I send a v2?

I'm not clear on why this is x86 only?

It looks like it would work on any arch, or is there some toolchain
requirement?

cheers

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1277450 — Re: [kernel-hardening] Re: [PATCH 1/2] x86: introduce post-init read-only memory

On Tue, Nov 24, 2015 at 4:54 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> On Tue, 2015-11-24 at 16:44 -0800, Kees Cook wrote:
>> On Tue, Nov 24, 2015 at 4:34 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> > On Nov 24, 2015 1:38 PM, "Kees Cook" <keescook@chromium.org> wrote:
>> > >
>> > > One of the easiest ways to protect the kernel from attack is to reduce
>> > > the internal attack surface exposed when a "write" flaw is available. By
>> > > making as much of the kernel read-only as possible, we reduce the
>> > > attack surface.
>> > >
>> > > Many things are written to only during __init, and never changed
>> > > again. These cannot be made "const" since the compiler will do the wrong
>> > > thing (we do actually need to write to them). Instead, move these items
>> > > into a memory region that will be made read-only during mark_rodata_ro()
>> > > which happens after all kernel __init code has finished.
>> > >
>> > > This introduces __read_only as a way to mark such memory, and adds some
>> > > documentation about the existing __read_mostly marking.
>> >
>> > Obligatory bikeshed:  __ro_after_init, please.  It's barely longer,
>> > and it directly explains what's going on.  __read_only makes me think
>> > that it's really read-only and could, for example, actually be in ROM.
>>
>> I'm fine with that. Anyone else want to chime in before I send a v2?
>
> I'm not clear on why this is x86 only?

I was initially looking at how __read_mostly got implemented, and it
seemed like section names were done on a per-arch basis. But it
doesn't seem like that needs to be true.

> It looks like it would work on any arch, or is there some toolchain
> requirement?

Given that the other sections are in the common linux.lds.h file, it
seems unlikely to me. I'll try it in an arch-agnostic way and see what
happens. :)

-Kees

-- 
Kees Cook
Chrome OS & Brillo Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1277868 — Re: [kernel-hardening] Re: [PATCH 1/2] x86: introduce post-init read-only memory

On Wed, 2015-11-25 at 07:03 -0800, Kees Cook wrote:
> On Tue, Nov 24, 2015 at 4:54 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> > On Tue, 2015-11-24 at 16:44 -0800, Kees Cook wrote:
> > > On Tue, Nov 24, 2015 at 4:34 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> > > > On Nov 24, 2015 1:38 PM, "Kees Cook" <keescook@chromium.org> wrote:
> > > > > 
> > > > > One of the easiest ways to protect the kernel from attack is to reduce
> > > > > the internal attack surface exposed when a "write" flaw is available. By
> > > > > making as much of the kernel read-only as possible, we reduce the
> > > > > attack surface.
> > > > > 
> > > > > Many things are written to only during __init, and never changed
> > > > > again. These cannot be made "const" since the compiler will do the wrong
> > > > > thing (we do actually need to write to them). Instead, move these items
> > > > > into a memory region that will be made read-only during mark_rodata_ro()
> > > > > which happens after all kernel __init code has finished.
> > > > > 
> > > > > This introduces __read_only as a way to mark such memory, and adds some
> > > > > documentation about the existing __read_mostly marking.
> > > > 
> > > > Obligatory bikeshed:  __ro_after_init, please.  It's barely longer,
> > > > and it directly explains what's going on.  __read_only makes me think
> > > > that it's really read-only and could, for example, actually be in ROM.
> > > 
> > > I'm fine with that. Anyone else want to chime in before I send a v2?
> > 
> > I'm not clear on why this is x86 only?
> 
> I was initially looking at how __read_mostly got implemented, and it
> seemed like section names were done on a per-arch basis. But it
> doesn't seem like that needs to be true.

Yeah I saw that too, but I couldn't see anything in the commit history that
explained why it was per-arch.

> > It looks like it would work on any arch, or is there some toolchain
> > requirement?
> 
> Given that the other sections are in the common linux.lds.h file, it
> seems unlikely to me. I'll try it in an arch-agnostic way and see what
> happens. :)

That'd be great, I can test on powerpc, and build test other arches too.

cheers

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1277879 — Re: [kernel-hardening] Re: [PATCH 1/2] x86: introduce post-init read-only memory

On Wed, Nov 25, 2015 at 3:05 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
> On Wed, 2015-11-25 at 07:03 -0800, Kees Cook wrote:
>> On Tue, Nov 24, 2015 at 4:54 PM, Michael Ellerman <mpe@ellerman.id.au> wrote:
>> > On Tue, 2015-11-24 at 16:44 -0800, Kees Cook wrote:
>> > > On Tue, Nov 24, 2015 at 4:34 PM, Andy Lutomirski <luto@amacapital.net> wrote:
>> > > > On Nov 24, 2015 1:38 PM, "Kees Cook" <keescook@chromium.org> wrote:
>> > > > >
>> > > > > One of the easiest ways to protect the kernel from attack is to reduce
>> > > > > the internal attack surface exposed when a "write" flaw is available. By
>> > > > > making as much of the kernel read-only as possible, we reduce the
>> > > > > attack surface.
>> > > > >
>> > > > > Many things are written to only during __init, and never changed
>> > > > > again. These cannot be made "const" since the compiler will do the wrong
>> > > > > thing (we do actually need to write to them). Instead, move these items
>> > > > > into a memory region that will be made read-only during mark_rodata_ro()
>> > > > > which happens after all kernel __init code has finished.
>> > > > >
>> > > > > This introduces __read_only as a way to mark such memory, and adds some
>> > > > > documentation about the existing __read_mostly marking.
>> > > >
>> > > > Obligatory bikeshed:  __ro_after_init, please.  It's barely longer,
>> > > > and it directly explains what's going on.  __read_only makes me think
>> > > > that it's really read-only and could, for example, actually be in ROM.
>> > >
>> > > I'm fine with that. Anyone else want to chime in before I send a v2?
>> >
>> > I'm not clear on why this is x86 only?
>>
>> I was initially looking at how __read_mostly got implemented, and it
>> seemed like section names were done on a per-arch basis. But it
>> doesn't seem like that needs to be true.
>
> Yeah I saw that too, but I couldn't see anything in the commit history that
> explained why it was per-arch.

Best I was able to see was that architectures weren't (aren't?) using
the common RODATA section macros in their linker scripts. From a quick
inspection, I think these are all okay now.

-Kees

>
>> > It looks like it would work on any arch, or is there some toolchain
>> > requirement?
>>
>> Given that the other sections are in the common linux.lds.h file, it
>> seems unlikely to me. I'll try it in an arch-agnostic way and see what
>> happens. :)
>
> That'd be great, I can test on powerpc, and build test other arches too.
>
> cheers
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/



-- 
Kees Cook
Chrome OS & Brillo Security
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1276779 — [PATCH 2/2] x86, vdso: mark vDSO read-only after init

The vDSO does not need to be writable after __init, so mark it as
__read_only. The result kills the exploit method of writing to the vDSO
from kernel space resulting in userspace executing the modified code,
as shown here to bypass SMEP restrictions: http://itszn.com/blog/?p=21

The memory map (with added vDSO address reporting) shows the vDSO moving
into read-only memory:

Before:
[    0.143067] vDSO @ ffffffff82004000
[    0.143551] vDSO @ ffffffff82006000
---[ High Kernel Mapping ]---
0xffffffff80000000-0xffffffff81000000      16M                         pmd
0xffffffff81000000-0xffffffff81800000       8M   ro     PSE     GLB x  pmd
0xffffffff81800000-0xffffffff819f3000    1996K   ro             GLB x  pte
0xffffffff819f3000-0xffffffff81a00000      52K   ro                 NX pte
0xffffffff81a00000-0xffffffff81e00000       4M   ro     PSE     GLB NX pmd
0xffffffff81e00000-0xffffffff81e05000      20K   ro             GLB NX pte
0xffffffff81e05000-0xffffffff82000000    2028K   ro                 NX pte
0xffffffff82000000-0xffffffff8214f000    1340K   RW             GLB NX pte
0xffffffff8214f000-0xffffffff82281000    1224K   RW                 NX pte
0xffffffff82281000-0xffffffff82400000    1532K   RW             GLB NX pte
0xffffffff82400000-0xffffffff83200000      14M   RW     PSE     GLB NX pmd
0xffffffff83200000-0xffffffffc0000000     974M                         pmd

After:
[    0.145062] vDSO @ ffffffff81da1000
[    0.146057] vDSO @ ffffffff81da4000
---[ High Kernel Mapping ]---
0xffffffff80000000-0xffffffff81000000      16M                         pmd
0xffffffff81000000-0xffffffff81800000       8M   ro     PSE     GLB x  pmd
0xffffffff81800000-0xffffffff819f3000    1996K   ro             GLB x  pte
0xffffffff819f3000-0xffffffff81a00000      52K   ro                 NX pte
0xffffffff81a00000-0xffffffff81e00000       4M   ro     PSE     GLB NX pmd
0xffffffff81e00000-0xffffffff81e0b000      44K   ro             GLB NX pte
0xffffffff81e0b000-0xffffffff82000000    2004K   ro                 NX pte
0xffffffff82000000-0xffffffff8214c000    1328K   RW             GLB NX pte
0xffffffff8214c000-0xffffffff8227e000    1224K   RW                 NX pte
0xffffffff8227e000-0xffffffff82400000    1544K   RW             GLB NX pte
0xffffffff82400000-0xffffffff83200000      14M   RW     PSE     GLB NX pmd
0xffffffff83200000-0xffffffffc0000000     974M                         pmd

Based on work by Brad Spengler.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/entry/vdso/vdso2c.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/entry/vdso/vdso2c.h b/arch/x86/entry/vdso/vdso2c.h
index 0224987556ce..677ce3ac4d34 100644
--- a/arch/x86/entry/vdso/vdso2c.h
+++ b/arch/x86/entry/vdso/vdso2c.h
@@ -140,7 +140,7 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
 	fprintf(outfile, "#include <asm/vdso.h>\n");
 	fprintf(outfile, "\n");
 	fprintf(outfile,
-		"static unsigned char raw_data[%lu] __page_aligned_data = {",
+		"static unsigned char raw_data[%lu] __read_only __aligned(PAGE_SIZE) = {",
 		mapping_size);
 	for (j = 0; j < stripped_len; j++) {
 		if (j % 10 == 0)
@@ -150,7 +150,7 @@ static void BITSFUNC(go)(void *raw_addr, size_t raw_len,
 	}
 	fprintf(outfile, "\n};\n\n");
 
-	fprintf(outfile, "static struct page *pages[%lu];\n\n",
+	fprintf(outfile, "static struct page *pages[%lu] __read_only;\n\n",
 		mapping_size / 4096);
 
 	fprintf(outfile, "const struct vdso_image %s = {\n", name);
-- 
1.9.1

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1277122 — Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory

On 24 November 2015 at 22:38, Kees Cook <keescook@chromium.org> wrote:
> Many things are written to only during __init, and never changed
> again. These cannot be made "const" since the compiler will do the wrong
> thing (we do actually need to write to them). Instead, move these items
> into a memory region that will be made read-only during mark_rodata_ro()
> which happens after all kernel __init code has finished.
>
> This introduces __read_only as a way to mark such memory, and uses it on
> the x86 vDSO to kill an extant kernel exploitation method.

...just some random notes on the experience with kernels implementing
such a feature for quite a lot of locations, not just the vDSO.

While having that annotation makes perfect sense, not only from a
security perspective but also from a micro-optimization point of view
(much like the already existing __read_mostly annotation), it has its
drawbacks. Violating the "r/o after init" rule by writing to such
annotated variables from non-init code goes unnoticed as far as it
concerns the toolchain. Neither the compiler nor the linker will flag
that incorrect use. It'll just trap at runtime and that's bad.

I myself had some educating experience seeing my machine triple fault
when resuming from a S3 sleep. The root cause was a variable that was
annotated __read_only but that was (unnecessarily) modified during CPU
bring-up phase. Debugging that kind of problems is sort of a PITA, you
could imagine.

So, prior extending the usage of the __read_only annotation some
toolchain support is needed. Maybe a gcc plugin that'll warn/error on
code that writes to such a variable but is not __init itself. The
initify and checker plugins from the PaX patch might be worth to look
at for that purpose, as they're doing similar things already. Adding
such a check to sparse might be worth it, too.
A modpost check probably won't work as it's unable to tell if it's a
legitimate access (r/o) or a violation (/w access). So the gcc plugin
is the way to go, IMHO.


Regards,
Mathias
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1277153 — Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory

Mathias Krause wrote:
> [...]
> So, prior extending the usage of the __read_only annotation some
> toolchain support is needed. Maybe a gcc plugin that'll warn/error on
> code that writes to such a variable but is not __init itself.

Or mark them as "const".  This would require the initialization code to
cast it away, probably with a helper macro.


Regards,
Clemens
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1277238 — Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory

On 25 Nov 2015 at 11:06, Clemens Ladisch wrote:

> Mathias Krause wrote:
> > [...]
> > So, prior extending the usage of the __read_only annotation some
> > toolchain support is needed. Maybe a gcc plugin that'll warn/error on
> > code that writes to such a variable but is not __init itself.
> 
> Or mark them as "const".  This would require the initialization code to
> cast it away, probably with a helper macro.

no, that'd be undefined behaviour and in practice gcc would take advantage
of it and produce unintended (and quite broken) code. if the constified object
is modified from a different translation unit then the compiler is free
to assume that it can constant propagate its initialization value into uses,
completely breaking the code that (forcibly) writes to it.

however as a poor man's detector for such __read_only violations it's
possible to just make the object const temporarily (without casting away
the write attempts!), recompile the tree and see if any writes outside
__init functions pop up.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1277205 — Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory

On 25 Nov 2015 at 10:13, Mathias Krause wrote:

> I myself had some educating experience seeing my machine triple fault
> when resuming from a S3 sleep. The root cause was a variable that was
> annotated __read_only but that was (unnecessarily) modified during CPU
> bring-up phase. Debugging that kind of problems is sort of a PITA, you
> could imagine.

actually the kernel could silently recover from this given how the
page fault handler could easily determine that the fault address fell
into the data..read_only section and just silently undo the read-only
property, log the event to dmesg and retry the faulting access.

> So, prior extending the usage of the __read_only annotation some
> toolchain support is needed. Maybe a gcc plugin that'll warn/error on
> code that writes to such a variable but is not __init itself.

this is exactly what i suggested earlier in the constify thread ;).
note that this will produce false positives because __init* annotations
are not propagated everywhere they could be.

> The initify and checker plugins from the PaX patch might be worth to
> look at for that purpose, as they're doing similar things already.

one of our plans for initify is to add the discovery and propagation
of _init* annotations as well, it'd not only fix the false positives
mentioned above but also help reduce the kernel size (code/data/rodata).

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1278090 — Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory

* PaX Team <pageexec@freemail.hu> wrote:

> On 25 Nov 2015 at 10:13, Mathias Krause wrote:
> 
> > I myself had some educating experience seeing my machine triple fault
> > when resuming from a S3 sleep. The root cause was a variable that was
> > annotated __read_only but that was (unnecessarily) modified during CPU
> > bring-up phase. Debugging that kind of problems is sort of a PITA, you
> > could imagine.

( Sidenote: I don't think a ro-faults typically result in triple faults, but yeah, 
  even having a regular oops (followed by a hang or reboot) during such an 
  undebuggable state of the system is a major PITA. )

> actually the kernel could silently recover from this given how the page fault 
> handler could easily determine that the fault address fell into the 
> data..read_only section and just silently undo the read-only property, log the 
> event to dmesg and retry the faulting access.

So a safer method would be to decode the faulting instruction, to skip it by 
fixing up the return RIP and to log the event. It would be mostly equivalent to 
trying to write to ROM (which get ignored as well), so it's a recoverable (and 
debuggable) event.

We have all the necessary code in place in the kprobes code, see 
arch/x86/lib/insn.c, it's a simplified x86 decoder that knows about instruction 
length (but not about semantics).

Simple skipping plus setting arithmetic flags to init value should be enough I 
think: I don't think we use fancy instructions to write to ro variables, such as 
PUSH/POP with other side effects. If such instructions exist we could minimally 
extend the decoder to do those fixups as well - in addition to double checking 
that we skip simple instructions only with no side effects.

Can you see any fragility in such a technique?

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1278123 — Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory

On 26 Nov 2015 at 9:54, Ingo Molnar wrote:

> * PaX Team <pageexec@freemail.hu> wrote:
> 
> > actually the kernel could silently recover from this given how the page fault 
> > handler could easily determine that the fault address fell into the 
> > data..read_only section and just silently undo the read-only property, log the 
> > event to dmesg and retry the faulting access.
> 
> So a safer method would be to decode the faulting instruction, to skip it by 
> fixing up the return RIP and to log the event. It would be mostly equivalent to 
> trying to write to ROM (which get ignored as well), so it's a recoverable (and 
> debuggable) event.

if by skipping you mean ignoring the write attempt then it's not a good idea
as it has a good chance to cause unexpected behaviour down the line.

e.g., imagine that the write was to a function pointer (even an entire ops
structure) or a boolean that controls some important feature for after-init
code. ignoring/dropping such writes could cause all kinds of logic bugs (if
not worse).

my somewhat related war story is that i once tried to constify machine_ops
(both the struct and the variable of the same name) directly and just forced
the writes in kvm/xen/etc via type casts. now i knew it was all undefined
behaviour but i didn't expect gcc to take advantage of it but it did (const
propagated the *initial* fptr values into the indirect calls by turning them
into direct calls) and which in turn prevented proper reboots for guests
(an event which obviously happens much later after init/boot to the great
puzzlement of end users and myself).

misusing __read_only and ignoring write attempts would effectively produce
the same misbehaviour as above so i strongly advise against it.

now i understand that the motivation is probably to preserve the read-only
property despite wrong use of __read_only but for this i'd suggest to simply
not make the silent recovery the default behaviour and enable it on the
kernel command line only. after all, this is expected to be a reproducible
problem and affected users can just reboot (in fact, they'd be advised to)
when it happens and get a proper report that they could then send back to lkml.

i also don't expect this to be a frequent problem as __read_only will have
to be handled carefully in every case and not just at the time of adding it
to a variable but also later during code evolution. as i suggested in the
constify plugin thread earlier, use of __read_only should be tied to compile
time checking of all uses of the affected variable to eliminate the entire
problem of problematic writes.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1278143 — Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory

* PaX Team <pageexec@freemail.hu> wrote:

> On 26 Nov 2015 at 9:54, Ingo Molnar wrote:
> 
> > * PaX Team <pageexec@freemail.hu> wrote:
> > 
> > > actually the kernel could silently recover from this given how the page fault 
> > > handler could easily determine that the fault address fell into the 
> > > data..read_only section and just silently undo the read-only property, log the 
> > > event to dmesg and retry the faulting access.
> > 
> > So a safer method would be to decode the faulting instruction, to skip it by 
> > fixing up the return RIP and to log the event. It would be mostly equivalent 
> > to trying to write to ROM (which get ignored as well), so it's a recoverable 
> > (and debuggable) event.
> 
> if by skipping you mean ignoring the write attempt then it's not a good idea as 
> it has a good chance to cause unexpected behaviour down the line.
> 
> e.g., imagine that the write was to a function pointer (even an entire ops 
> structure) or a boolean that controls some important feature for after-init 
> code. ignoring/dropping such writes could cause all kinds of logic bugs (if not 
> worse).

Well, the typical case is that it's a logic bug to _do_ the write: the structure 
was marked readonly for a reason but some init code re-runs during suspend or so.

But yes, logic bugs might trigger - but that is true in the opposite case as well, 
if we do the write despite it being marked readonly:

> my somewhat related war story is that i once tried to constify machine_ops (both 
> the struct and the variable of the same name) directly and just forced the 
> writes in kvm/xen/etc via type casts. now i knew it was all undefined behaviour 
> but i didn't expect gcc to take advantage of it but it did (const propagated the 
> *initial* fptr values into the indirect calls by turning them into direct calls) 
> and which in turn prevented proper reboots for guests (an event which obviously 
> happens much later after init/boot to the great puzzlement of end users and 
> myself).
> 
> misusing __read_only and ignoring write attempts would effectively produce the 
> same misbehaviour as above so i strongly advise against it.

No, the difference to the GCC related aliasing bug is that with my technique the 
kernel would immediately produce a very visible kernel warning, which is a very 
clear sign that is wrong - and with a very clear backtrace in the warning that 
points right to the problematic code - which signature shows us (and users) what 
is wrong.

So your example is not comparable at all.

Plus the truly paranoid might panic/halt the system on such warnings, so for 
highly secure systems there's a way to not even allow the possibility of logic 
bugs. (at the cost of stopping the system when a bug triggers.)

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1278171 — Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory

On 26 Nov 2015 at 11:42, Ingo Molnar wrote:

> * PaX Team <pageexec@freemail.hu> wrote:
> 
> > On 26 Nov 2015 at 9:54, Ingo Molnar wrote:
>
> > e.g., imagine that the write was to a function pointer (even an entire ops 
> > structure) or a boolean that controls some important feature for after-init 
> > code. ignoring/dropping such writes could cause all kinds of logic bugs (if not 
> > worse).
> 
> Well, the typical case is that it's a logic bug to _do_ the write: the structure 
> was marked readonly for a reason but some init code re-runs during suspend or so.

that's actually not the typical case in my experience, but rather these two:

1. initial mistake: someone didn't actually check whether the given object can
   be __read_only

2. code evolution: an object that was once written by __init code only (and
   thus proactively subjected to __read_only) gets modified by non-init code
   due to later changes

what you described above is a third case where there's a latent bug to begin
(unintended write) with that __read_only merely exposes but doesn't create
itself, unlike the two cases above (intended writes getting caught by wrong
use of __read_only).

> But yes, logic bugs might trigger - but that is true in the opposite case as well, 
> if we do the write despite it being marked readonly:

not really, the two cases above are not a priori bugs, they become bugs due
to using __read_only without due care (which is why i suggested to detect
them at compile time).

> > misusing __read_only and ignoring write attempts would effectively produce the 
> > same misbehaviour as above so i strongly advise against it.
> 
> No, the difference to the GCC related aliasing bug is that with my technique the 
> kernel would immediately produce a very visible kernel warning, which is a very 
> clear sign that is wrong - and with a very clear backtrace in the warning that 
> points right to the problematic code - which signature shows us (and users) what 
> is wrong.

my proposal would produce the exact same reports, the difference is in letting
the write attempt succeed vs. skipping it. this latter step is what is wrong
since it introduces at least a logic bug the same way the constprop optimization
created a logic bug.

> Plus the truly paranoid might panic/halt the system on such warnings, so for 
> highly secure systems there's a way to not even allow the possibility of logic 
> bugs. (at the cost of stopping the system when a bug triggers.)

this would/should be the default behaviour, i.e., no attempt at being smart by
either allowing or skipping the faulting insn, just report the event and trigger
whatever fancy future exploit reaction mechanism will get into the kernel.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1278604 — Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory

* PaX Team <pageexec@freemail.hu> wrote:

> On 26 Nov 2015 at 11:42, Ingo Molnar wrote:
> 
> > * PaX Team <pageexec@freemail.hu> wrote:
> > 
> > > On 26 Nov 2015 at 9:54, Ingo Molnar wrote:
> >
> > > e.g., imagine that the write was to a function pointer (even an entire ops 
> > > structure) or a boolean that controls some important feature for after-init 
> > > code. ignoring/dropping such writes could cause all kinds of logic bugs (if not 
> > > worse).
> > 
> > Well, the typical case is that it's a logic bug to _do_ the write: the structure 
> > was marked readonly for a reason but some init code re-runs during suspend or so.
> 
> that's actually not the typical case in my experience, but rather these two:
> 
> 1. initial mistake: someone didn't actually check whether the given object can
>    be __read_only
> 
> 2. code evolution: an object that was once written by __init code only (and
>    thus proactively subjected to __read_only) gets modified by non-init code
>    due to later changes
> 
> what you described above is a third case where there's a latent bug to begin 
> (unintended write) with that __read_only merely exposes but doesn't create 
> itself, unlike the two cases above (intended writes getting caught by wrong use 
> of __read_only).

You are right, I concede this part of the argument - what you describe is probably 
the most typical way to get ro-faults.

I do maintain the (sub-)argument that oopsing or relying on tooling help years 
down the line is vastly inferior to fixing up the problem and generating a 
one-time stack dump so that kernel developers have a chance to fix the bug. The 
sooner we detect and dump such information the more likely it is that such bugs 
don't get into end user kernel versions.

> my proposal would produce the exact same reports, the difference is in letting 
> the write attempt succeed vs. skipping it. this latter step is what is wrong 
> since it introduces at least a logic bug the same way the constprop optimization 
> created a logic bug.

Yes, you are right and I agree.

Does anyone want to submit such a patch for upstream? Looks like a good change.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1278849 — Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory

On 27 Nov 2015 at 9:05, Ingo Molnar wrote:

> * PaX Team <pageexec@freemail.hu> wrote:
> 
> > On 26 Nov 2015 at 11:42, Ingo Molnar wrote:
> > 
> > > * PaX Team <pageexec@freemail.hu> wrote:
> > that's actually not the typical case in my experience, but rather these two:
> > 
> > 1. initial mistake: someone didn't actually check whether the given object can
> >    be __read_only
> > 
> > 2. code evolution: an object that was once written by __init code only (and
> >    thus proactively subjected to __read_only) gets modified by non-init code
> >    due to later changes
> > 
> > what you described above is a third case where there's a latent bug to begin 
> > (unintended write) with that __read_only merely exposes but doesn't create 
> > itself, unlike the two cases above (intended writes getting caught by wrong use 
> > of __read_only).
> 
> You are right, I concede this part of the argument - what you describe is probably 
> the most typical way to get ro-faults.
> 
> I do maintain the (sub-)argument that oopsing or relying on tooling help years 
> down the line is vastly inferior to fixing up the problem and generating a 
> one-time stack dump so that kernel developers have a chance to fix the bug. The 
> sooner we detect and dump such information the more likely it is that such bugs 
> don't get into end user kernel versions.

i don't see the compile time vs. runtime detection as 'competing' approaches,
both have their own role. in general, i think it's safe to say that compile
time problem detection is preferred to the runtime one since it subjects less
users to the side effects of the bug. runtime detection is needed to augment
(even complete) the coverage that compile time detection may not be able to
provide.

that said, for __read_only related problems the compiler can actually do a
pretty good job, basically it could detect most of them except special cases
where the 'bad' write is somehow hidden from it. the only examples i recall
are like the one that Mathias already mentioned where the 'bad' write was
done from asm code or out-of-kernel code (think UEFI runtime services) that
is obviously not visible to the compiler (the resume/mmu_cr4_features problem
also happens to be an example where runtime detection did not help due to the
circumstances).

so let me summarize how i expect the runtime detection part to work:

1. in normal use any write attempt to read-only kernel data should only
   be reported as usual (the oops info already has rip/cr2/backtrace),
   but no smart recovery attempts should be made since they may end up
   actually helping a real exploit attempt.

2. if necessary for debugging purposes (i.e., when the above reporting
   mechanism didn't produce the necessary logs and the problem is
   reproducible and wasn't an attack), a kernel command line option can
   be used to make an attempt at smart recovery instead of oopsing (but
   the same information would still be reported of course).

   for this smart recovery we differ(ed?) in opinion, i say that allowing
   the write in this case (vs. ignoring it) is the least likely to introduce
   a logic bug (and its cascading effects) since the expected problem is
   to be case #1 or #2 above (i.e., the write is intended but prevented
   by __read_only).

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

#1278880 — Re: [kernel-hardening] [PATCH 0/2] introduce post-init read-only memory

On Fri, Nov 27, 2015 at 7:29 AM, PaX Team <pageexec@freemail.hu> wrote:
> On 27 Nov 2015 at 9:05, Ingo Molnar wrote:
>
>> * PaX Team <pageexec@freemail.hu> wrote:
>>
>> > On 26 Nov 2015 at 11:42, Ingo Molnar wrote:
>> >
>> > > * PaX Team <pageexec@freemail.hu> wrote:
>> > that's actually not the typical case in my experience, but rather these two:
>> >
>> > 1. initial mistake: someone didn't actually check whether the given object can
>> >    be __read_only
>> >
>> > 2. code evolution: an object that was once written by __init code only (and
>> >    thus proactively subjected to __read_only) gets modified by non-init code
>> >    due to later changes
>> >
>> > what you described above is a third case where there's a latent bug to begin
>> > (unintended write) with that __read_only merely exposes but doesn't create
>> > itself, unlike the two cases above (intended writes getting caught by wrong use
>> > of __read_only).
>>
>> You are right, I concede this part of the argument - what you describe is probably
>> the most typical way to get ro-faults.
>>
>> I do maintain the (sub-)argument that oopsing or relying on tooling help years
>> down the line is vastly inferior to fixing up the problem and generating a
>> one-time stack dump so that kernel developers have a chance to fix the bug. The
>> sooner we detect and dump such information the more likely it is that such bugs
>> don't get into end user kernel versions.
>
> i don't see the compile time vs. runtime detection as 'competing' approaches,
> both have their own role. in general, i think it's safe to say that compile
> time problem detection is preferred to the runtime one since it subjects less
> users to the side effects of the bug. runtime detection is needed to augment
> (even complete) the coverage that compile time detection may not be able to
> provide.
>
> that said, for __read_only related problems the compiler can actually do a
> pretty good job, basically it could detect most of them except special cases
> where the 'bad' write is somehow hidden from it. the only examples i recall
> are like the one that Mathias already mentioned where the 'bad' write was
> done from asm code or out-of-kernel code (think UEFI runtime services) that
> is obviously not visible to the compiler (the resume/mmu_cr4_features problem
> also happens to be an example where runtime detection did not help due to the
> circumstances).
>
> so let me summarize how i expect the runtime detection part to work:
>
> 1. in normal use any write attempt to read-only kernel data should only
>    be reported as usual (the oops info already has rip/cr2/backtrace),
>    but no smart recovery attempts should be made since they may end up
>    actually helping a real exploit attempt.
>
> 2. if necessary for debugging purposes (i.e., when the above reporting
>    mechanism didn't produce the necessary logs and the problem is
>    reproducible and wasn't an attack), a kernel command line option can
>    be used to make an attempt at smart recovery instead of oopsing (but
>    the same information would still be reported of course).
>
>    for this smart recovery we differ(ed?) in opinion, i say that allowing
>    the write in this case (vs. ignoring it) is the least likely to introduce
>    a logic bug (and its cascading effects) since the expected problem is
>    to be case #1 or #2 above (i.e., the write is intended but prevented
>    by __read_only).
>

So maybe we should think about doing this recovery as part of oops
processing.  That is, we oops as usual, but rather than killing the
task or spinning, we allow the post-oops code to try to recover (if
enabled).  That recovery step decodes the instruction and takes some
action.  In this example, if it's a write to ro-after-init memory,
then maybe we un-write-protect it and resume.

I agree with Linus' old sentiment as least insofar as trying to decode
things pre-OOPS is a bad idea: we don't want that decoding to
interfere with our primary goal, which is printing the OOPS.

I would argue that, if this is okay, we do exactly the same thing for
failed msr access.  We still oops, but we try to march on.

We could consider a default setting in which we "recover" from oops
until init starts and then we revert to old behavior (oopses kill the
task).

--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

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

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web