On Mon, Aug 03, 2020 at 10:50:53PM +0200, Alexander Graf wrote:

On 03.08.20 13:37, Vitaly Kuznetsov wrote:

quoted

quoted

+static int kvm_vm_ioctl_clear_msr_allowlist(struct kvm *kvm)
+{
+     mutex_lock(&kvm->lock);
+     kvm->arch.msr_allowlist_ranges_count = 0;
+     mutex_unlock(&kvm->lock);

Are we also supposed to kfree() bitmaps here?

Phew. Yes, because without the kfree() we're leaking memory. Unfortunately
if I just put in a kfree() here, we may allow a concurrently executing vCPU
to access already free'd memory.

So I'll also add locking around the range check. Let's hope it won't regress
performance too much.

What about using KVM's SRCU to protect the list?  The only thing I'm not 100%
on is whether holding kvm->lock across synchronize_srcu() is safe from a lock
inversion perspective.  I'm pretty sure KVM doesn't try to acquire kvm->lock
after grabbing SRCU, but that's hard to audit and there aren't any existing
flows that invoke synchronize_srcu() while holding kvm->lock.