Re: [RFC PATCH 0/4] SGX shmem backing store issue

Reinette Chatre
2022-04-29T18:50:56Z

Hi Dave,

On 4/28/2022 2:12 PM, Dave Hansen wrote:
> On 4/28/22 13:11, Reinette Chatre wrote:
>> ELDU returned 1073741837 (0x4000000d)
>> WARNING: CPU: 72 PID: 24407 at arch/x86/kernel/cpu/sgx/encl.c:81 sgx_encl_eldu+0x3cf/0x400
>> ...
>> Call Trace:
>> <TASK>
>> ? xa_load+0x6e/0xa0
>> __sgx_encl_load_page+0x3d/0x80
>> sgx_encl_load_page_in_vma+0x4a/0x60
>> sgx_vma_fault+0x7f/0x3b0
>
> First of all, thanks for all the work to narrow this down.
>
> It sounds like there are probably at least two failure modes at play here:
>
> 1. shmem_read_mapping_page_gfp() is called to retrieve an
> existing page, but an empty one is allocated instead. ELDU
> fails on the empty page. This one should be fixed by patch
> 4/4.

I am not yet comfortable calling patch 4/4 a fix. From what I understand
hitting that error means that the kernel lost a page. This patch detects
it earlier so that we can avoid the ELDU #GP but it does still seem
an issue to hit that error in the first place.

> 2. shmem_read_mapping_page_gfp() actually finds a page, but it
> still fails ELDU.
>
> Is that right?

Correct.

>
> If so, I'd probably delve deeper into what the page and the PCMD look
> like. I usually go after these kinds of things with tracing. I'd
> probably dump some representation of the PCMD and page contents with
> trace_printk(). Dump them when the at __sgx_encl_ewb() time, then also
> dump them where the warning is being hit. Pair the warning with a
> tracing_off().
>
> // A crude checksum:
> u64 sum_page(u64 *page)
> {
> u64 ret = 0
> int i;
>
> for (i = 0; i < PAGE_SIZE/sizeof(u64)); i++)
> ret += page[i];
>
> return ret;
> }
>
> Then, logically something like this:
>
> trace_printk("bad ELDU on shm page: %x sum: pcmd: %x %x...\n",
> page_to_pfn(shm_page), sum_page(page_kmap),
> &pcmd, ...);
>
> Both at EWB time and ELDU time. Let's see if the pages that are coming
> out of shmem are the same as the ones that were put in.
>
> When you hit the warning, tracing should turn itself off. Then, you can
> just grep through the trace for that same pfn.

Thank you very much for this guidance.

I added trace_printk() to the EWB and ELDU flows as below and found an
interesting result.

EWB:
__sgx_encl_ewb() {
...
trace_printk("EWB encl %px on SHM page: 0x%lx PCMD page: 0x%lx index %lu sum: 0x%llx \n",
encl_page->encl, page_to_pfn(backing->contents), page_to_pfn(backing->pcmd),
page_index, sum_page((u64 *)pginfo.contents));
...
}

ELDU:
__sgx_encl_eldu() {
...
trace_printk("WARN encl %px ELDU on SHM page: 0x%lx PCMD page: 0x%lx index %lu sum: 0x%llx \n",
encl, page_to_pfn(b.contents), page_to_pfn(b.pcmd), page_index, sum_page((u64 *)pginfo.contents));
...
}

Below is the last few entries of the trace buffer:
ksgxd-806 [022] ...2. 857.259294: __sgx_encl_ewb: EWB encl ffff90a2a38d0000 on SHM page: 0x27906c PCMD page: 0x27906d index 168866 sum: 0x0
ksgxd-806 [022] ...2. 857.259299: __sgx_encl_ewb: EWB encl ffff90a2a38d0000 on SHM page: 0x27906c PCMD page: 0x27906d index 168866 sum: 0x9f7dc28a239283bb
=> ksgxd-806 [022] ...2. 857.259321: __sgx_encl_ewb: EWB encl ffff90a2a38d0000 on SHM page: 0x27906e PCMD page: 0x27906d index 168867 sum: 0xcfe97176b1e8c386
ksgxd-806 [022] ...2. 857.259325: __sgx_encl_ewb: EWB encl ffff90a2a38d0000 on SHM page: 0x27906f PCMD page: 0x27906d index 168868 sum: 0xd1d012775db676d9
ksgxd-806 [022] ...2. 857.259329: __sgx_encl_ewb: EWB encl ffff90a2a38d0000 on SHM page: 0x279070 PCMD page: 0x27906d index 168869 sum: 0x1c568b08bd31d7bb
ksgxd-806 [022] ...2. 857.259333: __sgx_encl_ewb: EWB encl ffff90a2a38d0000 on SHM page: 0x279071 PCMD page: 0x27906d index 168870 sum: 0x4dba242d348982a7
=> test_sgx_large-4092 [030] ...2. 857.259341: sgx_encl_eldu: WARN encl ffff90a2a38d0000 ELDU on SHM page: 0x27906e PCMD page: 0x279052 index 168867 sum: 0xcfe97176b1e8c386

It seems that in this scenario a page is evicted from the enclave and then almost immediately
faulted back in. At the time it is faulted back in the sum matches with the original
encrypted enclave page at the time it was evicted from the enclave.
The interesting part is that the enclave page's PFN matches with the PFN when it was
evicted, but the PCMD PFN does not match.

I did search the log and I cannot find the PCMD PFN printed during ELDU used by the
enclave at all.

I am not familiar with this area - is the PFN expected to be consistent? Do
you perhaps have an idea why the PFN of the PCMD page may have changed? This
is running with this series applied so the ELDU flow would do a lookup of the
PCMD page and not attempt to allocate a new one.

Any hints would be appreciated.

Thank you very much

Reinette