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: Fri, 19 Apr 2024 15:12:27 +0000
From: "Wang, Wei W" <wei.w.wang@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: "pbonzini@...hat.com" <pbonzini@...hat.com>, "kvm@...r.kernel.org"
	<kvm@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [RFC PATCH v2 4/5] KVM: x86: Remove KVM_X86_OP_OPTIONAL

On Friday, April 19, 2024 9:42 PM, Sean Christopherson wrote:
> On Fri, Apr 19, 2024, Wei Wang wrote:
> > KVM_X86_OP and KVM_X86_OP_OPTIONAL were utilized to define and
> execute
> > static_call_update() calls on mandatory and optional hooks, respectively.
> > Mandatory hooks were invoked via static_call() and necessitated
> > definition due to the presumption that an undefined hook (i.e., NULL)
> > would cause
> > static_call() to fail. This assumption no longer holds true as
> > static_call() has been updated to treat a "NULL" hook as a NOP on x86.
> > Consequently, the so-called mandatory hooks are no longer required to
> > be defined, rendering them non-mandatory.
> 
> This is wrong.  They absolutely are mandatory.  The fact that static_call()
> doesn't blow up doesn't make them optional.  If a vendor neglects to
> implement a mandatory hook, KVM *will* break, just not immediately on the
> static_call().
> 
> The static_call() behavior is actually unfortunate, as KVM at least would prefer
> that it does explode on a NULL point.  I.e. better to crash the kernel (hopefully
> before getting to production) then to have a lurking bug just waiting to cause
> problems.
> 
> > This eliminates the need to differentiate between mandatory and
> > optional hooks, allowing a single KVM_X86_OP to suffice.
> >
> > So KVM_X86_OP_OPTIONAL and the WARN_ON() associated with
> KVM_X86_OP
> > are removed to simplify usage,
> 
> Just in case it isn't clear, I am very strongly opposed to removing
> KVM_X86_OP_OPTIONAL() and the WARN_ON() protection to ensure
> mandatory ops are implemented.

OK, we can drop patch 4 and 5.

Btw, may I know what is the boundary between mandatory and optional hooks?
For example, when adding a new hook, what criteria should we use to determine
whether it's mandatory, thereby requiring both SVM and VMX to implement it (and
seems need to be merged them together?)
(I searched a bit, but didn't find it)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ