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

Sean Christopherson
2025-09-17T21:47:55Z

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@google.com>
> >
> > Reviewed-by: Tom Lendacky <thomas.lendacky@amd.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.