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
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 <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
---
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
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 <[email protected]>
---
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
On Tue, Feb 26, 2019 at 3:37 PM Kees Cook <[email protected]> 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
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 <[email protected]>
---
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
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
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
On Wed, Feb 27, 2019 at 2:44 AM Solar Designer <[email protected]> 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