All of lore.kernel.org
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: sashiko-reviews@lists.linux.dev
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH v2 3/3] KVM: SVM: Remove VM from the GA Log notifier list before VM destruction
Date: Tue, 30 Jun 2026 14:27:33 -0700	[thread overview]
Message-ID: <akQ0xSvDXSVoLC6e@google.com> (raw)
In-Reply-To: <20260630211556.05EA91F000E9@smtp.kernel.org>

On Tue, Jun 30, 2026, sashiko-bot@kernel.org wrote:
> > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> > index d71a2fed1a08b..c5b1d294b15a2 100644
> > --- a/arch/x86/kvm/svm/avic.c
> > +++ b/arch/x86/kvm/svm/avic.c
> > @@ -374,9 +374,20 @@ int avic_vcpu_precreate(struct kvm *kvm)
> >  	return 0;
> >  }
> >  
> > +void avic_vm_pre_destroy(struct kvm *kvm)
> > +{
> > +	struct kvm_svm *kvm_svm = to_kvm_svm(kvm);
> > +
> > +	if (WARN_ON_ONCE(!enable_apicv) || !kvm_svm->avic_vm_id)
> > +		return;
> > +
> > +	guard(spinlock_irqsave)(&svm_vm_data_hash_lock);
> > +
> > +	hash_del(&kvm_svm->hnode);
> > +}
> 
> [Severity: High]
> This isn't a bug introduced by this patch, but could there still be a
> Time-Of-Check to Time-Of-Use race in avic_ga_log_notifier()?
> 
> If a notification arrives and avic_ga_log_notifier() finds the VM and its
> vCPU pointer under svm_vm_data_hash_lock, it drops the lock before calling
> kvm_vcpu_wake_up():
> 
> arch/x86/kvm/svm/avic.c:avic_ga_log_notifier() {
>     ...
>     spin_unlock_irqrestore(&svm_vm_data_hash_lock, flags);
> 
>     if (vcpu)
>         kvm_vcpu_wake_up(vcpu);
> }
> 
> If avic_ga_log_notifier() is preempted right after releasing the lock,
> couldn't avic_vm_pre_destroy() still run concurrently, remove the VM from
> the hash table here, and allow the remaining teardown process to free the
> vCPUs?
> 
> Would this result in avic_ga_log_notifier() calling kvm_vcpu_wake_up()
> on a freed vCPU pointer once it resumes execution?

Ugh, yes, that's theoretically possible.  I *think* we can simply move the call
to kvm_vcpu_wake_up() inside the spinlock.  There is a potential deadlock that's
near this code[*], but I don't think svm_vm_data_hash_lock will have the same
problem as ir_list_lock?  Regardless, I'll tackle this one in a separate patch,
unless it looks like we'll have to completely redo the entire scheme.

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

  reply	other threads:[~2026-06-30 21:27 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-30 21:01 [PATCH v2 0/3] KVM: SVM: Fix a (very) unlikely UAF for GA Log IRQs Sean Christopherson
2026-06-30 21:01 ` [PATCH v2 1/3] KVM: SVM: Make kvm_x86_ops.vcpu_precreate() hook fully AVIC specific Sean Christopherson
2026-07-03 16:25   ` Naveen N Rao
2026-06-30 21:01 ` [PATCH v2 2/3] KVM: SVM: Do all per-VM AVIC initialization during vCPU precreation phase Sean Christopherson
2026-06-30 21:18   ` sashiko-bot
2026-06-30 21:20     ` Sean Christopherson
2026-07-03 17:14   ` Naveen N Rao
2026-06-30 21:01 ` [PATCH v2 3/3] KVM: SVM: Remove VM from the GA Log notifier list before VM destruction Sean Christopherson
2026-06-30 21:15   ` sashiko-bot
2026-06-30 21:27     ` Sean Christopherson [this message]
2026-07-03 17:21   ` Naveen N Rao
2026-07-14 18:41 ` [PATCH v2 0/3] KVM: SVM: Fix a (very) unlikely UAF for GA Log IRQs Sean Christopherson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=akQ0xSvDXSVoLC6e@google.com \
    --to=seanjc@google.com \
    --cc=kvm@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.