* [PATCH] KVM: Harden against unpaired kvm_mmu_notifier_invalidate_range_end() calls
@ 2024-01-10 0:42 Sean Christopherson
2024-01-31 0:59 ` Sean Christopherson
0 siblings, 1 reply; 2+ messages in thread
From: Sean Christopherson @ 2024-01-10 0:42 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: kvm, linux-kernel, Sean Christopherson
When handling the end of an mmu_notifier invalidation, WARN if
mn_active_invalidate_count is already 0 do not decrement it further, i.e.
avoid causing mn_active_invalidate_count to underflow/wrap. In the worst
case scenario, effectively corrupting mn_active_invalidate_count could
cause kvm_swap_active_memslots() to hang indefinitely.
end() calls are *supposed* to be paired with start(), i.e. underflow can
only happen if there is a bug elsewhere in the kernel, but due to lack of
lockdep assertions in the mmu_notifier helpers, it's all too easy for a
bug to go unnoticed for some time, e.g. see the recently introduced
PAGEMAP_SCAN ioctl().
Ideally, mmu_notifiers would incorporate lockdep assertions, but users of
mmu_notifiers aren't required to hold any one specific lock, i.e. adding
the necessary annotations to make lockdep aware of all locks that are
mutally exclusive with mm_take_all_locks() isn't trivial.
Link: https://lore.kernel.org/all/000000000000f6d051060c6785bc@google.com
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
virt/kvm/kvm_main.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 10bfc88a69f7..8f03b56dafbd 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -890,7 +890,9 @@ static void kvm_mmu_notifier_invalidate_range_end(struct mmu_notifier *mn,
/* Pairs with the increment in range_start(). */
spin_lock(&kvm->mn_invalidate_lock);
- wake = (--kvm->mn_active_invalidate_count == 0);
+ if (!WARN_ON_ONCE(!kvm->mn_active_invalidate_count))
+ --kvm->mn_active_invalidate_count;
+ wake = !kvm->mn_active_invalidate_count;
spin_unlock(&kvm->mn_invalidate_lock);
/*
base-commit: 1c6d984f523f67ecfad1083bb04c55d91977bb15
--
2.43.0.472.g3155946c3a-goog
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] KVM: Harden against unpaired kvm_mmu_notifier_invalidate_range_end() calls
2024-01-10 0:42 [PATCH] KVM: Harden against unpaired kvm_mmu_notifier_invalidate_range_end() calls Sean Christopherson
@ 2024-01-31 0:59 ` Sean Christopherson
0 siblings, 0 replies; 2+ messages in thread
From: Sean Christopherson @ 2024-01-31 0:59 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, linux-kernel
On Tue, 09 Jan 2024 16:42:39 -0800, Sean Christopherson wrote:
> When handling the end of an mmu_notifier invalidation, WARN if
> mn_active_invalidate_count is already 0 do not decrement it further, i.e.
> avoid causing mn_active_invalidate_count to underflow/wrap. In the worst
> case scenario, effectively corrupting mn_active_invalidate_count could
> cause kvm_swap_active_memslots() to hang indefinitely.
>
> end() calls are *supposed* to be paired with start(), i.e. underflow can
> only happen if there is a bug elsewhere in the kernel, but due to lack of
> lockdep assertions in the mmu_notifier helpers, it's all too easy for a
> bug to go unnoticed for some time, e.g. see the recently introduced
> PAGEMAP_SCAN ioctl().
>
> [...]
Applied to kvm-x86 generic, thanks!
[1/1] KVM: Harden against unpaired kvm_mmu_notifier_invalidate_range_end() calls
https://github.com/kvm-x86/linux/commit/d489ec956583
--
https://github.com/kvm-x86/linux/tree/next
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2024-01-31 0:59 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-01-10 0:42 [PATCH] KVM: Harden against unpaired kvm_mmu_notifier_invalidate_range_end() calls Sean Christopherson
2024-01-31 0:59 ` Sean Christopherson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).