Skip to content

Commit 7de7ac8

Browse files
committed
Merge tag 'x86_urgent_for_v5.13_rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip
Pull x86 fixes from Borislav Petkov: - Fix how SEV handles MMIO accesses by forwarding potential page faults instead of killing the machine and by using the accessors with the exact functionality needed when accessing memory. - Fix a confusion with Clang LTO compiler switches passed to the it - Handle the case gracefully when VMGEXIT has been executed in userspace * tag 'x86_urgent_for_v5.13_rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip: x86/sev-es: Use __put_user()/__get_user() for data accesses x86/sev-es: Forward page-faults which happen during emulation x86/sev-es: Don't return NULL from sev_es_get_ghcb() x86/build: Fix location of '-plugin-opt=' flags x86/sev-es: Invalidate the GHCB after completing VMGEXIT x86/sev-es: Move sev_es_put_ghcb() in prep for follow on patch
2 parents 28ceac6 + 4954f5b commit 7de7ac8

File tree

3 files changed

+92
-57
lines changed

3 files changed

+92
-57
lines changed

arch/x86/Makefile

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -178,11 +178,6 @@ ifeq ($(ACCUMULATE_OUTGOING_ARGS), 1)
178178
KBUILD_CFLAGS += $(call cc-option,-maccumulate-outgoing-args,)
179179
endif
180180

181-
ifdef CONFIG_LTO_CLANG
182-
KBUILD_LDFLAGS += -plugin-opt=-code-model=kernel \
183-
-plugin-opt=-stack-alignment=$(if $(CONFIG_X86_32),4,8)
184-
endif
185-
186181
# Workaround for a gcc prelease that unfortunately was shipped in a suse release
187182
KBUILD_CFLAGS += -Wno-sign-compare
188183
#
@@ -202,7 +197,12 @@ ifdef CONFIG_RETPOLINE
202197
endif
203198
endif
204199

205-
KBUILD_LDFLAGS := -m elf_$(UTS_MACHINE)
200+
KBUILD_LDFLAGS += -m elf_$(UTS_MACHINE)
201+
202+
ifdef CONFIG_LTO_CLANG
203+
KBUILD_LDFLAGS += -plugin-opt=-code-model=kernel \
204+
-plugin-opt=-stack-alignment=$(if $(CONFIG_X86_32),4,8)
205+
endif
206206

207207
ifdef CONFIG_X86_NEED_RELOCS
208208
LDFLAGS_vmlinux := --emit-relocs --discard-none

arch/x86/kernel/sev-shared.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ static bool sev_es_negotiate_protocol(void)
6363

6464
static __always_inline void vc_ghcb_invalidate(struct ghcb *ghcb)
6565
{
66+
ghcb->save.sw_exit_code = 0;
6667
memset(ghcb->save.valid_bitmap, 0, sizeof(ghcb->save.valid_bitmap));
6768
}
6869

arch/x86/kernel/sev.c

Lines changed: 85 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -203,8 +203,18 @@ static __always_inline struct ghcb *sev_es_get_ghcb(struct ghcb_state *state)
203203
if (unlikely(data->ghcb_active)) {
204204
/* GHCB is already in use - save its contents */
205205

206-
if (unlikely(data->backup_ghcb_active))
207-
return NULL;
206+
if (unlikely(data->backup_ghcb_active)) {
207+
/*
208+
* Backup-GHCB is also already in use. There is no way
209+
* to continue here so just kill the machine. To make
210+
* panic() work, mark GHCBs inactive so that messages
211+
* can be printed out.
212+
*/
213+
data->ghcb_active = false;
214+
data->backup_ghcb_active = false;
215+
216+
panic("Unable to handle #VC exception! GHCB and Backup GHCB are already in use");
217+
}
208218

209219
/* Mark backup_ghcb active before writing to it */
210220
data->backup_ghcb_active = true;
@@ -221,24 +231,6 @@ static __always_inline struct ghcb *sev_es_get_ghcb(struct ghcb_state *state)
221231
return ghcb;
222232
}
223233

224-
static __always_inline void sev_es_put_ghcb(struct ghcb_state *state)
225-
{
226-
struct sev_es_runtime_data *data;
227-
struct ghcb *ghcb;
228-
229-
data = this_cpu_read(runtime_data);
230-
ghcb = &data->ghcb_page;
231-
232-
if (state->ghcb) {
233-
/* Restore GHCB from Backup */
234-
*ghcb = *state->ghcb;
235-
data->backup_ghcb_active = false;
236-
state->ghcb = NULL;
237-
} else {
238-
data->ghcb_active = false;
239-
}
240-
}
241-
242234
/* Needed in vc_early_forward_exception */
243235
void do_early_exception(struct pt_regs *regs, int trapnr);
244236

@@ -323,31 +315,44 @@ static enum es_result vc_write_mem(struct es_em_ctxt *ctxt,
323315
u16 d2;
324316
u8 d1;
325317

326-
/* If instruction ran in kernel mode and the I/O buffer is in kernel space */
327-
if (!user_mode(ctxt->regs) && !access_ok(target, size)) {
328-
memcpy(dst, buf, size);
329-
return ES_OK;
330-
}
331-
318+
/*
319+
* This function uses __put_user() independent of whether kernel or user
320+
* memory is accessed. This works fine because __put_user() does no
321+
* sanity checks of the pointer being accessed. All that it does is
322+
* to report when the access failed.
323+
*
324+
* Also, this function runs in atomic context, so __put_user() is not
325+
* allowed to sleep. The page-fault handler detects that it is running
326+
* in atomic context and will not try to take mmap_sem and handle the
327+
* fault, so additional pagefault_enable()/disable() calls are not
328+
* needed.
329+
*
330+
* The access can't be done via copy_to_user() here because
331+
* vc_write_mem() must not use string instructions to access unsafe
332+
* memory. The reason is that MOVS is emulated by the #VC handler by
333+
* splitting the move up into a read and a write and taking a nested #VC
334+
* exception on whatever of them is the MMIO access. Using string
335+
* instructions here would cause infinite nesting.
336+
*/
332337
switch (size) {
333338
case 1:
334339
memcpy(&d1, buf, 1);
335-
if (put_user(d1, target))
340+
if (__put_user(d1, target))
336341
goto fault;
337342
break;
338343
case 2:
339344
memcpy(&d2, buf, 2);
340-
if (put_user(d2, target))
345+
if (__put_user(d2, target))
341346
goto fault;
342347
break;
343348
case 4:
344349
memcpy(&d4, buf, 4);
345-
if (put_user(d4, target))
350+
if (__put_user(d4, target))
346351
goto fault;
347352
break;
348353
case 8:
349354
memcpy(&d8, buf, 8);
350-
if (put_user(d8, target))
355+
if (__put_user(d8, target))
351356
goto fault;
352357
break;
353358
default:
@@ -378,30 +383,43 @@ static enum es_result vc_read_mem(struct es_em_ctxt *ctxt,
378383
u16 d2;
379384
u8 d1;
380385

381-
/* If instruction ran in kernel mode and the I/O buffer is in kernel space */
382-
if (!user_mode(ctxt->regs) && !access_ok(s, size)) {
383-
memcpy(buf, src, size);
384-
return ES_OK;
385-
}
386-
386+
/*
387+
* This function uses __get_user() independent of whether kernel or user
388+
* memory is accessed. This works fine because __get_user() does no
389+
* sanity checks of the pointer being accessed. All that it does is
390+
* to report when the access failed.
391+
*
392+
* Also, this function runs in atomic context, so __get_user() is not
393+
* allowed to sleep. The page-fault handler detects that it is running
394+
* in atomic context and will not try to take mmap_sem and handle the
395+
* fault, so additional pagefault_enable()/disable() calls are not
396+
* needed.
397+
*
398+
* The access can't be done via copy_from_user() here because
399+
* vc_read_mem() must not use string instructions to access unsafe
400+
* memory. The reason is that MOVS is emulated by the #VC handler by
401+
* splitting the move up into a read and a write and taking a nested #VC
402+
* exception on whatever of them is the MMIO access. Using string
403+
* instructions here would cause infinite nesting.
404+
*/
387405
switch (size) {
388406
case 1:
389-
if (get_user(d1, s))
407+
if (__get_user(d1, s))
390408
goto fault;
391409
memcpy(buf, &d1, 1);
392410
break;
393411
case 2:
394-
if (get_user(d2, s))
412+
if (__get_user(d2, s))
395413
goto fault;
396414
memcpy(buf, &d2, 2);
397415
break;
398416
case 4:
399-
if (get_user(d4, s))
417+
if (__get_user(d4, s))
400418
goto fault;
401419
memcpy(buf, &d4, 4);
402420
break;
403421
case 8:
404-
if (get_user(d8, s))
422+
if (__get_user(d8, s))
405423
goto fault;
406424
memcpy(buf, &d8, 8);
407425
break;
@@ -461,6 +479,29 @@ static enum es_result vc_slow_virt_to_phys(struct ghcb *ghcb, struct es_em_ctxt
461479
/* Include code shared with pre-decompression boot stage */
462480
#include "sev-shared.c"
463481

482+
static __always_inline void sev_es_put_ghcb(struct ghcb_state *state)
483+
{
484+
struct sev_es_runtime_data *data;
485+
struct ghcb *ghcb;
486+
487+
data = this_cpu_read(runtime_data);
488+
ghcb = &data->ghcb_page;
489+
490+
if (state->ghcb) {
491+
/* Restore GHCB from Backup */
492+
*ghcb = *state->ghcb;
493+
data->backup_ghcb_active = false;
494+
state->ghcb = NULL;
495+
} else {
496+
/*
497+
* Invalidate the GHCB so a VMGEXIT instruction issued
498+
* from userspace won't appear to be valid.
499+
*/
500+
vc_ghcb_invalidate(ghcb);
501+
data->ghcb_active = false;
502+
}
503+
}
504+
464505
void noinstr __sev_es_nmi_complete(void)
465506
{
466507
struct ghcb_state state;
@@ -1255,6 +1296,10 @@ static __always_inline void vc_forward_exception(struct es_em_ctxt *ctxt)
12551296
case X86_TRAP_UD:
12561297
exc_invalid_op(ctxt->regs);
12571298
break;
1299+
case X86_TRAP_PF:
1300+
write_cr2(ctxt->fi.cr2);
1301+
exc_page_fault(ctxt->regs, error_code);
1302+
break;
12581303
case X86_TRAP_AC:
12591304
exc_alignment_check(ctxt->regs, error_code);
12601305
break;
@@ -1284,7 +1329,6 @@ static __always_inline bool on_vc_fallback_stack(struct pt_regs *regs)
12841329
*/
12851330
DEFINE_IDTENTRY_VC_SAFE_STACK(exc_vmm_communication)
12861331
{
1287-
struct sev_es_runtime_data *data = this_cpu_read(runtime_data);
12881332
irqentry_state_t irq_state;
12891333
struct ghcb_state state;
12901334
struct es_em_ctxt ctxt;
@@ -1310,16 +1354,6 @@ DEFINE_IDTENTRY_VC_SAFE_STACK(exc_vmm_communication)
13101354
*/
13111355

13121356
ghcb = sev_es_get_ghcb(&state);
1313-
if (!ghcb) {
1314-
/*
1315-
* Mark GHCBs inactive so that panic() is able to print the
1316-
* message.
1317-
*/
1318-
data->ghcb_active = false;
1319-
data->backup_ghcb_active = false;
1320-
1321-
panic("Unable to handle #VC exception! GHCB and Backup GHCB are already in use");
1322-
}
13231357

13241358
vc_ghcb_invalidate(ghcb);
13251359
result = vc_init_em_ctxt(&ctxt, regs, error_code);

0 commit comments

Comments
 (0)