Skip to content

Commit cfe1cb0

Browse files
committed
Merge tag 'x86_sgx_for_v5.19_rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
Pull x86 SGX updates from Dave Hansen: "A set of patches to prevent crashes in SGX enclaves under heavy memory pressure: SGX uses normal RAM allocated from special shmem files as backing storage when it runs out of SGX memory (EPC). The code was overly aggressive when freeing shmem pages and was inadvertently freeing perfectly good data. This resulted in failures in the SGX instructions used to swap data back into SGX memory. This turned out to be really hard to trigger in mainline. It was originally encountered testing the out-of-tree "SGX2" patches, but later reproduced on mainline. Fix the data loss by being more careful about truncating pages out of the backing storage and more judiciously setting pages dirty" * tag 'x86_sgx_for_v5.19_rc1' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip: x86/sgx: Ensure no data in PCMD page after truncate x86/sgx: Fix race between reclaimer and page fault handler x86/sgx: Obtain backing storage page with enclave mutex held x86/sgx: Mark PCMD page as dirty when modifying contents x86/sgx: Disconnect backing page references from dirty status
2 parents d6ecaa0 + e3a3bbe commit cfe1cb0

File tree

3 files changed

+114
-14
lines changed

3 files changed

+114
-14
lines changed

arch/x86/kernel/cpu/sgx/encl.c

Lines changed: 104 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,92 @@
1212
#include "encls.h"
1313
#include "sgx.h"
1414

15+
#define PCMDS_PER_PAGE (PAGE_SIZE / sizeof(struct sgx_pcmd))
16+
/*
17+
* 32 PCMD entries share a PCMD page. PCMD_FIRST_MASK is used to
18+
* determine the page index associated with the first PCMD entry
19+
* within a PCMD page.
20+
*/
21+
#define PCMD_FIRST_MASK GENMASK(4, 0)
22+
23+
/**
24+
* reclaimer_writing_to_pcmd() - Query if any enclave page associated with
25+
* a PCMD page is in process of being reclaimed.
26+
* @encl: Enclave to which PCMD page belongs
27+
* @start_addr: Address of enclave page using first entry within the PCMD page
28+
*
29+
* When an enclave page is reclaimed some Paging Crypto MetaData (PCMD) is
30+
* stored. The PCMD data of a reclaimed enclave page contains enough
31+
* information for the processor to verify the page at the time
32+
* it is loaded back into the Enclave Page Cache (EPC).
33+
*
34+
* The backing storage to which enclave pages are reclaimed is laid out as
35+
* follows:
36+
* Encrypted enclave pages:SECS page:PCMD pages
37+
*
38+
* Each PCMD page contains the PCMD metadata of
39+
* PAGE_SIZE/sizeof(struct sgx_pcmd) enclave pages.
40+
*
41+
* A PCMD page can only be truncated if it is (a) empty, and (b) not in the
42+
* process of getting data (and thus soon being non-empty). (b) is tested with
43+
* a check if an enclave page sharing the PCMD page is in the process of being
44+
* reclaimed.
45+
*
46+
* The reclaimer sets the SGX_ENCL_PAGE_BEING_RECLAIMED flag when it
47+
* intends to reclaim that enclave page - it means that the PCMD page
48+
* associated with that enclave page is about to get some data and thus
49+
* even if the PCMD page is empty, it should not be truncated.
50+
*
51+
* Context: Enclave mutex (&sgx_encl->lock) must be held.
52+
* Return: 1 if the reclaimer is about to write to the PCMD page
53+
* 0 if the reclaimer has no intention to write to the PCMD page
54+
*/
55+
static int reclaimer_writing_to_pcmd(struct sgx_encl *encl,
56+
unsigned long start_addr)
57+
{
58+
int reclaimed = 0;
59+
int i;
60+
61+
/*
62+
* PCMD_FIRST_MASK is based on number of PCMD entries within
63+
* PCMD page being 32.
64+
*/
65+
BUILD_BUG_ON(PCMDS_PER_PAGE != 32);
66+
67+
for (i = 0; i < PCMDS_PER_PAGE; i++) {
68+
struct sgx_encl_page *entry;
69+
unsigned long addr;
70+
71+
addr = start_addr + i * PAGE_SIZE;
72+
73+
/*
74+
* Stop when reaching the SECS page - it does not
75+
* have a page_array entry and its reclaim is
76+
* started and completed with enclave mutex held so
77+
* it does not use the SGX_ENCL_PAGE_BEING_RECLAIMED
78+
* flag.
79+
*/
80+
if (addr == encl->base + encl->size)
81+
break;
82+
83+
entry = xa_load(&encl->page_array, PFN_DOWN(addr));
84+
if (!entry)
85+
continue;
86+
87+
/*
88+
* VA page slot ID uses same bit as the flag so it is important
89+
* to ensure that the page is not already in backing store.
90+
*/
91+
if (entry->epc_page &&
92+
(entry->desc & SGX_ENCL_PAGE_BEING_RECLAIMED)) {
93+
reclaimed = 1;
94+
break;
95+
}
96+
}
97+
98+
return reclaimed;
99+
}
100+
15101
/*
16102
* Calculate byte offset of a PCMD struct associated with an enclave page. PCMD's
17103
* follow right after the EPC data in the backing storage. In addition to the
@@ -47,6 +133,7 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
47133
unsigned long va_offset = encl_page->desc & SGX_ENCL_PAGE_VA_OFFSET_MASK;
48134
struct sgx_encl *encl = encl_page->encl;
49135
pgoff_t page_index, page_pcmd_off;
136+
unsigned long pcmd_first_page;
50137
struct sgx_pageinfo pginfo;
51138
struct sgx_backing b;
52139
bool pcmd_page_empty;
@@ -58,6 +145,11 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
58145
else
59146
page_index = PFN_DOWN(encl->size);
60147

148+
/*
149+
* Address of enclave page using the first entry within the PCMD page.
150+
*/
151+
pcmd_first_page = PFN_PHYS(page_index & ~PCMD_FIRST_MASK) + encl->base;
152+
61153
page_pcmd_off = sgx_encl_get_backing_page_pcmd_offset(encl, page_index);
62154

63155
ret = sgx_encl_get_backing(encl, page_index, &b);
@@ -84,6 +176,7 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
84176
}
85177

86178
memset(pcmd_page + b.pcmd_offset, 0, sizeof(struct sgx_pcmd));
179+
set_page_dirty(b.pcmd);
87180

88181
/*
89182
* The area for the PCMD in the page was zeroed above. Check if the
@@ -94,12 +187,20 @@ static int __sgx_encl_eldu(struct sgx_encl_page *encl_page,
94187
kunmap_atomic(pcmd_page);
95188
kunmap_atomic((void *)(unsigned long)pginfo.contents);
96189

97-
sgx_encl_put_backing(&b, false);
190+
get_page(b.pcmd);
191+
sgx_encl_put_backing(&b);
98192

99193
sgx_encl_truncate_backing_page(encl, page_index);
100194

101-
if (pcmd_page_empty)
195+
if (pcmd_page_empty && !reclaimer_writing_to_pcmd(encl, pcmd_first_page)) {
102196
sgx_encl_truncate_backing_page(encl, PFN_DOWN(page_pcmd_off));
197+
pcmd_page = kmap_atomic(b.pcmd);
198+
if (memchr_inv(pcmd_page, 0, PAGE_SIZE))
199+
pr_warn("PCMD page not empty after truncate.\n");
200+
kunmap_atomic(pcmd_page);
201+
}
202+
203+
put_page(b.pcmd);
103204

104205
return ret;
105206
}
@@ -645,15 +746,9 @@ int sgx_encl_get_backing(struct sgx_encl *encl, unsigned long page_index,
645746
/**
646747
* sgx_encl_put_backing() - Unpin the backing storage
647748
* @backing: data for accessing backing storage for the page
648-
* @do_write: mark pages dirty
649749
*/
650-
void sgx_encl_put_backing(struct sgx_backing *backing, bool do_write)
750+
void sgx_encl_put_backing(struct sgx_backing *backing)
651751
{
652-
if (do_write) {
653-
set_page_dirty(backing->pcmd);
654-
set_page_dirty(backing->contents);
655-
}
656-
657752
put_page(backing->pcmd);
658753
put_page(backing->contents);
659754
}

arch/x86/kernel/cpu/sgx/encl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ void sgx_encl_release(struct kref *ref);
107107
int sgx_encl_mm_add(struct sgx_encl *encl, struct mm_struct *mm);
108108
int sgx_encl_get_backing(struct sgx_encl *encl, unsigned long page_index,
109109
struct sgx_backing *backing);
110-
void sgx_encl_put_backing(struct sgx_backing *backing, bool do_write);
110+
void sgx_encl_put_backing(struct sgx_backing *backing);
111111
int sgx_encl_test_and_clear_young(struct mm_struct *mm,
112112
struct sgx_encl_page *page);
113113

arch/x86/kernel/cpu/sgx/main.c

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,8 @@ static int __sgx_encl_ewb(struct sgx_epc_page *epc_page, void *va_slot,
191191
backing->pcmd_offset;
192192

193193
ret = __ewb(&pginfo, sgx_get_epc_virt_addr(epc_page), va_slot);
194+
set_page_dirty(backing->pcmd);
195+
set_page_dirty(backing->contents);
194196

195197
kunmap_atomic((void *)(unsigned long)(pginfo.metadata -
196198
backing->pcmd_offset));
@@ -308,6 +310,7 @@ static void sgx_reclaimer_write(struct sgx_epc_page *epc_page,
308310
sgx_encl_ewb(epc_page, backing);
309311
encl_page->epc_page = NULL;
310312
encl->secs_child_cnt--;
313+
sgx_encl_put_backing(backing);
311314

312315
if (!encl->secs_child_cnt && test_bit(SGX_ENCL_INITIALIZED, &encl->flags)) {
313316
ret = sgx_encl_get_backing(encl, PFN_DOWN(encl->size),
@@ -320,7 +323,7 @@ static void sgx_reclaimer_write(struct sgx_epc_page *epc_page,
320323
sgx_encl_free_epc_page(encl->secs.epc_page);
321324
encl->secs.epc_page = NULL;
322325

323-
sgx_encl_put_backing(&secs_backing, true);
326+
sgx_encl_put_backing(&secs_backing);
324327
}
325328

326329
out:
@@ -379,11 +382,14 @@ static void sgx_reclaim_pages(void)
379382
goto skip;
380383

381384
page_index = PFN_DOWN(encl_page->desc - encl_page->encl->base);
385+
386+
mutex_lock(&encl_page->encl->lock);
382387
ret = sgx_encl_get_backing(encl_page->encl, page_index, &backing[i]);
383-
if (ret)
388+
if (ret) {
389+
mutex_unlock(&encl_page->encl->lock);
384390
goto skip;
391+
}
385392

386-
mutex_lock(&encl_page->encl->lock);
387393
encl_page->desc |= SGX_ENCL_PAGE_BEING_RECLAIMED;
388394
mutex_unlock(&encl_page->encl->lock);
389395
continue;
@@ -411,7 +417,6 @@ static void sgx_reclaim_pages(void)
411417

412418
encl_page = epc_page->owner;
413419
sgx_reclaimer_write(epc_page, &backing[i]);
414-
sgx_encl_put_backing(&backing[i], true);
415420

416421
kref_put(&encl_page->encl->refcount, sgx_encl_release);
417422
epc_page->flags &= ~SGX_EPC_PAGE_RECLAIMER_TRACKED;

0 commit comments

Comments
 (0)