On Wed, 2023-10-25 at 07:31 -0700, Hansen, Dave wrote:
> On 10/19/23 19:53, Haitao Huang wrote:
> > In the EAUG on page fault path, VM_FAULT_OOM is returned when the
> > Enclave Page Cache (EPC) runs out. This may trigger unneeded OOM kill
> > that will not free any EPCs. Return VM_FAULT_SIGBUS instead.
>
> So, when picking an error code and we look the documentation for the
> bits, we see:
>
> > * @VM_FAULT_OOM: Out Of Memory
> > * @VM_FAULT_SIGBUS: Bad access
>
> So if anything we'll need a bit more changelog where you explain how
> running out of enclave memory is more "Bad access" than "Out Of Memory".
> Because on the surface this patch looks wrong.
>
> But that's just a naming thing. What *behavior* is bad here? With the
> old code, what happens? With the new code, what happens? Why is the
> old better than the new?
I think Haitao meant if we return OOM, the core-MM fault handler will believe
the fault couldn't be handled because of running out of memory, and then it
could invoke the OOM killer which might select an unrelated victim who might
have no EPC at all.
If we return SIGBUS, then the faulting process/enclave will get the signal and,
e.g., get killed.
static inline
void do_user_addr_fault(struct pt_regs *regs,
unsigned long error_code,
unsigned long address)
{
...
fault = handle_mm_fault(vma, address, ...);
...
done:
...
if (fault & VM_FAULT_OOM) {
/* Kernel mode? Handle exceptions or die: */
if (!user_mode(regs)) {
kernelmode_fixup_or_oops(regs, error_code, address,
SIGSEGV, SEGV_MAPERR,
ARCH_DEFAULT_PKEY);
return;
}
/*
* We ran out of memory, call the OOM killer, and return the
* userspace (which will retry the fault, or kill us if we got
* oom-killed):
*/
pagefault_out_of_memory();
} else {
if (fault & (VM_FAULT_SIGBUS|VM_FAULT_HWPOISON|
VM_FAULT_HWPOISON_LARGE))
do_sigbus(regs, error_code, address, fault);
else if (fault & VM_FAULT_SIGSEGV)
bad_area_nosemaphore(regs, error_code, address);
else
BUG();
}
}
Btw, Ingo has already queued this patch to tip/urgent:
https://lore.kernel.org/all/169778941056.3135.14169781154210769341.tip-bot2@tip-bot2/T/
(Also, currently the non-EAUG code path (ELDU) in sgx_vma_fault() also returns
SIGBUS if it fails to allocate EPC, so making EAUG code path return SIGBUS also
matches the ELDU path.)
