[PATCH v2] KVM: x86: put vcpu_create under kvm->srcu critical section

[PATCH v2] KVM: x86: put vcpu_create under kvm->srcu critical section

[Paolo Bonzini]

This is needed in case vcpu_create wants to access the memslots array.
Fixes this lockdep splat:

[26421.303750] ===============================
[26421.307952] [ INFO: suspicious RCU usage. ]
[26421.312161] 4.3.0-rc1+ #1 Not tainted
[26421.312161] -------------------------------
[26421.312162] include/linux/kvm_host.h:488 suspicious rcu_dereference_check() usage!
[26421.312163]
other info that might help us debug this:

[26421.312164]
rcu_scheduler_active = 1, debug_locks = 0
[26421.312165] 1 lock held by qemu-system-i38/17000:
[26421.312189]  #0:  (&(&kvm->mmu_lock)->rlock){+.+...}, at: [<ffffffffa02d6ab4>] kvm_zap_gfn_range+0x24/0x1a0 [kvm]
[26421.312189]
stack backtrace:
[26421.312191] CPU: 3 PID: 17000 Comm: qemu-system-i38 Not tainted 4.3.0-rc1+ #1
[26421.312192] Hardware name: To be filled by O.E.M. To be filled by O.E.M./M5A97 EVO R2.0, BIOS 1503 01/16/2013
[26421.312195]  0000000000000001 ffff880386c0fc90 ffffffff812c8c2a ffff880423f0c740
[26421.312197]  ffff880386c0fcc0 ffffffff8109e60d ffff880429ff8000 ffffffffffffffff
[26421.312199]  ffff880386844000 ffff880000000000 ffff880386c0fd30 ffffffffa02d6c18
[26421.312199] Call Trace:
[26421.312205]  [<ffffffff812c8c2a>] dump_stack+0x4e/0x84
[26421.312208]  [<ffffffff8109e60d>] lockdep_rcu_suspicious+0xfd/0x130
[26421.312223]  [<ffffffffa02d6c18>] kvm_zap_gfn_range+0x188/0x1a0 [kvm]
[26421.312235]  [<ffffffffa02bac7e>] kvm_set_cr0+0xde/0x1e0 [kvm]
[26421.312244]  [<ffffffffa0359e30>] init_vmcb+0x760/0xad0 [kvm_amd]
[26421.312246]  [<ffffffffa035a417>] svm_create_vcpu+0x197/0x250 [kvm_amd]
[26421.312259]  [<ffffffffa02c59c7>] kvm_arch_vcpu_create+0x47/0x70 [kvm]
[26421.312268]  [<ffffffffa02aff72>] kvm_vm_ioctl+0x302/0x7e0 [kvm]
[26421.312271]  [<ffffffff810a0ae1>] ? __lock_is_held+0x51/0x70
[26421.312273]  [<ffffffff811a0711>] ? __fget+0x101/0x210
[26421.312276]  [<ffffffff81194a54>] do_vfs_ioctl+0x2f4/0x560
[26421.312277]  [<ffffffff811a0889>] ? __fget_light+0x29/0x90
[26421.312279]  [<ffffffff81194d0c>] SyS_ioctl+0x4c/0x90
[26421.312282]  [<ffffffff816c495b>] entry_SYSCALL_64_fastpath+0x16/0x73

Reported-by: Borislav Petkov <bp@alien8.de>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 arch/x86/kvm/x86.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 991466bf8dee..6107e09fc5b6 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -7064,13 +7064,16 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
 						unsigned int id)
 {
 	struct kvm_vcpu *vcpu;
+	int idx;
 
 	if (check_tsc_unstable() && atomic_read(&kvm->online_vcpus) != 0)
 		printk_once(KERN_WARNING
 		"kvm: SMP vm created on host with unstable TSC; "
 		"guest TSC will not be reliable\n");
 
+	idx = srcu_read_lock(&kvm->srcu);
 	vcpu = kvm_x86_ops->vcpu_create(kvm, id);
+	srcu_read_unlock(&kvm->srcu, idx);
 
 	return vcpu;
 }
-- 
1.8.3.1

Re: [PATCH v2] KVM: x86: put vcpu_create under kvm->srcu critical section

[Borislav Petkov]

On Mon, Sep 21, 2015 at 08:02:56AM +0200, Paolo Bonzini wrote:
> This is needed in case vcpu_create wants to access the memslots array.
> Fixes this lockdep splat:
> 
> [26421.303750] ===============================
> [26421.307952] [ INFO: suspicious RCU usage. ]
> [26421.312161] 4.3.0-rc1+ #1 Not tainted
> [26421.312161] -------------------------------
> [26421.312162] include/linux/kvm_host.h:488 suspicious rcu_dereference_check() usage!
> [26421.312163]
> other info that might help us debug this:
> 
> [26421.312164]
> rcu_scheduler_active = 1, debug_locks = 0
> [26421.312165] 1 lock held by qemu-system-i38/17000:
> [26421.312189]  #0:  (&(&kvm->mmu_lock)->rlock){+.+...}, at: [<ffffffffa02d6ab4>] kvm_zap_gfn_range+0x24/0x1a0 [kvm]
> [26421.312189]
> stack backtrace:
> [26421.312191] CPU: 3 PID: 17000 Comm: qemu-system-i38 Not tainted 4.3.0-rc1+ #1
> [26421.312192] Hardware name: To be filled by O.E.M. To be filled by O.E.M./M5A97 EVO R2.0, BIOS 1503 01/16/2013
> [26421.312195]  0000000000000001 ffff880386c0fc90 ffffffff812c8c2a ffff880423f0c740
> [26421.312197]  ffff880386c0fcc0 ffffffff8109e60d ffff880429ff8000 ffffffffffffffff
> [26421.312199]  ffff880386844000 ffff880000000000 ffff880386c0fd30 ffffffffa02d6c18
> [26421.312199] Call Trace:
> [26421.312205]  [<ffffffff812c8c2a>] dump_stack+0x4e/0x84
> [26421.312208]  [<ffffffff8109e60d>] lockdep_rcu_suspicious+0xfd/0x130
> [26421.312223]  [<ffffffffa02d6c18>] kvm_zap_gfn_range+0x188/0x1a0 [kvm]
> [26421.312235]  [<ffffffffa02bac7e>] kvm_set_cr0+0xde/0x1e0 [kvm]
> [26421.312244]  [<ffffffffa0359e30>] init_vmcb+0x760/0xad0 [kvm_amd]
> [26421.312246]  [<ffffffffa035a417>] svm_create_vcpu+0x197/0x250 [kvm_amd]
> [26421.312259]  [<ffffffffa02c59c7>] kvm_arch_vcpu_create+0x47/0x70 [kvm]
> [26421.312268]  [<ffffffffa02aff72>] kvm_vm_ioctl+0x302/0x7e0 [kvm]
> [26421.312271]  [<ffffffff810a0ae1>] ? __lock_is_held+0x51/0x70
> [26421.312273]  [<ffffffff811a0711>] ? __fget+0x101/0x210
> [26421.312276]  [<ffffffff81194a54>] do_vfs_ioctl+0x2f4/0x560
> [26421.312277]  [<ffffffff811a0889>] ? __fget_light+0x29/0x90
> [26421.312279]  [<ffffffff81194d0c>] SyS_ioctl+0x4c/0x90
> [26421.312282]  [<ffffffff816c495b>] entry_SYSCALL_64_fastpath+0x16/0x73

You probably want to trim away all those numbers (timestamps and call
trace VAs) as they're useless.

> Reported-by: Borislav Petkov <bp@alien8.de>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
>  arch/x86/kvm/x86.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 991466bf8dee..6107e09fc5b6 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -7064,13 +7064,16 @@ struct kvm_vcpu *kvm_arch_vcpu_create(struct kvm *kvm,
>  						unsigned int id)
>  {
>  	struct kvm_vcpu *vcpu;
> +	int idx;
>  
>  	if (check_tsc_unstable() && atomic_read(&kvm->online_vcpus) != 0)
>  		printk_once(KERN_WARNING
>  		"kvm: SMP vm created on host with unstable TSC; "
>  		"guest TSC will not be reliable\n");
>  
> +	idx = srcu_read_lock(&kvm->srcu);
>  	vcpu = kvm_x86_ops->vcpu_create(kvm, id);
> +	srcu_read_unlock(&kvm->srcu, idx);
>  
>  	return vcpu;

Other than that, it works.

Tested-by: Borislav Petkov <bp@suse.de>

Thanks.

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.