linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* Re: [PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ
  2022-03-09 11:30 [PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ Yi Wang
@ 2022-03-09  9:01 ` Paolo Bonzini
  2022-03-09 10:27   ` wang.yi59
  0 siblings, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2022-03-09  9:01 UTC (permalink / raw)
  To: Yi Wang
  Cc: seanjc, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
	dave.hansen, x86, hpa, kvm, linux-kernel, xue.zhihong, up2wing,
	wang.liang82, Yi Liu

On 3/9/22 12:30, Yi Wang wrote:
> As guest_irq is coming from KVM_IRQFD API call, it may trigger
> crash in svm_update_pi_irte() due to out-of-bounds:
> 
> crash> bt
> PID: 22218  TASK: ffff951a6ad74980  CPU: 73  COMMAND: "vcpu8"
>   #0 [ffffb1ba6707fa40] machine_kexec at ffffffff8565b397
>   #1 [ffffb1ba6707fa90] __crash_kexec at ffffffff85788a6d
>   #2 [ffffb1ba6707fb58] crash_kexec at ffffffff8578995d
>   #3 [ffffb1ba6707fb70] oops_end at ffffffff85623c0d
>   #4 [ffffb1ba6707fb90] no_context at ffffffff856692c9
>   #5 [ffffb1ba6707fbf8] exc_page_fault at ffffffff85f95b51
>   #6 [ffffb1ba6707fc50] asm_exc_page_fault at ffffffff86000ace
>      [exception RIP: svm_update_pi_irte+227]
>      RIP: ffffffffc0761b53  RSP: ffffb1ba6707fd08  RFLAGS: 00010086
>      RAX: ffffb1ba6707fd78  RBX: ffffb1ba66d91000  RCX: 0000000000000001
>      RDX: 00003c803f63f1c0  RSI: 000000000000019a  RDI: ffffb1ba66db2ab8
>      RBP: 000000000000019a   R8: 0000000000000040   R9: ffff94ca41b82200
>      R10: ffffffffffffffcf  R11: 0000000000000001  R12: 0000000000000001
>      R13: 0000000000000001  R14: ffffffffffffffcf  R15: 000000000000005f
>      ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0018
>   #7 [ffffb1ba6707fdb8] kvm_irq_routing_update at ffffffffc09f19a1 [kvm]
>   #8 [ffffb1ba6707fde0] kvm_set_irq_routing at ffffffffc09f2133 [kvm]
>   #9 [ffffb1ba6707fe18] kvm_vm_ioctl at ffffffffc09ef544 [kvm]
> #10 [ffffb1ba6707ff10] __x64_sys_ioctl at ffffffff85935474
> #11 [ffffb1ba6707ff40] do_syscall_64 at ffffffff85f921d3
> #12 [ffffb1ba6707ff50] entry_SYSCALL_64_after_hwframe at ffffffff8600007c
>      RIP: 00007f143c36488b  RSP: 00007f143a4e04b8  RFLAGS: 00000246
>      RAX: ffffffffffffffda  RBX: 00007f05780041d0  RCX: 00007f143c36488b
>      RDX: 00007f05780041d0  RSI: 000000004008ae6a  RDI: 0000000000000020
>      RBP: 00000000000004e8   R8: 0000000000000008   R9: 00007f05780041e0
>      R10: 00007f0578004560  R11: 0000000000000246  R12: 00000000000004e0
>      R13: 000000000000001a  R14: 00007f1424001c60  R15: 00007f0578003bc0
>      ORIG_RAX: 0000000000000010  CS: 0033  SS: 002b
> 
> Vmx have been fix this in commit 3a8b0677fc61 (KVM: VMX: Do not BUG() on
> out-of-bounds guest IRQ), so we can just copy source from that to fix
> this.
> 
> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
> Signed-off-by: Yi Liu <liu.yi24@zte.com.cn>

Hi, the Signed-off-by chain is wrong.  Did Yi Liu write the patch (and 
you are just sending it)?

Paolo

> ---
>   arch/x86/kvm/svm/avic.c | 10 ++++++++--
>   1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
> index fb3e20791338..f59b93d8e95a 100644
> --- a/arch/x86/kvm/svm/avic.c
> +++ b/arch/x86/kvm/svm/avic.c
> @@ -783,7 +783,7 @@ int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
>   {
>   	struct kvm_kernel_irq_routing_entry *e;
>   	struct kvm_irq_routing_table *irq_rt;
> -	int idx, ret = -EINVAL;
> +	int idx, ret = 0;
>   
>   	if (!kvm_arch_has_assigned_device(kvm) ||
>   	    !irq_remapping_cap(IRQ_POSTING_CAP))
> @@ -794,7 +794,13 @@ int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
>   
>   	idx = srcu_read_lock(&kvm->irq_srcu);
>   	irq_rt = srcu_dereference(kvm->irq_routing, &kvm->irq_srcu);
> -	WARN_ON(guest_irq >= irq_rt->nr_rt_entries);
> +
> +	if (guest_irq >= irq_rt->nr_rt_entries ||
> +		hlist_empty(&irq_rt->map[guest_irq])) {
> +		pr_warn_once("no route for guest_irq %u/%u (broken user space?)\n",
> +			     guest_irq, irq_rt->nr_rt_entries);
> +		goto out;
> +	}
>   
>   	hlist_for_each_entry(e, &irq_rt->map[guest_irq], link) {
>   		struct vcpu_data vcpu_info;


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re:[PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ
  2022-03-09  9:01 ` Paolo Bonzini
@ 2022-03-09 10:27   ` wang.yi59
  2022-03-09 11:02     ` [PATCH] " Paolo Bonzini
  2022-03-09 12:17     ` [PATCH] " Borislav Petkov
  0 siblings, 2 replies; 7+ messages in thread
From: wang.yi59 @ 2022-03-09 10:27 UTC (permalink / raw)
  To: pbonzini
  Cc: seanjc, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
	dave.hansen, x86, hpa, kvm, linux-kernel, xue.zhihong, up2wing,
	wang.liang82, liu.yi24

Hi Paolo,

> On 3/9/22 12:30, Yi Wang wrote:
> > As guest_irq is coming from KVM_IRQFD API call, it may trigger
> > crash in svm_update_pi_irte() due to out-of-bounds:
> > 
> > crash> bt
> > PID: 22218  TASK: ffff951a6ad74980  CPU: 73  COMMAND: "vcpu8"
> >   #0 [ffffb1ba6707fa40] machine_kexec at ffffffff8565b397
> >   #1 [ffffb1ba6707fa90] __crash_kexec at ffffffff85788a6d
> >   #2 [ffffb1ba6707fb58] crash_kexec at ffffffff8578995d
> >   #3 [ffffb1ba6707fb70] oops_end at ffffffff85623c0d
> >   #4 [ffffb1ba6707fb90] no_context at ffffffff856692c9
> >   #5 [ffffb1ba6707fbf8] exc_page_fault at ffffffff85f95b51
> >   #6 [ffffb1ba6707fc50] asm_exc_page_fault at ffffffff86000ace
> >      [exception RIP: svm_update_pi_irte+227]
> >      RIP: ffffffffc0761b53  RSP: ffffb1ba6707fd08  RFLAGS: 00010086
> >      RAX: ffffb1ba6707fd78  RBX: ffffb1ba66d91000  RCX: 0000000000000001
> >      RDX: 00003c803f63f1c0  RSI: 000000000000019a  RDI: ffffb1ba66db2ab8
> >      RBP: 000000000000019a   R8: 0000000000000040   R9: ffff94ca41b82200
> >      R10: ffffffffffffffcf  R11: 0000000000000001  R12: 0000000000000001
> >      R13: 0000000000000001  R14: ffffffffffffffcf  R15: 000000000000005f
> >      ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0018
> >   #7 [ffffb1ba6707fdb8] kvm_irq_routing_update at ffffffffc09f19a1 [kvm]
> >   #8 [ffffb1ba6707fde0] kvm_set_irq_routing at ffffffffc09f2133 [kvm]
> >   #9 [ffffb1ba6707fe18] kvm_vm_ioctl at ffffffffc09ef544 [kvm]
> > #10 [ffffb1ba6707ff10] __x64_sys_ioctl at ffffffff85935474
> > #11 [ffffb1ba6707ff40] do_syscall_64 at ffffffff85f921d3
> > #12 [ffffb1ba6707ff50] entry_SYSCALL_64_after_hwframe at ffffffff8600007c
> >      RIP: 00007f143c36488b  RSP: 00007f143a4e04b8  RFLAGS: 00000246
> >      RAX: ffffffffffffffda  RBX: 00007f05780041d0  RCX: 00007f143c36488b
> >      RDX: 00007f05780041d0  RSI: 000000004008ae6a  RDI: 0000000000000020
> >      RBP: 00000000000004e8   R8: 0000000000000008   R9: 00007f05780041e0
> >      R10: 00007f0578004560  R11: 0000000000000246  R12: 00000000000004e0
> >      R13: 000000000000001a  R14: 00007f1424001c60  R15: 00007f0578003bc0
> >      ORIG_RAX: 0000000000000010  CS: 0033  SS: 002b
> > 
> > Vmx have been fix this in commit 3a8b0677fc61 (KVM: VMX: Do not BUG() on
> > out-of-bounds guest IRQ), so we can just copy source from that to fix
> > this.
> > 
> > Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
> > Signed-off-by: Yi Liu <liu.yi24@zte.com.cn>
> 
> Hi, the Signed-off-by chain is wrong.  Did Yi Liu write the patch (and 
> you are just sending it)?

The Signed-off-by chain is not wrong, I (Yi Wang) wrote this patch and Yi Liu
co-developed it.

> 
> Paolo

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ
  2022-03-09 10:27   ` wang.yi59
@ 2022-03-09 11:02     ` Paolo Bonzini
  2022-03-10  3:12       ` wang.yi59
  2022-03-09 12:17     ` [PATCH] " Borislav Petkov
  1 sibling, 1 reply; 7+ messages in thread
From: Paolo Bonzini @ 2022-03-09 11:02 UTC (permalink / raw)
  To: wang.yi59
  Cc: seanjc, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
	dave.hansen, x86, hpa, kvm, linux-kernel, xue.zhihong, up2wing,
	wang.liang82, liu.yi24

On 3/9/22 11:27, wang.yi59@zte.com.cn wrote:
>> Hi, the Signed-off-by chain is wrong.  Did Yi Liu write the patch (and
>> you are just sending it)?
> The Signed-off-by chain is not wrong, I (Yi Wang) wrote this patch and Yi Liu
> co-developed it.
> 

Ok, so it should be

Co-developed-by: Yi Liu <liu.yi24@zte.com.cn>
Signed-off-by: Yi Liu <liu.yi24@zte.com.cn>
Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>

I'll fix it myself - thanks for the quick reply!

Paolo


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ
@ 2022-03-09 11:30 Yi Wang
  2022-03-09  9:01 ` Paolo Bonzini
  0 siblings, 1 reply; 7+ messages in thread
From: Yi Wang @ 2022-03-09 11:30 UTC (permalink / raw)
  To: pbonzini
  Cc: seanjc, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
	dave.hansen, x86, hpa, kvm, linux-kernel, xue.zhihong, wang.yi59,
	up2wing, wang.liang82, Yi Liu

As guest_irq is coming from KVM_IRQFD API call, it may trigger
crash in svm_update_pi_irte() due to out-of-bounds:

crash> bt
PID: 22218  TASK: ffff951a6ad74980  CPU: 73  COMMAND: "vcpu8"
 #0 [ffffb1ba6707fa40] machine_kexec at ffffffff8565b397
 #1 [ffffb1ba6707fa90] __crash_kexec at ffffffff85788a6d
 #2 [ffffb1ba6707fb58] crash_kexec at ffffffff8578995d
 #3 [ffffb1ba6707fb70] oops_end at ffffffff85623c0d
 #4 [ffffb1ba6707fb90] no_context at ffffffff856692c9
 #5 [ffffb1ba6707fbf8] exc_page_fault at ffffffff85f95b51
 #6 [ffffb1ba6707fc50] asm_exc_page_fault at ffffffff86000ace
    [exception RIP: svm_update_pi_irte+227]
    RIP: ffffffffc0761b53  RSP: ffffb1ba6707fd08  RFLAGS: 00010086
    RAX: ffffb1ba6707fd78  RBX: ffffb1ba66d91000  RCX: 0000000000000001
    RDX: 00003c803f63f1c0  RSI: 000000000000019a  RDI: ffffb1ba66db2ab8
    RBP: 000000000000019a   R8: 0000000000000040   R9: ffff94ca41b82200
    R10: ffffffffffffffcf  R11: 0000000000000001  R12: 0000000000000001
    R13: 0000000000000001  R14: ffffffffffffffcf  R15: 000000000000005f
    ORIG_RAX: ffffffffffffffff  CS: 0010  SS: 0018
 #7 [ffffb1ba6707fdb8] kvm_irq_routing_update at ffffffffc09f19a1 [kvm]
 #8 [ffffb1ba6707fde0] kvm_set_irq_routing at ffffffffc09f2133 [kvm]
 #9 [ffffb1ba6707fe18] kvm_vm_ioctl at ffffffffc09ef544 [kvm]
#10 [ffffb1ba6707ff10] __x64_sys_ioctl at ffffffff85935474
#11 [ffffb1ba6707ff40] do_syscall_64 at ffffffff85f921d3
#12 [ffffb1ba6707ff50] entry_SYSCALL_64_after_hwframe at ffffffff8600007c
    RIP: 00007f143c36488b  RSP: 00007f143a4e04b8  RFLAGS: 00000246
    RAX: ffffffffffffffda  RBX: 00007f05780041d0  RCX: 00007f143c36488b
    RDX: 00007f05780041d0  RSI: 000000004008ae6a  RDI: 0000000000000020
    RBP: 00000000000004e8   R8: 0000000000000008   R9: 00007f05780041e0
    R10: 00007f0578004560  R11: 0000000000000246  R12: 00000000000004e0
    R13: 000000000000001a  R14: 00007f1424001c60  R15: 00007f0578003bc0
    ORIG_RAX: 0000000000000010  CS: 0033  SS: 002b

Vmx have been fix this in commit 3a8b0677fc61 (KVM: VMX: Do not BUG() on
out-of-bounds guest IRQ), so we can just copy source from that to fix
this.

Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
Signed-off-by: Yi Liu <liu.yi24@zte.com.cn>
---
 arch/x86/kvm/svm/avic.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c
index fb3e20791338..f59b93d8e95a 100644
--- a/arch/x86/kvm/svm/avic.c
+++ b/arch/x86/kvm/svm/avic.c
@@ -783,7 +783,7 @@ int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
 {
 	struct kvm_kernel_irq_routing_entry *e;
 	struct kvm_irq_routing_table *irq_rt;
-	int idx, ret = -EINVAL;
+	int idx, ret = 0;
 
 	if (!kvm_arch_has_assigned_device(kvm) ||
 	    !irq_remapping_cap(IRQ_POSTING_CAP))
@@ -794,7 +794,13 @@ int svm_update_pi_irte(struct kvm *kvm, unsigned int host_irq,
 
 	idx = srcu_read_lock(&kvm->irq_srcu);
 	irq_rt = srcu_dereference(kvm->irq_routing, &kvm->irq_srcu);
-	WARN_ON(guest_irq >= irq_rt->nr_rt_entries);
+
+	if (guest_irq >= irq_rt->nr_rt_entries ||
+		hlist_empty(&irq_rt->map[guest_irq])) {
+		pr_warn_once("no route for guest_irq %u/%u (broken user space?)\n",
+			     guest_irq, irq_rt->nr_rt_entries);
+		goto out;
+	}
 
 	hlist_for_each_entry(e, &irq_rt->map[guest_irq], link) {
 		struct vcpu_data vcpu_info;
-- 
2.33.0.rc0.dirty

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ
  2022-03-09 10:27   ` wang.yi59
  2022-03-09 11:02     ` [PATCH] " Paolo Bonzini
@ 2022-03-09 12:17     ` Borislav Petkov
  2022-03-10  3:17       ` wang.yi59
  1 sibling, 1 reply; 7+ messages in thread
From: Borislav Petkov @ 2022-03-09 12:17 UTC (permalink / raw)
  To: wang.yi59
  Cc: pbonzini, seanjc, vkuznets, wanpengli, jmattson, joro, tglx,
	mingo, dave.hansen, x86, hpa, kvm, linux-kernel, xue.zhihong,
	up2wing, wang.liang82, liu.yi24

On Wed, Mar 09, 2022 at 06:27:56PM +0800, wang.yi59@zte.com.cn wrote:
> The Signed-off-by chain is not wrong, I (Yi Wang) wrote this patch and Yi Liu
> co-developed it.

What to do in such cases is well documented. For the future, make sure
you look at

Documentation/process/submitting-patches.rst

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re:[PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ
  2022-03-09 11:02     ` [PATCH] " Paolo Bonzini
@ 2022-03-10  3:12       ` wang.yi59
  0 siblings, 0 replies; 7+ messages in thread
From: wang.yi59 @ 2022-03-10  3:12 UTC (permalink / raw)
  To: pbonzini
  Cc: seanjc, vkuznets, wanpengli, jmattson, joro, tglx, mingo, bp,
	dave.hansen, x86, hpa, kvm, linux-kernel, xue.zhihong, up2wing,
	wang.liang82, liu.yi24

> On 3/9/22 11:27, wang.yi59@zte.com.cn wrote:
> >> Hi, the Signed-off-by chain is wrong.  Did Yi Liu write the patch (and
> >> you are just sending it)?
> > The Signed-off-by chain is not wrong, I (Yi Wang) wrote this patch and Yi Liu
> > co-developed it.
> > 
> 
> Ok, so it should be
> 
> Co-developed-by: Yi Liu <liu.yi24@zte.com.cn>
> Signed-off-by: Yi Liu <liu.yi24@zte.com.cn>
> Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
> 
> I'll fix it myself - thanks for the quick reply!

Thanks, Paolo.

---
Best wishes
Yi Wang

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re:[PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ
  2022-03-09 12:17     ` [PATCH] " Borislav Petkov
@ 2022-03-10  3:17       ` wang.yi59
  0 siblings, 0 replies; 7+ messages in thread
From: wang.yi59 @ 2022-03-10  3:17 UTC (permalink / raw)
  To: bp
  Cc: pbonzini, seanjc, vkuznets, wanpengli, jmattson, joro, tglx,
	mingo, dave.hansen, x86, hpa, kvm, linux-kernel, xue.zhihong,
	up2wing, wang.liang82, liu.yi24

> On Wed, Mar 09, 2022 at 06:27:56PM +0800, wang.yi59@zte.com.cn wrote:
> > The Signed-off-by chain is not wrong, I (Yi Wang) wrote this patch and Yi Liu
> > co-developed it.
> 
> What to do in such cases is well documented. For the future, make sure
> you look at
> 
> Documentation/process/submitting-patches.rst

Thanks, Boris.

I will go over the doc and pay more attention on the patch format in the future.

---
Best wishes
Yi Wang

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2022-03-10  3:17 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-09 11:30 [PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ Yi Wang
2022-03-09  9:01 ` Paolo Bonzini
2022-03-09 10:27   ` wang.yi59
2022-03-09 11:02     ` [PATCH] " Paolo Bonzini
2022-03-10  3:12       ` wang.yi59
2022-03-09 12:17     ` [PATCH] " Borislav Petkov
2022-03-10  3:17       ` wang.yi59

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).