linux-kernel - Re: [PATCH v15 02/41] KVM: SEV: Read save fields from GHCB exactly once

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: <aMssiS12LPPmDjNK@google.com>
Date: Wed, 17 Sep 2025 14:47:53 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Tom Lendacky <thomas.lendacky@....com>
Cc: Paolo Bonzini <pbonzini@...hat.com>, kvm@...r.kernel.org, linux-kernel@...r.kernel.org, 
	Mathias Krause <minipli@...ecurity.net>, John Allen <john.allen@....com>, 
	Rick Edgecombe <rick.p.edgecombe@...el.com>, Chao Gao <chao.gao@...el.com>, 
	Maxim Levitsky <mlevitsk@...hat.com>, Xiaoyao Li <xiaoyao.li@...el.com>, 
	Zhang Yi Z <yi.z.zhang@...ux.intel.com>
Subject: Re: [PATCH v15 02/41] KVM: SEV: Read save fields from GHCB exactly once

On Mon, Sep 15, 2025, Sean Christopherson wrote:
> On Mon, Sep 15, 2025, Tom Lendacky wrote:
> > On 9/12/25 18:22, Sean Christopherson wrote:
> > > Wrap all reads of GHCB save fields with READ_ONCE() via a KVM-specific
> > > GHCB get() utility to help guard against TOCTOU bugs.  Using READ_ONCE()
> > > doesn't completely prevent such bugs, e.g. doesn't prevent KVM from
> > > redoing get() after checking the initial value, but at least addresses
> > > all potential TOCTOU issues in the current KVM code base.
> > > 
> > > Opportunistically reduce the indentation of the macro-defined helpers and
> > > clean up the alignment.
> > > 
> > > Fixes: 4e15a0ddc3ff ("KVM: SEV: snapshot the GHCB before accessing it")
> > > Signed-off-by: Sean Christopherson <seanjc@...gle.com>
> > 
> > Reviewed-by: Tom Lendacky <thomas.lendacky@....com>
> > 
> > Just wondering if we should make the kvm_ghcb_get_*() routines take just
> > a struct vcpu_svm routine so that they don't get confused with the
> > ghcb_get_*() routines? The current uses are just using svm->sev_es.ghcb
> > to set the ghcb variable that gets used anyway. That way the KVM
> > versions look specifically like KVM versions.
> 
> Yeah, that's a great idea.  I'll send a patch, 

Actually, I'll do that straightaway in this patch (need to send a v16 anyways).
Introducing kvm_ghcb_get_##field() and then immediately changing all callers is
ridiculous, and if this ends up getting backported to LTS kernels, it'd be better
to backport the final form, e.g. so that additional fixes don't generate conflicts
that could have been easily avoided.