Message ID | 20220309113025.44469-1-wang.yi59@zte.com.cn (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | KVM: SVM: fix panic on out-of-bounds guest IRQ | expand |
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;
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
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
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
> 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
> 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
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;