2022-03-09 03:56:49

by Yi Wang

[permalink] [raw]
Subject: [PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ

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 <[email protected]>
Signed-off-by: Yi Liu <[email protected]>
---
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


2022-03-09 09:51:16

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ

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 <[email protected]>
> Signed-off-by: Yi Liu <[email protected]>

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;

2022-03-09 10:41:17

by Yi Wang

[permalink] [raw]
Subject: Re:[PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ

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 <[email protected]>
> > Signed-off-by: Yi Liu <[email protected]>
>
> 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

2022-03-09 12:46:25

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ

On Wed, Mar 09, 2022 at 06:27:56PM +0800, [email protected] 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

2022-03-09 16:23:13

by Paolo Bonzini

[permalink] [raw]
Subject: Re: [PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ

On 3/9/22 11:27, [email protected] 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 <[email protected]>
Signed-off-by: Yi Liu <[email protected]>
Signed-off-by: Yi Wang <[email protected]>

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

Paolo

2022-03-10 06:01:21

by Yi Wang

[permalink] [raw]
Subject: Re:[PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ

> On Wed, Mar 09, 2022 at 06:27:56PM +0800, [email protected] 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

2022-03-10 06:36:38

by Yi Wang

[permalink] [raw]
Subject: Re:[PATCH] KVM: SVM: fix panic on out-of-bounds guest IRQ

> On 3/9/22 11:27, [email protected] 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 <[email protected]>
> Signed-off-by: Yi Liu <[email protected]>
> Signed-off-by: Yi Wang <[email protected]>
>
> I'll fix it myself - thanks for the quick reply!

Thanks, Paolo.

---
Best wishes
Yi Wang