From owner-svn-src-releng@freebsd.org Tue Sep 15 21:43:43 2020 Return-Path: Delivered-To: svn-src-releng@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 266DE3E1158; Tue, 15 Sep 2020 21:43:43 +0000 (UTC) (envelope-from gordon@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4BrcDb0Z8Rz3SZk; Tue, 15 Sep 2020 21:43:43 +0000 (UTC) (envelope-from gordon@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id EBED1EABE; Tue, 15 Sep 2020 21:43:42 +0000 (UTC) (envelope-from gordon@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 08FLhgVR046283; Tue, 15 Sep 2020 21:43:42 GMT (envelope-from gordon@FreeBSD.org) Received: (from gordon@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 08FLhfOm046275; Tue, 15 Sep 2020 21:43:41 GMT (envelope-from gordon@FreeBSD.org) Message-Id: <202009152143.08FLhfOm046275@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: gordon set sender to gordon@FreeBSD.org using -f From: Gordon Tetlow Date: Tue, 15 Sep 2020 21:43:41 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-releng@freebsd.org Subject: svn commit: r365779 - in releng: 11.3/sys/amd64/vmm/amd 11.3/sys/amd64/vmm/intel 11.4/sys/amd64/vmm/amd 11.4/sys/amd64/vmm/intel 12.1/sys/amd64/vmm/amd 12.1/sys/amd64/vmm/intel 12.2/sys/amd64/vmm/a... X-SVN-Group: releng X-SVN-Commit-Author: gordon X-SVN-Commit-Paths: in releng: 11.3/sys/amd64/vmm/amd 11.3/sys/amd64/vmm/intel 11.4/sys/amd64/vmm/amd 11.4/sys/amd64/vmm/intel 12.1/sys/amd64/vmm/amd 12.1/sys/amd64/vmm/intel 12.2/sys/amd64/vmm/amd 12.2/sys/amd64/vmm/int... X-SVN-Commit-Revision: 365779 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-releng@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the release engineering / security commits to the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Sep 2020 21:43:43 -0000 Author: gordon Date: Tue Sep 15 21:43:41 2020 New Revision: 365779 URL: https://svnweb.freebsd.org/changeset/base/365779 Log: Fix bhyve privilege escalation via VMCS access. Approved by: so Approved by: re (implicit for releng/12.2) Security: FreeBSD-SA-20:28.bhyve_vmcs Security: CVE-2020-24718 Modified: releng/11.3/sys/amd64/vmm/amd/svm.c releng/11.3/sys/amd64/vmm/intel/vmx.c releng/11.4/sys/amd64/vmm/amd/svm.c releng/11.4/sys/amd64/vmm/intel/vmx.c releng/12.1/sys/amd64/vmm/amd/svm.c releng/12.1/sys/amd64/vmm/intel/vmx.c releng/12.2/sys/amd64/vmm/amd/svm.c releng/12.2/sys/amd64/vmm/intel/vmx.c Modified: releng/11.3/sys/amd64/vmm/amd/svm.c ============================================================================== --- releng/11.3/sys/amd64/vmm/amd/svm.c Tue Sep 15 21:42:05 2020 (r365778) +++ releng/11.3/sys/amd64/vmm/amd/svm.c Tue Sep 15 21:43:41 2020 (r365779) @@ -469,11 +469,24 @@ vmcb_init(struct svm_softc *sc, int vcpu, uint64_t iop svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_SHUTDOWN); svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_FERR_FREEZE); + svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_INVD); + svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_INVLPGA); svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_MONITOR); svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_MWAIT); /* + * Intercept SVM instructions since AMD enables them in guests otherwise. + * Non-intercepted VMMCALL causes #UD, skip it. + */ + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_VMLOAD); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_VMSAVE); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_STGI); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_CLGI); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_SKINIT); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_ICEBP); + + /* * From section "Canonicalization and Consistency Checks" in APMv2 * the VMRUN intercept bit must be set to pass the consistency check. */ @@ -1217,43 +1230,45 @@ emulate_rdmsr(struct svm_softc *sc, int vcpu, u_int nu static const char * exit_reason_to_str(uint64_t reason) { + int i; static char reasonbuf[32]; + static const struct { + int reason; + const char *str; + } reasons[] = { + { .reason = VMCB_EXIT_INVALID, .str = "invalvmcb" }, + { .reason = VMCB_EXIT_SHUTDOWN, .str = "shutdown" }, + { .reason = VMCB_EXIT_NPF, .str = "nptfault" }, + { .reason = VMCB_EXIT_PAUSE, .str = "pause" }, + { .reason = VMCB_EXIT_HLT, .str = "hlt" }, + { .reason = VMCB_EXIT_CPUID, .str = "cpuid" }, + { .reason = VMCB_EXIT_IO, .str = "inout" }, + { .reason = VMCB_EXIT_MC, .str = "mchk" }, + { .reason = VMCB_EXIT_INTR, .str = "extintr" }, + { .reason = VMCB_EXIT_NMI, .str = "nmi" }, + { .reason = VMCB_EXIT_VINTR, .str = "vintr" }, + { .reason = VMCB_EXIT_MSR, .str = "msr" }, + { .reason = VMCB_EXIT_IRET, .str = "iret" }, + { .reason = VMCB_EXIT_MONITOR, .str = "monitor" }, + { .reason = VMCB_EXIT_MWAIT, .str = "mwait" }, + { .reason = VMCB_EXIT_VMRUN, .str = "vmrun" }, + { .reason = VMCB_EXIT_VMMCALL, .str = "vmmcall" }, + { .reason = VMCB_EXIT_VMLOAD, .str = "vmload" }, + { .reason = VMCB_EXIT_VMSAVE, .str = "vmsave" }, + { .reason = VMCB_EXIT_STGI, .str = "stgi" }, + { .reason = VMCB_EXIT_CLGI, .str = "clgi" }, + { .reason = VMCB_EXIT_SKINIT, .str = "skinit" }, + { .reason = VMCB_EXIT_ICEBP, .str = "icebp" }, + { .reason = VMCB_EXIT_INVD, .str = "invd" }, + { .reason = VMCB_EXIT_INVLPGA, .str = "invlpga" }, + }; - switch (reason) { - case VMCB_EXIT_INVALID: - return ("invalvmcb"); - case VMCB_EXIT_SHUTDOWN: - return ("shutdown"); - case VMCB_EXIT_NPF: - return ("nptfault"); - case VMCB_EXIT_PAUSE: - return ("pause"); - case VMCB_EXIT_HLT: - return ("hlt"); - case VMCB_EXIT_CPUID: - return ("cpuid"); - case VMCB_EXIT_IO: - return ("inout"); - case VMCB_EXIT_MC: - return ("mchk"); - case VMCB_EXIT_INTR: - return ("extintr"); - case VMCB_EXIT_NMI: - return ("nmi"); - case VMCB_EXIT_VINTR: - return ("vintr"); - case VMCB_EXIT_MSR: - return ("msr"); - case VMCB_EXIT_IRET: - return ("iret"); - case VMCB_EXIT_MONITOR: - return ("monitor"); - case VMCB_EXIT_MWAIT: - return ("mwait"); - default: - snprintf(reasonbuf, sizeof(reasonbuf), "%#lx", reason); - return (reasonbuf); + for (i = 0; i < nitems(reasons); i++) { + if (reasons[i].reason == reason) + return (reasons[i].str); } + snprintf(reasonbuf, sizeof(reasonbuf), "%#lx", reason); + return (reasonbuf); } #endif /* KTR */ @@ -1505,6 +1520,20 @@ svm_vmexit(struct svm_softc *svm_sc, int vcpu, struct case VMCB_EXIT_MWAIT: vmexit->exitcode = VM_EXITCODE_MWAIT; break; + case VMCB_EXIT_SHUTDOWN: + case VMCB_EXIT_VMRUN: + case VMCB_EXIT_VMMCALL: + case VMCB_EXIT_VMLOAD: + case VMCB_EXIT_VMSAVE: + case VMCB_EXIT_STGI: + case VMCB_EXIT_CLGI: + case VMCB_EXIT_SKINIT: + case VMCB_EXIT_ICEBP: + case VMCB_EXIT_INVD: + case VMCB_EXIT_INVLPGA: + vm_inject_ud(svm_sc->vm, vcpu); + handled = 1; + break; default: vmm_stat_incr(svm_sc->vm, vcpu, VMEXIT_UNKNOWN, 1); break; @@ -2173,8 +2202,11 @@ svm_setreg(void *arg, int vcpu, int ident, uint64_t va return (svm_modify_intr_shadow(svm_sc, vcpu, val)); } - if (vmcb_write(svm_sc, vcpu, ident, val) == 0) { - return (0); + /* Do not permit user write access to VMCB fields by offset. */ + if (!VMCB_ACCESS_OK(ident)) { + if (vmcb_write(svm_sc, vcpu, ident, val) == 0) { + return (0); + } } reg = swctx_regptr(svm_get_guest_regctx(svm_sc, vcpu), ident); Modified: releng/11.3/sys/amd64/vmm/intel/vmx.c ============================================================================== --- releng/11.3/sys/amd64/vmm/intel/vmx.c Tue Sep 15 21:42:05 2020 (r365778) +++ releng/11.3/sys/amd64/vmm/intel/vmx.c Tue Sep 15 21:43:41 2020 (r365779) @@ -3185,6 +3185,10 @@ vmx_setreg(void *arg, int vcpu, int reg, uint64_t val) if (vmxctx_setreg(&vmx->ctx[vcpu], reg, val) == 0) return (0); + /* Do not permit user write access to VMCS fields by offset. */ + if (reg < 0) + return (EINVAL); + error = vmcs_setreg(&vmx->vmcs[vcpu], running, reg, val); if (error == 0) { Modified: releng/11.4/sys/amd64/vmm/amd/svm.c ============================================================================== --- releng/11.4/sys/amd64/vmm/amd/svm.c Tue Sep 15 21:42:05 2020 (r365778) +++ releng/11.4/sys/amd64/vmm/amd/svm.c Tue Sep 15 21:43:41 2020 (r365779) @@ -469,11 +469,24 @@ vmcb_init(struct svm_softc *sc, int vcpu, uint64_t iop svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_SHUTDOWN); svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_FERR_FREEZE); + svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_INVD); + svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_INVLPGA); svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_MONITOR); svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_MWAIT); /* + * Intercept SVM instructions since AMD enables them in guests otherwise. + * Non-intercepted VMMCALL causes #UD, skip it. + */ + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_VMLOAD); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_VMSAVE); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_STGI); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_CLGI); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_SKINIT); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_ICEBP); + + /* * From section "Canonicalization and Consistency Checks" in APMv2 * the VMRUN intercept bit must be set to pass the consistency check. */ @@ -1217,43 +1230,45 @@ emulate_rdmsr(struct svm_softc *sc, int vcpu, u_int nu static const char * exit_reason_to_str(uint64_t reason) { + int i; static char reasonbuf[32]; + static const struct { + int reason; + const char *str; + } reasons[] = { + { .reason = VMCB_EXIT_INVALID, .str = "invalvmcb" }, + { .reason = VMCB_EXIT_SHUTDOWN, .str = "shutdown" }, + { .reason = VMCB_EXIT_NPF, .str = "nptfault" }, + { .reason = VMCB_EXIT_PAUSE, .str = "pause" }, + { .reason = VMCB_EXIT_HLT, .str = "hlt" }, + { .reason = VMCB_EXIT_CPUID, .str = "cpuid" }, + { .reason = VMCB_EXIT_IO, .str = "inout" }, + { .reason = VMCB_EXIT_MC, .str = "mchk" }, + { .reason = VMCB_EXIT_INTR, .str = "extintr" }, + { .reason = VMCB_EXIT_NMI, .str = "nmi" }, + { .reason = VMCB_EXIT_VINTR, .str = "vintr" }, + { .reason = VMCB_EXIT_MSR, .str = "msr" }, + { .reason = VMCB_EXIT_IRET, .str = "iret" }, + { .reason = VMCB_EXIT_MONITOR, .str = "monitor" }, + { .reason = VMCB_EXIT_MWAIT, .str = "mwait" }, + { .reason = VMCB_EXIT_VMRUN, .str = "vmrun" }, + { .reason = VMCB_EXIT_VMMCALL, .str = "vmmcall" }, + { .reason = VMCB_EXIT_VMLOAD, .str = "vmload" }, + { .reason = VMCB_EXIT_VMSAVE, .str = "vmsave" }, + { .reason = VMCB_EXIT_STGI, .str = "stgi" }, + { .reason = VMCB_EXIT_CLGI, .str = "clgi" }, + { .reason = VMCB_EXIT_SKINIT, .str = "skinit" }, + { .reason = VMCB_EXIT_ICEBP, .str = "icebp" }, + { .reason = VMCB_EXIT_INVD, .str = "invd" }, + { .reason = VMCB_EXIT_INVLPGA, .str = "invlpga" }, + }; - switch (reason) { - case VMCB_EXIT_INVALID: - return ("invalvmcb"); - case VMCB_EXIT_SHUTDOWN: - return ("shutdown"); - case VMCB_EXIT_NPF: - return ("nptfault"); - case VMCB_EXIT_PAUSE: - return ("pause"); - case VMCB_EXIT_HLT: - return ("hlt"); - case VMCB_EXIT_CPUID: - return ("cpuid"); - case VMCB_EXIT_IO: - return ("inout"); - case VMCB_EXIT_MC: - return ("mchk"); - case VMCB_EXIT_INTR: - return ("extintr"); - case VMCB_EXIT_NMI: - return ("nmi"); - case VMCB_EXIT_VINTR: - return ("vintr"); - case VMCB_EXIT_MSR: - return ("msr"); - case VMCB_EXIT_IRET: - return ("iret"); - case VMCB_EXIT_MONITOR: - return ("monitor"); - case VMCB_EXIT_MWAIT: - return ("mwait"); - default: - snprintf(reasonbuf, sizeof(reasonbuf), "%#lx", reason); - return (reasonbuf); + for (i = 0; i < nitems(reasons); i++) { + if (reasons[i].reason == reason) + return (reasons[i].str); } + snprintf(reasonbuf, sizeof(reasonbuf), "%#lx", reason); + return (reasonbuf); } #endif /* KTR */ @@ -1505,6 +1520,20 @@ svm_vmexit(struct svm_softc *svm_sc, int vcpu, struct case VMCB_EXIT_MWAIT: vmexit->exitcode = VM_EXITCODE_MWAIT; break; + case VMCB_EXIT_SHUTDOWN: + case VMCB_EXIT_VMRUN: + case VMCB_EXIT_VMMCALL: + case VMCB_EXIT_VMLOAD: + case VMCB_EXIT_VMSAVE: + case VMCB_EXIT_STGI: + case VMCB_EXIT_CLGI: + case VMCB_EXIT_SKINIT: + case VMCB_EXIT_ICEBP: + case VMCB_EXIT_INVD: + case VMCB_EXIT_INVLPGA: + vm_inject_ud(svm_sc->vm, vcpu); + handled = 1; + break; default: vmm_stat_incr(svm_sc->vm, vcpu, VMEXIT_UNKNOWN, 1); break; @@ -2173,8 +2202,11 @@ svm_setreg(void *arg, int vcpu, int ident, uint64_t va return (svm_modify_intr_shadow(svm_sc, vcpu, val)); } - if (vmcb_write(svm_sc, vcpu, ident, val) == 0) { - return (0); + /* Do not permit user write access to VMCB fields by offset. */ + if (!VMCB_ACCESS_OK(ident)) { + if (vmcb_write(svm_sc, vcpu, ident, val) == 0) { + return (0); + } } reg = swctx_regptr(svm_get_guest_regctx(svm_sc, vcpu), ident); Modified: releng/11.4/sys/amd64/vmm/intel/vmx.c ============================================================================== --- releng/11.4/sys/amd64/vmm/intel/vmx.c Tue Sep 15 21:42:05 2020 (r365778) +++ releng/11.4/sys/amd64/vmm/intel/vmx.c Tue Sep 15 21:43:41 2020 (r365779) @@ -3215,6 +3215,10 @@ vmx_setreg(void *arg, int vcpu, int reg, uint64_t val) if (vmxctx_setreg(&vmx->ctx[vcpu], reg, val) == 0) return (0); + /* Do not permit user write access to VMCS fields by offset. */ + if (reg < 0) + return (EINVAL); + error = vmcs_setreg(&vmx->vmcs[vcpu], running, reg, val); if (error == 0) { Modified: releng/12.1/sys/amd64/vmm/amd/svm.c ============================================================================== --- releng/12.1/sys/amd64/vmm/amd/svm.c Tue Sep 15 21:42:05 2020 (r365778) +++ releng/12.1/sys/amd64/vmm/amd/svm.c Tue Sep 15 21:43:41 2020 (r365779) @@ -469,11 +469,24 @@ vmcb_init(struct svm_softc *sc, int vcpu, uint64_t iop svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_SHUTDOWN); svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_FERR_FREEZE); + svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_INVD); + svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_INVLPGA); svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_MONITOR); svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_MWAIT); /* + * Intercept SVM instructions since AMD enables them in guests otherwise. + * Non-intercepted VMMCALL causes #UD, skip it. + */ + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_VMLOAD); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_VMSAVE); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_STGI); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_CLGI); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_SKINIT); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_ICEBP); + + /* * From section "Canonicalization and Consistency Checks" in APMv2 * the VMRUN intercept bit must be set to pass the consistency check. */ @@ -1217,43 +1230,45 @@ emulate_rdmsr(struct svm_softc *sc, int vcpu, u_int nu static const char * exit_reason_to_str(uint64_t reason) { + int i; static char reasonbuf[32]; + static const struct { + int reason; + const char *str; + } reasons[] = { + { .reason = VMCB_EXIT_INVALID, .str = "invalvmcb" }, + { .reason = VMCB_EXIT_SHUTDOWN, .str = "shutdown" }, + { .reason = VMCB_EXIT_NPF, .str = "nptfault" }, + { .reason = VMCB_EXIT_PAUSE, .str = "pause" }, + { .reason = VMCB_EXIT_HLT, .str = "hlt" }, + { .reason = VMCB_EXIT_CPUID, .str = "cpuid" }, + { .reason = VMCB_EXIT_IO, .str = "inout" }, + { .reason = VMCB_EXIT_MC, .str = "mchk" }, + { .reason = VMCB_EXIT_INTR, .str = "extintr" }, + { .reason = VMCB_EXIT_NMI, .str = "nmi" }, + { .reason = VMCB_EXIT_VINTR, .str = "vintr" }, + { .reason = VMCB_EXIT_MSR, .str = "msr" }, + { .reason = VMCB_EXIT_IRET, .str = "iret" }, + { .reason = VMCB_EXIT_MONITOR, .str = "monitor" }, + { .reason = VMCB_EXIT_MWAIT, .str = "mwait" }, + { .reason = VMCB_EXIT_VMRUN, .str = "vmrun" }, + { .reason = VMCB_EXIT_VMMCALL, .str = "vmmcall" }, + { .reason = VMCB_EXIT_VMLOAD, .str = "vmload" }, + { .reason = VMCB_EXIT_VMSAVE, .str = "vmsave" }, + { .reason = VMCB_EXIT_STGI, .str = "stgi" }, + { .reason = VMCB_EXIT_CLGI, .str = "clgi" }, + { .reason = VMCB_EXIT_SKINIT, .str = "skinit" }, + { .reason = VMCB_EXIT_ICEBP, .str = "icebp" }, + { .reason = VMCB_EXIT_INVD, .str = "invd" }, + { .reason = VMCB_EXIT_INVLPGA, .str = "invlpga" }, + }; - switch (reason) { - case VMCB_EXIT_INVALID: - return ("invalvmcb"); - case VMCB_EXIT_SHUTDOWN: - return ("shutdown"); - case VMCB_EXIT_NPF: - return ("nptfault"); - case VMCB_EXIT_PAUSE: - return ("pause"); - case VMCB_EXIT_HLT: - return ("hlt"); - case VMCB_EXIT_CPUID: - return ("cpuid"); - case VMCB_EXIT_IO: - return ("inout"); - case VMCB_EXIT_MC: - return ("mchk"); - case VMCB_EXIT_INTR: - return ("extintr"); - case VMCB_EXIT_NMI: - return ("nmi"); - case VMCB_EXIT_VINTR: - return ("vintr"); - case VMCB_EXIT_MSR: - return ("msr"); - case VMCB_EXIT_IRET: - return ("iret"); - case VMCB_EXIT_MONITOR: - return ("monitor"); - case VMCB_EXIT_MWAIT: - return ("mwait"); - default: - snprintf(reasonbuf, sizeof(reasonbuf), "%#lx", reason); - return (reasonbuf); + for (i = 0; i < nitems(reasons); i++) { + if (reasons[i].reason == reason) + return (reasons[i].str); } + snprintf(reasonbuf, sizeof(reasonbuf), "%#lx", reason); + return (reasonbuf); } #endif /* KTR */ @@ -1505,6 +1520,20 @@ svm_vmexit(struct svm_softc *svm_sc, int vcpu, struct case VMCB_EXIT_MWAIT: vmexit->exitcode = VM_EXITCODE_MWAIT; break; + case VMCB_EXIT_SHUTDOWN: + case VMCB_EXIT_VMRUN: + case VMCB_EXIT_VMMCALL: + case VMCB_EXIT_VMLOAD: + case VMCB_EXIT_VMSAVE: + case VMCB_EXIT_STGI: + case VMCB_EXIT_CLGI: + case VMCB_EXIT_SKINIT: + case VMCB_EXIT_ICEBP: + case VMCB_EXIT_INVD: + case VMCB_EXIT_INVLPGA: + vm_inject_ud(svm_sc->vm, vcpu); + handled = 1; + break; default: vmm_stat_incr(svm_sc->vm, vcpu, VMEXIT_UNKNOWN, 1); break; @@ -2179,8 +2208,11 @@ svm_setreg(void *arg, int vcpu, int ident, uint64_t va return (svm_modify_intr_shadow(svm_sc, vcpu, val)); } - if (vmcb_write(svm_sc, vcpu, ident, val) == 0) { - return (0); + /* Do not permit user write access to VMCB fields by offset. */ + if (!VMCB_ACCESS_OK(ident)) { + if (vmcb_write(svm_sc, vcpu, ident, val) == 0) { + return (0); + } } reg = swctx_regptr(svm_get_guest_regctx(svm_sc, vcpu), ident); Modified: releng/12.1/sys/amd64/vmm/intel/vmx.c ============================================================================== --- releng/12.1/sys/amd64/vmm/intel/vmx.c Tue Sep 15 21:42:05 2020 (r365778) +++ releng/12.1/sys/amd64/vmm/intel/vmx.c Tue Sep 15 21:43:41 2020 (r365779) @@ -3193,6 +3193,10 @@ vmx_setreg(void *arg, int vcpu, int reg, uint64_t val) if (vmxctx_setreg(&vmx->ctx[vcpu], reg, val) == 0) return (0); + /* Do not permit user write access to VMCS fields by offset. */ + if (reg < 0) + return (EINVAL); + error = vmcs_setreg(&vmx->vmcs[vcpu], running, reg, val); if (error == 0) { Modified: releng/12.2/sys/amd64/vmm/amd/svm.c ============================================================================== --- releng/12.2/sys/amd64/vmm/amd/svm.c Tue Sep 15 21:42:05 2020 (r365778) +++ releng/12.2/sys/amd64/vmm/amd/svm.c Tue Sep 15 21:43:41 2020 (r365779) @@ -466,11 +466,24 @@ vmcb_init(struct svm_softc *sc, int vcpu, uint64_t iop svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_SHUTDOWN); svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_FERR_FREEZE); + svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_INVD); + svm_enable_intercept(sc, vcpu, VMCB_CTRL1_INTCPT, VMCB_INTCPT_INVLPGA); svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_MONITOR); svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_MWAIT); /* + * Intercept SVM instructions since AMD enables them in guests otherwise. + * Non-intercepted VMMCALL causes #UD, skip it. + */ + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_VMLOAD); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_VMSAVE); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_STGI); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_CLGI); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_SKINIT); + svm_enable_intercept(sc, vcpu, VMCB_CTRL2_INTCPT, VMCB_INTCPT_ICEBP); + + /* * From section "Canonicalization and Consistency Checks" in APMv2 * the VMRUN intercept bit must be set to pass the consistency check. */ @@ -1214,43 +1227,45 @@ emulate_rdmsr(struct svm_softc *sc, int vcpu, u_int nu static const char * exit_reason_to_str(uint64_t reason) { + int i; static char reasonbuf[32]; + static const struct { + int reason; + const char *str; + } reasons[] = { + { .reason = VMCB_EXIT_INVALID, .str = "invalvmcb" }, + { .reason = VMCB_EXIT_SHUTDOWN, .str = "shutdown" }, + { .reason = VMCB_EXIT_NPF, .str = "nptfault" }, + { .reason = VMCB_EXIT_PAUSE, .str = "pause" }, + { .reason = VMCB_EXIT_HLT, .str = "hlt" }, + { .reason = VMCB_EXIT_CPUID, .str = "cpuid" }, + { .reason = VMCB_EXIT_IO, .str = "inout" }, + { .reason = VMCB_EXIT_MC, .str = "mchk" }, + { .reason = VMCB_EXIT_INTR, .str = "extintr" }, + { .reason = VMCB_EXIT_NMI, .str = "nmi" }, + { .reason = VMCB_EXIT_VINTR, .str = "vintr" }, + { .reason = VMCB_EXIT_MSR, .str = "msr" }, + { .reason = VMCB_EXIT_IRET, .str = "iret" }, + { .reason = VMCB_EXIT_MONITOR, .str = "monitor" }, + { .reason = VMCB_EXIT_MWAIT, .str = "mwait" }, + { .reason = VMCB_EXIT_VMRUN, .str = "vmrun" }, + { .reason = VMCB_EXIT_VMMCALL, .str = "vmmcall" }, + { .reason = VMCB_EXIT_VMLOAD, .str = "vmload" }, + { .reason = VMCB_EXIT_VMSAVE, .str = "vmsave" }, + { .reason = VMCB_EXIT_STGI, .str = "stgi" }, + { .reason = VMCB_EXIT_CLGI, .str = "clgi" }, + { .reason = VMCB_EXIT_SKINIT, .str = "skinit" }, + { .reason = VMCB_EXIT_ICEBP, .str = "icebp" }, + { .reason = VMCB_EXIT_INVD, .str = "invd" }, + { .reason = VMCB_EXIT_INVLPGA, .str = "invlpga" }, + }; - switch (reason) { - case VMCB_EXIT_INVALID: - return ("invalvmcb"); - case VMCB_EXIT_SHUTDOWN: - return ("shutdown"); - case VMCB_EXIT_NPF: - return ("nptfault"); - case VMCB_EXIT_PAUSE: - return ("pause"); - case VMCB_EXIT_HLT: - return ("hlt"); - case VMCB_EXIT_CPUID: - return ("cpuid"); - case VMCB_EXIT_IO: - return ("inout"); - case VMCB_EXIT_MC: - return ("mchk"); - case VMCB_EXIT_INTR: - return ("extintr"); - case VMCB_EXIT_NMI: - return ("nmi"); - case VMCB_EXIT_VINTR: - return ("vintr"); - case VMCB_EXIT_MSR: - return ("msr"); - case VMCB_EXIT_IRET: - return ("iret"); - case VMCB_EXIT_MONITOR: - return ("monitor"); - case VMCB_EXIT_MWAIT: - return ("mwait"); - default: - snprintf(reasonbuf, sizeof(reasonbuf), "%#lx", reason); - return (reasonbuf); + for (i = 0; i < nitems(reasons); i++) { + if (reasons[i].reason == reason) + return (reasons[i].str); } + snprintf(reasonbuf, sizeof(reasonbuf), "%#lx", reason); + return (reasonbuf); } #endif /* KTR */ @@ -1502,6 +1517,20 @@ svm_vmexit(struct svm_softc *svm_sc, int vcpu, struct case VMCB_EXIT_MWAIT: vmexit->exitcode = VM_EXITCODE_MWAIT; break; + case VMCB_EXIT_SHUTDOWN: + case VMCB_EXIT_VMRUN: + case VMCB_EXIT_VMMCALL: + case VMCB_EXIT_VMLOAD: + case VMCB_EXIT_VMSAVE: + case VMCB_EXIT_STGI: + case VMCB_EXIT_CLGI: + case VMCB_EXIT_SKINIT: + case VMCB_EXIT_ICEBP: + case VMCB_EXIT_INVD: + case VMCB_EXIT_INVLPGA: + vm_inject_ud(svm_sc->vm, vcpu); + handled = 1; + break; default: vmm_stat_incr(svm_sc->vm, vcpu, VMEXIT_UNKNOWN, 1); break; @@ -2176,8 +2205,11 @@ svm_setreg(void *arg, int vcpu, int ident, uint64_t va return (svm_modify_intr_shadow(svm_sc, vcpu, val)); } - if (vmcb_write(svm_sc, vcpu, ident, val) == 0) { - return (0); + /* Do not permit user write access to VMCB fields by offset. */ + if (!VMCB_ACCESS_OK(ident)) { + if (vmcb_write(svm_sc, vcpu, ident, val) == 0) { + return (0); + } } reg = swctx_regptr(svm_get_guest_regctx(svm_sc, vcpu), ident); Modified: releng/12.2/sys/amd64/vmm/intel/vmx.c ============================================================================== --- releng/12.2/sys/amd64/vmm/intel/vmx.c Tue Sep 15 21:42:05 2020 (r365778) +++ releng/12.2/sys/amd64/vmm/intel/vmx.c Tue Sep 15 21:43:41 2020 (r365779) @@ -3236,6 +3236,10 @@ vmx_setreg(void *arg, int vcpu, int reg, uint64_t val) if (vmxctx_setreg(&vmx->ctx[vcpu], reg, val) == 0) return (0); + /* Do not permit user write access to VMCS fields by offset. */ + if (reg < 0) + return (EINVAL); + error = vmcs_setreg(&vmx->vmcs[vcpu], running, reg, val); if (error == 0) {