Patch "kvm/x86: Avoid async PF preempting the kernel incorrectly" has been added to the 4.13-stable tree

PATCH [email protected] 1 participant 1 message Oct 10, 2017

Subsystems (4)

Stable branch x86 KVM x86 KVM paravirt

lore

Share & tools

lore

[email protected]

author maintainer Applied

2017-10-10 15:29 UTC #0 lore raw

This is a note to let you know that I've just added the patch titled

kvm/x86: Avoid async PF preempting the kernel incorrectly

to the 4.13-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
kvm-x86-avoid-async-pf-preempting-the-kernel-incorrectly.patch
and it can be found in the queue-4.13 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From a2b7861bb33b2538420bb5d8554153484d3f961f Mon Sep 17 00:00:00 2001

From: Boqun Feng <[email protected]>
Date: Tue, 3 Oct 2017 21:36:51 +0800
Subject: kvm/x86: Avoid async PF preempting the kernel incorrectly
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

From: Boqun Feng <[email protected]>

commit a2b7861bb33b2538420bb5d8554153484d3f961f upstream.

Currently, in PREEMPT_COUNT=n kernel, kvm_async_pf_task_wait() could call schedule() to reschedule in some cases. This could result in accidentally ending the current RCU read-side critical section early, causing random memory corruption in the guest, or otherwise preempting the currently running task inside between preempt_disable and preempt_enable.

The difficulty to handle this well is because we don't know whether an async PF delivered in a preemptible section or RCU read-side critical section for PREEMPT_COUNT=n, since preempt_disable()/enable() and rcu_read_lock/unlock() are both no-ops in that case.

To cure this, we treat any async PF interrupting a kernel context as one that cannot be preempted, preventing kvm_async_pf_task_wait() from choosing the schedule() path in that case.

To do so, a second parameter for kvm_async_pf_task_wait() is introduced, so that we know whether it's called from a context interrupting the kernel, and the parameter is set properly in all the callsites.

Cc "Paul E. McKenney" <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Wanpeng Li <[email protected]>

Signed-off-by Boqun Feng <[email protected]>

Signed-off-by Radim Krčmář <[email protected]>

Signed-off-by Greg Kroah-Hartman <[email protected]>

---
 arch/x86/include/asm/kvm_para.h |    4 ++--
 arch/x86/kernel/kvm.c           |   14 ++++++++++----
 arch/x86/kvm/mmu.c              |    2 +-
 3 files changed, 13 insertions(+), 7 deletions(-)

arch/x86/include/asm/kvm_para.h +2 -2

@@ -88,7 +88,7 @@ static inline long kvm_hypercall4(unsign
 bool kvm_para_available(void);
 unsigned int kvm_arch_para_features(void);
 void __init kvm_guest_init(void);
-void kvm_async_pf_task_wait(u32 token);
+void kvm_async_pf_task_wait(u32 token, int interrupt_kernel);
 void kvm_async_pf_task_wake(u32 token);
 u32 kvm_read_and_reset_pf_reason(void);
 extern void kvm_disable_steal_time(void);

@@ -103,7 +103,7 @@ static inline void kvm_spinlock_init(voi
 
 #else /* CONFIG_KVM_GUEST */
 #define kvm_guest_init() do {} while (0)
-#define kvm_async_pf_task_wait(T) do {} while(0)
+#define kvm_async_pf_task_wait(T, I) do {} while(0)
 #define kvm_async_pf_task_wake(T) do {} while(0)
 
 static inline bool kvm_para_available(void)

arch/x86/kernel/kvm.c +10 -4

@@ -117,7 +117,11 @@ static struct kvm_task_sleep_node *_find
 	return NULL;
 }
 
-void kvm_async_pf_task_wait(u32 token)
+/*
+ * @interrupt_kernel: Is this called from a routine which interrupts the kernel
+ * 		      (other than user space)?
+ */
+void kvm_async_pf_task_wait(u32 token, int interrupt_kernel)
 {
 	u32 key = hash_32(token, KVM_TASK_SLEEP_HASHBITS);
 	struct kvm_task_sleep_head *b = &async_pf_sleepers[key];

@@ -140,8 +144,10 @@ void kvm_async_pf_task_wait(u32 token)
 
 	n.token = token;
 	n.cpu = smp_processor_id();
-	n.halted = is_idle_task(current) || preempt_count() > 1 ||
-		   rcu_preempt_depth();
+	n.halted = is_idle_task(current) ||
+		   (IS_ENABLED(CONFIG_PREEMPT_COUNT)
+		    ? preempt_count() > 1 || rcu_preempt_depth()
+		    : interrupt_kernel);
 	init_swait_queue_head(&n.wq);
 	hlist_add_head(&n.link, &b->list);
 	raw_spin_unlock(&b->lock);

@@ -269,7 +275,7 @@ do_async_page_fault(struct pt_regs *regs
 	case KVM_PV_REASON_PAGE_NOT_PRESENT:
 		/* page is swapped out by the host. */
 		prev_state = exception_enter();
-		kvm_async_pf_task_wait((u32)read_cr2());
+		kvm_async_pf_task_wait((u32)read_cr2(), !user_mode(regs));
 		exception_exit(prev_state);
 		break;
 	case KVM_PV_REASON_PAGE_READY:

arch/x86/kvm/mmu.c +1 -1

@@ -3799,7 +3799,7 @@ int kvm_handle_page_fault(struct kvm_vcp
 	case KVM_PV_REASON_PAGE_NOT_PRESENT:
 		vcpu->arch.apf.host_apf_reason = 0;
 		local_irq_disable();
-		kvm_async_pf_task_wait(fault_address);
+		kvm_async_pf_task_wait(fault_address, 0);
 		local_irq_enable();
 		break;
 	case KVM_PV_REASON_PAGE_READY:

Patches currently in stable-queue which might be from [email protected] are

queue-4.13/kvm-x86-avoid-async-pf-preempting-the-kernel-incorrectly.patch