* Re: [RFC PATCH v3 19/24] x86/cet/shstk: Introduce WRUSS instruction
From: Yu-cheng Yu @ 2018-08-31 21:49 UTC (permalink / raw)
To: Andy Lutomirski, Jann Horn
Cc: the arch/x86 maintainers, H . Peter Anvin, Thomas Gleixner,
Ingo Molnar, kernel list, linux-doc, Linux-MM, linux-arch,
Linux API, Arnd Bergmann, Balbir Singh, Cyrill Gorcunov,
Dave Hansen, Florian Weimer, H. J. Lu, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek
In-Reply-To: <1535646146.26689.11.camel@intel.com>
On Thu, 2018-08-30 at 09:22 -0700, Yu-cheng Yu wrote:
> On Thu, 2018-08-30 at 08:55 -0700, Andy Lutomirski wrote:
> >
> > On Thu, Aug 30, 2018 at 8:39 AM, Jann Horn <jannh@google.com>
> > wrote:
> > >
> > >
> > > On Thu, Aug 30, 2018 at 4:44 PM Yu-cheng Yu <yu-cheng.yu@intel.c
> > > om
> > > >
> > > > wrote:
> > > >
> > > >
> > > > WRUSS is a new kernel-mode instruction but writes directly
> > > > to user shadow stack memory. This is used to construct
> > > > a return address on the shadow stack for the signal
> > > > handler.
> > > >
> > > > This instruction can fault if the user shadow stack is
> > > > invalid shadow stack memory. In that case, the kernel does
> > > > fixup.
> > > >
> > > > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > [...]
> > > >
> > > >
> > > > +static inline int write_user_shstk_64(unsigned long addr,
> > > > unsigned long val)
> > > > +{
> > > > + int err = 0;
> > > > +
> > > > + asm volatile("1: wrussq %1, (%0)\n"
> > > > + "2:\n"
> > > > + _ASM_EXTABLE_HANDLE(1b, 2b,
> > > > ex_handler_wruss)
> > > > + :
> > > > + : "r" (addr), "r" (val));
> > > > +
> > > > + return err;
> > > > +}
> > > What's up with "err"? You set it to zero, and then you return
> > > it,
> > > but
> > > nothing can ever set it to non-zero, right?
> > >
> > > >
> > > >
> > > > +__visible bool ex_handler_wruss(const struct
> > > > exception_table_entry *fixup,
> > > > + struct pt_regs *regs, int
> > > > trapnr)
> > > > +{
> > > > + regs->ip = ex_fixup_addr(fixup);
> > > > + regs->ax = -1;
> > > > + return true;
> > > > +}
> > > And here you just write into regs->ax, but your "asm volatile"
> > > doesn't
> > > reserve that register. This looks wrong to me.
> > >
> > > I think you probably want to add something like an explicit
> > > `"+&a"(err)` output to the asm statements.
> > We require asm goto support these days. How about using
> > that? You
> > won't even need a special exception handler.
Maybe something like this? It looks simple now.
static inline int write_user_shstk_64(unsigned long addr, unsigned
long val)
{
asm_volatile_goto("wrussq %1, (%0)\n"
"jmp %l[ok]\n"
".section .fixup,\"ax\"n"
"jmp %l[fail]\n"
".previous\n"
:: "r" (addr), "r" (val)
:: ok, fail);
ok:
return 0;
fail:
return -1;
}
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Dave Hansen @ 2018-08-31 17:52 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Jann Horn, Yu-cheng Yu, the arch/x86 maintainers, H . Peter Anvin,
Thomas Gleixner, Ingo Molnar, kernel list, linux-doc, Linux-MM,
linux-arch, Linux API, Arnd Bergmann, Balbir Singh,
Cyrill Gorcunov, Florian Weimer, H. J. Lu, Jonathan Corbet,
Kees Cook, Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel
In-Reply-To: <CALCETrUE6mY-+YCaJjGJuEqE_OBQc=QUR1XMnPW9VwTb8=HK4w@mail.gmail.com>
On 08/31/2018 10:46 AM, Andy Lutomirski wrote:
> On Thu, Aug 30, 2018 at 11:55 AM, Dave Hansen
>> That little hunk will definitely need to get updated with something like:
>>
>> On processors enumerating support for CET, the processor will on
>> set the dirty flag on paging structure entries in which the W
>> flag is 1.
>
> Can we get something much stronger, perhaps? Like this:
>
> On processors enumerating support for CET, the processor will write to
> the accessed and/or dirty flags atomically, as if using the LOCK
> CMPXCHG instruction. The memory access, any cached entries in any
> paging-structure caches, and the values in the paging-structure entry
> before and after writing the A and/or D bits will all be consistent.
There's some talk of this already in: 8.1.2.1 Automatic Locking:
> When updating page-directory and page-table entries — When updating
> page-directory and page-table entries, the processor uses locked
> cycles to set the accessed and dirty flag in the page-directory and
> page-table entries.
As for the A/D consistency, I'll see if I can share that before it hits
the SDM for real and see if it's sufficient for everybody.
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Andy Lutomirski @ 2018-08-31 17:46 UTC (permalink / raw)
To: Dave Hansen
Cc: Jann Horn, Yu-cheng Yu, the arch/x86 maintainers, H . Peter Anvin,
Thomas Gleixner, Ingo Molnar, kernel list, linux-doc, Linux-MM,
linux-arch, Linux API, Arnd Bergmann, Balbir Singh,
Cyrill Gorcunov, Florian Weimer, H. J. Lu, Jonathan Corbet,
Kees Cook, Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel
In-Reply-To: <e164a320-25a4-a9fc-3256-901b778468f3@linux.intel.com>
On Thu, Aug 30, 2018 at 11:55 AM, Dave Hansen
<dave.hansen@linux.intel.com> wrote:
> On 08/30/2018 10:34 AM, Andy Lutomirski wrote:
>>> But, to keep B's TLB from picking up the entry, I think we can just make
>>> it !Present for a moment. No TLB can cache it, and I believe the same
>>> "don't set Dirty on a !Writable entry" logic also holds for !Present
>>> (modulo a weird erratum or two).
>> Can we get documentation? Pretty please?
>
> The accessed bit description in the SDM looks pretty good to me today:
>
>> Whenever the processor uses a paging-structure entry as part of
>> linear-address translation, it sets the accessed flag in that entry
>> (if it is not already set).
> If it's !Present, it can't used as part of a translation so can't be
> set. I think that covers the thing I was unsure about.
>
> But, Dirty is a bit, er, muddier, but mostly because it only gets set on
> leaf entries:
>
>> Whenever there is a write to a linear address, the processor sets the
>> dirty flag (if it is not already set) in the paging- structure entry
>> that identifies the final physical address for the linear address
>> (either a PTE or a paging-structure entry in which the PS flag is
>> 1).
>
> That little hunk will definitely need to get updated with something like:
>
> On processors enumerating support for CET, the processor will on
> set the dirty flag on paging structure entries in which the W
> flag is 1.
Can we get something much stronger, perhaps? Like this:
On processors enumerating support for CET, the processor will write to
the accessed and/or dirty flags atomically, as if using the LOCK
CMPXCHG instruction. The memory access, any cached entries in any
paging-structure caches, and the values in the paging-structure entry
before and after writing the A and/or D bits will all be consistent.
I'm sure this could be worded better. The point is that the CPU
should, atomically, load the PTE, check if it allows the access, set A
and/or D appropriately, write the new value to the TLB, and use that
value for the access. This is clearly a little bit slower than what
old CPUs could do when writing to an already-in-TLB writable non-dirty
entry, but new CPUs are going to have to atomically check the W bit.
(I assume that even old CPUs will *atomically* set the D bit as if by
LOCK BTS, but this is all very vague in the SDM IIRC.)
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Dave Hansen @ 2018-08-31 16:29 UTC (permalink / raw)
To: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Cyrill Gorcunov,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra <pet>
In-Reply-To: <20180830143904.3168-13-yu-cheng.yu@intel.com>
On 08/30/2018 07:38 AM, Yu-cheng Yu wrote:
> + * Some processors can start a write, but ending up seeing
> + * a read-only PTE by the time they get to the Dirty bit.
> + * In this case, they will set the Dirty bit, leaving a
> + * read-only, Dirty PTE which looks like a Shadow Stack PTE.
> + *
> + * However, this behavior has been improved and will not occur
> + * on processors supporting Shadow Stacks. Without this
> + * guarantee, a transition to a non-present PTE and flush the
> + * TLB would be needed.
Did we publicly document this behavior anywhere? I can't seem to find it.
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Peter Zijlstra @ 2018-08-31 16:29 UTC (permalink / raw)
To: Dave Hansen
Cc: Yu-cheng Yu, Jann Horn, the arch/x86 maintainers, H . Peter Anvin,
Thomas Gleixner, Ingo Molnar, kernel list, linux-doc, Linux-MM,
linux-arch, Linux API, Arnd Bergmann, Andy Lutomirski,
Balbir Singh, Cyrill Gorcunov, Florian Weimer, hjl.tools,
Jonathan Corbet, keescook, Mike Kravetz, Nadav Amit,
Oleg Nesterov
In-Reply-To: <6d31bd30-6d5b-bbde-1e97-1d8255eff76d@linux.intel.com>
On Fri, Aug 31, 2018 at 08:58:39AM -0700, Dave Hansen wrote:
> On 08/31/2018 08:48 AM, Yu-cheng Yu wrote:
> > To trigger a race in ptep_set_wrprotect(), we need to fork from one of
> > three pthread siblings.
> >
> > Or do we measure only how much this affects fork?
> > If there is no racing, the effect should be minimal.
>
> We don't need a race.
>
> I think the cmpxchg will be slower, even without a race, than the code
> that was there before. The cmpxchg is a simple, straightforward
> solution, but we're putting it in place of a plain memory write, which
> is suboptimal.
Note quite, the clear_bit() is LOCK prefixed.
^ permalink raw reply
* Re: [RFC PATCH v3 06/24] x86/cet: Control protection exception handler
From: Yu-cheng Yu @ 2018-08-31 16:20 UTC (permalink / raw)
To: Jann Horn
Cc: the arch/x86 maintainers, H . Peter Anvin, Thomas Gleixner,
Ingo Molnar, kernel list, linux-doc, Linux-MM, linux-arch,
Linux API, Arnd Bergmann, Andy Lutomirski, Balbir Singh,
Cyrill Gorcunov, Dave Hansen, Florian Weimer, hjl.tools,
Jonathan Corbet, keescook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek <pave>
In-Reply-To: <CAG48ez0jvsDw189=YoCCa8tmJUENeUd_ypcP5bYJ+eLMPCYCOQ@mail.gmail.com>
On Fri, 2018-08-31 at 17:01 +0200, Jann Horn wrote:
> Is there a reason why all the code in this patch isn't #ifdef'ed
> away
> on builds that don't support CET? It looks like the CET handler is
> hooked up to the IDT conditionally, but the handler code is always
> built?
Yes, in idt.c, it should have been:
#ifdef CONFIG_X86_64
INTG(X86_TRAP_CP, control_protection),
#endif
I will fix it.
> > +dotraplinkage void
> > +do_control_protection(struct pt_regs *regs, long error_code)
> > +{
> > + struct task_struct *tsk;
> > +
> > + RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't
> > wake RCU");
> > + if (notify_die(DIE_TRAP, "control protection fault", regs,
> > + error_code, X86_TRAP_CP, SIGSEGV) ==
> > NOTIFY_STOP)
> > + return;
> > + cond_local_irq_enable(regs);
> > +
> > + if (!user_mode(regs))
> > + die("kernel control protection fault", regs,
> > error_code);
> > +
> > + if (!static_cpu_has(X86_FEATURE_SHSTK) &&
> > + !static_cpu_has(X86_FEATURE_IBT))
> > + WARN_ONCE(1, "CET is disabled but got control "
> > + "protection fault\n");
> > +
> > + tsk = current;
> > + tsk->thread.error_code = error_code;
> > + tsk->thread.trap_nr = X86_TRAP_CP;
> > +
> > + if (show_unhandled_signals && unhandled_signal(tsk,
> > SIGSEGV) &&
> > + printk_ratelimit()) {
> > + unsigned int max_err;
> > +
> > + max_err = ARRAY_SIZE(control_protection_err) - 1;
> > + if ((error_code < 0) || (error_code > max_err))
> > + error_code = 0;
> > + pr_info("%s[%d] control protection ip:%lx sp:%lx
> > error:%lx(%s)",
> > + tsk->comm, task_pid_nr(tsk),
> > + regs->ip, regs->sp, error_code,
> > + control_protection_err[error_code]);
> > + print_vma_addr(" in ", regs->ip);
> Shouldn't this be using KERN_CONT, like other callers of
> print_vma_addr(), to get the desired output?
I will change it.
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Dave Hansen @ 2018-08-31 15:58 UTC (permalink / raw)
To: Yu-cheng Yu, Peter Zijlstra, Jann Horn
Cc: the arch/x86 maintainers, H . Peter Anvin, Thomas Gleixner,
Ingo Molnar, kernel list, linux-doc, Linux-MM, linux-arch,
Linux API, Arnd Bergmann, Andy Lutomirski, Balbir Singh,
Cyrill Gorcunov, Florian Weimer, hjl.tools, Jonathan Corbet,
keescook, Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
ravi.v.shankar, vedvya
In-Reply-To: <1535730524.501.13.camel@intel.com>
On 08/31/2018 08:48 AM, Yu-cheng Yu wrote:
> To trigger a race in ptep_set_wrprotect(), we need to fork from one of
> three pthread siblings.
>
> Or do we measure only how much this affects fork?
> If there is no racing, the effect should be minimal.
We don't need a race.
I think the cmpxchg will be slower, even without a race, than the code
that was there before. The cmpxchg is a simple, straightforward
solution, but we're putting it in place of a plain memory write, which
is suboptimal.
But, before I nitpick the performance, I wanted to see if we could even
detect a delta.
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Yu-cheng Yu @ 2018-08-31 15:48 UTC (permalink / raw)
To: Dave Hansen, Peter Zijlstra, Jann Horn
Cc: the arch/x86 maintainers, H . Peter Anvin, Thomas Gleixner,
Ingo Molnar, kernel list, linux-doc, Linux-MM, linux-arch,
Linux API, Arnd Bergmann, Andy Lutomirski, Balbir Singh,
Cyrill Gorcunov, Florian Weimer, hjl.tools, Jonathan Corbet,
keescook, Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
ravi.v.shankar, vedvya
In-Reply-To: <f5a36e32-7c5f-91fe-9e98-fb44867fda11@linux.intel.com>
On Fri, 2018-08-31 at 07:47 -0700, Dave Hansen wrote:
> On 08/31/2018 07:33 AM, Yu-cheng Yu wrote:
> >
> > Please use the form:
> >
> > pte_t new_pte, pte = READ_ONCE(*ptep);
> > do {
> > new_pte = /* ... */;
> > } while (!try_cmpxchg(ptep, &pte, new_pte);
> It's probably also worth doing some testing to see if you can detect
> the
> cost of the cmpxchg. It's definitely more than the old code.
>
> A loop that does mprotect(PROT_READ) followed by
> mprotect(PROT_READ|PROT_WRITE) should do it.
I created the test,
https://github.com/yyu168/cet-smoke-test/blob/quick/quick/mprotect_ben
ch.c
then realized this won't work.
To trigger a race in ptep_set_wrprotect(), we need to fork from one of
three pthread siblings.
Or do we measure only how much this affects fork?
If there is no racing, the effect should be minimal.
Yu-cheng
^ permalink raw reply
* Re: [RFC PATCH v3 06/24] x86/cet: Control protection exception handler
From: Jann Horn @ 2018-08-31 15:01 UTC (permalink / raw)
To: yu-cheng.yu
Cc: the arch/x86 maintainers, H . Peter Anvin, Thomas Gleixner,
Ingo Molnar, kernel list, linux-doc, Linux-MM, linux-arch,
Linux API, Arnd Bergmann, Andy Lutomirski, Balbir Singh,
Cyrill Gorcunov, Dave Hansen, Florian Weimer, hjl.tools,
Jonathan Corbet, keescook, Mike Kravetz, Nadav Amit,
Oleg Nesterov
In-Reply-To: <20180830143904.3168-7-yu-cheng.yu@intel.com>
On Thu, Aug 30, 2018 at 4:43 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> A control protection exception is triggered when a control flow transfer
> attempt violated shadow stack or indirect branch tracking constraints.
> For example, the return address for a RET instruction differs from the
> safe copy on the shadow stack; or a JMP instruction arrives at a non-
> ENDBR instruction.
>
> The control protection exception handler works in a similar way as the
> general protection fault handler.
Is there a reason why all the code in this patch isn't #ifdef'ed away
on builds that don't support CET? It looks like the CET handler is
hooked up to the IDT conditionally, but the handler code is always
built?
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
> arch/x86/entry/entry_64.S | 2 +-
> arch/x86/include/asm/traps.h | 3 ++
> arch/x86/kernel/idt.c | 4 +++
> arch/x86/kernel/traps.c | 58 ++++++++++++++++++++++++++++++++++++
> 4 files changed, 66 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/entry/entry_64.S b/arch/x86/entry/entry_64.S
> index 957dfb693ecc..5f4914e988df 100644
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -1000,7 +1000,7 @@ idtentry spurious_interrupt_bug do_spurious_interrupt_bug has_error_code=0
> idtentry coprocessor_error do_coprocessor_error has_error_code=0
> idtentry alignment_check do_alignment_check has_error_code=1
> idtentry simd_coprocessor_error do_simd_coprocessor_error has_error_code=0
> -
> +idtentry control_protection do_control_protection has_error_code=1
>
> /*
> * Reload gs selector with exception handling
> diff --git a/arch/x86/include/asm/traps.h b/arch/x86/include/asm/traps.h
> index 3de69330e6c5..5196050ff3d5 100644
> --- a/arch/x86/include/asm/traps.h
> +++ b/arch/x86/include/asm/traps.h
> @@ -26,6 +26,7 @@ asmlinkage void invalid_TSS(void);
> asmlinkage void segment_not_present(void);
> asmlinkage void stack_segment(void);
> asmlinkage void general_protection(void);
> +asmlinkage void control_protection(void);
> asmlinkage void page_fault(void);
> asmlinkage void async_page_fault(void);
> asmlinkage void spurious_interrupt_bug(void);
> @@ -77,6 +78,7 @@ dotraplinkage void do_stack_segment(struct pt_regs *, long);
> dotraplinkage void do_double_fault(struct pt_regs *, long);
> #endif
> dotraplinkage void do_general_protection(struct pt_regs *, long);
> +dotraplinkage void do_control_protection(struct pt_regs *, long);
> dotraplinkage void do_page_fault(struct pt_regs *, unsigned long);
> dotraplinkage void do_spurious_interrupt_bug(struct pt_regs *, long);
> dotraplinkage void do_coprocessor_error(struct pt_regs *, long);
> @@ -142,6 +144,7 @@ enum {
> X86_TRAP_AC, /* 17, Alignment Check */
> X86_TRAP_MC, /* 18, Machine Check */
> X86_TRAP_XF, /* 19, SIMD Floating-Point Exception */
> + X86_TRAP_CP = 21, /* 21 Control Protection Fault */
> X86_TRAP_IRET = 32, /* 32, IRET Exception */
> };
>
> diff --git a/arch/x86/kernel/idt.c b/arch/x86/kernel/idt.c
> index 01adea278a71..2d02fdd599a2 100644
> --- a/arch/x86/kernel/idt.c
> +++ b/arch/x86/kernel/idt.c
> @@ -104,6 +104,10 @@ static const __initconst struct idt_data def_idts[] = {
> #elif defined(CONFIG_X86_32)
> SYSG(IA32_SYSCALL_VECTOR, entry_INT80_32),
> #endif
> +
> +#ifdef CONFIG_X86_INTEL_CET
> + INTG(X86_TRAP_CP, control_protection),
> +#endif
> };
>
> /*
> diff --git a/arch/x86/kernel/traps.c b/arch/x86/kernel/traps.c
> index e6db475164ed..21a713b96148 100644
> --- a/arch/x86/kernel/traps.c
> +++ b/arch/x86/kernel/traps.c
> @@ -578,6 +578,64 @@ do_general_protection(struct pt_regs *regs, long error_code)
> }
> NOKPROBE_SYMBOL(do_general_protection);
>
> +static const char *control_protection_err[] =
> +{
> + "unknown",
> + "near-ret",
> + "far-ret/iret",
> + "endbranch",
> + "rstorssp",
> + "setssbsy",
> +};
> +
> +/*
> + * When a control protection exception occurs, send a signal
> + * to the responsible application. Currently, control
> + * protection is only enabled for the user mode. This
> + * exception should not come from the kernel mode.
> + */
> +dotraplinkage void
> +do_control_protection(struct pt_regs *regs, long error_code)
> +{
> + struct task_struct *tsk;
> +
> + RCU_LOCKDEP_WARN(!rcu_is_watching(), "entry code didn't wake RCU");
> + if (notify_die(DIE_TRAP, "control protection fault", regs,
> + error_code, X86_TRAP_CP, SIGSEGV) == NOTIFY_STOP)
> + return;
> + cond_local_irq_enable(regs);
> +
> + if (!user_mode(regs))
> + die("kernel control protection fault", regs, error_code);
> +
> + if (!static_cpu_has(X86_FEATURE_SHSTK) &&
> + !static_cpu_has(X86_FEATURE_IBT))
> + WARN_ONCE(1, "CET is disabled but got control "
> + "protection fault\n");
> +
> + tsk = current;
> + tsk->thread.error_code = error_code;
> + tsk->thread.trap_nr = X86_TRAP_CP;
> +
> + if (show_unhandled_signals && unhandled_signal(tsk, SIGSEGV) &&
> + printk_ratelimit()) {
> + unsigned int max_err;
> +
> + max_err = ARRAY_SIZE(control_protection_err) - 1;
> + if ((error_code < 0) || (error_code > max_err))
> + error_code = 0;
> + pr_info("%s[%d] control protection ip:%lx sp:%lx error:%lx(%s)",
> + tsk->comm, task_pid_nr(tsk),
> + regs->ip, regs->sp, error_code,
> + control_protection_err[error_code]);
> + print_vma_addr(" in ", regs->ip);
Shouldn't this be using KERN_CONT, like other callers of
print_vma_addr(), to get the desired output?
> + pr_cont("\n");
> + }
> +
> + force_sig_info(SIGSEGV, SEND_SIG_PRIV, tsk);
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Dave Hansen @ 2018-08-31 14:47 UTC (permalink / raw)
To: Yu-cheng Yu, Peter Zijlstra, Jann Horn
Cc: the arch/x86 maintainers, H . Peter Anvin, Thomas Gleixner,
Ingo Molnar, kernel list, linux-doc, Linux-MM, linux-arch,
Linux API, Arnd Bergmann, Andy Lutomirski, Balbir Singh,
Cyrill Gorcunov, Florian Weimer, hjl.tools, Jonathan Corbet,
keescook, Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
ravi.v.shankar, vedvya
In-Reply-To: <1535726032.32537.0.camel@intel.com>
On 08/31/2018 07:33 AM, Yu-cheng Yu wrote:
> Please use the form:
>
> pte_t new_pte, pte = READ_ONCE(*ptep);
> do {
> new_pte = /* ... */;
> } while (!try_cmpxchg(ptep, &pte, new_pte);
It's probably also worth doing some testing to see if you can detect the
cost of the cmpxchg. It's definitely more than the old code.
A loop that does mprotect(PROT_READ) followed by
mprotect(PROT_READ|PROT_WRITE) should do it.
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Yu-cheng Yu @ 2018-08-31 14:33 UTC (permalink / raw)
To: Peter Zijlstra, Jann Horn
Cc: Dave Hansen, the arch/x86 maintainers, H . Peter Anvin,
Thomas Gleixner, Ingo Molnar, kernel list, linux-doc, Linux-MM,
linux-arch, Linux API, Arnd Bergmann, Andy Lutomirski,
Balbir Singh, Cyrill Gorcunov, Florian Weimer, hjl.tools,
Jonathan Corbet, keescook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek <pave>
In-Reply-To: <20180831095300.GF24124@hirez.programming.kicks-ass.net>
On Fri, 2018-08-31 at 11:53 +0200, Peter Zijlstra wrote:
> On Thu, Aug 30, 2018 at 11:47:16PM +0200, Jann Horn wrote:
> >
> > do {
> > pte = pte_wrprotect(pte);
> > /* note: relies on _PAGE_DIRTY_HW < _PAGE_DIRTY_SW
> > */
> > /* dirty direct bit-twiddling; you can probably
> > write
> > this in a nicer way */
> > pte.pte |= (pte.pte & _PAGE_DIRTY_HW) >>
> > _PAGE_BIT_DIRTY_HW << _PAGE_BIT_DIRTY_SW;
> > pte.pte &= ~_PAGE_DIRTY_HW;
> > pte = cmpxchg(ptep, pte, new_pte);
> > } while (pte != new_pte);
> Please use the form:
>
> pte_t new_pte, pte = READ_ONCE(*ptep);
> do {
> new_pte = /* ... */;
> } while (!try_cmpxchg(ptep, &pte, new_pte);
>
> Also, this will fail to build on i386-PAE, but I suspect this code
> will
> be under some CONFIG option specific to x86_64 anyway.
Thanks! I will work on it.
Yu-cheng
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Peter Zijlstra @ 2018-08-31 9:53 UTC (permalink / raw)
To: Jann Horn
Cc: yu-cheng.yu, Dave Hansen, the arch/x86 maintainers,
H . Peter Anvin, Thomas Gleixner, Ingo Molnar, kernel list,
linux-doc, Linux-MM, linux-arch, Linux API, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Cyrill Gorcunov, Florian Weimer,
hjl.tools, Jonathan Corbet, keescook, Mike Kravetz, Nadav Amit,
Oleg Nesterov
In-Reply-To: <CAG48ez3S3+DzAyo_SnoUW1GO0Cpd_x0A83MOx2p_MkogoAatLQ@mail.gmail.com>
On Thu, Aug 30, 2018 at 11:47:16PM +0200, Jann Horn wrote:
> do {
> pte = pte_wrprotect(pte);
> /* note: relies on _PAGE_DIRTY_HW < _PAGE_DIRTY_SW */
> /* dirty direct bit-twiddling; you can probably write
> this in a nicer way */
> pte.pte |= (pte.pte & _PAGE_DIRTY_HW) >>
> _PAGE_BIT_DIRTY_HW << _PAGE_BIT_DIRTY_SW;
> pte.pte &= ~_PAGE_DIRTY_HW;
> pte = cmpxchg(ptep, pte, new_pte);
> } while (pte != new_pte);
Please use the form:
pte_t new_pte, pte = READ_ONCE(*ptep);
do {
new_pte = /* ... */;
} while (!try_cmpxchg(ptep, &pte, new_pte);
Also, this will fail to build on i386-PAE, but I suspect this code will
be under some CONFIG option specific to x86_64 anyway.
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Andy Lutomirski @ 2018-08-31 1:23 UTC (permalink / raw)
To: Jann Horn
Cc: yu-cheng.yu, Dave Hansen, the arch/x86 maintainers,
H . Peter Anvin, Thomas Gleixner, Ingo Molnar, kernel list,
linux-doc, Linux-MM, linux-arch, Linux API, Arnd Bergmann,
Balbir Singh, Cyrill Gorcunov, Florian Weimer, hjl.tools,
Jonathan Corbet, keescook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek
In-Reply-To: <CAG48ez3ixWROuQc6WZze6qPL6q0e_gCnMU4XF11JUWziePsBJg@mail.gmail.com>
> On Aug 30, 2018, at 10:59 AM, Jann Horn <jannh@google.com> wrote:
>
>> On Thu, Aug 30, 2018 at 7:58 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>>
>>> On Thu, 2018-08-30 at 10:33 -0700, Dave Hansen wrote:
>>>> On 08/30/2018 10:26 AM, Yu-cheng Yu wrote:
>>>>
>>>> We don't have the guard page now, but there is a shadow stack
>>>> token
>>>> there, which cannot be used as a return address.
>>> The overall concern is that we could overflow into a page that we
>>> did
>>> not intend. Either another actual shadow stack or something that a
>>> page
>>> that the attacker constructed, like the transient scenario Jann
>>> described.
>>>
>>
>> A task could go beyond the bottom of its shadow stack by doing either
>> 'ret' or 'incssp'. If it is the 'ret' case, the token prevents it.
>> If it is the 'incssp' case, a guard page cannot prevent it entirely,
>> right?
>
> I mean the other direction, on "call".
I still think that shadow stacks should work just like mmap and that mmap should learn to add guard pages for all non-MAP_FIXED allocations.
^ permalink raw reply
* Re: [RFC PATCH v3 05/24] Documentation/x86: Add CET description
From: Yu-cheng Yu @ 2018-08-30 22:49 UTC (permalink / raw)
To: Pavel Machek
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Cyrill Gorcunov, Dave Hansen,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Peter Zijlstra
In-Reply-To: <20180830203948.GB1936@amd>
On Thu, 2018-08-30 at 22:39 +0200, Pavel Machek wrote:
> Hi!
>
> >
> > diff --git a/Documentation/admin-guide/kernel-parameters.txt
> > b/Documentation/admin-guide/kernel-parameters.txt
> > index 9871e649ffef..b090787188b4 100644
> > --- a/Documentation/admin-guide/kernel-parameters.txt
> > +++ b/Documentation/admin-guide/kernel-parameters.txt
> > @@ -2764,6 +2764,12 @@
> > noexec=on: enable non-executable mappings
> > (default)
> > noexec=off: disable non-executable
> > mappings
> >
> > + no_cet_ibt [X86-64] Disable indirect branch
> > tracking for user-mode
> > + applications
> > +
> > + no_cet_shstk [X86-64] Disable shadow stack support
> > for user-mode
> > + applications
> Hmm, not too consistent with "nosmap" below. Would it make sense to
> have cet=on/off/ibt/shstk instead?
>
> >
> > +++ b/Documentation/x86/intel_cet.rst
> > @@ -0,0 +1,252 @@
> > +=========================================
> > +Control Flow Enforcement Technology (CET)
> > +=========================================
> > +
> > +[1] Overview
> > +============
> > +
> > +Control Flow Enforcement Technology (CET) provides protection
> > against
> > +return/jump-oriented programing (ROP) attacks.
> Can you add something like "It attempts to protect process from
> running arbitrary code even after attacker has control of its stack"
> -- for people that don't know what ROP is, and perhaps link to
> wikipedia explaining ROP or something...
>
> >
> > It can be implemented
> > +to protect both the kernel and applications. In the first phase,
> > +only the user-mode protection is implemented for the 64-bit
> > kernel.
> > +Thirty-two bit applications are supported under the compatibility
> 32-bit (for consistency).
>
> Ok, so CET stops execution of malicious code before architectural
> effects are visible, correct? Does it prevent micro-architectural
> effects of the malicious code? (cache content would be one example;
> see Spectre).
>
> >
> > +[3] Application Enabling
> > +========================
> "Enabling CET in applications" ?
>
> >
> > +Signal
> > +------
> > +
> > +The main program and its signal handlers use the same
> > SHSTK. Because
> > +the SHSTK stores only return addresses, we can estimate a large
> > +enough SHSTK to cover the condition that both the program stack
> > and
> > +the sigaltstack run out.
> English? Is it estimate or is it large enough? "a large" -- "a"
> should
> be deleted AFAICT.
>
I will work on these, thanks!
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Jann Horn @ 2018-08-30 21:47 UTC (permalink / raw)
To: yu-cheng.yu
Cc: Dave Hansen, the arch/x86 maintainers, H . Peter Anvin,
Thomas Gleixner, Ingo Molnar, kernel list, linux-doc, Linux-MM,
linux-arch, Linux API, Arnd Bergmann, Andy Lutomirski,
Balbir Singh, Cyrill Gorcunov, Florian Weimer, hjl.tools,
Jonathan Corbet, keescook, Mike Kravetz, Nadav Amit,
Oleg Nesterov
In-Reply-To: <CAG48ez0mkr95_TbLQnDGuGUd6G+eJVLZ-fEjDkwA6dSrm+9tLw@mail.gmail.com>
On Thu, Aug 30, 2018 at 11:01 PM Jann Horn <jannh@google.com> wrote:
>
> On Thu, Aug 30, 2018 at 10:57 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> >
> > On Thu, 2018-08-30 at 22:44 +0200, Jann Horn wrote:
> > > On Thu, Aug 30, 2018 at 10:25 PM Yu-cheng Yu <yu-cheng.yu@intel.com>
> > > wrote:
> > ...
> > > > In the flow you described, if C writes to the overflow page before
> > > > B
> > > > gets in with a 'call', the return address is still correct for
> > > > B. To
> > > > make an attack, C needs to write again before the TLB flush. I
> > > > agree
> > > > that is possible.
> > > >
> > > > Assume we have a guard page, can someone in the short window do
> > > > recursive calls in B, move ssp to the end of the guard page, and
> > > > trigger the same again? He can simply take the incssp route.
> > > I don't understand what you're saying. If the shadow stack is
> > > between
> > > guard pages, you should never be able to move SSP past that area's
> > > guard pages without an appropriate shadow stack token (not even with
> > > INCSSP, since that has a maximum range of PAGE_SIZE/2), and
> > > therefore,
> > > it shouldn't matter whether memory outside that range is incorrectly
> > > marked as shadow stack. Am I missing something?
> >
> > INCSSP has a range of 256, but we can do multiple of that.
> > But I realize the key is not to have the transient SHSTK page at all.
> > The guard page is !pte_write() and even we have flaws in
> > ptep_set_wrprotect(), there will not be any transient SHSTK pages. I
> > will add guard pages to both ends.
> >
> > Still thinking how to fix ptep_set_wrprotect().
>
> cmpxchg loop? Or is that slow?
Something like this:
static inline void ptep_set_wrprotect(struct mm_struct *mm,
unsigned long addr, pte_t *ptep)
{
pte_t pte = READ_ONCE(*ptep), new_pte;
/* ... your comment about not needing a TLB shootdown here ... */
do {
pte = pte_wrprotect(pte);
/* note: relies on _PAGE_DIRTY_HW < _PAGE_DIRTY_SW */
/* dirty direct bit-twiddling; you can probably write
this in a nicer way */
pte.pte |= (pte.pte & _PAGE_DIRTY_HW) >>
_PAGE_BIT_DIRTY_HW << _PAGE_BIT_DIRTY_SW;
pte.pte &= ~_PAGE_DIRTY_HW;
pte = cmpxchg(ptep, pte, new_pte);
} while (pte != new_pte);
}
I think this has the advantage of not generating weird spurious pagefaults.
It's not compatible with Xen PV, but I'm guessing that this whole
feature isn't going to support Xen PV anyway? So you could switch
between two implementations of ptep_set_wrprotect using the pvop
mechanism or so - one for environments that support shadow stacks, one
for all other environments.
Or is there some arcane reason why cmpxchg doesn't work here the way I
think it should?
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Jann Horn @ 2018-08-30 21:01 UTC (permalink / raw)
To: yu-cheng.yu
Cc: Dave Hansen, the arch/x86 maintainers, H . Peter Anvin,
Thomas Gleixner, Ingo Molnar, kernel list, linux-doc, Linux-MM,
linux-arch, Linux API, Arnd Bergmann, Andy Lutomirski,
Balbir Singh, Cyrill Gorcunov, Florian Weimer, hjl.tools,
Jonathan Corbet, keescook, Mike Kravetz, Nadav Amit,
Oleg Nesterov
In-Reply-To: <1535662366.28781.6.camel@intel.com>
On Thu, Aug 30, 2018 at 10:57 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> On Thu, 2018-08-30 at 22:44 +0200, Jann Horn wrote:
> > On Thu, Aug 30, 2018 at 10:25 PM Yu-cheng Yu <yu-cheng.yu@intel.com>
> > wrote:
> ...
> > > In the flow you described, if C writes to the overflow page before
> > > B
> > > gets in with a 'call', the return address is still correct for
> > > B. To
> > > make an attack, C needs to write again before the TLB flush. I
> > > agree
> > > that is possible.
> > >
> > > Assume we have a guard page, can someone in the short window do
> > > recursive calls in B, move ssp to the end of the guard page, and
> > > trigger the same again? He can simply take the incssp route.
> > I don't understand what you're saying. If the shadow stack is
> > between
> > guard pages, you should never be able to move SSP past that area's
> > guard pages without an appropriate shadow stack token (not even with
> > INCSSP, since that has a maximum range of PAGE_SIZE/2), and
> > therefore,
> > it shouldn't matter whether memory outside that range is incorrectly
> > marked as shadow stack. Am I missing something?
>
> INCSSP has a range of 256, but we can do multiple of that.
> But I realize the key is not to have the transient SHSTK page at all.
> The guard page is !pte_write() and even we have flaws in
> ptep_set_wrprotect(), there will not be any transient SHSTK pages. I
> will add guard pages to both ends.
>
> Still thinking how to fix ptep_set_wrprotect().
cmpxchg loop? Or is that slow?
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Yu-cheng Yu @ 2018-08-30 20:52 UTC (permalink / raw)
To: Jann Horn
Cc: Dave Hansen, the arch/x86 maintainers, H . Peter Anvin,
Thomas Gleixner, Ingo Molnar, kernel list, linux-doc, Linux-MM,
linux-arch, Linux API, Arnd Bergmann, Andy Lutomirski,
Balbir Singh, Cyrill Gorcunov, Florian Weimer, hjl.tools,
Jonathan Corbet, keescook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek <pave>
In-Reply-To: <CAG48ez0yOuDhqxB779aO3Kss3gQ3cZTJL1VphDXQm+_M9jFPvQ@mail.gmail.com>
On Thu, 2018-08-30 at 22:44 +0200, Jann Horn wrote:
> On Thu, Aug 30, 2018 at 10:25 PM Yu-cheng Yu <yu-cheng.yu@intel.com>
> wrote:
...
> > In the flow you described, if C writes to the overflow page before
> > B
> > gets in with a 'call', the return address is still correct for
> > B. To
> > make an attack, C needs to write again before the TLB flush. I
> > agree
> > that is possible.
> >
> > Assume we have a guard page, can someone in the short window do
> > recursive calls in B, move ssp to the end of the guard page, and
> > trigger the same again? He can simply take the incssp route.
> I don't understand what you're saying. If the shadow stack is
> between
> guard pages, you should never be able to move SSP past that area's
> guard pages without an appropriate shadow stack token (not even with
> INCSSP, since that has a maximum range of PAGE_SIZE/2), and
> therefore,
> it shouldn't matter whether memory outside that range is incorrectly
> marked as shadow stack. Am I missing something?
INCSSP has a range of 256, but we can do multiple of that.
But I realize the key is not to have the transient SHSTK page at all.
The guard page is !pte_write() and even we have flaws in
ptep_set_wrprotect(), there will not be any transient SHSTK pages. I
will add guard pages to both ends.
Still thinking how to fix ptep_set_wrprotect().
Yu-cheng
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Jann Horn @ 2018-08-30 20:44 UTC (permalink / raw)
To: yu-cheng.yu
Cc: Dave Hansen, the arch/x86 maintainers, H . Peter Anvin,
Thomas Gleixner, Ingo Molnar, kernel list, linux-doc, Linux-MM,
linux-arch, Linux API, Arnd Bergmann, Andy Lutomirski,
Balbir Singh, Cyrill Gorcunov, Florian Weimer, hjl.tools,
Jonathan Corbet, keescook, Mike Kravetz, Nadav Amit,
Oleg Nesterov
In-Reply-To: <1535660494.28258.36.camel@intel.com>
On Thu, Aug 30, 2018 at 10:25 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> On Thu, 2018-08-30 at 19:59 +0200, Jann Horn wrote:
> > On Thu, Aug 30, 2018 at 7:58 PM Yu-cheng Yu <yu-cheng.yu@intel.com>
> > wrote:
> > >
> > >
> > > On Thu, 2018-08-30 at 10:33 -0700, Dave Hansen wrote:
> > > >
> > > > On 08/30/2018 10:26 AM, Yu-cheng Yu wrote:
> > > > >
> > > > >
> > > > > We don't have the guard page now, but there is a shadow stack
> > > > > token
> > > > > there, which cannot be used as a return address.
> > > > The overall concern is that we could overflow into a page that
> > > > we
> > > > did
> > > > not intend. Either another actual shadow stack or something
> > > > that a
> > > > page
> > > > that the attacker constructed, like the transient scenario Jann
> > > > described.
> > > >
> > > A task could go beyond the bottom of its shadow stack by doing
> > > either
> > > 'ret' or 'incssp'. If it is the 'ret' case, the token prevents
> > > it.
> > > If it is the 'incssp' case, a guard page cannot prevent it
> > > entirely,
> > > right?
> > I mean the other direction, on "call".
>
> In the flow you described, if C writes to the overflow page before B
> gets in with a 'call', the return address is still correct for B. To
> make an attack, C needs to write again before the TLB flush. I agree
> that is possible.
>
> Assume we have a guard page, can someone in the short window do
> recursive calls in B, move ssp to the end of the guard page, and
> trigger the same again? He can simply take the incssp route.
I don't understand what you're saying. If the shadow stack is between
guard pages, you should never be able to move SSP past that area's
guard pages without an appropriate shadow stack token (not even with
INCSSP, since that has a maximum range of PAGE_SIZE/2), and therefore,
it shouldn't matter whether memory outside that range is incorrectly
marked as shadow stack. Am I missing something?
^ permalink raw reply
* Re: [RFC PATCH v3 05/24] Documentation/x86: Add CET description
From: Pavel Machek @ 2018-08-30 20:39 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar, linux-kernel,
linux-doc, linux-mm, linux-arch, linux-api, Arnd Bergmann,
Andy Lutomirski, Balbir Singh, Cyrill Gorcunov, Dave Hansen,
Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
Mike Kravetz, Nadav Amit, Oleg Nesterov, Peter Zijlstra
In-Reply-To: <20180830143904.3168-6-yu-cheng.yu@intel.com>
[-- Attachment #1: Type: text/plain, Size: 2419 bytes --]
Hi!
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 9871e649ffef..b090787188b4 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -2764,6 +2764,12 @@
> noexec=on: enable non-executable mappings (default)
> noexec=off: disable non-executable mappings
>
> + no_cet_ibt [X86-64] Disable indirect branch tracking for user-mode
> + applications
> +
> + no_cet_shstk [X86-64] Disable shadow stack support for user-mode
> + applications
Hmm, not too consistent with "nosmap" below. Would it make sense to
have cet=on/off/ibt/shstk instead?
> +++ b/Documentation/x86/intel_cet.rst
> @@ -0,0 +1,252 @@
> +=========================================
> +Control Flow Enforcement Technology (CET)
> +=========================================
> +
> +[1] Overview
> +============
> +
> +Control Flow Enforcement Technology (CET) provides protection against
> +return/jump-oriented programing (ROP) attacks.
Can you add something like "It attempts to protect process from
running arbitrary code even after attacker has control of its stack"
-- for people that don't know what ROP is, and perhaps link to
wikipedia explaining ROP or something...
> It can be implemented
> +to protect both the kernel and applications. In the first phase,
> +only the user-mode protection is implemented for the 64-bit kernel.
> +Thirty-two bit applications are supported under the compatibility
32-bit (for consistency).
Ok, so CET stops execution of malicious code before architectural
effects are visible, correct? Does it prevent micro-architectural
effects of the malicious code? (cache content would be one example;
see Spectre).
> +[3] Application Enabling
> +========================
"Enabling CET in applications" ?
> +Signal
> +------
> +
> +The main program and its signal handlers use the same SHSTK. Because
> +the SHSTK stores only return addresses, we can estimate a large
> +enough SHSTK to cover the condition that both the program stack and
> +the sigaltstack run out.
English? Is it estimate or is it large enough? "a large" -- "a" should
be deleted AFAICT.
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 181 bytes --]
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Yu-cheng Yu @ 2018-08-30 20:23 UTC (permalink / raw)
To: Randy Dunlap, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Cyrill Gorcunov,
Dave Hansen, Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet,
Kees Cook, Mike Kravetz, Nadav Amit, Oleg Nesterov, Pave
In-Reply-To: <9879c17a-24da-d84a-5a7c-30bcbb473914@infradead.org>
On Thu, 2018-08-30 at 12:59 -0700, Randy Dunlap wrote:
> On 08/30/2018 07:38 AM, Yu-cheng Yu wrote:
> >
> > When Shadow Stack is enabled, the read-only and PAGE_DIRTY_HW PTE
> > setting is reserved only for the Shadow Stack. To track dirty of
> > non-Shadow Stack read-only PTEs, we use PAGE_DIRTY_SW.
> >
> > Update ptep_set_wrprotect() and pmdp_set_wrprotect().
> >
> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> > ---
> > arch/x86/include/asm/pgtable.h | 42
> > ++++++++++++++++++++++++++++++++++
> > 1 file changed, 42 insertions(+)
> >
> > diff --git a/arch/x86/include/asm/pgtable.h
> > b/arch/x86/include/asm/pgtable.h
> > index 4d50de77ea96..556ef258eeff 100644
> > --- a/arch/x86/include/asm/pgtable.h
> > +++ b/arch/x86/include/asm/pgtable.h
> > @@ -1203,7 +1203,28 @@ static inline pte_t
> > ptep_get_and_clear_full(struct mm_struct *mm,
> > static inline void ptep_set_wrprotect(struct mm_struct *mm,
> > unsigned long addr, pte_t
> > *ptep)
> > {
> > + pte_t pte;
> > +
> > clear_bit(_PAGE_BIT_RW, (unsigned long *)&ptep->pte);
> > + pte = *ptep;
> > +
> > + /*
> > + * Some processors can start a write, but ending up
> > seeing
> but end up seeing
>
> >
> > + * a read-only PTE by the time they get to the Dirty bit.
> > + * In this case, they will set the Dirty bit, leaving a
> > + * read-only, Dirty PTE which looks like a Shadow Stack
> > PTE.
> > + *
> > + * However, this behavior has been improved and will not
> > occur
> > + * on processors supporting Shadow Stacks. Without this
> > + * guarantee, a transition to a non-present PTE and flush
> > the
> > + * TLB would be needed.
> > + *
> > + * When change a writable PTE to read-only and if the PTE
> > has
> changing
>
> >
> > + * _PAGE_DIRTY_HW set, we move that bit to _PAGE_DIRTY_SW
> > so
> > + * that the PTE is not a valid Shadow Stack PTE.
> > + */
> > + pte = pte_move_flags(pte, _PAGE_DIRTY_HW,
> > _PAGE_DIRTY_SW);
> > + set_pte_at(mm, addr, ptep, pte);
> > }
> >
> > #define flush_tlb_fix_spurious_fault(vma, address) do { } while
> > (0)
> > @@ -1266,7 +1287,28 @@ static inline pud_t
> > pudp_huge_get_and_clear(struct mm_struct *mm,
> > static inline void pmdp_set_wrprotect(struct mm_struct *mm,
> > unsigned long addr, pmd_t
> > *pmdp)
> > {
> > + pmd_t pmd;
> > +
> > clear_bit(_PAGE_BIT_RW, (unsigned long *)pmdp);
> > + pmd = *pmdp;
> > +
> > + /*
> > + * Some processors can start a write, but ending up
> > seeing
> but end up seeing
>
> >
> > + * a read-only PTE by the time they get to the Dirty bit.
> > + * In this case, they will set the Dirty bit, leaving a
> > + * read-only, Dirty PTE which looks like a Shadow Stack
> > PTE.
> > + *
> > + * However, this behavior has been improved and will not
> > occur
> > + * on processors supporting Shadow Stacks. Without this
> > + * guarantee, a transition to a non-present PTE and flush
> > the
> > + * TLB would be needed.
> > + *
> > + * When change a writable PTE to read-only and if the PTE
> > has
> changing
>
> >
> > + * _PAGE_DIRTY_HW set, we move that bit to _PAGE_DIRTY_SW
> > so
> > + * that the PTE is not a valid Shadow Stack PTE.
> > + */
> > + pmd = pmd_move_flags(pmd, _PAGE_DIRTY_HW,
> > _PAGE_DIRTY_SW);
> > + set_pmd_at(mm, addr, pmdp, pmd);
> > }
> >
> > #define pud_write pud_write
> >
>
Thanks, I will fix it!
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Yu-cheng Yu @ 2018-08-30 20:21 UTC (permalink / raw)
To: Jann Horn
Cc: Dave Hansen, the arch/x86 maintainers, H . Peter Anvin,
Thomas Gleixner, Ingo Molnar, kernel list, linux-doc, Linux-MM,
linux-arch, Linux API, Arnd Bergmann, Andy Lutomirski,
Balbir Singh, Cyrill Gorcunov, Florian Weimer, hjl.tools,
Jonathan Corbet, keescook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pavel Machek <pave>
In-Reply-To: <CAG48ez3ixWROuQc6WZze6qPL6q0e_gCnMU4XF11JUWziePsBJg@mail.gmail.com>
On Thu, 2018-08-30 at 19:59 +0200, Jann Horn wrote:
> On Thu, Aug 30, 2018 at 7:58 PM Yu-cheng Yu <yu-cheng.yu@intel.com>
> wrote:
> >
> >
> > On Thu, 2018-08-30 at 10:33 -0700, Dave Hansen wrote:
> > >
> > > On 08/30/2018 10:26 AM, Yu-cheng Yu wrote:
> > > >
> > > >
> > > > We don't have the guard page now, but there is a shadow stack
> > > > token
> > > > there, which cannot be used as a return address.
> > > The overall concern is that we could overflow into a page that
> > > we
> > > did
> > > not intend. Either another actual shadow stack or something
> > > that a
> > > page
> > > that the attacker constructed, like the transient scenario Jann
> > > described.
> > >
> > A task could go beyond the bottom of its shadow stack by doing
> > either
> > 'ret' or 'incssp'. If it is the 'ret' case, the token prevents
> > it.
> > If it is the 'incssp' case, a guard page cannot prevent it
> > entirely,
> > right?
> I mean the other direction, on "call".
In the flow you described, if C writes to the overflow page before B
gets in with a 'call', the return address is still correct for B. To
make an attack, C needs to write again before the TLB flush. I agree
that is possible.
Assume we have a guard page, can someone in the short window do
recursive calls in B, move ssp to the end of the guard page, and
trigger the same again? He can simply take the incssp route.
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Randy Dunlap @ 2018-08-30 19:59 UTC (permalink / raw)
To: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Cyrill Gorcunov,
Dave Hansen, Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet,
Kees Cook, Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel
In-Reply-To: <20180830143904.3168-13-yu-cheng.yu@intel.com>
On 08/30/2018 07:38 AM, Yu-cheng Yu wrote:
> When Shadow Stack is enabled, the read-only and PAGE_DIRTY_HW PTE
> setting is reserved only for the Shadow Stack. To track dirty of
> non-Shadow Stack read-only PTEs, we use PAGE_DIRTY_SW.
>
> Update ptep_set_wrprotect() and pmdp_set_wrprotect().
>
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
> arch/x86/include/asm/pgtable.h | 42 ++++++++++++++++++++++++++++++++++
> 1 file changed, 42 insertions(+)
>
> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> index 4d50de77ea96..556ef258eeff 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -1203,7 +1203,28 @@ static inline pte_t ptep_get_and_clear_full(struct mm_struct *mm,
> static inline void ptep_set_wrprotect(struct mm_struct *mm,
> unsigned long addr, pte_t *ptep)
> {
> + pte_t pte;
> +
> clear_bit(_PAGE_BIT_RW, (unsigned long *)&ptep->pte);
> + pte = *ptep;
> +
> + /*
> + * Some processors can start a write, but ending up seeing
but end up seeing
> + * a read-only PTE by the time they get to the Dirty bit.
> + * In this case, they will set the Dirty bit, leaving a
> + * read-only, Dirty PTE which looks like a Shadow Stack PTE.
> + *
> + * However, this behavior has been improved and will not occur
> + * on processors supporting Shadow Stacks. Without this
> + * guarantee, a transition to a non-present PTE and flush the
> + * TLB would be needed.
> + *
> + * When change a writable PTE to read-only and if the PTE has
changing
> + * _PAGE_DIRTY_HW set, we move that bit to _PAGE_DIRTY_SW so
> + * that the PTE is not a valid Shadow Stack PTE.
> + */
> + pte = pte_move_flags(pte, _PAGE_DIRTY_HW, _PAGE_DIRTY_SW);
> + set_pte_at(mm, addr, ptep, pte);
> }
>
> #define flush_tlb_fix_spurious_fault(vma, address) do { } while (0)
> @@ -1266,7 +1287,28 @@ static inline pud_t pudp_huge_get_and_clear(struct mm_struct *mm,
> static inline void pmdp_set_wrprotect(struct mm_struct *mm,
> unsigned long addr, pmd_t *pmdp)
> {
> + pmd_t pmd;
> +
> clear_bit(_PAGE_BIT_RW, (unsigned long *)pmdp);
> + pmd = *pmdp;
> +
> + /*
> + * Some processors can start a write, but ending up seeing
but end up seeing
> + * a read-only PTE by the time they get to the Dirty bit.
> + * In this case, they will set the Dirty bit, leaving a
> + * read-only, Dirty PTE which looks like a Shadow Stack PTE.
> + *
> + * However, this behavior has been improved and will not occur
> + * on processors supporting Shadow Stacks. Without this
> + * guarantee, a transition to a non-present PTE and flush the
> + * TLB would be needed.
> + *
> + * When change a writable PTE to read-only and if the PTE has
changing
> + * _PAGE_DIRTY_HW set, we move that bit to _PAGE_DIRTY_SW so
> + * that the PTE is not a valid Shadow Stack PTE.
> + */
> + pmd = pmd_move_flags(pmd, _PAGE_DIRTY_HW, _PAGE_DIRTY_SW);
> + set_pmd_at(mm, addr, pmdp, pmd);
> }
>
> #define pud_write pud_write
>
--
~Randy
^ permalink raw reply
* Re: [RFC PATCH v3 1/8] x86/cet/ibt: Add Kconfig option for user-mode Indirect Branch Tracking
From: Randy Dunlap @ 2018-08-30 19:38 UTC (permalink / raw)
To: Yu-cheng Yu, x86, H. Peter Anvin, Thomas Gleixner, Ingo Molnar,
linux-kernel, linux-doc, linux-mm, linux-arch, linux-api,
Arnd Bergmann, Andy Lutomirski, Balbir Singh, Cyrill Gorcunov,
Dave Hansen, Florian Weimer, H.J. Lu, Jann Horn, Jonathan Corbet,
Kees Cook, Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel
In-Reply-To: <20180830144009.3314-2-yu-cheng.yu@intel.com>
On 08/30/2018 07:40 AM, Yu-cheng Yu wrote:
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 2cfe11e1cf7f..0d97b03f35f6 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1941,6 +1941,18 @@ config X86_INTEL_SHADOW_STACK_USER
>
> If unsure, say y.
>
> +config X86_INTEL_BRANCH_TRACKING_USER
> + prompt "Intel Indirect Branch Tracking for user-mode"
> + def_bool n
> + depends on CPU_SUP_INTEL && X86_64
> + select X86_INTEL_CET
> + select ARCH_HAS_PROGRAM_PROPERTIES
> + ---help---
> + Indirect Branch Tracking provides hardware protection against return-/jmp-
> + oriented programing attacks.
programming
> +
> + If unsure, say y
> +
> config EFI
> bool "EFI runtime service support"
> depends on ACPI
--
~Randy
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Dave Hansen @ 2018-08-30 18:55 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Jann Horn, yu-cheng.yu, the arch/x86 maintainers, H . Peter Anvin,
Thomas Gleixner, Ingo Molnar, kernel list, linux-doc, Linux-MM,
linux-arch, Linux API, Arnd Bergmann, Balbir Singh,
Cyrill Gorcunov, Florian Weimer, hjl.tools, Jonathan Corbet,
keescook, Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek,
Peter Zijlstra <pe>
In-Reply-To: <B2222C69-337B-44F2-9DA6-69E685AA469B@amacapital.net>
On 08/30/2018 10:34 AM, Andy Lutomirski wrote:
>> But, to keep B's TLB from picking up the entry, I think we can just make
>> it !Present for a moment. No TLB can cache it, and I believe the same
>> "don't set Dirty on a !Writable entry" logic also holds for !Present
>> (modulo a weird erratum or two).
> Can we get documentation? Pretty please?
The accessed bit description in the SDM looks pretty good to me today:
> Whenever the processor uses a paging-structure entry as part of
> linear-address translation, it sets the accessed flag in that entry
> (if it is not already set).
If it's !Present, it can't used as part of a translation so can't be
set. I think that covers the thing I was unsure about.
But, Dirty is a bit, er, muddier, but mostly because it only gets set on
leaf entries:
> Whenever there is a write to a linear address, the processor sets the
> dirty flag (if it is not already set) in the paging- structure entry
> that identifies the final physical address for the linear address
> (either a PTE or a paging-structure entry in which the PS flag is
> 1).
That little hunk will definitely need to get updated with something like:
On processors enumerating support for CET, the processor will on
set the dirty flag on paging structure entries in which the W
flag is 1.
^ permalink raw reply
* Re: [RFC PATCH v3 12/24] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Jann Horn @ 2018-08-30 17:59 UTC (permalink / raw)
To: yu-cheng.yu
Cc: Dave Hansen, the arch/x86 maintainers, H . Peter Anvin,
Thomas Gleixner, Ingo Molnar, kernel list, linux-doc, Linux-MM,
linux-arch, Linux API, Arnd Bergmann, Andy Lutomirski,
Balbir Singh, Cyrill Gorcunov, Florian Weimer, hjl.tools,
Jonathan Corbet, keescook, Mike Kravetz, Nadav Amit,
Oleg Nesterov
In-Reply-To: <1535651666.27823.6.camel@intel.com>
On Thu, Aug 30, 2018 at 7:58 PM Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
>
> On Thu, 2018-08-30 at 10:33 -0700, Dave Hansen wrote:
> > On 08/30/2018 10:26 AM, Yu-cheng Yu wrote:
> > >
> > > We don't have the guard page now, but there is a shadow stack
> > > token
> > > there, which cannot be used as a return address.
> > The overall concern is that we could overflow into a page that we
> > did
> > not intend. Either another actual shadow stack or something that a
> > page
> > that the attacker constructed, like the transient scenario Jann
> > described.
> >
>
> A task could go beyond the bottom of its shadow stack by doing either
> 'ret' or 'incssp'. If it is the 'ret' case, the token prevents it.
> If it is the 'incssp' case, a guard page cannot prevent it entirely,
> right?
I mean the other direction, on "call".
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox