linux-kernel - Re: [PATCH v3 2/3] KVM: x86: Use kvzalloc() to allocate VM struct

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]

Message-ID: <aC8pSfEBdHZW9Ze7@google.com>
Date: Thu, 22 May 2025 06:40:25 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Kai Huang <kai.huang@...el.com>
Cc: "kvm@...r.kernel.org" <kvm@...r.kernel.org>, "pbonzini@...hat.com" <pbonzini@...hat.com>, 
	"vipinsh@...gle.com" <vipinsh@...gle.com>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v3 2/3] KVM: x86: Use kvzalloc() to allocate VM struct

On Wed, May 21, 2025, Kai Huang wrote:
> On Wed, 2025-05-21 at 10:12 -0700, Sean Christopherson wrote:
> > > e.g., if we export kvm_x86_ops, we could unwind them.
> > 
> > Maaaybe.  I mean, yes, we could fully unwind kvm_x86_ops, but doing so would make
> > the overall code far more brittle.  E.g. simply updating kvm_x86_ops won't suffice,
> > as the static_calls also need to be patched, and we would have to be very careful
> > not to touch anything in kvm_x86_ops that might have been consumed between here
> > and the call to tdx_bringup().
> 
> Right.  Maybe exporting kvm_ops_update() is better.

A bit, but KVM would still need to be careful not to modify the parts of
vt_x86_ops that have already been consumed.

While I agree that leaving TDX breadcrumbs in kvm_x86_ops when TDX is disabled is
undesirable, the behavior is known, i.e. we know exactly what TDX state is being
left behind.  And failure to load the TDX Module should be very, very rare for
any setup that is actually trying to enable TDX.

Whereas providing a way to modify kvm_x86_ops creates the possibility for "bad"
updates.  KVM's initialization code is a lot like the kernel's boot code (and
probably most bootstrapping code): it's inherently fragile because avoiding
dependencies is practically impossible.

E.g. I ran into a relevant ordering problem[*] just a few days ago, where checking
for VMX capabilities during PMU initialization always failed because the VMCS
config hadn't yet been parsed.  Those types of bugs are especially dangerous
because they're very easy to overlook when modifying existing code, e.g. the
only sign that anything is broken is an optional feature being missing.

[*] https://lore.kernel.org/all/aCU2YEpU0dOk7RTk@google.com