* Re: pkeys: Reserve PKEY_DISABLE_READ
From: Florian Weimer @ 2018-11-08 20:23 UTC (permalink / raw)
To: Ram Pai; +Cc: linux-mm, Dave Hansen, linuxppc-dev, linux-api
In-Reply-To: <20181108201231.GE5481@ram.oc3035372033.ibm.com>
* Ram Pai:
> Florian,
>
> I can. But I am struggling to understand the requirement. Why is
> this needed? Are we proposing a enhancement to the sys_pkey_alloc(),
> to be able to allocate keys that are initialied to disable-read
> only?
Yes, I think that would be a natural consequence.
However, my immediate need comes from the fact that the AMR register can
contain a flag combination that is not possible to represent with the
existing PKEY_DISABLE_WRITE and PKEY_DISABLE_ACCESS flags. User code
could write to AMR directly, so I cannot rule out that certain flag
combinations exist there.
So I came up with this:
int
pkey_get (int key)
{
if (key < 0 || key > PKEY_MAX)
{
__set_errno (EINVAL);
return -1;
}
unsigned int index = pkey_index (key);
unsigned long int amr = pkey_read ();
unsigned int bits = (amr >> index) & 3;
/* Translate from AMR values. PKEY_AMR_READ standing alone is not
currently representable. */
if (bits & PKEY_AMR_READ)
return PKEY_DISABLE_ACCESS;
else if (bits == PKEY_AMR_WRITE)
return PKEY_DISABLE_WRITE;
return 0;
}
And this is not ideal. I would prefer something like this instead:
switch (bits)
{
case PKEY_AMR_READ | PKEY_AMR_WRITE:
return PKEY_DISABLE_ACCESS;
case PKEY_AMR_READ:
return PKEY_DISABLE_READ;
case PKEY_AMR_WRITE:
return PKEY_DISABLE_WRITE;
case 0:
return 0;
}
By the way, is the AMR register 64-bit or 32-bit on 32-bit POWER?
Thanks,
Florian
^ permalink raw reply
* Re: pkeys: Reserve PKEY_DISABLE_READ
From: Ram Pai @ 2018-11-08 20:12 UTC (permalink / raw)
To: Florian Weimer; +Cc: linux-mm, Dave Hansen, linuxppc-dev, linux-api
In-Reply-To: <87k1ln8o7u.fsf@oldenburg.str.redhat.com>
On Thu, Nov 08, 2018 at 06:37:41PM +0100, Florian Weimer wrote:
> * Dave Hansen:
>
> > On 11/8/18 7:01 AM, Florian Weimer wrote:
> >> Ideally, PKEY_DISABLE_READ | PKEY_DISABLE_WRITE and PKEY_DISABLE_READ |
> >> PKEY_DISABLE_ACCESS would be treated as PKEY_DISABLE_ACCESS both, and a
> >> line PKEY_DISABLE_READ would result in an EINVAL failure.
> >
> > Sounds reasonable to me.
> >
> > I don't see any urgency to do this right now. It could easily go in
> > alongside the ppc patches when those get merged.
>
> POWER support has already been merged, so we need to do something here
> now, before I can complete the userspace side.
>
> > The only thing I'd suggest is that we make it something slightly
> > higher than 0x4. It'll make the code easier to deal with in the
> > kernel if we have the ABI and the hardware mirror each other, and if
> > we pick 0x4 in the ABI for PKEY_DISABLE_READ, it might get messy if
> > the harware choose 0x4 for PKEY_DISABLE_EXECUTE or something.
> >
> > So, let's make it 0x80 or something on x86 at least.
>
> I don't have a problem with that if that's what it takes.
>
> > Also, I'll be happy to review and ack the patch to do this, but I'd
> > expect the ppc guys (hi Ram!) to actually put it together.
>
> Ram, do you want to write a patch?
Florian,
I can. But I am struggling to understand the requirement. Why is
this needed? Are we proposing a enhancement to the sys_pkey_alloc(),
to be able to allocate keys that are initialied to disable-read
only?
RP
>
> I'll promise I finish the glibc support for this. 8-)
>
> Thanks,
> Florian
--
Ram Pai
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Dave Hansen @ 2018-11-08 20:10 UTC (permalink / raw)
To: Andy Lutomirski, Christopherson, Sean J
Cc: Andrew Lutomirski, Jann Horn, Linus Torvalds, Rich Felker,
Dave Hansen, Jethro Beekman, Jarkko Sakkinen, Florian Weimer,
Linux API, X86 ML, linux-arch, LKML, Peter Zijlstra, nhorman,
npmccallum, Ayoun, Serge, shay.katz-zamir, linux-sgx,
Andy Shevchenko, Thomas Gleixner, Ingo Molnar,
Borislav Petkov <bp@
In-Reply-To: <CALCETrUCQbPMfBJVRRg=0apEU=2U2ps74UPTrN8bFqi1k0K4VA@mail.gmail.com>
On 11/8/18 12:05 PM, Andy Lutomirski wrote:
> Hmm. The idea being that the SDK preserves RBP but not RSP. That's
> not the most terrible thing in the world. But could the SDK live with
> something more like my suggestion where the vDSO supplies a normal
> function that takes a struct containing registers that are visible to
> the enclave? This would make it extremely awkward for the enclave to
> use the untrusted stack per se, but it would make it quite easy (I
> think) for the untrusted part of the SDK to allocate some extra memory
> and just tell the enclave that *that* memory is the stack.
I really think the enclave should keep its grubby mitts off the
untrusted stack. There are lots of ways to get memory, even with
stack-like semantics, that don't involve mucking with the stack itself.
I have not heard a good, hard argument for why there is an absolute
*need* to store things on the actual untrusted stack.
We could quite easily have the untrusted code just promise to allocate a
stack-sized virtual area (even derived from the stack rlimit size) and
pass that into the enclave for parameter use.
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Andy Lutomirski @ 2018-11-08 20:05 UTC (permalink / raw)
To: Christopherson, Sean J
Cc: Dave Hansen, Andrew Lutomirski, Jann Horn, Linus Torvalds,
Rich Felker, Dave Hansen, Jethro Beekman, Jarkko Sakkinen,
Florian Weimer, Linux API, X86 ML, linux-arch, LKML,
Peter Zijlstra, nhorman, npmccallum, Ayoun, Serge,
shay.katz-zamir, linux-sgx, Andy Shevchenko, Thomas Gleixner
In-Reply-To: <20181108195420.GA14715@linux.intel.com>
On Thu, Nov 8, 2018 at 11:54 AM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> On Tue, Nov 06, 2018 at 01:07:54PM -0800, Andy Lutomirski wrote:
> >
> >
> > > On Nov 6, 2018, at 1:00 PM, Dave Hansen <dave.hansen@intel.com> wrote:
> > >
> > >> On 11/6/18 12:12 PM, Andy Lutomirski wrote:
> > >> True, but what if we have a nasty enclave that writes to memory just
> > >> below SP *before* decrementing SP?
> > >
> > > Yeah, that would be unfortunate. If an enclave did this (roughly):
> > >
> > > 1. EENTER
> > > 2. Hardware sets eenter_hwframe->sp = %sp
> > > 3. Enclave runs... wants to do out-call
> > > 4. Enclave sets up parameters:
> > > memcpy(&eenter_hwframe->sp[-offset], arg1, size);
> > > ...
> > > 5. Enclave sets eenter_hwframe->sp -= offset
> > >
> > > If we got a signal between 4 and 5, we'd clobber the copy of 'arg1' that
> > > was on the stack. The enclave could easily fix this by moving ->sp first.
> > >
> > > But, this is one of those "fun" parts of the ABI that I think we need to
> > > talk about. If we do this, we also basically require that the code
> > > which handles asynchronous exits must *not* write to the stack. That's
> > > not hard because it's typically just a single ERESUME instruction, but
> > > it *is* a requirement.
> > >
> >
> > I was assuming that the async exit stuff was completely hidden by the
> > API. The AEP code would decide whether the exit got fixed up by the
> > kernel (which may or may not be easy to tell — can the code even tell
> > without kernel help whether it was, say, an IRQ vs #UD?) and then either
> > do ERESUME or cause sgx_enter_enclave() to return with an appropriate
> > return value.
>
> Ok, SDK folks came up with an idea that would allow them to use vDSO,
> albeit with a bit of ugliness and potentially a ROP-attack issue.
> Definitely some weirdness, but the weirdness is well contained, unlike
> the magic prefix approach.
>
> Provide two enter_enclave() vDSO "functions". The first is a normal
> function with a normal C interface. The second is a blob of code that
> is "called" and "returns" via indirect jmp, and can be used by SGX
> runtimes that want to use the untrusted stack for out-calls from the
> enclave.
>
> For the indirect jmp "function", use %rbp to stash the return address
> of the caller (either in %rbp itself or in memory pointed to by %rbp).
> It works because hardware also saves/restores %rbp along with %rsp when
> doing enclave transitions, and the SDK can live with %rbp being
> off-limits. Fault info is passed via registers.
Hmm. The idea being that the SDK preserves RBP but not RSP. That's
not the most terrible thing in the world. But could the SDK live with
something more like my suggestion where the vDSO supplies a normal
function that takes a struct containing registers that are visible to
the enclave? This would make it extremely awkward for the enclave to
use the untrusted stack per se, but it would make it quite easy (I
think) for the untrusted part of the SDK to allocate some extra memory
and just tell the enclave that *that* memory is the stack.
AFAFICS we do have two registers that genuinely are preserved: FSBASE
and GSBASE. Which is a good thing, because otherwise SGX enablement
would currently be a privilege escalation issue due to making GSBASE
writable when it should not be.
This whole thing is a mess. I'm starting to think that the cleanest
solution would be to provide a way to just tell the kernel that
certain RIP values have exception fixups.
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Sean Christopherson @ 2018-11-08 19:54 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Dave Hansen, Andy Lutomirski, Jann Horn, Linus Torvalds,
Rich Felker, Dave Hansen, Jethro Beekman, Jarkko Sakkinen,
Florian Weimer, Linux API, X86 ML, linux-arch, LKML,
Peter Zijlstra, nhorman, npmccallum, Ayoun, Serge,
shay.katz-zamir, linux-sgx, Andy Shevchenko, Thomas Gleixner
In-Reply-To: <9B76E95B-5745-412E-8007-7FAA7F83D6FB@amacapital.net>
On Tue, Nov 06, 2018 at 01:07:54PM -0800, Andy Lutomirski wrote:
>
>
> > On Nov 6, 2018, at 1:00 PM, Dave Hansen <dave.hansen@intel.com> wrote:
> >
> >> On 11/6/18 12:12 PM, Andy Lutomirski wrote:
> >> True, but what if we have a nasty enclave that writes to memory just
> >> below SP *before* decrementing SP?
> >
> > Yeah, that would be unfortunate. If an enclave did this (roughly):
> >
> > 1. EENTER
> > 2. Hardware sets eenter_hwframe->sp = %sp
> > 3. Enclave runs... wants to do out-call
> > 4. Enclave sets up parameters:
> > memcpy(&eenter_hwframe->sp[-offset], arg1, size);
> > ...
> > 5. Enclave sets eenter_hwframe->sp -= offset
> >
> > If we got a signal between 4 and 5, we'd clobber the copy of 'arg1' that
> > was on the stack. The enclave could easily fix this by moving ->sp first.
> >
> > But, this is one of those "fun" parts of the ABI that I think we need to
> > talk about. If we do this, we also basically require that the code
> > which handles asynchronous exits must *not* write to the stack. That's
> > not hard because it's typically just a single ERESUME instruction, but
> > it *is* a requirement.
> >
>
> I was assuming that the async exit stuff was completely hidden by the
> API. The AEP code would decide whether the exit got fixed up by the
> kernel (which may or may not be easy to tell — can the code even tell
> without kernel help whether it was, say, an IRQ vs #UD?) and then either
> do ERESUME or cause sgx_enter_enclave() to return with an appropriate
> return value.
Ok, SDK folks came up with an idea that would allow them to use vDSO,
albeit with a bit of ugliness and potentially a ROP-attack issue.
Definitely some weirdness, but the weirdness is well contained, unlike
the magic prefix approach.
Provide two enter_enclave() vDSO "functions". The first is a normal
function with a normal C interface. The second is a blob of code that
is "called" and "returns" via indirect jmp, and can be used by SGX
runtimes that want to use the untrusted stack for out-calls from the
enclave.
For the indirect jmp "function", use %rbp to stash the return address
of the caller (either in %rbp itself or in memory pointed to by %rbp).
It works because hardware also saves/restores %rbp along with %rsp when
doing enclave transitions, and the SDK can live with %rbp being
off-limits. Fault info is passed via registers.
Basic idea for the "functions" below. The fixup stuff is obviously not
wired up correctly, just trying to convey the concept.
struct enclu_fault_info {
unsigned int leaf;
unsigned int trapnr;
unsigned int error_code;
unsigned long address;
};
int __vdso_enter_enclave(void *tcs, struct enclu_fault_info *fault_info)
{
unsigned int leaf, trapnr;
asm volatile (
"lea 2f(%%rip), %%rcx\n\t"
"1: enclu\n\t"
"jmp 3f\n\t"
/* ERESUME trampoline */
"2: enclu\n\t"
"ud2\n\t"
/* out: */
"3:\n"
/* EENTER fixup */
".pushsection .fixup,\"ax\"\n\t"
"4:\n\t"
"mov %%eax, %%edi\n\t"
"movl $"__stringify(SGX_EENTER)", %%eax\n\t"
"jmp 3b\n\t"
".popsection\n\t"
_ASM_EXTABLE_FAULT(1b, 4b)
/* ERESUME FIXUP */
".pushsection .fixup,\"ax\"\n\t"
"5:\n\t"
"mov %%eax, %%edi\n\t"
"movl $"__stringify(SGX_ERESUME)", %%eax\n\t"
"jmp 3b\n\t"
".popsection\n\t"
_ASM_EXTABLE_FAULT(2b, 5b)
: "=a"(leaf), "=D" (trapnr)
: "a" (SGX_EENTER), "b" (tcs)
: "cc", "memory", "rcx", "rdx", "rsi", "r8", "r9", "r10",
"r11", "r12", "r13", "r14", "r15"
);
if (leaf == SGX_EEXIT)
return 0;
if (fault_info) {
fault_info->leaf = leaf;
fault_info->trapnr = trapnr;
fault_info->error_code = 0;
fault_info->address = 0;
}
return -EFAULT;
}
GLOBAL(__vdso_enter_enclave_no_stack)
endbr64
/* %rbp = return target, %rbx = tcs */
leaq 3f(%rip), %rcx
movl $2, %eax
1: enclu
/* "return" to "caller" */
2: jmp *%rbp
/* ERESUME trampoline */
3: enclu
ud2
/* EENTER fixup handler */
4: movq %rax, %rdi
movl $2, %eax
/* %rsi = error code, %rdx = address */
jmp 2b
/* ERESUME fixup handler */
5: movq %rax, %rdi
movl $3, %eax
/* %rsi = error code, %rdx = address */
jmp 2b
^ permalink raw reply
* Re: pkeys: Reserve PKEY_DISABLE_READ
From: Ram Pai @ 2018-11-08 19:22 UTC (permalink / raw)
To: Florian Weimer; +Cc: linux-api, dave.hansen, linuxppc-dev, linux-mm
In-Reply-To: <877ehnbwqy.fsf@oldenburg.str.redhat.com>
On Thu, Nov 08, 2018 at 01:05:09PM +0100, Florian Weimer wrote:
> Would it be possible to reserve a bit for PKEY_DISABLE_READ?
>
> I think the POWER implementation can disable read access at the hardware
> level, but not write access, and that cannot be expressed with the
> current PKEY_DISABLE_ACCESS and PKEY_DISABLE_WRITE bits.
POWER hardware can disable-read and can **also disable-write**
at the hardware level. It can disable-execute aswell at the
hardware level. For example if the key bits for a given key in the AMR
register is
0b01 it is read-disable
0b10 it is write-disable
To support access-disable, we make the key value 0b11.
So in case if you want to know if the key is read-disable 'bitwise-and' it
against 0x1. i.e (x & 0x1)
RP
^ permalink raw reply
* Re: [PATCH v5 04/27] x86/fpu/xstate: Add XSAVES system states for shadow stack
From: Borislav Petkov @ 2018-11-08 18:40 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,
Eugene Syromiatnikov, Florian Weimer, H.J. Lu, Jann Horn,
Jonathan Corbet, Kees Cook, Mike Kravetz, Nadav Amit,
Oleg Nesterov, Pa
In-Reply-To: <20181011151523.27101-5-yu-cheng.yu@intel.com>
On Thu, Oct 11, 2018 at 08:15:00AM -0700, Yu-cheng Yu wrote:
> Intel Control-flow Enforcement Technology (CET) introduces the
> following MSRs into the XSAVES system states.
>
> IA32_U_CET (user-mode CET settings),
> IA32_PL3_SSP (user-mode shadow stack),
> IA32_PL0_SSP (kernel-mode shadow stack),
> IA32_PL1_SSP (ring-1 shadow stack),
> IA32_PL2_SSP (ring-2 shadow stack).
And?
That commit message got chopped off here, it seems.
> Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
> ---
> arch/x86/include/asm/fpu/types.h | 22 +++++++++++++++++++++
> arch/x86/include/asm/fpu/xstate.h | 4 +++-
> arch/x86/include/uapi/asm/processor-flags.h | 2 ++
> arch/x86/kernel/fpu/xstate.c | 10 ++++++++++
> 4 files changed, 37 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/include/asm/fpu/types.h b/arch/x86/include/asm/fpu/types.h
> index 202c53918ecf..e55d51d172f1 100644
> --- a/arch/x86/include/asm/fpu/types.h
> +++ b/arch/x86/include/asm/fpu/types.h
> @@ -114,6 +114,9 @@ enum xfeature {
> XFEATURE_Hi16_ZMM,
> XFEATURE_PT_UNIMPLEMENTED_SO_FAR,
> XFEATURE_PKRU,
> + XFEATURE_RESERVED,
> + XFEATURE_SHSTK_USER,
> + XFEATURE_SHSTK_KERNEL,
>
> XFEATURE_MAX,
> };
> @@ -128,6 +131,8 @@ enum xfeature {
> #define XFEATURE_MASK_Hi16_ZMM (1 << XFEATURE_Hi16_ZMM)
> #define XFEATURE_MASK_PT (1 << XFEATURE_PT_UNIMPLEMENTED_SO_FAR)
> #define XFEATURE_MASK_PKRU (1 << XFEATURE_PKRU)
> +#define XFEATURE_MASK_SHSTK_USER (1 << XFEATURE_SHSTK_USER)
> +#define XFEATURE_MASK_SHSTK_KERNEL (1 << XFEATURE_SHSTK_KERNEL)
>
> #define XFEATURE_MASK_FPSSE (XFEATURE_MASK_FP | XFEATURE_MASK_SSE)
> #define XFEATURE_MASK_AVX512 (XFEATURE_MASK_OPMASK \
> @@ -229,6 +234,23 @@ struct pkru_state {
> u32 pad;
> } __packed;
>
> +/*
> + * State component 11 is Control flow Enforcement user states
Why the Camel-cased naming?
"Control" then "flow" then capitalized again "Enforcement".
Fix all occurrences pls, especially the user-visible strings.
> + */
> +struct cet_user_state {
> + u64 u_cet; /* user control flow settings */
> + u64 user_ssp; /* user shadow stack pointer */
Prefix both with "usr_" instead.
> +} __packed;
> +
> +/*
> + * State component 12 is Control flow Enforcement kernel states
> + */
> +struct cet_kernel_state {
> + u64 kernel_ssp; /* kernel shadow stack */
> + u64 pl1_ssp; /* ring-1 shadow stack */
> + u64 pl2_ssp; /* ring-2 shadow stack */
Just write "privilege level" everywhere - not "ring".
Btw, do you see how the type and the name of all those other fields in
that file are tabulated? Except yours...
> +} __packed;
> +
> struct xstate_header {
> u64 xfeatures;
> u64 xcomp_bv;
> diff --git a/arch/x86/include/asm/fpu/xstate.h b/arch/x86/include/asm/fpu/xstate.h
> index d8e2ec99f635..18b60748a34d 100644
> --- a/arch/x86/include/asm/fpu/xstate.h
> +++ b/arch/x86/include/asm/fpu/xstate.h
> @@ -28,7 +28,9 @@
> XFEATURE_MASK_Hi16_ZMM | \
> XFEATURE_MASK_PKRU | \
> XFEATURE_MASK_BNDREGS | \
> - XFEATURE_MASK_BNDCSR)
> + XFEATURE_MASK_BNDCSR | \
> + XFEATURE_MASK_SHSTK_USER | \
> + XFEATURE_MASK_SHSTK_KERNEL)
>
> #ifdef CONFIG_X86_64
> #define REX_PREFIX "0x48, "
> diff --git a/arch/x86/include/uapi/asm/processor-flags.h b/arch/x86/include/uapi/asm/processor-flags.h
> index bcba3c643e63..25311ec4b731 100644
> --- a/arch/x86/include/uapi/asm/processor-flags.h
> +++ b/arch/x86/include/uapi/asm/processor-flags.h
> @@ -130,6 +130,8 @@
> #define X86_CR4_SMAP _BITUL(X86_CR4_SMAP_BIT)
> #define X86_CR4_PKE_BIT 22 /* enable Protection Keys support */
> #define X86_CR4_PKE _BITUL(X86_CR4_PKE_BIT)
> +#define X86_CR4_CET_BIT 23 /* enable Control flow Enforcement */
> +#define X86_CR4_CET _BITUL(X86_CR4_CET_BIT)
>
> /*
> * x86-64 Task Priority Register, CR8
> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
> index 605ec6decf3e..ad36ea28bfd1 100644
> --- a/arch/x86/kernel/fpu/xstate.c
> +++ b/arch/x86/kernel/fpu/xstate.c
> @@ -35,6 +35,9 @@ static const char *xfeature_names[] =
> "Processor Trace (unused)" ,
> "Protection Keys User registers",
> "unknown xstate feature" ,
> + "Control flow User registers" ,
> + "Control flow Kernel registers" ,
> + "unknown xstate feature" ,
So there are two "unknown xstate feature" array elems now...
> static short xsave_cpuid_features[] __initdata = {
> @@ -48,6 +51,9 @@ static short xsave_cpuid_features[] __initdata = {
> X86_FEATURE_AVX512F,
> X86_FEATURE_INTEL_PT,
> X86_FEATURE_PKU,
> + 0, /* Unused */
What's that for?
> + X86_FEATURE_SHSTK, /* XFEATURE_SHSTK_USER */
> + X86_FEATURE_SHSTK, /* XFEATURE_SHSTK_KERNEL */
> };
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Jarkko Sakkinen @ 2018-11-08 15:11 UTC (permalink / raw)
To: Sean Christopherson
Cc: Rich Felker, Andy Lutomirski, Dave Hansen, Jann Horn,
Linus Torvalds, Dave Hansen, Jethro Beekman, Florian Weimer,
Linux API, X86 ML, linux-arch, LKML, Peter Zijlstra, nhorman,
npmccallum, Ayoun, Serge, shay.katz-zamir, linux-sgx,
Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav Petkov
In-Reply-To: <20181107214058.GG27170@linux.intel.com>
On Wed, Nov 07, 2018 at 01:40:59PM -0800, Sean Christopherson wrote:
> > In that case it seems like the only way to use SGX that's not a gaping
> > security hole is to run the SGX enclave in its own fully-seccomp (or
> > equivalent) process, with no host application in the same address
> > space. Since the host application can't see the contents of the
> > enclave to make any determination of whether it's safe to run, running
> > it in the same address space only makes sense if the cpu provides
> > protection against unwanted accesses to the host's memory from the
> > enclave -- and according to you, it doesn't.
>
> The enclave's code (and any initial data) isn't encrypted until the
> pages are loaded into the Enclave Page Cache (EPC), which can only
> be done by the kernel (via ENCLS[EADD]). In other words, both the
> kernel and userspace can vet the code/data before running an enclave.
>
> Practically speaking, an enclave will be coupled with an untrusted
> userspace runtime, i.e. it's loader. Enclaves are also measured
> as part of their build process, and so the enclave loader needs to
> know which pages to add to the measurement, and in what order. I
> guess technically speaking an enclave could have zero pages added
> to its measurement, but that'd probably be a big red flag that said
> enclave is up to something fishy.
IMHO the whole idea adds too much policy into kernel even if it would
be doable. You can easily spawn untrusted run-time and enclave to its
own process.
Seccomp limits the syscall space and enclaves cannot do syscalls in the
first place. It is the URT that will do them behalf of the enclave.
/Jarkko
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Jarkko Sakkinen @ 2018-11-08 15:04 UTC (permalink / raw)
To: Dave Hansen
Cc: Sean Christopherson, Andy Lutomirski, Jann Horn, Linus Torvalds,
Rich Felker, Dave Hansen, Jethro Beekman, Florian Weimer,
Linux API, X86 ML, linux-arch, LKML, Peter Zijlstra, nhorman,
npmccallum, Ayoun, Serge, shay.katz-zamir, linux-sgx,
Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav
In-Reply-To: <662fa2c8-c9a8-b2a8-ee8b-92a8a89c0630@intel.com>
On Wed, Nov 07, 2018 at 12:56:58PM -0800, Dave Hansen wrote:
> On 11/7/18 11:01 AM, Sean Christopherson wrote:
> > Going off comments in similar code related to UMIP, we'd need to figure
> > out how to handle protection keys.
>
> There are two options:
> 1. Don't depend on the userspace mapping. Do get_user_pages() to find
> the instruction in the kernel direct map, and use that.
> 2. Do a WRPKRU that allows read access, do the read, then put PKRU back.
> This is a pain because of preemption and all that jazz.
>
> Right now, we just let the prefetch instruction detection fail if you
> mark it unreadable with pkeys. Tough cookies, basically. But, that's
> just the kernel being nice, but you need it for functionality, so it's
> tougher.
I would go with one because it is the stable way to do it and we are
100% sure to not conflict with pk's.
/Jarkko
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Sean Christopherson @ 2018-11-07 21:40 UTC (permalink / raw)
To: Rich Felker
Cc: Andy Lutomirski, Dave Hansen, Jann Horn, Linus Torvalds,
Dave Hansen, Jethro Beekman, Jarkko Sakkinen, Florian Weimer,
Linux API, X86 ML, linux-arch, LKML, Peter Zijlstra, nhorman,
npmccallum, Ayoun, Serge, shay.katz-zamir, linux-sgx,
Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav
In-Reply-To: <20181107212758.GU5150@brightrain.aerifal.cx>
On Wed, Nov 07, 2018 at 04:27:58PM -0500, Rich Felker wrote:
> On Tue, Nov 06, 2018 at 03:26:16PM -0800, Sean Christopherson wrote:
> > On Tue, Nov 06, 2018 at 06:17:30PM -0500, Rich Felker wrote:
> > > On Tue, Nov 06, 2018 at 11:02:11AM -0800, Andy Lutomirski wrote:
> > > > On Tue, Nov 6, 2018 at 10:41 AM Dave Hansen <dave.hansen@intel.com> wrote:
> > > > >
> > > > > On 11/6/18 10:20 AM, Andy Lutomirski wrote:
> > > > > > I almost feel like the right solution is to call into SGX on its own
> > > > > > private stack or maybe even its own private address space.
> > > > >
> > > > > Yeah, I had the same gut feeling. Couldn't the debugger even treat the
> > > > > enclave like its own "thread" with its own stack and its own set of
> > > > > registers and context? That seems like a much more workable model than
> > > > > trying to weave it together with the EENTER context.
> > > >
> > > > So maybe the API should be, roughly
> > > >
> > > > sgx_exit_reason_t sgx_enter_enclave(pointer_to_enclave, struct
> > > > host_state *state);
> > > > sgx_exit_reason_t sgx_resume_enclave(same args);
> > > >
> > > > where host_state is something like:
> > > >
> > > > struct host_state {
> > > > unsigned long bp, sp, ax, bx, cx, dx, si, di;
> > > > };
> > > >
> > > > and the values in host_state explicitly have nothing to do with the
> > > > actual host registers. So, if you want to use the outcall mechanism,
> > > > you'd allocate some memory, point sp to that memory, call
> > > > sgx_enter_enclave(), and then read that memory to do the outcall.
> > > >
> > > > Actually implementing this would be distinctly nontrivial, and would
> > > > almost certainly need some degree of kernel help to avoid an explosion
> > > > when a signal gets delivered while we have host_state.sp loaded into
> > > > the actual SP register. Maybe rseq could help with this?
> > > >
> > > > The ISA here is IMO not well thought through.
> > >
> > > Maybe I'm mistaken about some fundamentals here, but my understanding
> > > of SGX is that the whole point is that the host application and the
> > > code running in the enclave are mutually adversarial towards one
> > > another. Do any or all of the proposed protocols here account for this
> > > and fully protect the host application from malicious code in the
> > > enclave? It seems that having control over the register file on exit
> > > from the enclave is fundamentally problematic but I assume there must
> > > be some way I'm missing that this is fixed up.
> >
> > SGX provides protections for the enclave but not the other way around.
> > The kernel has all of its normal non-SGX protections in place, but the
> > enclave can certainly wreak havoc on its userspace process. The basic
> > design idea is that the enclave is a specialized .so that gets extra
> > security protections but is still effectively part of the overall
> > application, e.g. it has full access to its host userspace process'
> > virtual memory.
>
> In that case it seems like the only way to use SGX that's not a gaping
> security hole is to run the SGX enclave in its own fully-seccomp (or
> equivalent) process, with no host application in the same address
> space. Since the host application can't see the contents of the
> enclave to make any determination of whether it's safe to run, running
> it in the same address space only makes sense if the cpu provides
> protection against unwanted accesses to the host's memory from the
> enclave -- and according to you, it doesn't.
The enclave's code (and any initial data) isn't encrypted until the
pages are loaded into the Enclave Page Cache (EPC), which can only
be done by the kernel (via ENCLS[EADD]). In other words, both the
kernel and userspace can vet the code/data before running an enclave.
Practically speaking, an enclave will be coupled with an untrusted
userspace runtime, i.e. it's loader. Enclaves are also measured
as part of their build process, and so the enclave loader needs to
know which pages to add to the measurement, and in what order. I
guess technically speaking an enclave could have zero pages added
to its measurement, but that'd probably be a big red flag that said
enclave is up to something fishy.
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Andy Lutomirski @ 2018-11-07 21:33 UTC (permalink / raw)
To: Rich Felker
Cc: Christopherson, Sean J, Andrew Lutomirski, Dave Hansen, Jann Horn,
Linus Torvalds, Dave Hansen, Jethro Beekman, Jarkko Sakkinen,
Florian Weimer, Linux API, X86 ML, linux-arch, LKML,
Peter Zijlstra, nhorman, npmccallum, Ayoun, Serge,
shay.katz-zamir, linux-sgx, Andy Shevchenko, Thomas
In-Reply-To: <20181107212758.GU5150@brightrain.aerifal.cx>
On Wed, Nov 7, 2018 at 1:28 PM Rich Felker <dalias@libc.org> wrote:
>
> On Tue, Nov 06, 2018 at 03:26:16PM -0800, Sean Christopherson wrote:
> > On Tue, Nov 06, 2018 at 06:17:30PM -0500, Rich Felker wrote:
> > > On Tue, Nov 06, 2018 at 11:02:11AM -0800, Andy Lutomirski wrote:
> > > > On Tue, Nov 6, 2018 at 10:41 AM Dave Hansen <dave.hansen@intel.com> wrote:
> > > > >
> > > > > On 11/6/18 10:20 AM, Andy Lutomirski wrote:
> > > > > > I almost feel like the right solution is to call into SGX on its own
> > > > > > private stack or maybe even its own private address space.
> > > > >
> > > > > Yeah, I had the same gut feeling. Couldn't the debugger even treat the
> > > > > enclave like its own "thread" with its own stack and its own set of
> > > > > registers and context? That seems like a much more workable model than
> > > > > trying to weave it together with the EENTER context.
> > > >
> > > > So maybe the API should be, roughly
> > > >
> > > > sgx_exit_reason_t sgx_enter_enclave(pointer_to_enclave, struct
> > > > host_state *state);
> > > > sgx_exit_reason_t sgx_resume_enclave(same args);
> > > >
> > > > where host_state is something like:
> > > >
> > > > struct host_state {
> > > > unsigned long bp, sp, ax, bx, cx, dx, si, di;
> > > > };
> > > >
> > > > and the values in host_state explicitly have nothing to do with the
> > > > actual host registers. So, if you want to use the outcall mechanism,
> > > > you'd allocate some memory, point sp to that memory, call
> > > > sgx_enter_enclave(), and then read that memory to do the outcall.
> > > >
> > > > Actually implementing this would be distinctly nontrivial, and would
> > > > almost certainly need some degree of kernel help to avoid an explosion
> > > > when a signal gets delivered while we have host_state.sp loaded into
> > > > the actual SP register. Maybe rseq could help with this?
> > > >
> > > > The ISA here is IMO not well thought through.
> > >
> > > Maybe I'm mistaken about some fundamentals here, but my understanding
> > > of SGX is that the whole point is that the host application and the
> > > code running in the enclave are mutually adversarial towards one
> > > another. Do any or all of the proposed protocols here account for this
> > > and fully protect the host application from malicious code in the
> > > enclave? It seems that having control over the register file on exit
> > > from the enclave is fundamentally problematic but I assume there must
> > > be some way I'm missing that this is fixed up.
> >
> > SGX provides protections for the enclave but not the other way around.
> > The kernel has all of its normal non-SGX protections in place, but the
> > enclave can certainly wreak havoc on its userspace process. The basic
> > design idea is that the enclave is a specialized .so that gets extra
> > security protections but is still effectively part of the overall
> > application, e.g. it has full access to its host userspace process'
> > virtual memory.
>
> In that case it seems like the only way to use SGX that's not a gaping
> security hole is to run the SGX enclave in its own fully-seccomp (or
> equivalent) process, with no host application in the same address
> space. Since the host application can't see the contents of the
> enclave to make any determination of whether it's safe to run, running
> it in the same address space only makes sense if the cpu provides
> protection against unwanted accesses to the host's memory from the
> enclave -- and according to you, it doesn't.
>
I think the theory is that the enclave is shipped with the host application.
That being said, a way to run the enclave in an address space that has
basically nothing else (except an ENCLU instruction as a trampoline)
would be quite nice.
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Rich Felker @ 2018-11-07 21:27 UTC (permalink / raw)
To: Sean Christopherson
Cc: Andy Lutomirski, Dave Hansen, Jann Horn, Linus Torvalds,
Dave Hansen, Jethro Beekman, Jarkko Sakkinen, Florian Weimer,
Linux API, X86 ML, linux-arch, LKML, Peter Zijlstra, nhorman,
npmccallum, Ayoun, Serge, shay.katz-zamir, linux-sgx,
Andy Shevchenko, Thomas Gleixner, Ingo Molnar, Borislav
In-Reply-To: <20181106232616.GA11101@linux.intel.com>
On Tue, Nov 06, 2018 at 03:26:16PM -0800, Sean Christopherson wrote:
> On Tue, Nov 06, 2018 at 06:17:30PM -0500, Rich Felker wrote:
> > On Tue, Nov 06, 2018 at 11:02:11AM -0800, Andy Lutomirski wrote:
> > > On Tue, Nov 6, 2018 at 10:41 AM Dave Hansen <dave.hansen@intel.com> wrote:
> > > >
> > > > On 11/6/18 10:20 AM, Andy Lutomirski wrote:
> > > > > I almost feel like the right solution is to call into SGX on its own
> > > > > private stack or maybe even its own private address space.
> > > >
> > > > Yeah, I had the same gut feeling. Couldn't the debugger even treat the
> > > > enclave like its own "thread" with its own stack and its own set of
> > > > registers and context? That seems like a much more workable model than
> > > > trying to weave it together with the EENTER context.
> > >
> > > So maybe the API should be, roughly
> > >
> > > sgx_exit_reason_t sgx_enter_enclave(pointer_to_enclave, struct
> > > host_state *state);
> > > sgx_exit_reason_t sgx_resume_enclave(same args);
> > >
> > > where host_state is something like:
> > >
> > > struct host_state {
> > > unsigned long bp, sp, ax, bx, cx, dx, si, di;
> > > };
> > >
> > > and the values in host_state explicitly have nothing to do with the
> > > actual host registers. So, if you want to use the outcall mechanism,
> > > you'd allocate some memory, point sp to that memory, call
> > > sgx_enter_enclave(), and then read that memory to do the outcall.
> > >
> > > Actually implementing this would be distinctly nontrivial, and would
> > > almost certainly need some degree of kernel help to avoid an explosion
> > > when a signal gets delivered while we have host_state.sp loaded into
> > > the actual SP register. Maybe rseq could help with this?
> > >
> > > The ISA here is IMO not well thought through.
> >
> > Maybe I'm mistaken about some fundamentals here, but my understanding
> > of SGX is that the whole point is that the host application and the
> > code running in the enclave are mutually adversarial towards one
> > another. Do any or all of the proposed protocols here account for this
> > and fully protect the host application from malicious code in the
> > enclave? It seems that having control over the register file on exit
> > from the enclave is fundamentally problematic but I assume there must
> > be some way I'm missing that this is fixed up.
>
> SGX provides protections for the enclave but not the other way around.
> The kernel has all of its normal non-SGX protections in place, but the
> enclave can certainly wreak havoc on its userspace process. The basic
> design idea is that the enclave is a specialized .so that gets extra
> security protections but is still effectively part of the overall
> application, e.g. it has full access to its host userspace process'
> virtual memory.
In that case it seems like the only way to use SGX that's not a gaping
security hole is to run the SGX enclave in its own fully-seccomp (or
equivalent) process, with no host application in the same address
space. Since the host application can't see the contents of the
enclave to make any determination of whether it's safe to run, running
it in the same address space only makes sense if the cpu provides
protection against unwanted accesses to the host's memory from the
enclave -- and according to you, it doesn't.
Rich
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Dave Hansen @ 2018-11-07 20:56 UTC (permalink / raw)
To: Sean Christopherson, Andy Lutomirski
Cc: Jann Horn, Linus Torvalds, Rich Felker, Dave Hansen,
Jethro Beekman, Jarkko Sakkinen, Florian Weimer, Linux API,
X86 ML, linux-arch, LKML, Peter Zijlstra, nhorman, npmccallum,
Ayoun, Serge, shay.katz-zamir, linux-sgx, Andy Shevchenko,
Thomas Gleixner, Ingo Molnar, Borislav Petkov
In-Reply-To: <20181107190114.GA26603@linux.intel.com>
On 11/7/18 11:01 AM, Sean Christopherson wrote:
> Going off comments in similar code related to UMIP, we'd need to figure
> out how to handle protection keys.
There are two options:
1. Don't depend on the userspace mapping. Do get_user_pages() to find
the instruction in the kernel direct map, and use that.
2. Do a WRPKRU that allows read access, do the read, then put PKRU back.
This is a pain because of preemption and all that jazz.
Right now, we just let the prefetch instruction detection fail if you
mark it unreadable with pkeys. Tough cookies, basically. But, that's
just the kernel being nice, but you need it for functionality, so it's
tougher.
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Sean Christopherson @ 2018-11-07 19:01 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Dave Hansen, Jann Horn, Linus Torvalds, Rich Felker, Dave Hansen,
Jethro Beekman, Jarkko Sakkinen, Florian Weimer, Linux API,
X86 ML, linux-arch, LKML, Peter Zijlstra, nhorman, npmccallum,
Ayoun, Serge, shay.katz-zamir, linux-sgx, Andy Shevchenko,
Thomas Gleixner, Ingo Molnar, Borislav Petkov <bp@
In-Reply-To: <20181107153452.GB22972@linux.intel.com>
On Wed, Nov 07, 2018 at 07:34:52AM -0800, Sean Christopherson wrote:
> On Tue, Nov 06, 2018 at 05:17:14PM -0800, Andy Lutomirski wrote:
> > On Tue, Nov 6, 2018 at 4:02 PM Sean Christopherson
> > <sean.j.christopherson@intel.com> wrote:
> > >
> > > On Tue, Nov 06, 2018 at 03:39:48PM -0800, Andy Lutomirski wrote:
> > > > On Tue, Nov 6, 2018 at 3:35 PM Sean Christopherson
> > > > <sean.j.christopherson@intel.com> wrote:
> > > > >
> > > > > Sorry if I'm beating a dead horse, but what if we only did fixup on ENCLU
> > > > > with a specific (ignored) prefix pattern? I.e. effectively make the magic
> > > > > fixup opt-in, falling back to signals. Jamming RIP to skip ENCLU isn't
> > > > > that far off the architecture, e.g. EENTER stuffs RCX with the next RIP so
> > > > > that the enclave can EEXIT to immediately after the EENTER location.
> > > > >
> > > >
> > > > How does that even work, though? On an AEX, RIP points to the ERESUME
> > > > instruction, not the EENTER instruction, so if we skip it we just end
> > > > up in lala land.
> > >
> > > Userspace would obviously need to be aware of the fixup behavior, but
> > > it actually works out fairly nicely to have a separate path for ERESUME
> > > fixup since a fault on EENTER is generally fatal, whereas as a fault on
> > > ERESUME might be recoverable.
> > >
> >
> > Hmm.
> >
> > >
> > > do_eenter:
> > > mov tcs, %rbx
> > > lea async_exit, %rcx
> > > mov $EENTER, %rax
> > > ENCLU
> >
> > Or SOME_SILLY_PREFIX ENCLU?
>
> Yeah, forgot to include that.
>
> > >
> > > /*
> > > * EEXIT or EENTER faulted. In the latter case, %RAX already holds some
> > > * fault indicator, e.g. -EFAULT.
> > > */
> > > eexit_or_eenter_fault:
> > > ret
> >
> > But userspace wants to know whether it was a fault or not. So I think
> > we either need two landing pads or we need to hijack a flag bit (are
> > there any known-zeroed flag bits after EEXIT?) to say whether it was a
> > fault. And, if it was a fault, we should give the vector, the
> > sanitized error code, and possibly CR2.
>
> As Jethro mentioned, RAX will always be 4 on a successful EEXIT, so we
> can use RAX to indicate a fault. That's what I was trying to imply with
> EFAULT. Here's the reg stuffing I use for the POC:
>
> regs->ax = EFAULT;
> regs->di = trapnr;
> regs->si = error_code;
> regs->dx = address;
>
>
> Well-known RAX values also means the kernel fault handlers only need to
> look for SOME_SILLY_PREFIX ENCLU if RAX==2 || RAX==3, i.e. the fault
> occurred on EENTER or in an enclave (RAX is set to ERESUME's leaf as
> part of the asynchronous enlcave exit flow).
POC kernel code, 64-bit only.
Limiting this to 64-bit isn't necessary, but it makes the code prettier
and allows using REX as the magic prefix. I like the idea of using REX
because it seems least likely to be repurposed for yet another new
feature. I have no idea if 64-bit only will fly with the SDK folks.
Going off comments in similar code related to UMIP, we'd need to figure
out how to handle protection keys.
/* REX with all bits set, ignored by ENCLU. */
#define SGX_DO_ENCLU_FIXUP 0x4F
#define SGX_ENCLU_OPCODE0 0x0F
#define SGX_ENCLU_OPCODE1 0x01
#define SGX_ENCLU_OPCODE2 0xD7
/* ENCLU is a three-byte opcode, plus one byte for the magic prefix. */
#define SGX_ENCLU_FIXUP_INSN_LEN 4
static int sgx_detect_enclu(struct pt_regs *regs)
{
unsigned char buf[SGX_ENCLU_FIXUP_INSN_LEN];
/* Look for EENTER or ERESUME in RAX, 64-bit mode only. */
if (!regs || (regs->ax != 2 && regs->ax != 3) || !user_64bit_mode(regs))
return 0;
if (copy_from_user(buf, (void __user *)(regs->ip), sizeof(buf)))
return 0;
if (buf[0] == SGX_DO_ENCLU_FIXUP &&
buf[1] == SGX_ENCLU_OPCODE0 &&
buf[2] == SGX_ENCLU_OPCODE1 &&
buf[3] == SGX_ENCLU_OPCODE2)
return SGX_ENCLU_FIXUP_INSN_LEN;
return 0;
}
bool sgx_fixup_enclu_fault(struct pt_regs *regs, int trapnr,
unsigned long error_code, unsigned long address)
{
int insn_len;
insn_len = sgx_detect_enclu(regs);
if (!insn_len)
return false;
regs->ip += insn_len;
regs->ax = EFAULT;
regs->di = trapnr;
regs->si = error_code;
regs->dx = address;
return true;
}
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Sean Christopherson @ 2018-11-07 15:34 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Dave Hansen, Jann Horn, Linus Torvalds, Rich Felker, Dave Hansen,
Jethro Beekman, Jarkko Sakkinen, Florian Weimer, Linux API,
X86 ML, linux-arch, LKML, Peter Zijlstra, nhorman, npmccallum,
Ayoun, Serge, shay.katz-zamir, linux-sgx, Andy Shevchenko,
Thomas Gleixner, Ingo Molnar, Borislav Petkov <bp@
In-Reply-To: <CALCETrV_sMOeQGRkxoA8KMxEizagB9bD-mB2T-ro3F6O6OX9Xw@mail.gmail.com>
On Tue, Nov 06, 2018 at 05:17:14PM -0800, Andy Lutomirski wrote:
> On Tue, Nov 6, 2018 at 4:02 PM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
> >
> > On Tue, Nov 06, 2018 at 03:39:48PM -0800, Andy Lutomirski wrote:
> > > On Tue, Nov 6, 2018 at 3:35 PM Sean Christopherson
> > > <sean.j.christopherson@intel.com> wrote:
> > > >
> > > > On Tue, Nov 06, 2018 at 03:00:56PM -0800, Andy Lutomirski wrote:
> > > > >
> > > > >
> > > > > >> On Nov 6, 2018, at 1:59 PM, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
> > > > > >>
> > > > > >>> On Tue, 2018-11-06 at 13:41 -0800, Andy Lutomirski wrote:
> > > > > >> Sean, how does the current SDK AEX handler decide whether to do
> > > > > >> EENTER, ERESUME, or just bail and consider the enclave dead? It seems
> > > > > >> like the *CPU* could give a big hint, but I don't see where there is
> > > > > >> any architectural indication of why the AEX code got called or any
> > > > > >> obvious way for the user code to know whether the exit was fixed up by
> > > > > >> the kernel?
> > > > > >
> > > > > > The SDK "unconditionally" does ERESUME at the AEP location, but that's
> > > > > > bit misleading because its signal handler may muck with the context's
> > > > > > RIP, e.g. to abort the enclave on a fatal fault.
> > > > > >
> > > > > > On an event/exception from within an enclave, the event is immediately
> > > > > > delivered after loading synthetic state and changing RIP to the AEP.
> > > > > > In other words, jamming CPU state is essentially a bunch of vectoring
> > > > > > ucode preamble, but from software's perspective it's a normal event
> > > > > > that happens to point at the AEP instead of somewhere in the enclave.
> > > > > > And because the signals the SDK cares about are all synchronous, the
> > > > > > SDK can simply hardcode ERESUME at the AEP since all of the fault logic
> > > > > > resides in its signal handler. IRQs and whatnot simply trampoline back
> > > > > > into the enclave.
> > > > > >
> > > > > > Userspace can do something funky instead of ERESUME, but only *after*
> > > > > > IRET/RSM/VMRESUME has returned to the AEP location, and in Linux's
> > > > > > case, after the trap handler has run.
> > > > > >
> > > > > > Jumping back a bit, how much do we care about preventing userspace
> > > > > > from doing stupid things?
> > > > >
> > > > > My general feeling is that userspace should be allowed to do apparently
> > > > > stupid things. For example, as far as the kernel is concerned, Wine and
> > > > > DOSEMU are just user programs that do stupid things. Linux generally tries
> > > > > to provide a reasonably complete view of architectural behavior. This is
> > > > > in contrast to, say, Windows, where IIUC doing an unapproved WRFSBASE May
> > > > > cause very odd behavior indeed. So magic fixups that do non-architectural
> > > > > things are not so great.
> > > >
> > > > Sorry if I'm beating a dead horse, but what if we only did fixup on ENCLU
> > > > with a specific (ignored) prefix pattern? I.e. effectively make the magic
> > > > fixup opt-in, falling back to signals. Jamming RIP to skip ENCLU isn't
> > > > that far off the architecture, e.g. EENTER stuffs RCX with the next RIP so
> > > > that the enclave can EEXIT to immediately after the EENTER location.
> > > >
> > >
> > > How does that even work, though? On an AEX, RIP points to the ERESUME
> > > instruction, not the EENTER instruction, so if we skip it we just end
> > > up in lala land.
> >
> > Userspace would obviously need to be aware of the fixup behavior, but
> > it actually works out fairly nicely to have a separate path for ERESUME
> > fixup since a fault on EENTER is generally fatal, whereas as a fault on
> > ERESUME might be recoverable.
> >
>
> Hmm.
>
> >
> > do_eenter:
> > mov tcs, %rbx
> > lea async_exit, %rcx
> > mov $EENTER, %rax
> > ENCLU
>
> Or SOME_SILLY_PREFIX ENCLU?
Yeah, forgot to include that.
> >
> > /*
> > * EEXIT or EENTER faulted. In the latter case, %RAX already holds some
> > * fault indicator, e.g. -EFAULT.
> > */
> > eexit_or_eenter_fault:
> > ret
>
> But userspace wants to know whether it was a fault or not. So I think
> we either need two landing pads or we need to hijack a flag bit (are
> there any known-zeroed flag bits after EEXIT?) to say whether it was a
> fault. And, if it was a fault, we should give the vector, the
> sanitized error code, and possibly CR2.
As Jethro mentioned, RAX will always be 4 on a successful EEXIT, so we
can use RAX to indicate a fault. That's what I was trying to imply with
EFAULT. Here's the reg stuffing I use for the POC:
regs->ax = EFAULT;
regs->di = trapnr;
regs->si = error_code;
regs->dx = address;
Well-known RAX values also means the kernel fault handlers only need to
look for SOME_SILLY_PREFIX ENCLU if RAX==2 || RAX==3, i.e. the fault
occurred on EENTER or in an enclave (RAX is set to ERESUME's leaf as
part of the asynchronous enlcave exit flow).
> >
> > async_exit:
> > ENCLU
>
> Same prefix here, right?
>
> >
> > fixup_handler:
> > <do fault stuff>
>
> This whole thing is a bit odd, but not necessarily a terrible idea.
^ permalink raw reply
* Re: [PATCH v5 02/15] sched/core: make sched_setattr able to tune the current policy
From: Peter Zijlstra @ 2018-11-07 13:58 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan,
linux-api
In-Reply-To: <20181107135039.GD14309@e110439-lin>
On Wed, Nov 07, 2018 at 01:50:39PM +0000, Patrick Bellasi wrote:
> On 07-Nov 13:11, Peter Zijlstra wrote:
> > On Mon, Oct 29, 2018 at 06:32:56PM +0000, Patrick Bellasi wrote:
> >
> > > @@ -50,11 +52,13 @@
> > > #define SCHED_FLAG_RESET_ON_FORK 0x01
> > > #define SCHED_FLAG_RECLAIM 0x02
> > > #define SCHED_FLAG_DL_OVERRUN 0x04
> > > -#define SCHED_FLAG_UTIL_CLAMP 0x08
> > > +#define SCHED_FLAG_TUNE_POLICY 0x08
> > > +#define SCHED_FLAG_UTIL_CLAMP 0x10
> >
> > That seems to suggest you want to do this patch first, but you didn't..
>
> I've kept it here just to better highlight this change, suggested by
> Juri, since we was not entirely sure you are fine with it...
>
> If you think it's ok adding a SCHED_FLAG_TUNE_POLICY behavior to the
> sched_setattr syscall, I can certainly squash into the previous patch,
> which gives more context on why we need it.
I'm fine with the idea I think. It's the details I worry about. Which
fields in particular are not updated with this. Are flags?
Also, I'm not too keen on the name; since it explicitly does not modify
the policy and its related parameters, so TUNE_POLICY is actively wrong.
But the thing that confused me most is how fiddled the numbers to fit
this before UTIL_CLAMP.
> Since we are at that, are we supposed to document some{where,how}
> these API changes ?
I'm pretty sure there's a manpage somewhere... SCHED_SETATTR(2) seems to
exist on my machine. So that wants updates.
^ permalink raw reply
* Re: [PATCH v5 02/15] sched/core: make sched_setattr able to tune the current policy
From: Patrick Bellasi @ 2018-11-07 13:50 UTC (permalink / raw)
To: Peter Zijlstra
Cc: linux-kernel, linux-pm, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan,
linux-api
In-Reply-To: <20181107121126.GN9781@hirez.programming.kicks-ass.net>
On 07-Nov 13:11, Peter Zijlstra wrote:
> On Mon, Oct 29, 2018 at 06:32:56PM +0000, Patrick Bellasi wrote:
>
> > @@ -50,11 +52,13 @@
> > #define SCHED_FLAG_RESET_ON_FORK 0x01
> > #define SCHED_FLAG_RECLAIM 0x02
> > #define SCHED_FLAG_DL_OVERRUN 0x04
> > -#define SCHED_FLAG_UTIL_CLAMP 0x08
> > +#define SCHED_FLAG_TUNE_POLICY 0x08
> > +#define SCHED_FLAG_UTIL_CLAMP 0x10
>
> That seems to suggest you want to do this patch first, but you didn't..
I've kept it here just to better highlight this change, suggested by
Juri, since we was not entirely sure you are fine with it...
If you think it's ok adding a SCHED_FLAG_TUNE_POLICY behavior to the
sched_setattr syscall, I can certainly squash into the previous patch,
which gives more context on why we need it.
Otherwise, if we want to keep these bits better isolated for possible
future bisects, I can also move it at the beginning of the series.
What do you like best ?
Since we are at that, are we supposed to document some{where,how}
these API changes ?
--
#include <best/regards.h>
Patrick Bellasi
^ permalink raw reply
* Re: [PATCH v5 02/15] sched/core: make sched_setattr able to tune the current policy
From: Peter Zijlstra @ 2018-11-07 12:11 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan,
linux-api
In-Reply-To: <20181029183311.29175-3-patrick.bellasi@arm.com>
On Mon, Oct 29, 2018 at 06:32:56PM +0000, Patrick Bellasi wrote:
> @@ -50,11 +52,13 @@
> #define SCHED_FLAG_RESET_ON_FORK 0x01
> #define SCHED_FLAG_RECLAIM 0x02
> #define SCHED_FLAG_DL_OVERRUN 0x04
> -#define SCHED_FLAG_UTIL_CLAMP 0x08
> +#define SCHED_FLAG_TUNE_POLICY 0x08
> +#define SCHED_FLAG_UTIL_CLAMP 0x10
That seems to suggest you want to do this patch first, but you didn't..
^ permalink raw reply
* Re: [PATCH v5 01/15] sched/core: uclamp: extend sched_setattr to support utilization clamping
From: Peter Zijlstra @ 2018-11-07 12:09 UTC (permalink / raw)
To: Patrick Bellasi
Cc: linux-kernel, linux-pm, Ingo Molnar, Tejun Heo,
Rafael J . Wysocki, Vincent Guittot, Viresh Kumar, Paul Turner,
Quentin Perret, Dietmar Eggemann, Morten Rasmussen, Juri Lelli,
Todd Kjos, Joel Fernandes, Steve Muckle, Suren Baghdasaryan,
Randy Dunlap, linux-api
In-Reply-To: <20181029183311.29175-2-patrick.bellasi@arm.com>
On Mon, Oct 29, 2018 at 06:32:55PM +0000, Patrick Bellasi wrote:
> diff --git a/include/uapi/linux/sched.h b/include/uapi/linux/sched.h
> index 22627f80063e..c27d6e81517b 100644
> --- a/include/uapi/linux/sched.h
> +++ b/include/uapi/linux/sched.h
> @@ -50,9 +50,11 @@
> #define SCHED_FLAG_RESET_ON_FORK 0x01
> #define SCHED_FLAG_RECLAIM 0x02
> #define SCHED_FLAG_DL_OVERRUN 0x04
> +#define SCHED_FLAG_UTIL_CLAMP 0x08
>
> #define SCHED_FLAG_ALL (SCHED_FLAG_RESET_ON_FORK | \
> SCHED_FLAG_RECLAIM | \
> - SCHED_FLAG_DL_OVERRUN)
> + SCHED_FLAG_DL_OVERRUN | \
> + SCHED_FLAG_UTIL_CLAMP)
>
> #endif /* _UAPI_LINUX_SCHED_H */
> diff --git a/include/uapi/linux/sched/types.h b/include/uapi/linux/sched/types.h
> index 10fbb8031930..fde7301ed28c 100644
> --- a/include/uapi/linux/sched/types.h
> +++ b/include/uapi/linux/sched/types.h
> @@ -9,6 +9,7 @@ struct sched_param {
> };
>
> #define SCHED_ATTR_SIZE_VER0 48 /* sizeof first published struct */
> +#define SCHED_ATTR_SIZE_VER1 56 /* add: util_{min,max} */
>
> /*
> * Extended scheduling parameters data structure.
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -4533,6 +4533,10 @@ static int sched_copy_attr(struct sched_
if (ret)
return -EFAULT;
+ if ((attr->sched_flags & SCHED_FLAG_UTIL_CLAMP) &&
+ size < SCHED_ATTR_SIZE_VER1)
+ return -EINVAL;
+
/*
* XXX: Do we want to be lenient like existing syscalls; or do we want
* to be strict and return an error on out-of-bounds values?
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Jethro Beekman @ 2018-11-07 6:47 UTC (permalink / raw)
To: Andy Lutomirski, Christopherson, Sean J
Cc: Dave Hansen, Jann Horn, Linus Torvalds, Rich Felker, Dave Hansen,
Jarkko Sakkinen, Florian Weimer, Linux API, X86 ML, linux-arch,
LKML, Peter Zijlstra, nhorman@redhat.com, npmccallum@redhat.com,
Ayoun, Serge, shay.katz-zamir@intel.com,
linux-sgx@vger.kernel.org, Andy Shevchenko, Thomas
In-Reply-To: <CALCETrV_sMOeQGRkxoA8KMxEizagB9bD-mB2T-ro3F6O6OX9Xw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 797 bytes --]
On 2018-11-07 02:17, Andy Lutomirski wrote:
> On Tue, Nov 6, 2018 at 4:02 PM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
>>
>> /*
>> * EEXIT or EENTER faulted. In the latter case, %RAX already holds some
>> * fault indicator, e.g. -EFAULT.
>> */
>> eexit_or_eenter_fault:
>> ret
>
> But userspace wants to know whether it was a fault or not. So I think
> we either need two landing pads or we need to hijack a flag bit (are
> there any known-zeroed flag bits after EEXIT?) to say whether it was a
> fault. And, if it was a fault, we should give the vector, the
> sanitized error code, and possibly CR2.
On AEX, %rax will contain ENCLU_LEAF_ERESUME (0x3). On EEXIT, %rax will
contain ENCLU_LEAF_EEXIT (0x4).
--
Jethro Beekman | Fortanix
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3990 bytes --]
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Andy Lutomirski @ 2018-11-07 1:17 UTC (permalink / raw)
To: Christopherson, Sean J
Cc: Andrew Lutomirski, Dave Hansen, Jann Horn, Linus Torvalds,
Rich Felker, Dave Hansen, Jethro Beekman, Jarkko Sakkinen,
Florian Weimer, Linux API, X86 ML, linux-arch, LKML,
Peter Zijlstra, nhorman, npmccallum, Ayoun, Serge,
shay.katz-zamir, linux-sgx, Andy Shevchenko, Thomas Gleixner
In-Reply-To: <20181107000235.GC11101@linux.intel.com>
On Tue, Nov 6, 2018 at 4:02 PM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> On Tue, Nov 06, 2018 at 03:39:48PM -0800, Andy Lutomirski wrote:
> > On Tue, Nov 6, 2018 at 3:35 PM Sean Christopherson
> > <sean.j.christopherson@intel.com> wrote:
> > >
> > > On Tue, Nov 06, 2018 at 03:00:56PM -0800, Andy Lutomirski wrote:
> > > >
> > > >
> > > > >> On Nov 6, 2018, at 1:59 PM, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
> > > > >>
> > > > >>> On Tue, 2018-11-06 at 13:41 -0800, Andy Lutomirski wrote:
> > > > >> Sean, how does the current SDK AEX handler decide whether to do
> > > > >> EENTER, ERESUME, or just bail and consider the enclave dead? It seems
> > > > >> like the *CPU* could give a big hint, but I don't see where there is
> > > > >> any architectural indication of why the AEX code got called or any
> > > > >> obvious way for the user code to know whether the exit was fixed up by
> > > > >> the kernel?
> > > > >
> > > > > The SDK "unconditionally" does ERESUME at the AEP location, but that's
> > > > > bit misleading because its signal handler may muck with the context's
> > > > > RIP, e.g. to abort the enclave on a fatal fault.
> > > > >
> > > > > On an event/exception from within an enclave, the event is immediately
> > > > > delivered after loading synthetic state and changing RIP to the AEP.
> > > > > In other words, jamming CPU state is essentially a bunch of vectoring
> > > > > ucode preamble, but from software's perspective it's a normal event
> > > > > that happens to point at the AEP instead of somewhere in the enclave.
> > > > > And because the signals the SDK cares about are all synchronous, the
> > > > > SDK can simply hardcode ERESUME at the AEP since all of the fault logic
> > > > > resides in its signal handler. IRQs and whatnot simply trampoline back
> > > > > into the enclave.
> > > > >
> > > > > Userspace can do something funky instead of ERESUME, but only *after*
> > > > > IRET/RSM/VMRESUME has returned to the AEP location, and in Linux's
> > > > > case, after the trap handler has run.
> > > > >
> > > > > Jumping back a bit, how much do we care about preventing userspace
> > > > > from doing stupid things?
> > > >
> > > > My general feeling is that userspace should be allowed to do apparently
> > > > stupid things. For example, as far as the kernel is concerned, Wine and
> > > > DOSEMU are just user programs that do stupid things. Linux generally tries
> > > > to provide a reasonably complete view of architectural behavior. This is
> > > > in contrast to, say, Windows, where IIUC doing an unapproved WRFSBASE May
> > > > cause very odd behavior indeed. So magic fixups that do non-architectural
> > > > things are not so great.
> > >
> > > Sorry if I'm beating a dead horse, but what if we only did fixup on ENCLU
> > > with a specific (ignored) prefix pattern? I.e. effectively make the magic
> > > fixup opt-in, falling back to signals. Jamming RIP to skip ENCLU isn't
> > > that far off the architecture, e.g. EENTER stuffs RCX with the next RIP so
> > > that the enclave can EEXIT to immediately after the EENTER location.
> > >
> >
> > How does that even work, though? On an AEX, RIP points to the ERESUME
> > instruction, not the EENTER instruction, so if we skip it we just end
> > up in lala land.
>
> Userspace would obviously need to be aware of the fixup behavior, but
> it actually works out fairly nicely to have a separate path for ERESUME
> fixup since a fault on EENTER is generally fatal, whereas as a fault on
> ERESUME might be recoverable.
>
Hmm.
>
> do_eenter:
> mov tcs, %rbx
> lea async_exit, %rcx
> mov $EENTER, %rax
> ENCLU
Or SOME_SILLY_PREFIX ENCLU?
>
> /*
> * EEXIT or EENTER faulted. In the latter case, %RAX already holds some
> * fault indicator, e.g. -EFAULT.
> */
> eexit_or_eenter_fault:
> ret
But userspace wants to know whether it was a fault or not. So I think
we either need two landing pads or we need to hijack a flag bit (are
there any known-zeroed flag bits after EEXIT?) to say whether it was a
fault. And, if it was a fault, we should give the vector, the
sanitized error code, and possibly CR2.
>
> async_exit:
> ENCLU
Same prefix here, right?
>
> fixup_handler:
> <do fault stuff>
This whole thing is a bit odd, but not necessarily a terrible idea.
>
> > How averse would everyone be to making enclave entry be a syscall?
> > The user code would do sys_sgx_enter_enclave(), and the kernel would
> > stash away the register state (vm86()-style), point RIP to the vDSO's
> > ENCLU instruction, point RCX to another vDSO ENCLU instruction, and
> > SYSRET. The trap handlers would understand what's going on and
> > restore register state accordingly.
>
> Wouldn't that blast away any stack changes made by the enclave?
Yes, but I was imagining that it would stash the registers into the
struct host_state thing I made up :)
^ permalink raw reply
* Re: [PATCH v2 1/3] bpf: allow zero-initializing hash map seed
From: Song Liu @ 2018-11-07 0:39 UTC (permalink / raw)
To: lmb; +Cc: Alexei Starovoitov, Daniel Borkmann, Networking, linux-api
In-Reply-To: <CACAyw98RnRZtqMYcS5GZxhf-F-z=sW+Ki+_PBADG+V_qZiuA1Q@mail.gmail.com>
On Thu, Oct 25, 2018 at 8:12 AM Lorenz Bauer <lmb@cloudflare.com> wrote:
>
> On Tue, 9 Oct 2018 at 01:08, Song Liu <liu.song.a23@gmail.com> wrote:
> >
> > > --- a/include/uapi/linux/bpf.h
> > > +++ b/include/uapi/linux/bpf.h
> > > @@ -253,6 +253,8 @@ enum bpf_attach_type {
> > > #define BPF_F_NO_COMMON_LRU (1U << 1)
> > > /* Specify numa node during map creation */
> > > #define BPF_F_NUMA_NODE (1U << 2)
> > > +/* Zero-initialize hash function seed. This should only be used for testing. */
> > > +#define BPF_F_ZERO_SEED (1U << 6)
> >
> > Please add this line after
> > #define BPF_F_STACK_BUILD_ID (1U << 5)
>
> I wanted to keep the flags for BPF_MAP_CREATE grouped together.
> Maybe the correct value is (1U << 3)? It seemed like the other flags
> were allocated to avoid
> overlap between different BPF commands, however, so I tried to follow suit.
I think it should be (1U << 6). We probably should move BPF_F_QUERY_EFFECTIVE
to after BPF_F_STACK_BUILD_ID (and BPF_F_ZERO_SEED).
Also, please rebase against the latest bpf-next tree and resubmit the set.
Thanks,
Song
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Sean Christopherson @ 2018-11-07 0:02 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Dave Hansen, Jann Horn, Linus Torvalds, Rich Felker, Dave Hansen,
Jethro Beekman, Jarkko Sakkinen, Florian Weimer, Linux API,
X86 ML, linux-arch, LKML, Peter Zijlstra, nhorman, npmccallum,
Ayoun, Serge, shay.katz-zamir, linux-sgx, Andy Shevchenko,
Thomas Gleixner, Ingo Molnar, Borislav Petkov <bp@
In-Reply-To: <CALCETrVySfV64YN7DWf3rsAxfiugJKsRJCNmEn-AKQ4dPYeG4Q@mail.gmail.com>
On Tue, Nov 06, 2018 at 03:39:48PM -0800, Andy Lutomirski wrote:
> On Tue, Nov 6, 2018 at 3:35 PM Sean Christopherson
> <sean.j.christopherson@intel.com> wrote:
> >
> > On Tue, Nov 06, 2018 at 03:00:56PM -0800, Andy Lutomirski wrote:
> > >
> > >
> > > >> On Nov 6, 2018, at 1:59 PM, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
> > > >>
> > > >>> On Tue, 2018-11-06 at 13:41 -0800, Andy Lutomirski wrote:
> > > >> Sean, how does the current SDK AEX handler decide whether to do
> > > >> EENTER, ERESUME, or just bail and consider the enclave dead? It seems
> > > >> like the *CPU* could give a big hint, but I don't see where there is
> > > >> any architectural indication of why the AEX code got called or any
> > > >> obvious way for the user code to know whether the exit was fixed up by
> > > >> the kernel?
> > > >
> > > > The SDK "unconditionally" does ERESUME at the AEP location, but that's
> > > > bit misleading because its signal handler may muck with the context's
> > > > RIP, e.g. to abort the enclave on a fatal fault.
> > > >
> > > > On an event/exception from within an enclave, the event is immediately
> > > > delivered after loading synthetic state and changing RIP to the AEP.
> > > > In other words, jamming CPU state is essentially a bunch of vectoring
> > > > ucode preamble, but from software's perspective it's a normal event
> > > > that happens to point at the AEP instead of somewhere in the enclave.
> > > > And because the signals the SDK cares about are all synchronous, the
> > > > SDK can simply hardcode ERESUME at the AEP since all of the fault logic
> > > > resides in its signal handler. IRQs and whatnot simply trampoline back
> > > > into the enclave.
> > > >
> > > > Userspace can do something funky instead of ERESUME, but only *after*
> > > > IRET/RSM/VMRESUME has returned to the AEP location, and in Linux's
> > > > case, after the trap handler has run.
> > > >
> > > > Jumping back a bit, how much do we care about preventing userspace
> > > > from doing stupid things?
> > >
> > > My general feeling is that userspace should be allowed to do apparently
> > > stupid things. For example, as far as the kernel is concerned, Wine and
> > > DOSEMU are just user programs that do stupid things. Linux generally tries
> > > to provide a reasonably complete view of architectural behavior. This is
> > > in contrast to, say, Windows, where IIUC doing an unapproved WRFSBASE May
> > > cause very odd behavior indeed. So magic fixups that do non-architectural
> > > things are not so great.
> >
> > Sorry if I'm beating a dead horse, but what if we only did fixup on ENCLU
> > with a specific (ignored) prefix pattern? I.e. effectively make the magic
> > fixup opt-in, falling back to signals. Jamming RIP to skip ENCLU isn't
> > that far off the architecture, e.g. EENTER stuffs RCX with the next RIP so
> > that the enclave can EEXIT to immediately after the EENTER location.
> >
>
> How does that even work, though? On an AEX, RIP points to the ERESUME
> instruction, not the EENTER instruction, so if we skip it we just end
> up in lala land.
Userspace would obviously need to be aware of the fixup behavior, but
it actually works out fairly nicely to have a separate path for ERESUME
fixup since a fault on EENTER is generally fatal, whereas as a fault on
ERESUME might be recoverable.
do_eenter:
mov tcs, %rbx
lea async_exit, %rcx
mov $EENTER, %rax
ENCLU
/*
* EEXIT or EENTER faulted. In the latter case, %RAX already holds some
* fault indicator, e.g. -EFAULT.
*/
eexit_or_eenter_fault:
ret
async_exit:
ENCLU
fixup_handler:
<do fault stuff>
> How averse would everyone be to making enclave entry be a syscall?
> The user code would do sys_sgx_enter_enclave(), and the kernel would
> stash away the register state (vm86()-style), point RIP to the vDSO's
> ENCLU instruction, point RCX to another vDSO ENCLU instruction, and
> SYSRET. The trap handlers would understand what's going on and
> restore register state accordingly.
Wouldn't that blast away any stack changes made by the enclave?
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Andy Lutomirski @ 2018-11-06 23:39 UTC (permalink / raw)
To: Christopherson, Sean J
Cc: Andrew Lutomirski, Dave Hansen, Jann Horn, Linus Torvalds,
Rich Felker, Dave Hansen, Jethro Beekman, Jarkko Sakkinen,
Florian Weimer, Linux API, X86 ML, linux-arch, LKML,
Peter Zijlstra, nhorman, npmccallum, Ayoun, Serge,
shay.katz-zamir, linux-sgx, Andy Shevchenko, Thomas Gleixner
In-Reply-To: <20181106233515.GB11101@linux.intel.com>
On Tue, Nov 6, 2018 at 3:35 PM Sean Christopherson
<sean.j.christopherson@intel.com> wrote:
>
> On Tue, Nov 06, 2018 at 03:00:56PM -0800, Andy Lutomirski wrote:
> >
> >
> > >> On Nov 6, 2018, at 1:59 PM, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
> > >>
> > >>> On Tue, 2018-11-06 at 13:41 -0800, Andy Lutomirski wrote:
> > >> Sean, how does the current SDK AEX handler decide whether to do
> > >> EENTER, ERESUME, or just bail and consider the enclave dead? It seems
> > >> like the *CPU* could give a big hint, but I don't see where there is
> > >> any architectural indication of why the AEX code got called or any
> > >> obvious way for the user code to know whether the exit was fixed up by
> > >> the kernel?
> > >
> > > The SDK "unconditionally" does ERESUME at the AEP location, but that's
> > > bit misleading because its signal handler may muck with the context's
> > > RIP, e.g. to abort the enclave on a fatal fault.
> > >
> > > On an event/exception from within an enclave, the event is immediately
> > > delivered after loading synthetic state and changing RIP to the AEP.
> > > In other words, jamming CPU state is essentially a bunch of vectoring
> > > ucode preamble, but from software's perspective it's a normal event
> > > that happens to point at the AEP instead of somewhere in the enclave.
> > > And because the signals the SDK cares about are all synchronous, the
> > > SDK can simply hardcode ERESUME at the AEP since all of the fault logic
> > > resides in its signal handler. IRQs and whatnot simply trampoline back
> > > into the enclave.
> > >
> > > Userspace can do something funky instead of ERESUME, but only *after*
> > > IRET/RSM/VMRESUME has returned to the AEP location, and in Linux's
> > > case, after the trap handler has run.
> > >
> > > Jumping back a bit, how much do we care about preventing userspace
> > > from doing stupid things?
> >
> > My general feeling is that userspace should be allowed to do apparently
> > stupid things. For example, as far as the kernel is concerned, Wine and
> > DOSEMU are just user programs that do stupid things. Linux generally tries
> > to provide a reasonably complete view of architectural behavior. This is
> > in contrast to, say, Windows, where IIUC doing an unapproved WRFSBASE May
> > cause very odd behavior indeed. So magic fixups that do non-architectural
> > things are not so great.
>
> Sorry if I'm beating a dead horse, but what if we only did fixup on ENCLU
> with a specific (ignored) prefix pattern? I.e. effectively make the magic
> fixup opt-in, falling back to signals. Jamming RIP to skip ENCLU isn't
> that far off the architecture, e.g. EENTER stuffs RCX with the next RIP so
> that the enclave can EEXIT to immediately after the EENTER location.
>
How does that even work, though? On an AEX, RIP points to the ERESUME
instruction, not the EENTER instruction, so if we skip it we just end
up in lala land.
How averse would everyone be to making enclave entry be a syscall?
The user code would do sys_sgx_enter_enclave(), and the kernel would
stash away the register state (vm86()-style), point RIP to the vDSO's
ENCLU instruction, point RCX to another vDSO ENCLU instruction, and
SYSRET. The trap handlers would understand what's going on and
restore register state accordingly.
On non-Meltdown hardware (hah!) this would even be fairly fast.
--Andy
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Sean Christopherson @ 2018-11-06 23:35 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andy Lutomirski, Dave Hansen, Jann Horn, Linus Torvalds,
Rich Felker, Dave Hansen, Jethro Beekman, Jarkko Sakkinen,
Florian Weimer, Linux API, X86 ML, linux-arch, LKML,
Peter Zijlstra, nhorman, npmccallum, Ayoun, Serge,
shay.katz-zamir, linux-sgx, Andy Shevchenko, Thomas Gleixner
In-Reply-To: <7FF4802E-FBC5-4E6D-A8F6-8A65114F18C7@amacapital.net>
On Tue, Nov 06, 2018 at 03:00:56PM -0800, Andy Lutomirski wrote:
>
>
> >> On Nov 6, 2018, at 1:59 PM, Sean Christopherson <sean.j.christopherson@intel.com> wrote:
> >>
> >>> On Tue, 2018-11-06 at 13:41 -0800, Andy Lutomirski wrote:
> >> Sean, how does the current SDK AEX handler decide whether to do
> >> EENTER, ERESUME, or just bail and consider the enclave dead? It seems
> >> like the *CPU* could give a big hint, but I don't see where there is
> >> any architectural indication of why the AEX code got called or any
> >> obvious way for the user code to know whether the exit was fixed up by
> >> the kernel?
> >
> > The SDK "unconditionally" does ERESUME at the AEP location, but that's
> > bit misleading because its signal handler may muck with the context's
> > RIP, e.g. to abort the enclave on a fatal fault.
> >
> > On an event/exception from within an enclave, the event is immediately
> > delivered after loading synthetic state and changing RIP to the AEP.
> > In other words, jamming CPU state is essentially a bunch of vectoring
> > ucode preamble, but from software's perspective it's a normal event
> > that happens to point at the AEP instead of somewhere in the enclave.
> > And because the signals the SDK cares about are all synchronous, the
> > SDK can simply hardcode ERESUME at the AEP since all of the fault logic
> > resides in its signal handler. IRQs and whatnot simply trampoline back
> > into the enclave.
> >
> > Userspace can do something funky instead of ERESUME, but only *after*
> > IRET/RSM/VMRESUME has returned to the AEP location, and in Linux's
> > case, after the trap handler has run.
> >
> > Jumping back a bit, how much do we care about preventing userspace
> > from doing stupid things?
>
> My general feeling is that userspace should be allowed to do apparently
> stupid things. For example, as far as the kernel is concerned, Wine and
> DOSEMU are just user programs that do stupid things. Linux generally tries
> to provide a reasonably complete view of architectural behavior. This is
> in contrast to, say, Windows, where IIUC doing an unapproved WRFSBASE May
> cause very odd behavior indeed. So magic fixups that do non-architectural
> things are not so great.
Sorry if I'm beating a dead horse, but what if we only did fixup on ENCLU
with a specific (ignored) prefix pattern? I.e. effectively make the magic
fixup opt-in, falling back to signals. Jamming RIP to skip ENCLU isn't
that far off the architecture, e.g. EENTER stuffs RCX with the next RIP so
that the enclave can EEXIT to immediately after the EENTER location.
> (How does the Windows case work? If there’s an exception after the untrusted
> stack allocation and before EEXIT and SEH tries to handle it, how does the
> unwinder figure out where to start?)
No clue, I'll ask and report back.
^ 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