lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 8 Feb 2024 11:31:03 +0100
From: Paolo Bonzini <pbonzini@...hat.com>
To: Yunhong Jiang <yunhong.jiang@...ux.intel.com>
Cc: Michal Wilczynski <michal.wilczynski@...el.com>, seanjc@...gle.com, mlevitsk@...hat.com, 
	tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, 
	dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, 
	kvm@...r.kernel.org, linux-kernel@...r.kernel.org, dedekind1@...il.com, 
	yuan.yao@...el.com, Zheyu Ma <zheyuma97@...il.com>
Subject: Re: [PATCH v2] KVM: x86: nSVM/nVMX: Fix handling triple fault on RSM instruction

On Thu, Jan 25, 2024 at 1:59 AM Yunhong Jiang
<yunhong.jiang@...ux.intel.com> wrote:
> Would it be ok to move the followed emulator_leave_smm() code into
> vmx_leave_smm, before setting nested_run_pending bit? It avoids changing
> the generic emulator code.
>
> #ifdef CONFIG_X86_64
>         if (guest_cpuid_has(vcpu, X86_FEATURE_LM))
>                 return rsm_load_state_64(ctxt, &smram.smram64);
>         else
> #endif
>                 return rsm_load_state_32(ctxt, &smram.smram32);

I agree with Michal that vendor code should not be in charge of
calling rsm_load_state_*.

However, I don't understand the problem or the fix.

The possible causes are two, and in neither case the fix is to clear
nested_run_pending:

1) if the problem is that setting nested_run_pending was premature,
the correct fix in my opinion is to split the leave_smm vendor
callback in "prepare" and "commit" parts; not to clear it later.  See
the attached, untested, patch.

2) otherwise, if the problem is that we have not gone through the
vmenter yet, then KVM needs to do that and _then_ inject the triple
fault. The fix is to merge the .triple_fault and .check_nested_events
callbacks, with something like the second attached patch - which
probably has so many problems that I haven't even tried to compile it.

The first patch should be equivalent to yours, and I guess it is
okay-ish with a few more comments that explain what's going on.

Paolo

View attachment "nested.patch" of type "text/x-patch" (8336 bytes)

View attachment "nested2.patch" of type "text/x-patch" (2691 bytes)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ