On Thu, Sep 30, 2021 at 11:42:01PM -0500, Tom Lendacky wrote:

After returning from a VMGEXIT NAE event, SW_EXITINFO1[31:0] is checked for a value of 1, which indicates an error and that SW_EXITINFO2 contains exception information. However, future versions of the GHCB specification may define new values for SW_EXITINFO1[31:0], so really any non-zero value should be treated as an error.

Fixes: 597cfe48212a ("x86/boot/compressed/64: Setup a GHCB-based VC Exception handler") Cc: stable@vger.kernel.org # 5.10+ Signed-off-by: Tom Lendacky thomas.lendacky@amd.com

---

arch/x86/kernel/sev-shared.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c index 34f20e08dc46..ff1e82ff52d9 100644 --- a/arch/x86/kernel/sev-shared.c +++ b/arch/x86/kernel/sev-shared.c @@ -130,6 +130,8 @@ static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb, } else { ret = ES_VMM_ERROR; }

• } else if (ghcb->save.sw_exit_info_1 & 0xffffffff) {

• ret = ES_VMM_ERROR;
  

  } else { ret = ES_OK; }

--

So I wanna do this ontop. Might wanna apply it and look at the result - it shows better what the changes are.

--- From: Borislav Petkov bp@suse.de Date: Fri, 1 Oct 2021 11:41:05 +0200 Subject: [PATCH] x86/sev: Carve out HV call return value verification

Carve out the verification of the HV call return value into a separate helper and make it more readable.

No it more readable.

Signed-off-by: Borislav Petkov bp@suse.de --- arch/x86/kernel/sev-shared.c | 53 ++++++++++++++++++++---------------- 1 file changed, 29 insertions(+), 24 deletions(-)

diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c index bf1033a62e48..f2933f740fa7 100644 --- a/arch/x86/kernel/sev-shared.c +++ b/arch/x86/kernel/sev-shared.c @@ -94,25 +94,15 @@ static void vc_finish_insn(struct es_em_ctxt *ctxt) ctxt->regs->ip += ctxt->insn.length; }

-static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb, - struct es_em_ctxt *ctxt, - u64 exit_code, u64 exit_info_1, - u64 exit_info_2) +static enum es_result verify_exception_info(struct ghcb *ghcb, struct es_em_ctxt *ctxt) { - enum es_result ret; + int ret;

- /* Fill in protocol and format specifiers */ - ghcb->protocol_version = GHCB_PROTOCOL_MAX; - ghcb->ghcb_usage = GHCB_DEFAULT_USAGE; + ret = ghcb->save.sw_exit_info_1 & 0xffffffff; + if (!ret) + return ES_OK;

- ghcb_set_sw_exit_code(ghcb, exit_code); - ghcb_set_sw_exit_info_1(ghcb, exit_info_1); - ghcb_set_sw_exit_info_2(ghcb, exit_info_2); - - sev_es_wr_ghcb_msr(__pa(ghcb)); - VMGEXIT(); - - if ((ghcb->save.sw_exit_info_1 & 0xffffffff) == 1) { + if (ret == 1) { u64 info = ghcb->save.sw_exit_info_2; unsigned long v;

@@ -124,19 +114,34 @@ static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb, ((v == X86_TRAP_GP) || (v == X86_TRAP_UD)) && ((info & SVM_EVTINJ_TYPE_MASK) == SVM_EVTINJ_TYPE_EXEPT)) { ctxt->fi.vector = v; + if (info & SVM_EVTINJ_VALID_ERR) ctxt->fi.error_code = info >> 32; - ret = ES_EXCEPTION; - } else { - ret = ES_VMM_ERROR; + + return ES_EXCEPTION; } - } else if (ghcb->save.sw_exit_info_1 & 0xffffffff) { - ret = ES_VMM_ERROR; - } else { - ret = ES_OK; }

- return ret; + return ES_VMM_ERROR; +} + +static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb, + struct es_em_ctxt *ctxt, + u64 exit_code, u64 exit_info_1, + u64 exit_info_2) +{ + /* Fill in protocol and format specifiers */ + ghcb->protocol_version = GHCB_PROTOCOL_MAX; + ghcb->ghcb_usage = GHCB_DEFAULT_USAGE; + + ghcb_set_sw_exit_code(ghcb, exit_code); + ghcb_set_sw_exit_info_1(ghcb, exit_info_1); + ghcb_set_sw_exit_info_2(ghcb, exit_info_2); + + sev_es_wr_ghcb_msr(__pa(ghcb)); + VMGEXIT(); + + return verify_exception_info(ghcb, ctxt); }

/*

The following commit has been merged into the x86/urgent branch of tip:

Commit-ID: 06f2ac3d4219bbbfd93d79e01966a42053084f11 Gitweb: https://git.kernel.org/tip/06f2ac3d4219bbbfd93d79e01966a42053084f11 Author: Tom Lendacky thomas.lendacky@amd.com AuthorDate: Thu, 30 Sep 2021 23:42:01 -05:00 Committer: Borislav Petkov bp@suse.de CommitterDate: Fri, 01 Oct 2021 11:14:41 +02:00

x86/sev: Return an error on a returned non-zero SW_EXITINFO1[31:0]

After returning from a VMGEXIT NAE event, SW_EXITINFO1[31:0] is checked for a value of 1, which indicates an error and that SW_EXITINFO2 contains exception information. However, future versions of the GHCB specification may define new values for SW_EXITINFO1[31:0], so really any non-zero value should be treated as an error.

Fixes: 597cfe48212a ("x86/boot/compressed/64: Setup a GHCB-based VC Exception handler") Signed-off-by: Tom Lendacky thomas.lendacky@amd.com Signed-off-by: Borislav Petkov bp@suse.de Cc: stable@vger.kernel.org # 5.10+ Link: https://lkml.kernel.org/r/efc772af831e9e7f517f0439b13b41f56bad8784.163306332... --- arch/x86/kernel/sev-shared.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/arch/x86/kernel/sev-shared.c b/arch/x86/kernel/sev-shared.c index 9f90f46..bf1033a 100644 --- a/arch/x86/kernel/sev-shared.c +++ b/arch/x86/kernel/sev-shared.c @@ -130,6 +130,8 @@ static enum es_result sev_es_ghcb_hv_call(struct ghcb *ghcb, } else { ret = ES_VMM_ERROR; } + } else if (ghcb->save.sw_exit_info_1 & 0xffffffff) { + ret = ES_VMM_ERROR; } else { ret = ES_OK; }