[PATCH v2] x86/kvm: Avoid freeing stack-allocated node in kvm_async_pf_queue_task

[Ryosuke Yasuoka <ryasuoka@redhat.com>]

kvm_async_pf_queue_task() can incorrectly try to kfree() a node
allocated on the stack of kvm_async_pf_task_wait_schedule().

This occurs when a task requests a PF while another task's PF request
with the same token is still pending. Since the token is derived from
the (u32)address in exc_page_fault(), two different tasks can generate
the same token.

Currently, kvm_async_pf_queue_task() assumes that any entry found in the
list is a dummy entry and tries to kfree() it. To fix this, add a flag
to the node structure to distinguish stack-allocated nodes, and only
kfree() the node if it is a dummy entry.

Signed-off-by: Ryosuke Yasuoka <ryasuoka@redhat.com>
---

v2:
Based on Vitaly's comment,
* Update comment in kvm_async_pf_queue_task
* Set n->dummy false in kvm_async_pf_queue_task
* Add explanation about what token is in commit message.

v1:
https://lore.kernel.org/all/87cy4vlmv8.fsf@redhat.com/


 arch/x86/kernel/kvm.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
index df78ddee0abb..37dc8465e0f5 100644
--- a/arch/x86/kernel/kvm.c
+++ b/arch/x86/kernel/kvm.c
@@ -89,6 +89,7 @@ struct kvm_task_sleep_node {
 	struct swait_queue_head wq;
 	u32 token;
 	int cpu;
+	bool dummy;
 };
 
 static struct kvm_task_sleep_head {
@@ -120,15 +121,26 @@ static bool kvm_async_pf_queue_task(u32 token, struct kvm_task_sleep_node *n)
 	raw_spin_lock(&b->lock);
 	e = _find_apf_task(b, token);
 	if (e) {
-		/* dummy entry exist -> wake up was delivered ahead of PF */
-		hlist_del(&e->link);
+		struct kvm_task_sleep_node *dummy = NULL;
+
+		/*
+		 * The entry can either be a 'dummy' entry (which is put on the
+		 * list when wake-up happens ahead of APF handling completion)
+		 * or a token from another task which should not be touched.
+		 */
+		if (e->dummy) {
+			hlist_del(&e->link);
+			dummy = e;
+		}
+
 		raw_spin_unlock(&b->lock);
-		kfree(e);
+		kfree(dummy);
 		return false;
 	}
 
 	n->token = token;
 	n->cpu = smp_processor_id();
+	n->dummy = false;
 	init_swait_queue_head(&n->wq);
 	hlist_add_head(&n->link, &b->list);
 	raw_spin_unlock(&b->lock);
@@ -231,6 +243,7 @@ static void kvm_async_pf_task_wake(u32 token)
 		}
 		dummy->token = token;
 		dummy->cpu = smp_processor_id();
+		dummy->dummy = true;
 		init_swait_queue_head(&dummy->wq);
 		hlist_add_head(&dummy->link, &b->list);
 		dummy = NULL;

base-commit: 416f99c3b16f582a3fc6d64a1f77f39d94b76de5
-- 
2.52.0