From: Kees Cook <keescook@chromium.org> To: Ian Campbell <ijc@hellion.org.uk> Cc: "kernel-hardening@lists.openwall.com" <kernel-hardening@lists.openwall.com>, Mark Rutland <mark.rutland@arm.com>, Andy Lutomirski <luto@kernel.org>, Hoeun Ryu <hoeun.ryu@gmail.com>, PaX Team <pageexec@freemail.hu>, Emese Revfy <re.emese@gmail.com>, Russell King <linux@armlinux.org.uk>, "x86@kernel.org" <x86@kernel.org>, LKML <linux-kernel@vger.kernel.org>, "linux-arm-kernel@lists.infradead.org" <linux-arm-kernel@lists.infradead.org> Subject: Re: [kernel-hardening] [RFC v2][PATCH 02/11] lkdtm: add test for rare_write() infrastructure Date: Thu, 30 Mar 2017 09:16:29 -0700 [thread overview] Message-ID: <CAGXu5jLMKA3OwshsQn4yc1eXFwSuD3xbaYY56RY_Uu3TBAWG3A@mail.gmail.com> (raw) In-Reply-To: <1490866441.10874.44.camel@hellion.org.uk> 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
WARNING: multiple messages have this Message-ID (diff)
From: keescook@chromium.org (Kees Cook) To: linux-arm-kernel@lists.infradead.org Subject: [kernel-hardening] [RFC v2][PATCH 02/11] lkdtm: add test for rare_write() infrastructure Date: Thu, 30 Mar 2017 09:16:29 -0700 [thread overview] Message-ID: <CAGXu5jLMKA3OwshsQn4yc1eXFwSuD3xbaYY56RY_Uu3TBAWG3A@mail.gmail.com> (raw) In-Reply-To: <1490866441.10874.44.camel@hellion.org.uk> 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
next prev parent reply other threads:[~2017-03-30 16:16 UTC|newest] Thread overview: 188+ messages / expand[flat|nested] mbox.gz Atom feed top 2017-03-29 18:15 [RFC v2] Introduce rare_write() infrastructure Kees Cook 2017-03-29 18:15 ` [kernel-hardening] " Kees Cook 2017-03-29 18:15 ` Kees Cook 2017-03-29 18:15 ` [RFC v2][PATCH 01/11] " Kees Cook 2017-03-29 18:15 ` [kernel-hardening] " Kees Cook 2017-03-29 18:15 ` Kees Cook 2017-03-29 18:23 ` Kees Cook 2017-03-29 18:23 ` [kernel-hardening] " Kees Cook 2017-03-29 18:23 ` Kees Cook 2017-03-30 7:44 ` Ho-Eun Ryu 2017-03-30 7:44 ` [kernel-hardening] " Ho-Eun Ryu 2017-03-30 7:44 ` Ho-Eun Ryu 2017-03-30 17:02 ` Kees Cook 2017-03-30 17:02 ` [kernel-hardening] " Kees Cook 2017-03-30 17:02 ` Kees Cook 2017-04-07 8:09 ` Ho-Eun Ryu 2017-04-07 8:09 ` [kernel-hardening] " Ho-Eun Ryu 2017-04-07 8:09 ` Ho-Eun Ryu 2017-04-07 20:38 ` Kees Cook 2017-04-07 20:38 ` [kernel-hardening] " Kees Cook 2017-04-07 20:38 ` Kees Cook 2017-03-29 18:15 ` [RFC v2][PATCH 02/11] lkdtm: add test for " Kees Cook 2017-03-29 18:15 ` [kernel-hardening] " Kees Cook 2017-03-29 18:15 ` Kees Cook 2017-03-30 9:34 ` [kernel-hardening] " Ian Campbell 2017-03-30 9:34 ` Ian Campbell 2017-03-30 16:16 ` Kees Cook [this message] 2017-03-30 16:16 ` Kees Cook 2017-03-30 16:16 ` Kees Cook 2017-03-29 18:15 ` [RFC v2][PATCH 03/11] net: switch sock_diag handlers to rare_write() Kees Cook 2017-03-29 18:15 ` [kernel-hardening] " Kees Cook 2017-03-29 18:15 ` Kees Cook 2017-03-29 18:15 ` [RFC v2][PATCH 04/11] x86: Implement __arch_rare_write_begin/unmap() Kees Cook 2017-03-29 18:15 ` [kernel-hardening] " Kees Cook 2017-03-29 18:15 ` Kees Cook 2017-03-29 22:38 ` Andy Lutomirski 2017-03-29 22:38 ` [kernel-hardening] " Andy Lutomirski 2017-03-29 22:38 ` Andy Lutomirski 2017-03-30 1:41 ` Kees Cook 2017-03-30 1:41 ` [kernel-hardening] " Kees Cook 2017-03-30 1:41 ` Kees Cook 2017-04-05 23:57 ` Andy Lutomirski 2017-04-05 23:57 ` [kernel-hardening] " Andy Lutomirski 2017-04-05 23:57 ` Andy Lutomirski 2017-04-06 0:14 ` Kees Cook 2017-04-06 0:14 ` [kernel-hardening] " Kees Cook 2017-04-06 0:14 ` Kees Cook 2017-04-06 15:59 ` Andy Lutomirski 2017-04-06 15:59 ` [kernel-hardening] " Andy Lutomirski 2017-04-06 15:59 ` Andy Lutomirski 2017-04-07 8:34 ` [kernel-hardening] " Mathias Krause 2017-04-07 8:34 ` Mathias Krause 2017-04-07 8:34 ` Mathias Krause 2017-04-07 9:46 ` Thomas Gleixner 2017-04-07 9:46 ` Thomas Gleixner 2017-04-07 9:46 ` Thomas Gleixner 2017-04-07 10:51 ` Mathias Krause 2017-04-07 10:51 ` Mathias Krause 2017-04-07 10:51 ` Mathias Krause 2017-04-07 13:14 ` Thomas Gleixner 2017-04-07 13:14 ` Thomas Gleixner 2017-04-07 13:14 ` Thomas Gleixner 2017-04-07 13:30 ` Mathias Krause 2017-04-07 13:30 ` Mathias Krause 2017-04-07 13:30 ` Mathias Krause 2017-04-07 16:14 ` Andy Lutomirski 2017-04-07 16:14 ` Andy Lutomirski 2017-04-07 16:14 ` Andy Lutomirski 2017-04-07 16:22 ` Mark Rutland 2017-04-07 16:22 ` Mark Rutland 2017-04-07 16:22 ` Mark Rutland 2017-04-07 19:58 ` PaX Team 2017-04-07 19:58 ` PaX Team 2017-04-07 19:58 ` PaX Team 2017-04-08 4:58 ` Andy Lutomirski 2017-04-08 4:58 ` Andy Lutomirski 2017-04-08 4:58 ` Andy Lutomirski 2017-04-09 12:47 ` PaX Team 2017-04-09 12:47 ` PaX Team 2017-04-09 12:47 ` PaX Team 2017-04-10 0:10 ` Andy Lutomirski 2017-04-10 0:10 ` Andy Lutomirski 2017-04-10 0:10 ` Andy Lutomirski 2017-04-10 10:42 ` PaX Team 2017-04-10 10:42 ` PaX Team 2017-04-10 10:42 ` PaX Team 2017-04-10 16:01 ` Andy Lutomirski 2017-04-10 16:01 ` Andy Lutomirski 2017-04-10 16:01 ` Andy Lutomirski 2017-04-07 20:44 ` Thomas Gleixner 2017-04-07 20:44 ` Thomas Gleixner 2017-04-07 20:44 ` Thomas Gleixner 2017-04-07 21:20 ` Kees Cook 2017-04-07 21:20 ` Kees Cook 2017-04-07 21:20 ` Kees Cook 2017-04-08 4:12 ` Daniel Micay 2017-04-08 4:12 ` Daniel Micay 2017-04-08 4:12 ` Daniel Micay 2017-04-08 4:13 ` Daniel Micay 2017-04-08 4:13 ` Daniel Micay 2017-04-08 4:13 ` Daniel Micay 2017-04-08 4:21 ` Daniel Micay 2017-04-08 4:21 ` Daniel Micay 2017-04-08 4:21 ` Daniel Micay 2017-04-08 5:07 ` Andy Lutomirski 2017-04-08 5:07 ` Andy Lutomirski 2017-04-08 5:07 ` Andy Lutomirski 2017-04-08 7:33 ` Daniel Micay 2017-04-08 7:33 ` Daniel Micay 2017-04-08 7:33 ` Daniel Micay 2017-04-08 15:20 ` Andy Lutomirski 2017-04-08 15:20 ` Andy Lutomirski 2017-04-08 15:20 ` Andy Lutomirski 2017-04-09 10:53 ` Ingo Molnar 2017-04-09 10:53 ` Ingo Molnar 2017-04-09 10:53 ` Ingo Molnar 2017-04-10 10:22 ` Mark Rutland 2017-04-10 10:22 ` Mark Rutland 2017-04-10 10:22 ` Mark Rutland 2017-04-09 20:24 ` PaX Team 2017-04-09 20:24 ` PaX Team 2017-04-09 20:24 ` PaX Team 2017-04-10 0:31 ` Andy Lutomirski 2017-04-10 0:31 ` Andy Lutomirski 2017-04-10 0:31 ` Andy Lutomirski 2017-04-10 19:47 ` PaX Team 2017-04-10 19:47 ` PaX Team 2017-04-10 19:47 ` PaX Team 2017-04-10 20:27 ` Andy Lutomirski 2017-04-10 20:27 ` Andy Lutomirski 2017-04-10 20:27 ` Andy Lutomirski 2017-04-10 20:13 ` Kees Cook 2017-04-10 20:13 ` Kees Cook 2017-04-10 20:13 ` Kees Cook 2017-04-10 20:17 ` Andy Lutomirski 2017-04-10 20:17 ` Andy Lutomirski 2017-04-10 20:17 ` Andy Lutomirski 2017-04-07 19:25 ` Thomas Gleixner 2017-04-07 19:25 ` Thomas Gleixner 2017-04-07 19:25 ` Thomas Gleixner 2017-04-07 14:45 ` Peter Zijlstra 2017-04-07 14:45 ` Peter Zijlstra 2017-04-07 14:45 ` Peter Zijlstra 2017-04-10 10:29 ` Mark Rutland 2017-04-10 10:29 ` Mark Rutland 2017-04-10 10:29 ` Mark Rutland 2017-04-07 19:52 ` PaX Team 2017-04-07 19:52 ` PaX Team 2017-04-07 19:52 ` PaX Team 2017-04-10 8:26 ` Thomas Gleixner 2017-04-10 8:26 ` Thomas Gleixner 2017-04-10 8:26 ` Thomas Gleixner 2017-04-10 19:55 ` PaX Team 2017-04-10 19:55 ` PaX Team 2017-04-10 19:55 ` PaX Team 2017-04-07 9:37 ` Peter Zijlstra 2017-04-07 9:37 ` [kernel-hardening] " Peter Zijlstra 2017-04-07 9:37 ` Peter Zijlstra 2017-03-29 18:15 ` [RFC v2][PATCH 05/11] ARM: mm: dump: Add domain to output Kees Cook 2017-03-29 18:15 ` [kernel-hardening] " Kees Cook 2017-03-29 18:15 ` Kees Cook 2017-03-29 18:15 ` [RFC v2][PATCH 06/11] ARM: domains: Extract common USER domain init Kees Cook 2017-03-29 18:15 ` [kernel-hardening] " Kees Cook 2017-03-29 18:15 ` Kees Cook 2017-03-29 18:15 ` [RFC v2][PATCH 07/11] ARM: mm: set DOMAIN_WR_RARE for rodata Kees Cook 2017-03-29 18:15 ` [kernel-hardening] " Kees Cook 2017-03-29 18:15 ` Kees Cook 2017-03-29 18:16 ` [RFC v2][PATCH 08/11] ARM: Implement __arch_rare_write_begin/end() Kees Cook 2017-03-29 18:16 ` [kernel-hardening] " Kees Cook 2017-03-29 18:16 ` Kees Cook 2017-04-07 9:36 ` Peter Zijlstra 2017-04-07 9:36 ` [kernel-hardening] " Peter Zijlstra 2017-04-07 9:36 ` Peter Zijlstra 2017-03-29 18:16 ` [RFC v2][PATCH 09/11] list: add rare_write() list helpers Kees Cook 2017-03-29 18:16 ` [kernel-hardening] " Kees Cook 2017-03-29 18:16 ` Kees Cook 2017-03-29 18:16 ` [RFC v2][PATCH 10/11] gcc-plugins: Add constify plugin Kees Cook 2017-03-29 18:16 ` [kernel-hardening] " Kees Cook 2017-03-29 18:16 ` Kees Cook 2017-03-29 18:16 ` [RFC v2][PATCH 11/11] cgroups: force all struct cftype const Kees Cook 2017-03-29 18:16 ` [kernel-hardening] " Kees Cook 2017-03-29 18:16 ` Kees Cook 2017-03-29 19:00 ` [RFC v2] Introduce rare_write() infrastructure Russell King - ARM Linux 2017-03-29 19:00 ` [kernel-hardening] " Russell King - ARM Linux 2017-03-29 19:00 ` Russell King - ARM Linux 2017-03-29 19:14 ` Kees Cook 2017-03-29 19:14 ` [kernel-hardening] " Kees Cook 2017-03-29 19:14 ` Kees Cook
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=CAGXu5jLMKA3OwshsQn4yc1eXFwSuD3xbaYY56RY_Uu3TBAWG3A@mail.gmail.com \ --to=keescook@chromium.org \ --cc=hoeun.ryu@gmail.com \ --cc=ijc@hellion.org.uk \ --cc=kernel-hardening@lists.openwall.com \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=luto@kernel.org \ --cc=mark.rutland@arm.com \ --cc=pageexec@freemail.hu \ --cc=re.emese@gmail.com \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.