From nobody Sat Feb 7 12:18:22 2026 Received: from mail-yb1-f201.google.com (mail-yb1-f201.google.com [209.85.219.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id D223215C0 for ; Wed, 10 Jan 2024 01:15:38 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=flex--seanjc.bounces.google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="qiW5NejF" Received: by mail-yb1-f201.google.com with SMTP id 3f1490d57ef6-dbeac7a5b53so3337975276.2 for ; Tue, 09 Jan 2024 17:15:38 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1704849338; x=1705454138; darn=vger.kernel.org; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:from:to:cc:subject:date:message-id:reply-to; bh=XYuPpMHg4kkSf61Majt6ZB1e5OqyEZJQxirPTzJ0Fdw=; b=qiW5NejF4maQ9b2Dlimzdpcp53dwS+KZAernKfMXgv/Mb97qv5NqoSKLaPtniKbhcZ f1vvOnnK9tXYTqFpTY+gT9GQi/f6eEOTLDFcbnpNJEgF8vxFegXRrQVDv5pZ9pf8gn7Y TJ5D9H5Y5cTs5orpASXqH5JHTzvKGFygvxZK7hFUHuVrxc5ihEhm5sDvojOUfuGDWgpQ KIdVqRw6lefSGTyoJ8WokVSOKXbdH59EcN96maj8DWfkbvYPCHJfhZiPmVYMvLWchPOd 92lKhnvqv5lfjKQ53fvwkdJPCfKEgw5rV4u2RO9dxxoi0wjzXGOuRi1b1jeAEECfXoQ7 kuxA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1704849338; x=1705454138; h=cc:to:from:subject:message-id:references:mime-version:in-reply-to :date:reply-to:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=XYuPpMHg4kkSf61Majt6ZB1e5OqyEZJQxirPTzJ0Fdw=; b=wlbRMjKKGu9GhDwS53TSvD4kQEceGScumzGZEjvMlPdJjBf3DQG2i9yPenbwSsqC8x E5ZE2XtLJV2rY5pE4bBIl2hoJp5uMdxNAAUKTbZbkslO/iO5cPlRBcQ7dImdV8pkvwci h4cXbT2hifEk5PrbcAnp8bM7ZArUf0IGlASgCI4XOJztB45UMApdkdYfLrsso2KOlBKj jANorvx7MMQFD8uslV/NE9ez4WbufHB6khXb6vDi2NQSjvRCnMCCSvYu6KBE/8cjWbVM 9uq+8slQB1a4eCLu5WNgzb+pW6LGSxM5h6agDGAar99b6S4PvyVBNx+v2Biisqs8fEXJ jo8A== X-Gm-Message-State: AOJu0YziXsRvrs1F05Tv/xPXtBqg6/oJQA/xfZOdrvfXFdAr3C3mSj+F 1ONWfk4SS8IQZ0taQO29KG6s23eYemGhBC3P3g== X-Google-Smtp-Source: AGHT+IHPHYzTfLV1eyWciUMErntB/kDHFOk79hccdy8996kR1y9nEE7EfVhcF4BWE9mB/Y6Qjh4D0pBEn50= X-Received: from zagreus.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:5c37]) (user=seanjc job=sendgmr) by 2002:a25:8002:0:b0:dbe:4772:395e with SMTP id m2-20020a258002000000b00dbe4772395emr6664ybk.5.1704849337952; Tue, 09 Jan 2024 17:15:37 -0800 (PST) Reply-To: Sean Christopherson Date: Tue, 9 Jan 2024 17:15:30 -0800 In-Reply-To: <20240110011533.503302-1-seanjc@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 References: <20240110011533.503302-1-seanjc@google.com> X-Mailer: git-send-email 2.43.0.472.g3155946c3a-goog Message-ID: <20240110011533.503302-2-seanjc@google.com> Subject: [PATCH 1/4] KVM: Always flush async #PF workqueue when vCPU is being destroyed From: Sean Christopherson To: Paolo Bonzini Cc: kvm@vger.kernel.org, linux-kernel@vger.kernel.org, David Matlack , Xu Yilun , Sean Christopherson Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" Always flush the per-vCPU async #PF workqueue when a vCPU is clearing its completion queue, e.g. when a VM and all its vCPUs is being destroyed. KVM must ensure that none of its workqueue callbacks is running when the last reference to the KVM _module_ is put. Gifting a reference to the associated VM prevents the workqueue callback from dereferencing freed vCPU/VM memory, but does not prevent the KVM module from being unloaded before the callback completes. Drop the misguided VM refcount gifting, as calling kvm_put_kvm() from async_pf_execute() if kvm_put_kvm() flushes the async #PF workqueue will result in deadlock. async_pf_execute() can't return until kvm_put_kvm() finishes, and kvm_put_kvm() can't return until async_pf_execute() finishes: WARNING: CPU: 8 PID: 251 at virt/kvm/kvm_main.c:1435 kvm_put_kvm+0x2d/0x32= 0 [kvm] Modules linked in: vhost_net vhost vhost_iotlb tap kvm_intel kvm irqbypass CPU: 8 PID: 251 Comm: kworker/8:1 Tainted: G W 6.6.0-rc1-e= 7af8d17224a-x86/gmem-vm #119 Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015 Workqueue: events async_pf_execute [kvm] RIP: 0010:kvm_put_kvm+0x2d/0x320 [kvm] Call Trace: async_pf_execute+0x198/0x260 [kvm] process_one_work+0x145/0x2d0 worker_thread+0x27e/0x3a0 kthread+0xba/0xe0 ret_from_fork+0x2d/0x50 ret_from_fork_asm+0x11/0x20 ---[ end trace 0000000000000000 ]--- INFO: task kworker/8:1:251 blocked for more than 120 seconds. Tainted: G W 6.6.0-rc1-e7af8d17224a-x86/gmem-vm #119 "echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message. task:kworker/8:1 state:D stack:0 pid:251 ppid:2 flags:0x000= 04000 Workqueue: events async_pf_execute [kvm] Call Trace: __schedule+0x33f/0xa40 schedule+0x53/0xc0 schedule_timeout+0x12a/0x140 __wait_for_common+0x8d/0x1d0 __flush_work.isra.0+0x19f/0x2c0 kvm_clear_async_pf_completion_queue+0x129/0x190 [kvm] kvm_arch_destroy_vm+0x78/0x1b0 [kvm] kvm_put_kvm+0x1c1/0x320 [kvm] async_pf_execute+0x198/0x260 [kvm] process_one_work+0x145/0x2d0 worker_thread+0x27e/0x3a0 kthread+0xba/0xe0 ret_from_fork+0x2d/0x50 ret_from_fork_asm+0x11/0x20 If kvm_clear_async_pf_completion_queue() actually flushes the workqueue, then there's no need to gift async_pf_execute() a reference because all invocations of async_pf_execute() will be forced to complete before the vCPU and its VM are destroyed/freed. And that in turn fixes the module unloading bug as __fput() won't do module_put() on the last vCPU reference until the vCPU has been freed, e.g. if closing the vCPU file also puts the last reference to the KVM module. Note that kvm_check_async_pf_completion() may also take the work item off the completion queue and so also needs to flush the work queue, as the work will not be seen by kvm_clear_async_pf_completion_queue(). Waiting on the workqueue could theoretically delay a vCPU due to waiting for the work to complete, but that's a very, very small chance, and likely a very small delay. kvm_arch_async_page_present_queued() unconditionally makes a new request, i.e. will effectively delay entering the guest, so the remaining work is really just: trace_kvm_async_pf_completed(addr, cr2_or_gpa); __kvm_vcpu_wake_up(vcpu); mmput(mm); and mmput() can't drop the last reference to the page tables if the vCPU is still alive, i.e. the vCPU won't get stuck tearing down page tables. Add a helper to do the flushing, specifically to deal with "wakeup all" work items, as they aren't actually work items, i.e. are never placed in a workqueue. Trying to flush a bogus workqueue entry rightly makes __flush_work() complain (kudos to whoever added that sanity check). Note, commit 5f6de5cbebee ("KVM: Prevent module exit until all VMs are freed") *tried* to fix the module refcounting issue by having VMs grab a reference to the module, but that only made the bug slightly harder to hit as it gave async_pf_execute() a bit more time to complete before the KVM module could be unloaded. Fixes: af585b921e5d ("KVM: Halt vcpu if page it tries to access is swapped = out") Cc: stable@vger.kernel.org Cc: David Matlack Cc: Xu Yilun Signed-off-by: Sean Christopherson Reviewed-by: Vitaly Kuznetsov --- virt/kvm/async_pf.c | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c index e033c79d528e..876927a558ad 100644 --- a/virt/kvm/async_pf.c +++ b/virt/kvm/async_pf.c @@ -87,7 +87,25 @@ static void async_pf_execute(struct work_struct *work) __kvm_vcpu_wake_up(vcpu); =20 mmput(mm); - kvm_put_kvm(vcpu->kvm); +} + +static void kvm_flush_and_free_async_pf_work(struct kvm_async_pf *work) +{ + /* + * The async #PF is "done", but KVM must wait for the work item itself, + * i.e. async_pf_execute(), to run to completion. If KVM is a module, + * KVM must ensure *no* code owned by the KVM (the module) can be run + * after the last call to module_put(), i.e. after the last reference + * to the last vCPU's file is put. + * + * Wake all events skip the queue and go straight done, i.e. don't + * need to be flushed (but sanity check that the work wasn't queued). + */ + if (work->wakeup_all) + WARN_ON_ONCE(work->work.func); + else + flush_work(&work->work); + kmem_cache_free(async_pf_cache, work); } =20 void kvm_clear_async_pf_completion_queue(struct kvm_vcpu *vcpu) @@ -114,7 +132,6 @@ void kvm_clear_async_pf_completion_queue(struct kvm_vcp= u *vcpu) #else if (cancel_work_sync(&work->work)) { mmput(work->mm); - kvm_put_kvm(vcpu->kvm); /* =3D=3D work->vcpu->kvm */ kmem_cache_free(async_pf_cache, work); } #endif @@ -126,7 +143,18 @@ void kvm_clear_async_pf_completion_queue(struct kvm_vc= pu *vcpu) list_first_entry(&vcpu->async_pf.done, typeof(*work), link); list_del(&work->link); - kmem_cache_free(async_pf_cache, work); + + spin_unlock(&vcpu->async_pf.lock); + + /* + * The async #PF is "done", but KVM must wait for the work item + * itself, i.e. async_pf_execute(), to run to completion. If + * KVM is a module, KVM must ensure *no* code owned by the KVM + * (the module) can be run after the last call to module_put(), + * i.e. after the last reference to the last vCPU's file is put. + */ + kvm_flush_and_free_async_pf_work(work); + spin_lock(&vcpu->async_pf.lock); } spin_unlock(&vcpu->async_pf.lock); =20 @@ -151,7 +179,7 @@ void kvm_check_async_pf_completion(struct kvm_vcpu *vcp= u) =20 list_del(&work->queue); vcpu->async_pf.queued--; - kmem_cache_free(async_pf_cache, work); + kvm_flush_and_free_async_pf_work(work); } } =20 @@ -186,7 +214,6 @@ bool kvm_setup_async_pf(struct kvm_vcpu *vcpu, gpa_t cr= 2_or_gpa, work->arch =3D *arch; work->mm =3D current->mm; mmget(work->mm); - kvm_get_kvm(work->vcpu->kvm); =20 INIT_WORK(&work->work, async_pf_execute); =20 --=20 2.43.0.472.g3155946c3a-goog