Linux userland API discussions
 help / color / mirror / Atom feed
* Re: [PATCH 24/32] vfs: syscall: Add fsopen() to prepare for superblock creation [ver #9]
From: Jonathan Corbet @ 2018-07-11 15:51 UTC (permalink / raw)
  To: David Howells; +Cc: viro, linux-api, linux-fsdevel, torvalds, linux-kernel
In-Reply-To: <153126264966.14533.3388004240803696769.stgit@warthog.procyon.org.uk>

On Tue, 10 Jul 2018 23:44:09 +0100
David Howells <dhowells@redhat.com> wrote:

> 	sfd = fsopen("ext4", FSOPEN_CLOEXEC);
> 	write(sfd, "s /dev/sdb1"); // note I'm ignoring write's length arg
> 	write(sfd, "o noatime");
> 	write(sfd, "o acl");
> 	write(sfd, "o user_attr");
> 	write(sfd, "o iversion");
> 	write(sfd, "o ");
> 	write(sfd, "r /my/container"); // root inside the fs
> 	write(sfd, "x create"); // create the superblock

A minor detail but ... the "r" operation mentioned above is not actually
implemented in this system call.

jon

^ permalink raw reply

* Re: [RFC PATCH v2 17/27] x86/cet/shstk: User-mode shadow stack support
From: Yu-cheng Yu @ 2018-07-11 15:45 UTC (permalink / raw)
  To: Peter Zijlstra
  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, Pavel Machek, Ravi 
In-Reply-To: <20180711093454.GX2476@hirez.programming.kicks-ass.net>

On Wed, 2018-07-11 at 11:34 +0200, Peter Zijlstra wrote:
> On Tue, Jul 10, 2018 at 03:26:29PM -0700, Yu-cheng Yu wrote:
> > 
> > +/* MSR_IA32_U_CET and MSR_IA32_S_CET bits */
> > +#define MSR_IA32_CET_SHSTK_EN		0x0000000000000001
> > +#define MSR_IA32_CET_WRSS_EN		0x0000000000000002
> > +#define MSR_IA32_CET_ENDBR_EN		0x0000000000000004
> > +#define MSR_IA32_CET_LEG_IW_EN		0x0000000000000008
> > +#define MSR_IA32_CET_NO_TRACK_EN	0x0000000000000010
> Do those want a ULL literal suffix?

I will fix it.

^ permalink raw reply

* Re: [RFC PATCH v2 18/27] x86/cet/shstk: Introduce WRUSS instruction
From: Yu-cheng Yu @ 2018-07-11 15:41 UTC (permalink / raw)
  To: Peter Zijlstra
  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, Pavel Machek, Ravi 
In-Reply-To: <20180711152754.GJ2476@hirez.programming.kicks-ass.net>

On Wed, 2018-07-11 at 17:27 +0200, Peter Zijlstra wrote:
> On Wed, Jul 11, 2018 at 07:58:09AM -0700, Yu-cheng Yu wrote:
> > 
> > On Wed, 2018-07-11 at 11:45 +0200, Peter Zijlstra wrote:
> > > 
> > > On Tue, Jul 10, 2018 at 03:26:30PM -0700, Yu-cheng Yu wrote:
> > > > 
> > > > 
> > > > diff --git a/arch/x86/lib/x86-opcode-map.txt
> > > > b/arch/x86/lib/x86-
> > > > opcode-map.txt
> > > > index e0b85930dd77..72bb7c48a7df 100644
> > > > --- a/arch/x86/lib/x86-opcode-map.txt
> > > > +++ b/arch/x86/lib/x86-opcode-map.txt
> > > > @@ -789,7 +789,7 @@ f0: MOVBE Gy,My | MOVBE Gw,Mw (66) | CRC32
> > > > Gd,Eb (F2) | CRC32 Gd,Eb (66&F2)
> > > >  f1: MOVBE My,Gy | MOVBE Mw,Gw (66) | CRC32 Gd,Ey (F2) | CRC32
> > > > Gd,Ew (66&F2)
> > > >  f2: ANDN Gy,By,Ey (v)
> > > >  f3: Grp17 (1A)
> > > > -f5: BZHI Gy,Ey,By (v) | PEXT Gy,By,Ey (F3),(v) | PDEP Gy,By,Ey
> > > > (F2),(v)
> > > > +f5: BZHI Gy,Ey,By (v) | PEXT Gy,By,Ey (F3),(v) | PDEP Gy,By,Ey
> > > > (F2),(v) | WRUSS Pq,Qq (66),REX.W
> > > >  f6: ADCX Gy,Ey (66) | ADOX Gy,Ey (F3) | MULX By,Gy,rDX,Ey
> > > > (F2),(v)
> > > >  f7: BEXTR Gy,Ey,By (v) | SHLX Gy,Ey,By (66),(v) | SARX
> > > > Gy,Ey,By
> > > > (F3),(v) | SHRX Gy,Ey,By (F2),(v)
> > > >  EndTable
> > > Where are all the other instructions? ISTR that documentation
> > > patch
> > > listing a whole bunch of new instructions, not just wuss.
> > Currently we only use WRUSS in the kernel code.  Do we want to add
> > all
> > instructions here?
> Yes, since we also use the in-kernel decoder to decode random
> userspace
> code.

I will add other instructions.

Yu-cheng

^ permalink raw reply

* Re: [RFC PATCH v2 25/27] x86/cet: Add PTRACE interface for CET
From: Yu-cheng Yu @ 2018-07-11 15:40 UTC (permalink / raw)
  To: Ingo Molnar
  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, Pavel Machek
In-Reply-To: <20180711102035.GB8574@gmail.com>

On Wed, 2018-07-11 at 12:20 +0200, Ingo Molnar wrote:
> * Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:
> 
> > 
> > Add PTRACE interface for CET MSRs.
> Please *always* describe new ABIs in the changelog, in a precise,
> well-documented 
> way.

Ok!

> > 
> > diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
> > index e2ee403865eb..ac2bc3a18427 100644
> > --- a/arch/x86/kernel/ptrace.c
> > +++ b/arch/x86/kernel/ptrace.c
> > @@ -49,7 +49,9 @@ enum x86_regset {
> >  	REGSET_IOPERM64 = REGSET_XFP,
> >  	REGSET_XSTATE,
> >  	REGSET_TLS,
> > +	REGSET_CET64 = REGSET_TLS,
> >  	REGSET_IOPERM32,
> > +	REGSET_CET32,
> >  };
> Why does REGSET_CET64 alias on REGSET_TLS?

In x86_64_regsets[], there is no [REGSET_TLS].  The core dump code
cannot handle holes in the array.

> 
> > 
> >  struct pt_regs_offset {
> > @@ -1276,6 +1278,13 @@ static struct user_regset x86_64_regsets[]
> > __ro_after_init = {
> >  		.size = sizeof(long), .align = sizeof(long),
> >  		.active = ioperm_active, .get = ioperm_get
> >  	},
> > +	[REGSET_CET64] = {
> > +		.core_note_type = NT_X86_CET,
> > +		.n = sizeof(struct cet_user_state) / sizeof(u64),
> > +		.size = sizeof(u64), .align = sizeof(u64),
> > +		.active = cetregs_active, .get = cetregs_get,
> > +		.set = cetregs_set
> > +	},
> Ok, could we first please make this part of the regset code more
> readable and 
> start the series with a standalone clean-up patch that changes these
> initializers 
> to something more readable:
> 
> 	[REGSET_CET64] = {
> 		.core_note_type	= NT_X86_CET,
> 		.n		= sizeof(struct cet_user_state) /
> sizeof(u64),
> 		.size		= sizeof(u64),
> 		.align		= sizeof(u64),
> 		.active		= cetregs_active,
> 		.get		= cetregs_get,
> 		.set		= cetregs_set
> 	},
> 
> ? (I'm demonstrating the cleanup based on REGSET_CET64, but this
> should be done on 
> every other entry first.)
> 

I will fix it.

> 
> > 
> > --- a/include/uapi/linux/elf.h
> > +++ b/include/uapi/linux/elf.h
> > @@ -401,6 +401,7 @@ typedef struct elf64_shdr {
> >  #define NT_386_TLS	0x200		/* i386 TLS slots
> > (struct user_desc) */
> >  #define NT_386_IOPERM	0x201		/* x86 io
> > permission bitmap (1=deny) */
> >  #define NT_X86_XSTATE	0x202		/* x86 extended
> > state using xsave */
> > +#define NT_X86_CET	0x203		/* x86 cet state */
> Acronyms in comments should be in capital letters.
> 
> Also, I think I asked this before: why does "Control Flow
> Enforcement" abbreviate 
> to "CET" (which is a well-known acronym for "Central European Time"),
> not to CFE?
> 

I don't know if I can change that, will find out.

Thanks,
Yu-cheng

^ permalink raw reply

* Re: [RFC PATCH v2 18/27] x86/cet/shstk: Introduce WRUSS instruction
From: Peter Zijlstra @ 2018-07-11 15:30 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, Pavel Machek, Ravi 
In-Reply-To: <1531321615.13297.9.camel@intel.com>

On Wed, Jul 11, 2018 at 08:06:55AM -0700, Yu-cheng Yu wrote:
> On Wed, 2018-07-11 at 11:44 +0200, Peter Zijlstra wrote:

> > What happened to:
> > 
> >   https://lkml.kernel.org/r/1528729376.4526.0.camel@2b52.sc.intel.com
> 
> Yes, I put that in once and realized we only need to skip the
> instruction and return err.  Do you think we still need a handler for
> that?

I find that other form more readable, but then there's Nadav doing asm
macros to shrink inline asm thingies so maybe he has another suggestion.

^ permalink raw reply

* Re: [RFC PATCH v2 18/27] x86/cet/shstk: Introduce WRUSS instruction
From: Peter Zijlstra @ 2018-07-11 15:27 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, Pavel Machek, Ravi 
In-Reply-To: <1531321089.13297.4.camel@intel.com>

On Wed, Jul 11, 2018 at 07:58:09AM -0700, Yu-cheng Yu wrote:
> On Wed, 2018-07-11 at 11:45 +0200, Peter Zijlstra wrote:
> > On Tue, Jul 10, 2018 at 03:26:30PM -0700, Yu-cheng Yu wrote:
> > > 
> > > diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-
> > > opcode-map.txt
> > > index e0b85930dd77..72bb7c48a7df 100644
> > > --- a/arch/x86/lib/x86-opcode-map.txt
> > > +++ b/arch/x86/lib/x86-opcode-map.txt
> > > @@ -789,7 +789,7 @@ f0: MOVBE Gy,My | MOVBE Gw,Mw (66) | CRC32
> > > Gd,Eb (F2) | CRC32 Gd,Eb (66&F2)
> > >  f1: MOVBE My,Gy | MOVBE Mw,Gw (66) | CRC32 Gd,Ey (F2) | CRC32
> > > Gd,Ew (66&F2)
> > >  f2: ANDN Gy,By,Ey (v)
> > >  f3: Grp17 (1A)
> > > -f5: BZHI Gy,Ey,By (v) | PEXT Gy,By,Ey (F3),(v) | PDEP Gy,By,Ey
> > > (F2),(v)
> > > +f5: BZHI Gy,Ey,By (v) | PEXT Gy,By,Ey (F3),(v) | PDEP Gy,By,Ey
> > > (F2),(v) | WRUSS Pq,Qq (66),REX.W
> > >  f6: ADCX Gy,Ey (66) | ADOX Gy,Ey (F3) | MULX By,Gy,rDX,Ey (F2),(v)
> > >  f7: BEXTR Gy,Ey,By (v) | SHLX Gy,Ey,By (66),(v) | SARX Gy,Ey,By
> > > (F3),(v) | SHRX Gy,Ey,By (F2),(v)
> > >  EndTable
> > Where are all the other instructions? ISTR that documentation patch
> > listing a whole bunch of new instructions, not just wuss.
> 
> Currently we only use WRUSS in the kernel code.  Do we want to add all
> instructions here?

Yes, since we also use the in-kernel decoder to decode random userspace
code.

^ permalink raw reply

* Re: [RFC PATCH v2 05/27] Documentation/x86: Add CET description
From: Yu-cheng Yu @ 2018-07-11 15:25 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: <20180711082739.GA18919@amd>

On Wed, 2018-07-11 at 10:27 +0200, Pavel Machek wrote:
> On Tue 2018-07-10 15:26:17, Yu-cheng Yu wrote:
> > 
> > Explain how CET works and the no_cet_shstk/no_cet_ibt kernel
> > parameters.
> > 
> > 
> > --- /dev/null
> > +++ b/Documentation/x86/intel_cet.txt
> > @@ -0,0 +1,250 @@
> > +=========================================
> > +Control Flow Enforcement Technology (CET)
> > +=========================================
> We normally use .rst for this kind of formatted text.

I will change this to a .rst file.

> 
> 
> > 
> > +[6] The implementation of the SHSTK
> > +===================================
> > +
> > +SHSTK size
> > +----------
> > +
> > +A task's SHSTK is allocated from memory to a fixed size that can
> > +support 32 KB nested function calls; that is 256 KB for a 64-bit
> > +application and 128 KB for a 32-bit application.  The system admin
> > +can change the default size.
> How does admin change that? We already have ulimit for stack size,
> should those be somehow tied together?
> 
> $ ulimit -a
> ...
> stack size              (kbytes, -s) 8192
> 

We can do that.  This makes sense to me.

Yu-cheng

^ permalink raw reply

* Re: [RFC PATCH v2 18/27] x86/cet/shstk: Introduce WRUSS instruction
From: Yu-cheng Yu @ 2018-07-11 15:06 UTC (permalink / raw)
  To: Peter Zijlstra
  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, Pavel Machek, Ravi 
In-Reply-To: <20180711094448.GZ2476@hirez.programming.kicks-ass.net>

On Wed, 2018-07-11 at 11:44 +0200, Peter Zijlstra wrote:
> On Tue, Jul 10, 2018 at 03:26:30PM -0700, Yu-cheng Yu 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.
> > 
> > 
> > +static inline int write_user_shstk_64(unsigned long addr, unsigned
> > long val)
> > +{
> > +	int err = 0;
> > +
> > +	asm volatile("1: wrussq %[val], (%[addr])\n"
> > +		     "xor %[err], %[err]\n"
> this XOR is superfluous, you already cleared @err above.

I will fix it.

> 
> > 
> > +		     "2:\n"
> > +		     ".section .fixup,\"ax\"\n"
> > +		     "3: mov $-1, %[err]; jmp 2b\n"
> > +		     ".previous\n"
> > +		     _ASM_EXTABLE(1b, 3b)
> > +		     : [err] "=a" (err)
> > +		     : [val] "S" (val), [addr] "D" (addr));
> > +
> > +	return err;
> > +}
> > +#endif /* CONFIG_X86_INTEL_CET */
> > +
> >  #define nop() asm volatile ("nop")
> What happened to:
> 
>   https://lkml.kernel.org/r/1528729376.4526.0.camel@2b52.sc.intel.com

Yes, I put that in once and realized we only need to skip the
instruction and return err.  Do you think we still need a handler for
that?

Yu-cheng

^ permalink raw reply

* Re: [RFC PATCH v2 18/27] x86/cet/shstk: Introduce WRUSS instruction
From: Yu-cheng Yu @ 2018-07-11 14:58 UTC (permalink / raw)
  To: Peter Zijlstra
  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, Pavel Machek, Ravi 
In-Reply-To: <20180711094549.GA2476@hirez.programming.kicks-ass.net>

On Wed, 2018-07-11 at 11:45 +0200, Peter Zijlstra wrote:
> On Tue, Jul 10, 2018 at 03:26:30PM -0700, Yu-cheng Yu wrote:
> > 
> > diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-
> > opcode-map.txt
> > index e0b85930dd77..72bb7c48a7df 100644
> > --- a/arch/x86/lib/x86-opcode-map.txt
> > +++ b/arch/x86/lib/x86-opcode-map.txt
> > @@ -789,7 +789,7 @@ f0: MOVBE Gy,My | MOVBE Gw,Mw (66) | CRC32
> > Gd,Eb (F2) | CRC32 Gd,Eb (66&F2)
> >  f1: MOVBE My,Gy | MOVBE Mw,Gw (66) | CRC32 Gd,Ey (F2) | CRC32
> > Gd,Ew (66&F2)
> >  f2: ANDN Gy,By,Ey (v)
> >  f3: Grp17 (1A)
> > -f5: BZHI Gy,Ey,By (v) | PEXT Gy,By,Ey (F3),(v) | PDEP Gy,By,Ey
> > (F2),(v)
> > +f5: BZHI Gy,Ey,By (v) | PEXT Gy,By,Ey (F3),(v) | PDEP Gy,By,Ey
> > (F2),(v) | WRUSS Pq,Qq (66),REX.W
> >  f6: ADCX Gy,Ey (66) | ADOX Gy,Ey (F3) | MULX By,Gy,rDX,Ey (F2),(v)
> >  f7: BEXTR Gy,Ey,By (v) | SHLX Gy,Ey,By (66),(v) | SARX Gy,Ey,By
> > (F3),(v) | SHRX Gy,Ey,By (F2),(v)
> >  EndTable
> Where are all the other instructions? ISTR that documentation patch
> listing a whole bunch of new instructions, not just wuss.

Currently we only use WRUSS in the kernel code.  Do we want to add all
instructions here?

Yu-cheng

^ permalink raw reply

* Re: [RFC PATCH v2 05/27] Documentation/x86: Add CET description
From: Yu-cheng Yu @ 2018-07-11 14:53 UTC (permalink / raw)
  To: H.J. Lu, Florian Weimer
  Cc: the arch/x86 maintainers, H. Peter Anvin, Thomas Gleixner,
	Ingo Molnar, LKML, linux-doc, Linux-MM, linux-arch, Linux API,
	Arnd Bergmann, Andy Lutomirski, Balbir Singh, Cyrill Gorcunov,
	Dave Hansen, Jann Horn, Jonathan Corbet, Kees Cook, Mike Kravetz,
	Nadav Amit, Oleg Nesterov, Pavel Machek
In-Reply-To: <CAMe9rOqo3ZSMuwNZf8HbrL72OY1aQ0S0Huwqj7rsVY9_ZOfF_A@mail.gmail.com>

On Wed, 2018-07-11 at 06:47 -0700, H.J. Lu wrote:
> On Wed, Jul 11, 2018 at 2:57 AM, Florian Weimer <fweimer@redhat.com>
> wrote:
> > 
> > On 07/11/2018 12:26 AM, Yu-cheng Yu wrote:
> > 
> > > 
> > > +To build a CET-enabled kernel, Binutils v2.30 and GCC v8.1 or
> > > later
> > > +are required.  To build a CET-enabled application, GLIBC v2.29
> > > or
> > > +later is also requried.
> > 
> > Have you given up on getting the required changes into glibc 2.28?
> > 
> This is a typo.  We are still targeting for 2.28.  All pieces are
> there.
> 

Ok, I will fix it.

Yu-cheng

^ permalink raw reply

* Re: [RFC PATCH v2 05/27] Documentation/x86: Add CET description
From: H.J. Lu @ 2018-07-11 13:47 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Yu-cheng Yu, the arch/x86 maintainers, H. Peter Anvin,
	Thomas Gleixner, Ingo Molnar, LKML, linux-doc, Linux-MM,
	linux-arch, Linux API, Arnd Bergmann, Andy Lutomirski,
	Balbir Singh, Cyrill Gorcunov, Dave Hansen, Jann Horn,
	Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
	Oleg Nesterov
In-Reply-To: <ae3e2013-9c90-af39-f9da-278bf7af6f73@redhat.com>

On Wed, Jul 11, 2018 at 2:57 AM, Florian Weimer <fweimer@redhat.com> wrote:
> On 07/11/2018 12:26 AM, Yu-cheng Yu wrote:
>
>> +To build a CET-enabled kernel, Binutils v2.30 and GCC v8.1 or later
>> +are required.  To build a CET-enabled application, GLIBC v2.29 or
>> +later is also requried.
>
>
> Have you given up on getting the required changes into glibc 2.28?
>

This is a typo.  We are still targeting for 2.28.  All pieces are there.

-- 
H.J.

^ permalink raw reply

* Re: [RFC PATCH v2 27/27] x86/cet: Add arch_prctl functions for CET
From: Florian Weimer @ 2018-07-11 12:19 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, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
	Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek
In-Reply-To: <20180710222639.8241-28-yu-cheng.yu@intel.com>

On 07/11/2018 12:26 AM, Yu-cheng Yu wrote:
> arch_prctl(ARCH_CET_DISABLE, unsigned long features)
>      Disable SHSTK and/or IBT specified in 'features'.  Return -EPERM
>      if CET is locked out.
> 
> arch_prctl(ARCH_CET_LOCK)
>      Lock out CET feature.

Isn't it a “lock in” rather than a “lock out”?

Thanks,
Florian

^ permalink raw reply

* Re: [RFC PATCH v2 20/27] x86/cet/shstk: ELF header parsing of CET
From: Florian Weimer @ 2018-07-11 11:12 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, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
	Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek
In-Reply-To: <20180710222639.8241-21-yu-cheng.yu@intel.com>

On 07/11/2018 12:26 AM, Yu-cheng Yu wrote:
> +	/*
> +	 * PT_NOTE segment is small.  Read at most
> +	 * PAGE_SIZE.
> +	 */
> +	if (note_size > PAGE_SIZE)
> +		note_size = PAGE_SIZE;

That's not really true.  There are some huge PT_NOTE segments out there.

Why can't you check the notes after the executable has been mapped?

Thanks,
Florian

^ permalink raw reply

* Re: [RFC PATCH v2 25/27] x86/cet: Add PTRACE interface for CET
From: Ingo Molnar @ 2018-07-11 10:20 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, Pavel Machek
In-Reply-To: <20180710222639.8241-26-yu-cheng.yu@intel.com>


* Yu-cheng Yu <yu-cheng.yu@intel.com> wrote:

> Add PTRACE interface for CET MSRs.

Please *always* describe new ABIs in the changelog, in a precise, well-documented 
way.

> diff --git a/arch/x86/kernel/ptrace.c b/arch/x86/kernel/ptrace.c
> index e2ee403865eb..ac2bc3a18427 100644
> --- a/arch/x86/kernel/ptrace.c
> +++ b/arch/x86/kernel/ptrace.c
> @@ -49,7 +49,9 @@ enum x86_regset {
>  	REGSET_IOPERM64 = REGSET_XFP,
>  	REGSET_XSTATE,
>  	REGSET_TLS,
> +	REGSET_CET64 = REGSET_TLS,
>  	REGSET_IOPERM32,
> +	REGSET_CET32,
>  };

Why does REGSET_CET64 alias on REGSET_TLS?

>  struct pt_regs_offset {
> @@ -1276,6 +1278,13 @@ static struct user_regset x86_64_regsets[] __ro_after_init = {
>  		.size = sizeof(long), .align = sizeof(long),
>  		.active = ioperm_active, .get = ioperm_get
>  	},
> +	[REGSET_CET64] = {
> +		.core_note_type = NT_X86_CET,
> +		.n = sizeof(struct cet_user_state) / sizeof(u64),
> +		.size = sizeof(u64), .align = sizeof(u64),
> +		.active = cetregs_active, .get = cetregs_get,
> +		.set = cetregs_set
> +	},

Ok, could we first please make this part of the regset code more readable and 
start the series with a standalone clean-up patch that changes these initializers 
to something more readable:

	[REGSET_CET64] = {
		.core_note_type	= NT_X86_CET,
		.n		= sizeof(struct cet_user_state) / sizeof(u64),
		.size		= sizeof(u64),
		.align		= sizeof(u64),
		.active		= cetregs_active,
		.get		= cetregs_get,
		.set		= cetregs_set
	},

? (I'm demonstrating the cleanup based on REGSET_CET64, but this should be done on 
every other entry first.)


> --- a/include/uapi/linux/elf.h
> +++ b/include/uapi/linux/elf.h
> @@ -401,6 +401,7 @@ typedef struct elf64_shdr {
>  #define NT_386_TLS	0x200		/* i386 TLS slots (struct user_desc) */
>  #define NT_386_IOPERM	0x201		/* x86 io permission bitmap (1=deny) */
>  #define NT_X86_XSTATE	0x202		/* x86 extended state using xsave */
> +#define NT_X86_CET	0x203		/* x86 cet state */

Acronyms in comments should be in capital letters.

Also, I think I asked this before: why does "Control Flow Enforcement" abbreviate 
to "CET" (which is a well-known acronym for "Central European Time"), not to CFE?

Thanks,

	Ingo

^ permalink raw reply

* Re: [RFC PATCH v2 05/27] Documentation/x86: Add CET description
From: Florian Weimer @ 2018-07-11  9:57 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, H.J. Lu, Jann Horn, Jonathan Corbet, Kees Cook,
	Mike Kravetz, Nadav Amit, Oleg Nesterov, Pavel Machek
In-Reply-To: <20180710222639.8241-6-yu-cheng.yu@intel.com>

On 07/11/2018 12:26 AM, Yu-cheng Yu wrote:

> +To build a CET-enabled kernel, Binutils v2.30 and GCC v8.1 or later
> +are required.  To build a CET-enabled application, GLIBC v2.29 or
> +later is also requried.

Have you given up on getting the required changes into glibc 2.28?

Thanks,
Florian

^ permalink raw reply

* Re: [RFC PATCH v2 18/27] x86/cet/shstk: Introduce WRUSS instruction
From: Peter Zijlstra @ 2018-07-11  9:45 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, Pavel Machek, Ravi 
In-Reply-To: <20180710222639.8241-19-yu-cheng.yu@intel.com>

On Tue, Jul 10, 2018 at 03:26:30PM -0700, Yu-cheng Yu wrote:
> diff --git a/arch/x86/lib/x86-opcode-map.txt b/arch/x86/lib/x86-opcode-map.txt
> index e0b85930dd77..72bb7c48a7df 100644
> --- a/arch/x86/lib/x86-opcode-map.txt
> +++ b/arch/x86/lib/x86-opcode-map.txt
> @@ -789,7 +789,7 @@ f0: MOVBE Gy,My | MOVBE Gw,Mw (66) | CRC32 Gd,Eb (F2) | CRC32 Gd,Eb (66&F2)
>  f1: MOVBE My,Gy | MOVBE Mw,Gw (66) | CRC32 Gd,Ey (F2) | CRC32 Gd,Ew (66&F2)
>  f2: ANDN Gy,By,Ey (v)
>  f3: Grp17 (1A)
> -f5: BZHI Gy,Ey,By (v) | PEXT Gy,By,Ey (F3),(v) | PDEP Gy,By,Ey (F2),(v)
> +f5: BZHI Gy,Ey,By (v) | PEXT Gy,By,Ey (F3),(v) | PDEP Gy,By,Ey (F2),(v) | WRUSS Pq,Qq (66),REX.W
>  f6: ADCX Gy,Ey (66) | ADOX Gy,Ey (F3) | MULX By,Gy,rDX,Ey (F2),(v)
>  f7: BEXTR Gy,Ey,By (v) | SHLX Gy,Ey,By (66),(v) | SARX Gy,Ey,By (F3),(v) | SHRX Gy,Ey,By (F2),(v)
>  EndTable

Where are all the other instructions? ISTR that documentation patch
listing a whole bunch of new instructions, not just wuss.

^ permalink raw reply

* Re: [RFC PATCH v2 18/27] x86/cet/shstk: Introduce WRUSS instruction
From: Peter Zijlstra @ 2018-07-11  9:44 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, Pavel Machek, Ravi 
In-Reply-To: <20180710222639.8241-19-yu-cheng.yu@intel.com>

On Tue, Jul 10, 2018 at 03:26:30PM -0700, Yu-cheng Yu 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.
> 

> +static inline int write_user_shstk_64(unsigned long addr, unsigned long val)
> +{
> +	int err = 0;
> +
> +	asm volatile("1: wrussq %[val], (%[addr])\n"
> +		     "xor %[err], %[err]\n"

this XOR is superfluous, you already cleared @err above.

> +		     "2:\n"
> +		     ".section .fixup,\"ax\"\n"
> +		     "3: mov $-1, %[err]; jmp 2b\n"
> +		     ".previous\n"
> +		     _ASM_EXTABLE(1b, 3b)
> +		     : [err] "=a" (err)
> +		     : [val] "S" (val), [addr] "D" (addr));
> +
> +	return err;
> +}
> +#endif /* CONFIG_X86_INTEL_CET */
> +
>  #define nop() asm volatile ("nop")

What happened to:

  https://lkml.kernel.org/r/1528729376.4526.0.camel@2b52.sc.intel.com

^ permalink raw reply

* Re: [RFC PATCH v2 17/27] x86/cet/shstk: User-mode shadow stack support
From: Peter Zijlstra @ 2018-07-11  9:36 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, Pavel Machek, Ravi 
In-Reply-To: <20180710222639.8241-18-yu-cheng.yu@intel.com>

On Tue, Jul 10, 2018 at 03:26:29PM -0700, Yu-cheng Yu wrote:
> +struct cet_status {
> +	unsigned long	shstk_base;
> +	unsigned long	shstk_size;
> +	unsigned int	shstk_enabled:1;
> +};

> @@ -498,6 +499,10 @@ struct thread_struct {
>  	unsigned int		sig_on_uaccess_err:1;
>  	unsigned int		uaccess_err:1;	/* uaccess failed */
>  
> +#ifdef CONFIG_X86_INTEL_CET
> +	struct cet_status	cet;
> +#endif
> +
>  	/* Floating point and extended processor state */
>  	struct fpu		fpu;
>  	/*

Why does that need a structure? That avoids folding the bitfields.

^ permalink raw reply

* Re: [RFC PATCH v2 17/27] x86/cet/shstk: User-mode shadow stack support
From: Peter Zijlstra @ 2018-07-11  9:34 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, Pavel Machek, Ravi 
In-Reply-To: <20180710222639.8241-18-yu-cheng.yu@intel.com>

On Tue, Jul 10, 2018 at 03:26:29PM -0700, Yu-cheng Yu wrote:
> +/* MSR_IA32_U_CET and MSR_IA32_S_CET bits */
> +#define MSR_IA32_CET_SHSTK_EN		0x0000000000000001
> +#define MSR_IA32_CET_WRSS_EN		0x0000000000000002
> +#define MSR_IA32_CET_ENDBR_EN		0x0000000000000004
> +#define MSR_IA32_CET_LEG_IW_EN		0x0000000000000008
> +#define MSR_IA32_CET_NO_TRACK_EN	0x0000000000000010

Do those want a ULL literal suffix?

^ permalink raw reply

* Re: [RFC PATCH v2 16/27] mm: Modify can_follow_write_pte/pmd for shadow stack
From: Peter Zijlstra @ 2018-07-11  9:29 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, Pavel Machek, Ravi 
In-Reply-To: <20180710222639.8241-17-yu-cheng.yu@intel.com>

On Tue, Jul 10, 2018 at 03:26:28PM -0700, Yu-cheng Yu wrote:
> There are three possible shadow stack PTE settings:
> 
>   Normal SHSTK PTE: (R/O + DIRTY_HW)
>   SHSTK PTE COW'ed: (R/O + DIRTY_HW)
>   SHSTK PTE shared as R/O data: (R/O + DIRTY_SW)

I count _2_ distinct states there.

> Update can_follow_write_pte/pmd for the shadow stack.

So the below disallows can_follow_write when shstk && _PAGE_DIRTY_SW,
but this here Changelog doesn't explain why. Doesn't even get close.

Also, the code is a right mess :/ Can't we try harder to not let this
shadow stack stuff escape arch code.

^ permalink raw reply

* Re: [RFC PATCH v2 10/27] x86/mm: Introduce _PAGE_DIRTY_SW
From: Peter Zijlstra @ 2018-07-11  9:21 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, Pavel Machek, Ravi 
In-Reply-To: <20180710222639.8241-11-yu-cheng.yu@intel.com>

On Tue, Jul 10, 2018 at 03:26:22PM -0700, Yu-cheng Yu wrote:
> +static inline bool is_shstk_pte(pte_t pte)
> +{
> +	pteval_t val;
> +
> +	val = pte_flags(pte) & (_PAGE_RW | _PAGE_DIRTY_HW);
> +	return (val == _PAGE_DIRTY_HW);
> +}

That's against naming convention here.

static inline bool pte_shstk(pte_t pte)
{
	return pte_flags(pte) & (_PAGE_RW | _PAGE_DIRTY_HW) == _PAGE_DIRTY_HW;
}

would be more in style with the rest of this code.

^ permalink raw reply

* Re: [RFC PATCH v2 15/27] mm/mprotect: Prevent mprotect from changing shadow stack
From: Peter Zijlstra @ 2018-07-11  9:12 UTC (permalink / raw)
  To: Dave Hansen
  Cc: 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
In-Reply-To: <04800c52-1f86-c485-ba7c-2216d8c4966f@linux.intel.com>

On Tue, Jul 10, 2018 at 04:10:08PM -0700, Dave Hansen wrote:
> On 07/10/2018 03:26 PM, Yu-cheng Yu wrote:
> > Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> 
> This still needs a changelog, even if you think it's simple.
> > --- a/mm/mprotect.c
> > +++ b/mm/mprotect.c
> > @@ -446,6 +446,15 @@ static int do_mprotect_pkey(unsigned long start, size_t len,
> >  	error = -ENOMEM;
> >  	if (!vma)
> >  		goto out;
> > +
> > +	/*
> > +	 * Do not allow changing shadow stack memory.
> > +	 */
> > +	if (vma->vm_flags & VM_SHSTK) {
> > +		error = -EINVAL;
> > +		goto out;
> > +	}
> > +
> 
> I think this is a _bit_ draconian.  Why shouldn't we be able to use
> protection keys with a shadow stack?  Or, set it to PROT_NONE?

Right, and then there's also madvise() and some of the other accessors.

Why do we need to disallow this? AFAICT the worst that can happen is
that a process wrecks itself, so what?

^ permalink raw reply

* Re: [RFC PATCH v2 14/27] mm: Handle THP/HugeTLB shadow stack page fault
From: Peter Zijlstra @ 2018-07-11  9:10 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, Pavel Machek, Ravi 
In-Reply-To: <20180710222639.8241-15-yu-cheng.yu@intel.com>

On Tue, Jul 10, 2018 at 03:26:26PM -0700, Yu-cheng Yu wrote:
> diff --git a/mm/memory.c b/mm/memory.c
> index a2695dbc0418..f7c46d61eaea 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -4108,7 +4108,13 @@ static int __handle_mm_fault(struct vm_area_struct *vma, unsigned long address,
>  			if (pmd_protnone(orig_pmd) && vma_is_accessible(vma))
>  				return do_huge_pmd_numa_page(&vmf, orig_pmd);
>  
> -			if (dirty && !pmd_write(orig_pmd)) {
> +			/*
> +			 * Shadow stack trans huge PMDs are copy-on-access,
> +			 * so wp_huge_pmd() on them no mater if we have a
> +			 * write fault or not.
> +			 */
> +			if (is_shstk_mapping(vma->vm_flags) ||
> +			    (dirty && !pmd_write(orig_pmd))) {
>  				ret = wp_huge_pmd(&vmf, orig_pmd);
>  				if (!(ret & VM_FAULT_FALLBACK))
>  					return ret;

Can't we do this (and the do_wp_page thing) by setting FAULT_FLAG_WRITE
in the arch fault handler on shadow stack faults?

^ permalink raw reply

* Re: [RFC PATCH v2 13/27] mm: Handle shadow stack page fault
From: Peter Zijlstra @ 2018-07-11  9:06 UTC (permalink / raw)
  To: Dave Hansen
  Cc: 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
In-Reply-To: <2f3ff321-c629-3e00-59f6-8bca510650d4@linux.intel.com>

On Tue, Jul 10, 2018 at 04:06:25PM -0700, Dave Hansen wrote:
> On 07/10/2018 03:26 PM, Yu-cheng Yu wrote:
> > +	if (is_shstk_mapping(vma->vm_flags))
> > +		entry = pte_mkdirty_shstk(entry);
> > +	else
> > +		entry = pte_mkdirty(entry);
> > +
> > +	entry = maybe_mkwrite(entry, vma);
> >  	if (ptep_set_access_flags(vma, vmf->address, vmf->pte, entry, 1))
> >  		update_mmu_cache(vma, vmf->address, vmf->pte);
> >  	pte_unmap_unlock(vmf->pte, vmf->ptl);
> > @@ -2526,7 +2532,11 @@ static int wp_page_copy(struct vm_fault *vmf)
> >  		}
> >  		flush_cache_page(vma, vmf->address, pte_pfn(vmf->orig_pte));
> >  		entry = mk_pte(new_page, vma->vm_page_prot);
> > -		entry = maybe_mkwrite(pte_mkdirty(entry), vma);
> > +		if (is_shstk_mapping(vma->vm_flags))
> > +			entry = pte_mkdirty_shstk(entry);
> > +		else
> > +			entry = pte_mkdirty(entry);
> > +		entry = maybe_mkwrite(entry, vma);
> 
> Do we want to lift this hunk of code and put it elsewhere?  Maybe:
> 
> 	entry = pte_set_vma_features(entry, vma);
> 
> and then:
> 
> pte_t pte_set_vma_features(pte_t entry, struct vm_area_struct)
> {
> 		/*
> 		 * Shadow stack PTEs are always dirty and always
> 		 * writable.  They have a different encoding for
> 		 * this than normal PTEs, though.
> 		 */
> 		if (is_shstk_mapping(vma->vm_flags))
> 			entry = pte_mkdirty_shstk(entry);
> 		else
> 			entry = pte_mkdirty(entry);
> 
> 		entry = maybe_mkwrite(entry, vma);
> 
> 	return entry;
> }

Yes, that wants a helper like that. Not sold on the name, but whatever.

Is there any way we can hide all the shadow stack magic in arch code?

^ permalink raw reply

* Re: [RFC PATCH v2 11/27] x86/mm: Modify ptep_set_wrprotect and pmdp_set_wrprotect for _PAGE_DIRTY_SW
From: Peter Zijlstra @ 2018-07-11  8:48 UTC (permalink / raw)
  To: Dave Hansen
  Cc: 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
In-Reply-To: <fbf45667-5388-44a6-1f22-07bcc03e1804@linux.intel.com>

On Tue, Jul 10, 2018 at 03:44:32PM -0700, Dave Hansen wrote:
> On 07/10/2018 03:26 PM, Yu-cheng Yu wrote:
> > +	/*
> > +	 * On platforms before CET, other threads could race to
> > +	 * create a RO and _PAGE_DIRTY_HW PMD again.  However,
> > +	 * on CET platforms, this is safe without a TLB flush.
> > +	 */
> 
> If I didn't work for Intel, I'd wonder what the heck CET is and what the
> heck it has to do with _PAGE_DIRTY_HW.  I think we need a better comment

And Changelog, the provided one is abysmal.

> than this.  How about:
> 
> 	Some processors can _start_ a write, but end up seeing
> 	a read-only PTE by the time they get to getting 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.

I'm still struggling. I think I get the first paragraph, but then what?

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox