On Fri, Nov 26, 2021, Colin Ian King wrote: > Currently variable info is being assigned twice, the second assignment > is redundant. Clean up the code by making the assignments at declaration > time and remove the latter two assignments. > > Signed-off-by: Colin Ian King <colin.i.king@xxxxxxxxx> Heh, checkpatch complains about the different email domains as it doesn't know that googlemail.com and gmail.com are more or less the same. Doubt it matters much, but probably worth having your SoB match your local git config. > --- > arch/x86/kernel/sev-shared.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c > index ce987688bbc0..6083d6f658c8 100644 > --- a/arch/x86/kernel/sev-shared.c > +++ b/arch/x86/kernel/sev-shared.c > @@ -104,10 +104,7 @@ static enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt > > if (ret == 1) { > u64 info = ghcb->save.sw_exit_info_2; > - unsigned long v; > - > - info = ghcb->save.sw_exit_info_2; > - v = info & SVM_EVTINJ_VEC_MASK; > + unsigned long v = info & SVM_EVTINJ_VEC_MASK; 'v' should really be a u8, and IMO 'v' is a pointlessly short. Opportunistically squash this in? --- arch/x86/kernel/sev-shared.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c index 6083d6f658c8..2feb7359ed3a 100644 --- a/arch/x86/kernel/sev-shared.c +++ b/arch/x86/kernel/sev-shared.c @@ -104,13 +104,13 @@ static enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt if (ret == 1) { u64 info = ghcb->save.sw_exit_info_2; - unsigned long v = info & SVM_EVTINJ_VEC_MASK; + u8 vector = info & SVM_EVTINJ_VEC_MASK; /* Check if exception information from hypervisor is sane. */ if ((info & SVM_EVTINJ_VALID) && - ((v == X86_TRAP_GP) || (v == X86_TRAP_UD)) && + ((vector == X86_TRAP_GP) || (vector == X86_TRAP_UD)) && ((info & SVM_EVTINJ_TYPE_MASK) == SVM_EVTINJ_TYPE_EXEPT)) { - ctxt->fi.vector = v; + ctxt->fi.vector = vector; if (info & SVM_EVTINJ_VALID_ERR) ctxt->fi.error_code = info >> 32; -- > /* Check if exception information from hypervisor is sane. */ s/sane/mostly sane, this code doesn't check that the hypervisor correctly provided an error. It doesn't really matter since ctxt->fi.error_code is zeroed, i.e. will have clean data for #GP, and is ignored by #UD. And nothing downstream should be doing anything more than reporting the error code anyways since the value is fully VMM controlled. But to be pedantic... From: Sean Christopherson <seanjc@xxxxxxxxxx> Date: Tue, 30 Nov 2021 18:40:30 -0800 Subject: [PATCH] x86/sev: Verify that VMM provides error code on #GP, and not on #UD When handling an "injected" exception after #VMGEXIT, verify that the VMM did (not) provide an error code as appropriate for the signaled vector. Treat an error code mismatch as a VMM error instead of synthesizing an exception. Fixes: 597cfe48212a ("x86/boot/compressed/64: Setup a GHCB-based VC Exception handler") Signed-off-by: Sean Christopherson <seanjc@xxxxxxxxxx> --- arch/x86/kernel/sev-shared.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c index 2feb7359ed3a..139d88a76adc 100644 --- a/arch/x86/kernel/sev-shared.c +++ b/arch/x86/kernel/sev-shared.c @@ -105,11 +105,13 @@ static enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt if (ret == 1) { u64 info = ghcb->save.sw_exit_info_2; u8 vector = info & SVM_EVTINJ_VEC_MASK; + bool want_error_code = (vector == X86_TRAP_GP); /* Check if exception information from hypervisor is sane. */ if ((info & SVM_EVTINJ_VALID) && ((vector == X86_TRAP_GP) || (vector == X86_TRAP_UD)) && - ((info & SVM_EVTINJ_TYPE_MASK) == SVM_EVTINJ_TYPE_EXEPT)) { + ((info & SVM_EVTINJ_TYPE_MASK) == SVM_EVTINJ_TYPE_EXEPT) && + (!!(info & SVM_EVTINJ_VALID_ERR) == want_error_code)) { ctxt->fi.vector = vector; if (info & SVM_EVTINJ_VALID_ERR) --