* [PATCH 0/3] x86/asm: More pinning
@ 2019-02-26 23:36 Kees Cook
2019-02-26 23:36 ` [PATCH 1/3] x86/asm: Pin sensitive CR0 bits Kees Cook
` (4 more replies)
0 siblings, 5 replies; 9+ messages in thread
From: Kees Cook @ 2019-02-26 23:36 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Kees Cook, Peter Zijlstra, Jann Horn, Sean Christopherson,
Dominik Brodowski, Kernel Hardening, linux-kernel
This adds CR0 pinning (for WP), and cleans up the CR4 pin to avoid
taking an exception from WARN before fixing up the desired pin.
Additionally adds lkdtm test (which depends on the CR4 patch, otherwise
I'd send it via Greg's tree).
Thanks!
-Kees
Kees Cook (3):
x86/asm: Pin sensitive CR0 bits
x86/asm: Avoid taking an exception before cr4 restore
lkdtm: Check for SMEP clearing protections
arch/x86/include/asm/special_insns.h | 33 +++++++++++++--
drivers/misc/lkdtm/bugs.c | 60 ++++++++++++++++++++++++++++
drivers/misc/lkdtm/core.c | 1 +
drivers/misc/lkdtm/lkdtm.h | 1 +
4 files changed, 91 insertions(+), 4 deletions(-)
--
2.17.1
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/3] x86/asm: Pin sensitive CR0 bits
2019-02-26 23:36 [PATCH 0/3] x86/asm: More pinning Kees Cook
@ 2019-02-26 23:36 ` Kees Cook
2019-02-27 10:44 ` Solar Designer
2019-02-26 23:36 ` [PATCH 2/3] x86/asm: Avoid taking an exception before cr4 restore Kees Cook
` (3 subsequent siblings)
4 siblings, 1 reply; 9+ messages in thread
From: Kees Cook @ 2019-02-26 23:36 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Kees Cook, Peter Zijlstra, Jann Horn, Sean Christopherson,
Dominik Brodowski, Kernel Hardening, linux-kernel
With sensitive CR4 bits pinned now, it's possible that the WP bit for CR0
might become a target as well. Following the same reasoning for the CR4
pinning, this pins CR0's WP bit (but this can be done with a static value).
As before, to convince the compiler to not optimize away the check for the
WP bit after the set, this marks "val" as an output from the asm() block.
This protects against just jumping into the function past where the masking
happens; we must check that the mask was applied after we do the set). Due
to how this function can be built by the compiler (especially due to the
removal of frame pointers), jumping into the middle of the function
frequently doesn't require stack manipulation to construct a stack frame
(there may only a retq without pops, which is sufficient for use with
exploits like timer overwrites).
Additionally, this avoids WARN()ing before resetting the bit, just to
minimize any race conditions with leaving the bit unset.
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/x86/include/asm/special_insns.h | 23 ++++++++++++++++++++++-
1 file changed, 22 insertions(+), 1 deletion(-)
diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
index fabda1400137..8416d6b31084 100644
--- a/arch/x86/include/asm/special_insns.h
+++ b/arch/x86/include/asm/special_insns.h
@@ -25,7 +25,28 @@ static inline unsigned long native_read_cr0(void)
static inline void native_write_cr0(unsigned long val)
{
- asm volatile("mov %0,%%cr0": : "r" (val), "m" (__force_order));
+ bool warn = false;
+
+again:
+ val |= X86_CR0_WP;
+ /*
+ * In order to have the compiler not optimize away the check
+ * in the WARN_ONCE(), mark "val" as being also an output ("+r")
+ * by this asm() block so it will perform an explicit check, as
+ * if it were "volatile".
+ */
+ asm volatile("mov %0,%%cr0": "+r" (val) : "m" (__force_order) : );
+ /*
+ * If the MOV above was used directly as a ROP gadget we can
+ * notice the lack of pinned bits in "val" and start the function
+ * from the beginning to gain the WP bit for sure. And do it
+ * without first taking the exception for a WARN().
+ */
+ if ((val & X86_CR0_WP) != X86_CR0_WP) {
+ warn = true;
+ goto again;
+ }
+ WARN_ONCE(warn, "Attempt to unpin X86_CR0_WP, cr0 bypass attack?!\n");
}
static inline unsigned long native_read_cr2(void)
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/3] x86/asm: Avoid taking an exception before cr4 restore
2019-02-26 23:36 [PATCH 0/3] x86/asm: More pinning Kees Cook
2019-02-26 23:36 ` [PATCH 1/3] x86/asm: Pin sensitive CR0 bits Kees Cook
@ 2019-02-26 23:36 ` Kees Cook
2019-02-26 23:36 ` [PATCH 3/3] lkdtm: Check for SMEP clearing protections Kees Cook
` (2 subsequent siblings)
4 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2019-02-26 23:36 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Kees Cook, Peter Zijlstra, Jann Horn, Sean Christopherson,
Dominik Brodowski, Kernel Hardening, linux-kernel
Instead of taking a full WARN() exception before restoring a potentially
missed CR4 bit, this retains the missing bit for later reporting. This
matches the logic done for the CR0 pinning.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/x86/include/asm/special_insns.h | 10 +++++++---
1 file changed, 7 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/special_insns.h b/arch/x86/include/asm/special_insns.h
index 8416d6b31084..6f649eaecc73 100644
--- a/arch/x86/include/asm/special_insns.h
+++ b/arch/x86/include/asm/special_insns.h
@@ -97,6 +97,8 @@ extern volatile unsigned long cr4_pin;
static inline void native_write_cr4(unsigned long val)
{
+ unsigned long warn = 0;
+
again:
val |= cr4_pin;
asm volatile("mov %0,%%cr4": : "r" (val), "m" (__force_order));
@@ -105,10 +107,12 @@ static inline void native_write_cr4(unsigned long val)
* notice the lack of pinned bits in "val" and start the function
* from the beginning to gain the cr4_pin bits for sure.
*/
- if (WARN_ONCE((val & cr4_pin) != cr4_pin,
- "Attempt to unpin cr4 bits: %lx, cr4 bypass attack?!",
- ~val & cr4_pin))
+ if ((val & cr4_pin) != cr4_pin) {
+ warn = ~val & cr4_pin;
goto again;
+ }
+ WARN_ONCE(warn, "Attempt to unpin cr4 bits: %lx; bypass attack?!\n",
+ warn);
}
#ifdef CONFIG_X86_64
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/3] lkdtm: Check for SMEP clearing protections
2019-02-26 23:36 [PATCH 0/3] x86/asm: More pinning Kees Cook
2019-02-26 23:36 ` [PATCH 1/3] x86/asm: Pin sensitive CR0 bits Kees Cook
2019-02-26 23:36 ` [PATCH 2/3] x86/asm: Avoid taking an exception before cr4 restore Kees Cook
@ 2019-02-26 23:36 ` Kees Cook
2019-02-26 23:40 ` Kees Cook
2019-02-26 23:45 ` [PATCH v2] " Kees Cook
2019-02-27 8:20 ` [PATCH 0/3] x86/asm: More pinning Greg KH
4 siblings, 1 reply; 9+ messages in thread
From: Kees Cook @ 2019-02-26 23:36 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Kees Cook, Peter Zijlstra, Jann Horn, Sean Christopherson,
Dominik Brodowski, Kernel Hardening, linux-kernel
This adds an x86-specific test for pinned cr4 bits. A successful test
will validate pinning and check the ROP-style call-middle-of-function
defense, if needed. For example, in the case of native_write_cr4()
looking like this:
ffffffff8171bce0 <native_write_cr4>:
ffffffff8171bce0: 48 8b 35 79 46 f2 00 mov 0xf24679(%rip),%rsi
ffffffff8171bce7: 48 09 f7 or %rsi,%rdi
ffffffff8171bcea: 0f 22 e7 mov %rdi,%cr4
...
ffffffff8171bd5a: c3 retq
The UNSET_SMEP test will jump to ffffffff8171bcea (the mov to cr4)
instead of ffffffff8171bce0 (native_write_cr4() entry) to simulate a
direct-call bypass attempt.
Expected successful results:
# echo UNSET_SMEP > /sys/kernel/debug/provoke-crash/DIRECT
# dmesg
[ 79.594433] lkdtm: Performing direct entry UNSET_SMEP
[ 79.596459] lkdtm: trying to clear SMEP normally
[ 79.598406] lkdtm: ok: SMEP did not get cleared
[ 79.599981] lkdtm: trying to clear SMEP with call gadget
[ 79.601810] ------------[ cut here ]------------
[ 79.603421] Attempt to unpin cr4 bits: 100000; bypass attack?!
...
[ 79.650170] ---[ end trace 2452ca0f6126242e ]---
[ 79.650937] lkdtm: ok: SMEP removal was reverted
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/misc/lkdtm/bugs.c | 60 ++++++++++++++++++++++++++++++++++++++
drivers/misc/lkdtm/core.c | 1 +
drivers/misc/lkdtm/lkdtm.h | 1 +
3 files changed, 62 insertions(+)
diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
index 7eebbdfbcacd..c79b43a5ba34 100644
--- a/drivers/misc/lkdtm/bugs.c
+++ b/drivers/misc/lkdtm/bugs.c
@@ -255,3 +255,63 @@ void lkdtm_STACK_GUARD_PAGE_TRAILING(void)
pr_err("FAIL: accessed page after stack!\n");
}
+
+void lkdtm_UNSET_SMEP(void)
+{
+#ifdef CONFIG_X86_64
+ void (*direct_write_cr4)(unsigned long val);
+ unsigned char *insn;
+ unsigned long cr4;
+ int i;
+
+ cr4 = native_read_cr4();
+
+ if ((cr4 & X86_CR4_SMEP) != X86_CR4_SMEP) {
+ pr_err("FAIL: SMEP not in use\n");
+ return;
+ }
+ cr4 &= ~(X86_CR4_SMEP);
+
+ pr_info("trying to clear SMEP normally\n");
+ native_write_cr4(cr4);
+ if (cr4 == native_read_cr4()) {
+ pr_err("FAIL: pinning SMEP failed!\n");
+ cr4 |= X86_CR4_SMEP;
+ pr_info("restoring SMEP\n");
+ native_write_cr4(cr4);
+ return;
+ }
+ pr_info("ok: SMEP did not get cleared\n");
+
+ /*
+ * To test the post-write pinning verification we need to call
+ * directly into the the middle of native_write_cr4() where the
+ * cr4 write happens, skipping the pinning. This searches for
+ * the cr4 writing instruction.
+ */
+ insn = (unsigned char *)native_write_cr4;
+ for (i = 0; i < 64; i++) {
+ /* mov %rdi, %cr4 */
+ if (insn[i] == 0x0f && insn[i+1] == 0x22 && insn[i+2] == 0xe7)
+ break;
+ }
+ if (i >= 256) {
+ pr_info("ok: cannot locate cr4 writing call gadget\n");
+ return;
+ }
+ direct_write_cr4 = (void *)(insn + i);
+
+ pr_info("trying to clear SMEP with call gadget\n");
+ direct_write_cr4(cr4);
+ if (native_read_cr4() & X86_CR4_SMEP) {
+ pr_info("ok: SMEP removal was reverted\n");
+ } else {
+ pr_err("FAIL: cleared SMEP not detected!\n");
+ cr4 |= X86_CR4_SMEP;
+ pr_info("restoring SMEP\n");
+ native_write_cr4(cr4);
+ }
+#else
+ pr_err("FAIL: this test is x86_64-only\n");
+#endif
+}
diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index 2837dc77478e..fd668776414b 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -132,6 +132,7 @@ static const struct crashtype crashtypes[] = {
CRASHTYPE(CORRUPT_LIST_ADD),
CRASHTYPE(CORRUPT_LIST_DEL),
CRASHTYPE(CORRUPT_USER_DS),
+ CRASHTYPE(UNSET_SMEP),
CRASHTYPE(CORRUPT_STACK),
CRASHTYPE(CORRUPT_STACK_STRONG),
CRASHTYPE(STACK_GUARD_PAGE_LEADING),
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 3c6fd327e166..9c78d7e21c13 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -26,6 +26,7 @@ void lkdtm_CORRUPT_LIST_DEL(void);
void lkdtm_CORRUPT_USER_DS(void);
void lkdtm_STACK_GUARD_PAGE_LEADING(void);
void lkdtm_STACK_GUARD_PAGE_TRAILING(void);
+void lkdtm_UNSET_SMEP(void);
/* lkdtm_heap.c */
void lkdtm_OVERWRITE_ALLOCATION(void);
--
2.17.1
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 3/3] lkdtm: Check for SMEP clearing protections
2019-02-26 23:36 ` [PATCH 3/3] lkdtm: Check for SMEP clearing protections Kees Cook
@ 2019-02-26 23:40 ` Kees Cook
0 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2019-02-26 23:40 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Peter Zijlstra, Jann Horn, Sean Christopherson,
Dominik Brodowski, Kernel Hardening, LKML
On Tue, Feb 26, 2019 at 3:37 PM Kees Cook <keescook@chromium.org> wrote:
> + for (i = 0; i < 64; i++) {
> ...
> + }
> + if (i >= 256) {
> + pr_info("ok: cannot locate cr4 writing call gadget\n");
> + return;
> + }
*brown paper bag*
Argh. I changed the depth of that search at the last moment and now
it's mismatched. *sigh* I will send a v2 for this with it fixed.
--
Kees Cook
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v2] lkdtm: Check for SMEP clearing protections
2019-02-26 23:36 [PATCH 0/3] x86/asm: More pinning Kees Cook
` (2 preceding siblings ...)
2019-02-26 23:36 ` [PATCH 3/3] lkdtm: Check for SMEP clearing protections Kees Cook
@ 2019-02-26 23:45 ` Kees Cook
2019-02-27 8:20 ` [PATCH 0/3] x86/asm: More pinning Greg KH
4 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2019-02-26 23:45 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Peter Zijlstra, Jann Horn, Sean Christopherson,
Dominik Brodowski, Kernel Hardening, LKML
This adds an x86-specific test for pinned cr4 bits. A successful test
will validate pinning and check the ROP-style call-middle-of-function
defense, if needed. For example, in the case of native_write_cr4()
looking like this:
ffffffff8171bce0 <native_write_cr4>:
ffffffff8171bce0: 48 8b 35 79 46 f2 00 mov 0xf24679(%rip),%rsi
ffffffff8171bce7: 48 09 f7 or %rsi,%rdi
ffffffff8171bcea: 0f 22 e7 mov %rdi,%cr4
...
ffffffff8171bd5a: c3 retq
The UNSET_SMEP test will jump to ffffffff8171bcea (the mov to cr4)
instead of ffffffff8171bce0 (native_write_cr4() entry) to simulate a
direct-call bypass attempt.
Expected successful results:
# echo UNSET_SMEP > /sys/kernel/debug/provoke-crash/DIRECT
# dmesg
[ 79.594433] lkdtm: Performing direct entry UNSET_SMEP
[ 79.596459] lkdtm: trying to clear SMEP normally
[ 79.598406] lkdtm: ok: SMEP did not get cleared
[ 79.599981] lkdtm: trying to clear SMEP with call gadget
[ 79.601810] ------------[ cut here ]------------
[ 79.603421] Attempt to unpin cr4 bits: 100000; bypass attack?!
...
[ 79.650170] ---[ end trace 2452ca0f6126242e ]---
[ 79.650937] lkdtm: ok: SMEP removal was reverted
Signed-off-by: Kees Cook <keescook@chromium.org>
---
v2: fix 64 vs 256 loop-end comparison, use a #define instead
---
drivers/misc/lkdtm/bugs.c | 61 ++++++++++++++++++++++++++++++++++++++
drivers/misc/lkdtm/core.c | 1 +
drivers/misc/lkdtm/lkdtm.h | 1 +
3 files changed, 63 insertions(+)
diff --git a/drivers/misc/lkdtm/bugs.c b/drivers/misc/lkdtm/bugs.c
index 7eebbdfbcacd..6176384b4f85 100644
--- a/drivers/misc/lkdtm/bugs.c
+++ b/drivers/misc/lkdtm/bugs.c
@@ -255,3 +255,64 @@ void lkdtm_STACK_GUARD_PAGE_TRAILING(void)
pr_err("FAIL: accessed page after stack!\n");
}
+
+void lkdtm_UNSET_SMEP(void)
+{
+#ifdef CONFIG_X86_64
+#define MOV_CR4_DEPTH 64
+ void (*direct_write_cr4)(unsigned long val);
+ unsigned char *insn;
+ unsigned long cr4;
+ int i;
+
+ cr4 = native_read_cr4();
+
+ if ((cr4 & X86_CR4_SMEP) != X86_CR4_SMEP) {
+ pr_err("FAIL: SMEP not in use\n");
+ return;
+ }
+ cr4 &= ~(X86_CR4_SMEP);
+
+ pr_info("trying to clear SMEP normally\n");
+ native_write_cr4(cr4);
+ if (cr4 == native_read_cr4()) {
+ pr_err("FAIL: pinning SMEP failed!\n");
+ cr4 |= X86_CR4_SMEP;
+ pr_info("restoring SMEP\n");
+ native_write_cr4(cr4);
+ return;
+ }
+ pr_info("ok: SMEP did not get cleared\n");
+
+ /*
+ * To test the post-write pinning verification we need to call
+ * directly into the the middle of native_write_cr4() where the
+ * cr4 write happens, skipping the pinning. This searches for
+ * the cr4 writing instruction.
+ */
+ insn = (unsigned char *)native_write_cr4;
+ for (i = 0; i < MOV_CR4_DEPTH; i++) {
+ /* mov %rdi, %cr4 */
+ if (insn[i] == 0x0f && insn[i+1] == 0x22 && insn[i+2] == 0xe7)
+ break;
+ }
+ if (i >= MOV_CR4_DEPTH) {
+ pr_info("ok: cannot locate cr4 writing call gadget\n");
+ return;
+ }
+ direct_write_cr4 = (void *)(insn + i);
+
+ pr_info("trying to clear SMEP with call gadget\n");
+ direct_write_cr4(cr4);
+ if (native_read_cr4() & X86_CR4_SMEP) {
+ pr_info("ok: SMEP removal was reverted\n");
+ } else {
+ pr_err("FAIL: cleared SMEP not detected!\n");
+ cr4 |= X86_CR4_SMEP;
+ pr_info("restoring SMEP\n");
+ native_write_cr4(cr4);
+ }
+#else
+ pr_err("FAIL: this test is x86_64-only\n");
+#endif
+}
diff --git a/drivers/misc/lkdtm/core.c b/drivers/misc/lkdtm/core.c
index 2837dc77478e..fd668776414b 100644
--- a/drivers/misc/lkdtm/core.c
+++ b/drivers/misc/lkdtm/core.c
@@ -132,6 +132,7 @@ static const struct crashtype crashtypes[] = {
CRASHTYPE(CORRUPT_LIST_ADD),
CRASHTYPE(CORRUPT_LIST_DEL),
CRASHTYPE(CORRUPT_USER_DS),
+ CRASHTYPE(UNSET_SMEP),
CRASHTYPE(CORRUPT_STACK),
CRASHTYPE(CORRUPT_STACK_STRONG),
CRASHTYPE(STACK_GUARD_PAGE_LEADING),
diff --git a/drivers/misc/lkdtm/lkdtm.h b/drivers/misc/lkdtm/lkdtm.h
index 3c6fd327e166..9c78d7e21c13 100644
--- a/drivers/misc/lkdtm/lkdtm.h
+++ b/drivers/misc/lkdtm/lkdtm.h
@@ -26,6 +26,7 @@ void lkdtm_CORRUPT_LIST_DEL(void);
void lkdtm_CORRUPT_USER_DS(void);
void lkdtm_STACK_GUARD_PAGE_LEADING(void);
void lkdtm_STACK_GUARD_PAGE_TRAILING(void);
+void lkdtm_UNSET_SMEP(void);
/* lkdtm_heap.c */
void lkdtm_OVERWRITE_ALLOCATION(void);
--
2.17.1
--
Kees Cook
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 0/3] x86/asm: More pinning
2019-02-26 23:36 [PATCH 0/3] x86/asm: More pinning Kees Cook
` (3 preceding siblings ...)
2019-02-26 23:45 ` [PATCH v2] " Kees Cook
@ 2019-02-27 8:20 ` Greg KH
4 siblings, 0 replies; 9+ messages in thread
From: Greg KH @ 2019-02-27 8:20 UTC (permalink / raw)
To: Kees Cook
Cc: Thomas Gleixner, Peter Zijlstra, Jann Horn, Sean Christopherson,
Dominik Brodowski, Kernel Hardening, linux-kernel
On Tue, Feb 26, 2019 at 03:36:44PM -0800, Kees Cook wrote:
> This adds CR0 pinning (for WP), and cleans up the CR4 pin to avoid
> taking an exception from WARN before fixing up the desired pin.
> Additionally adds lkdtm test (which depends on the CR4 patch, otherwise
> I'd send it via Greg's tree).
No objection from me to route around me, please do!
greg k-h
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] x86/asm: Pin sensitive CR0 bits
2019-02-26 23:36 ` [PATCH 1/3] x86/asm: Pin sensitive CR0 bits Kees Cook
@ 2019-02-27 10:44 ` Solar Designer
2019-02-27 19:45 ` Kees Cook
0 siblings, 1 reply; 9+ messages in thread
From: Solar Designer @ 2019-02-27 10:44 UTC (permalink / raw)
To: Kees Cook
Cc: Thomas Gleixner, Peter Zijlstra, Jann Horn, Sean Christopherson,
Dominik Brodowski, Kernel Hardening, linux-kernel
On Tue, Feb 26, 2019 at 03:36:45PM -0800, Kees Cook wrote:
> static inline void native_write_cr0(unsigned long val)
> {
> - asm volatile("mov %0,%%cr0": : "r" (val), "m" (__force_order));
> + bool warn = false;
> +
> +again:
> + val |= X86_CR0_WP;
> + /*
> + * In order to have the compiler not optimize away the check
> + * in the WARN_ONCE(), mark "val" as being also an output ("+r")
This comment is now slightly out of date: the check is no longer "in the
WARN_ONCE()". Ditto about the comment for CR4.
> + * by this asm() block so it will perform an explicit check, as
> + * if it were "volatile".
> + */
> + asm volatile("mov %0,%%cr0": "+r" (val) : "m" (__force_order) : );
> + /*
> + * If the MOV above was used directly as a ROP gadget we can
> + * notice the lack of pinned bits in "val" and start the function
> + * from the beginning to gain the WP bit for sure. And do it
> + * without first taking the exception for a WARN().
> + */
> + if ((val & X86_CR0_WP) != X86_CR0_WP) {
> + warn = true;
> + goto again;
> + }
> + WARN_ONCE(warn, "Attempt to unpin X86_CR0_WP, cr0 bypass attack?!\n");
> }
Alexander
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/3] x86/asm: Pin sensitive CR0 bits
2019-02-27 10:44 ` Solar Designer
@ 2019-02-27 19:45 ` Kees Cook
0 siblings, 0 replies; 9+ messages in thread
From: Kees Cook @ 2019-02-27 19:45 UTC (permalink / raw)
To: Solar Designer
Cc: Thomas Gleixner, Peter Zijlstra, Jann Horn, Sean Christopherson,
Dominik Brodowski, Kernel Hardening, LKML
On Wed, Feb 27, 2019 at 2:44 AM Solar Designer <solar@openwall.com> wrote:
>
> On Tue, Feb 26, 2019 at 03:36:45PM -0800, Kees Cook wrote:
> > static inline void native_write_cr0(unsigned long val)
> > {
> > - asm volatile("mov %0,%%cr0": : "r" (val), "m" (__force_order));
> > + bool warn = false;
> > +
> > +again:
> > + val |= X86_CR0_WP;
> > + /*
> > + * In order to have the compiler not optimize away the check
> > + * in the WARN_ONCE(), mark "val" as being also an output ("+r")
>
> This comment is now slightly out of date: the check is no longer "in the
> WARN_ONCE()". Ditto about the comment for CR4.
Ah yes, good point. I will adjust and send a v2 series.
>
> > + * by this asm() block so it will perform an explicit check, as
> > + * if it were "volatile".
> > + */
> > + asm volatile("mov %0,%%cr0": "+r" (val) : "m" (__force_order) : );
> > + /*
> > + * If the MOV above was used directly as a ROP gadget we can
> > + * notice the lack of pinned bits in "val" and start the function
> > + * from the beginning to gain the WP bit for sure. And do it
> > + * without first taking the exception for a WARN().
> > + */
> > + if ((val & X86_CR0_WP) != X86_CR0_WP) {
> > + warn = true;
> > + goto again;
> > + }
> > + WARN_ONCE(warn, "Attempt to unpin X86_CR0_WP, cr0 bypass attack?!\n");
> > }
>
> Alexander
--
Kees Cook
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2019-02-27 19:45 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-26 23:36 [PATCH 0/3] x86/asm: More pinning Kees Cook
2019-02-26 23:36 ` [PATCH 1/3] x86/asm: Pin sensitive CR0 bits Kees Cook
2019-02-27 10:44 ` Solar Designer
2019-02-27 19:45 ` Kees Cook
2019-02-26 23:36 ` [PATCH 2/3] x86/asm: Avoid taking an exception before cr4 restore Kees Cook
2019-02-26 23:36 ` [PATCH 3/3] lkdtm: Check for SMEP clearing protections Kees Cook
2019-02-26 23:40 ` Kees Cook
2019-02-26 23:45 ` [PATCH v2] " Kees Cook
2019-02-27 8:20 ` [PATCH 0/3] x86/asm: More pinning Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).