| Message ID | 1428679079-16499-1-git-send-email-andre.przywara@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/04/15 16:17, Andre Przywara wrote: > When userland injects a SPI via the KVM_IRQ_LINE ioctl we currently > only check it against a fixed limit, which historically is set > to 127. With the new dynamic IRQ allocation the effective limit may > actually be smaller (64). > So when now a malicious or buggy userland injects a SPI in that > range, we spill over on our VGIC bitmaps and bytemaps memory. > I could trigger a host kernel NULL pointer dereference with current > mainline by injecting some bogus IRQ number from a hacked kvmtool: > ----------------- > .... > DEBUG: kvm_vgic_inject_irq(kvm, cpu=0, irq=114, level=1) > DEBUG: vgic_update_irq_pending(kvm, cpu=0, irq=114, level=1) > DEBUG: IRQ #114 still in the game, writing to bytemap now... > Unable to handle kernel NULL pointer dereference at virtual address 00000000 > pgd = ffffffc07652e000 > [00000000] *pgd=00000000f658b003, *pud=00000000f658b003, *pmd=0000000000000000 > Internal error: Oops: 96000006 [#1] PREEMPT SMP > Modules linked in: > CPU: 1 PID: 1053 Comm: lkvm-msi-irqinj Not tainted 4.0.0-rc7+ #3027 > Hardware name: FVP Base (DT) > task: ffffffc0774e9680 ti: ffffffc0765a8000 task.ti: ffffffc0765a8000 > PC is at kvm_vgic_inject_irq+0x234/0x310 > LR is at kvm_vgic_inject_irq+0x30c/0x310 > pc : [<ffffffc0000ae0a8>] lr : [<ffffffc0000ae180>] pstate: 80000145 > ..... > > So this patch fixes this by checking the SPI number against the > actual limit. Also we remove the former legacy hard limit of > 127 in the ioctl code. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > CC: <stable@vger.kernel.org> # 4.0, 3.19, 3.18 Reviewed-by: Marc Zyngier <marc.zyngier@arm.com> It is getting really tight for 4.0, but hopefully I can squeeze it in a second pull request together with the missing barrier on 32bit. Thanks, M.
Hi Andre, On 04/10/2015 11:17 AM, Andre Przywara wrote: > When userland injects a SPI via the KVM_IRQ_LINE ioctl we currently > only check it against a fixed limit, which historically is set > to 127. With the new dynamic IRQ allocation the effective limit may > actually be smaller (64). > So when now a malicious or buggy userland injects a SPI in that > range, we spill over on our VGIC bitmaps and bytemaps memory. > I could trigger a host kernel NULL pointer dereference with current > mainline by injecting some bogus IRQ number from a hacked kvmtool: > --- a/arch/arm/include/uapi/asm/kvm.h > +++ b/arch/arm/include/uapi/asm/kvm.h > @@ -195,7 +195,11 @@ struct kvm_arch_memory_slot { > #define KVM_ARM_IRQ_CPU_IRQ 0 > #define KVM_ARM_IRQ_CPU_FIQ 1 > > -/* Highest supported SPI, from VGIC_NR_IRQS */ > +/* > + * This used to hold the highest supported SPI, but it is now obsolete > + * and only here to provide source code level compatibility with older > + * userland. The highest SPI number can be set via KVM_DEV_ARM_VGIC_GRP_NR_IRQS. > + */ > #define KVM_ARM_IRQ_GIC_MAX 127 If that's the case should it maybe only defined when __KERNEL__ is not defined? Chris
On 10/04/2015 17:25, Marc Zyngier wrote: > On 10/04/15 16:17, Andre Przywara wrote: >> When userland injects a SPI via the KVM_IRQ_LINE ioctl we currently >> only check it against a fixed limit, which historically is set >> to 127. With the new dynamic IRQ allocation the effective limit may >> actually be smaller (64). >> So when now a malicious or buggy userland injects a SPI in that >> range, we spill over on our VGIC bitmaps and bytemaps memory. >> I could trigger a host kernel NULL pointer dereference with current >> mainline by injecting some bogus IRQ number from a hacked kvmtool: >> ----------------- >> .... >> DEBUG: kvm_vgic_inject_irq(kvm, cpu=0, irq=114, level=1) >> DEBUG: vgic_update_irq_pending(kvm, cpu=0, irq=114, level=1) >> DEBUG: IRQ #114 still in the game, writing to bytemap now... >> Unable to handle kernel NULL pointer dereference at virtual address 00000000 >> pgd = ffffffc07652e000 >> [00000000] *pgd=00000000f658b003, *pud=00000000f658b003, *pmd=0000000000000000 >> Internal error: Oops: 96000006 [#1] PREEMPT SMP >> Modules linked in: >> CPU: 1 PID: 1053 Comm: lkvm-msi-irqinj Not tainted 4.0.0-rc7+ #3027 >> Hardware name: FVP Base (DT) >> task: ffffffc0774e9680 ti: ffffffc0765a8000 task.ti: ffffffc0765a8000 >> PC is at kvm_vgic_inject_irq+0x234/0x310 >> LR is at kvm_vgic_inject_irq+0x30c/0x310 >> pc : [<ffffffc0000ae0a8>] lr : [<ffffffc0000ae180>] pstate: 80000145 >> ..... >> >> So this patch fixes this by checking the SPI number against the >> actual limit. Also we remove the former legacy hard limit of >> 127 in the ioctl code. >> >> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >> CC: <stable@vger.kernel.org> # 4.0, 3.19, 3.18 > > Reviewed-by: Marc Zyngier <marc.zyngier@arm.com> > > It is getting really tight for 4.0, but hopefully I can squeeze it in a > second pull request together with the missing barrier on 32bit. I doubt I'll be able to send the pull request to Linus. Can't it really wait a couple of weeks? I'll include it in the second pull request for 4.1, together with (if you want) the lazy (lazier) FP/SIMD save/restore. Paolo
On 10/04/15 17:43, Paolo Bonzini wrote: > > > On 10/04/2015 17:25, Marc Zyngier wrote: >> On 10/04/15 16:17, Andre Przywara wrote: >>> When userland injects a SPI via the KVM_IRQ_LINE ioctl we currently >>> only check it against a fixed limit, which historically is set >>> to 127. With the new dynamic IRQ allocation the effective limit may >>> actually be smaller (64). >>> So when now a malicious or buggy userland injects a SPI in that >>> range, we spill over on our VGIC bitmaps and bytemaps memory. >>> I could trigger a host kernel NULL pointer dereference with current >>> mainline by injecting some bogus IRQ number from a hacked kvmtool: >>> ----------------- >>> .... >>> DEBUG: kvm_vgic_inject_irq(kvm, cpu=0, irq=114, level=1) >>> DEBUG: vgic_update_irq_pending(kvm, cpu=0, irq=114, level=1) >>> DEBUG: IRQ #114 still in the game, writing to bytemap now... >>> Unable to handle kernel NULL pointer dereference at virtual address 00000000 >>> pgd = ffffffc07652e000 >>> [00000000] *pgd=00000000f658b003, *pud=00000000f658b003, *pmd=0000000000000000 >>> Internal error: Oops: 96000006 [#1] PREEMPT SMP >>> Modules linked in: >>> CPU: 1 PID: 1053 Comm: lkvm-msi-irqinj Not tainted 4.0.0-rc7+ #3027 >>> Hardware name: FVP Base (DT) >>> task: ffffffc0774e9680 ti: ffffffc0765a8000 task.ti: ffffffc0765a8000 >>> PC is at kvm_vgic_inject_irq+0x234/0x310 >>> LR is at kvm_vgic_inject_irq+0x30c/0x310 >>> pc : [<ffffffc0000ae0a8>] lr : [<ffffffc0000ae180>] pstate: 80000145 >>> ..... >>> >>> So this patch fixes this by checking the SPI number against the >>> actual limit. Also we remove the former legacy hard limit of >>> 127 in the ioctl code. >>> >>> Signed-off-by: Andre Przywara <andre.przywara@arm.com> >>> CC: <stable@vger.kernel.org> # 4.0, 3.19, 3.18 >> >> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com> >> >> It is getting really tight for 4.0, but hopefully I can squeeze it in a >> second pull request together with the missing barrier on 32bit. > > I doubt I'll be able to send the pull request to Linus. Can't it really > wait a couple of weeks? I'll include it in the second pull request for > 4.1, together with (if you want) the lazy (lazier) FP/SIMD save/restore. That's what I meant (sorry if I wasn't clear). Second PR for 4.1 is just fine, we'll Cc stable anyway. Thanks, M.
Hi Christopher, On 10/04/15 16:29, Christopher Covington wrote: > Hi Andre, > > On 04/10/2015 11:17 AM, Andre Przywara wrote: >> When userland injects a SPI via the KVM_IRQ_LINE ioctl we currently >> only check it against a fixed limit, which historically is set >> to 127. With the new dynamic IRQ allocation the effective limit may >> actually be smaller (64). >> So when now a malicious or buggy userland injects a SPI in that >> range, we spill over on our VGIC bitmaps and bytemaps memory. >> I could trigger a host kernel NULL pointer dereference with current >> mainline by injecting some bogus IRQ number from a hacked kvmtool: > >> --- a/arch/arm/include/uapi/asm/kvm.h >> +++ b/arch/arm/include/uapi/asm/kvm.h >> @@ -195,7 +195,11 @@ struct kvm_arch_memory_slot { >> #define KVM_ARM_IRQ_CPU_IRQ 0 >> #define KVM_ARM_IRQ_CPU_FIQ 1 >> >> -/* Highest supported SPI, from VGIC_NR_IRQS */ >> +/* >> + * This used to hold the highest supported SPI, but it is now obsolete >> + * and only here to provide source code level compatibility with older >> + * userland. The highest SPI number can be set via KVM_DEV_ARM_VGIC_GRP_NR_IRQS. >> + */ >> #define KVM_ARM_IRQ_GIC_MAX 127 > > If that's the case should it maybe only defined when __KERNEL__ is not defined? Mmmh, I am not sure it's really worth the hassle. Actually it seems like that neither kvmtool nor QEMU use this definition, so it's more or less orphaned by now. I am confident we can avoid it sneaking in in the kernel again. Cheers, Andre.
On Fri, Apr 10, 2015 at 04:17:59PM +0100, Andre Przywara wrote: > When userland injects a SPI via the KVM_IRQ_LINE ioctl we currently > only check it against a fixed limit, which historically is set > to 127. With the new dynamic IRQ allocation the effective limit may > actually be smaller (64). > So when now a malicious or buggy userland injects a SPI in that > range, we spill over on our VGIC bitmaps and bytemaps memory. > I could trigger a host kernel NULL pointer dereference with current > mainline by injecting some bogus IRQ number from a hacked kvmtool: > ----------------- > .... > DEBUG: kvm_vgic_inject_irq(kvm, cpu=0, irq=114, level=1) > DEBUG: vgic_update_irq_pending(kvm, cpu=0, irq=114, level=1) > DEBUG: IRQ #114 still in the game, writing to bytemap now... > Unable to handle kernel NULL pointer dereference at virtual address 00000000 > pgd = ffffffc07652e000 > [00000000] *pgd=00000000f658b003, *pud=00000000f658b003, *pmd=0000000000000000 > Internal error: Oops: 96000006 [#1] PREEMPT SMP > Modules linked in: > CPU: 1 PID: 1053 Comm: lkvm-msi-irqinj Not tainted 4.0.0-rc7+ #3027 > Hardware name: FVP Base (DT) > task: ffffffc0774e9680 ti: ffffffc0765a8000 task.ti: ffffffc0765a8000 > PC is at kvm_vgic_inject_irq+0x234/0x310 > LR is at kvm_vgic_inject_irq+0x30c/0x310 > pc : [<ffffffc0000ae0a8>] lr : [<ffffffc0000ae180>] pstate: 80000145 > ..... > > So this patch fixes this by checking the SPI number against the > actual limit. Also we remove the former legacy hard limit of > 127 in the ioctl code. > > Signed-off-by: Andre Przywara <andre.przywara@arm.com> > CC: <stable@vger.kernel.org> # 4.0, 3.19, 3.18 Reviewed-by: Christoffer Dall <christoffer.dall@linaro.org>
On Fri, Apr 10, 2015 at 05:52:05PM +0100, Andre Przywara wrote: > Hi Christopher, > > On 10/04/15 16:29, Christopher Covington wrote: > > Hi Andre, > > > > On 04/10/2015 11:17 AM, Andre Przywara wrote: > >> When userland injects a SPI via the KVM_IRQ_LINE ioctl we currently > >> only check it against a fixed limit, which historically is set > >> to 127. With the new dynamic IRQ allocation the effective limit may > >> actually be smaller (64). > >> So when now a malicious or buggy userland injects a SPI in that > >> range, we spill over on our VGIC bitmaps and bytemaps memory. > >> I could trigger a host kernel NULL pointer dereference with current > >> mainline by injecting some bogus IRQ number from a hacked kvmtool: > > > >> --- a/arch/arm/include/uapi/asm/kvm.h > >> +++ b/arch/arm/include/uapi/asm/kvm.h > >> @@ -195,7 +195,11 @@ struct kvm_arch_memory_slot { > >> #define KVM_ARM_IRQ_CPU_IRQ 0 > >> #define KVM_ARM_IRQ_CPU_FIQ 1 > >> > >> -/* Highest supported SPI, from VGIC_NR_IRQS */ > >> +/* > >> + * This used to hold the highest supported SPI, but it is now obsolete > >> + * and only here to provide source code level compatibility with older > >> + * userland. The highest SPI number can be set via KVM_DEV_ARM_VGIC_GRP_NR_IRQS. > >> + */ > >> #define KVM_ARM_IRQ_GIC_MAX 127 > > > > If that's the case should it maybe only defined when __KERNEL__ is not defined? > > Mmmh, I am not sure it's really worth the hassle. Actually it seems like > that neither kvmtool nor QEMU use this definition, so it's more or less > orphaned by now. I am confident we can avoid it sneaking in in the > kernel again. > TBH, I wouldn't object against Marc enclosing the definition in an #ifdef __KERNEL__. Thanks, -Christoffer
On 13/04/15 11:04, Christoffer Dall wrote: > On Fri, Apr 10, 2015 at 05:52:05PM +0100, Andre Przywara wrote: >> Hi Christopher, >> >> On 10/04/15 16:29, Christopher Covington wrote: >>> Hi Andre, >>> >>> On 04/10/2015 11:17 AM, Andre Przywara wrote: >>>> When userland injects a SPI via the KVM_IRQ_LINE ioctl we currently >>>> only check it against a fixed limit, which historically is set >>>> to 127. With the new dynamic IRQ allocation the effective limit may >>>> actually be smaller (64). >>>> So when now a malicious or buggy userland injects a SPI in that >>>> range, we spill over on our VGIC bitmaps and bytemaps memory. >>>> I could trigger a host kernel NULL pointer dereference with current >>>> mainline by injecting some bogus IRQ number from a hacked kvmtool: >>> >>>> --- a/arch/arm/include/uapi/asm/kvm.h >>>> +++ b/arch/arm/include/uapi/asm/kvm.h >>>> @@ -195,7 +195,11 @@ struct kvm_arch_memory_slot { >>>> #define KVM_ARM_IRQ_CPU_IRQ 0 >>>> #define KVM_ARM_IRQ_CPU_FIQ 1 >>>> >>>> -/* Highest supported SPI, from VGIC_NR_IRQS */ >>>> +/* >>>> + * This used to hold the highest supported SPI, but it is now obsolete >>>> + * and only here to provide source code level compatibility with older >>>> + * userland. The highest SPI number can be set via KVM_DEV_ARM_VGIC_GRP_NR_IRQS. >>>> + */ >>>> #define KVM_ARM_IRQ_GIC_MAX 127 >>> >>> If that's the case should it maybe only defined when __KERNEL__ is not defined? >> >> Mmmh, I am not sure it's really worth the hassle. Actually it seems like >> that neither kvmtool nor QEMU use this definition, so it's more or less >> orphaned by now. I am confident we can avoid it sneaking in in the >> kernel again. >> > TBH, I wouldn't object against Marc enclosing the definition in an > #ifdef __KERNEL__. Yeah, I'll fix that up (assuming you mean #ifndef rather than #ifdef). Thanks, M.
Hi, On 13/04/15 11:04, Christoffer Dall wrote: > On Fri, Apr 10, 2015 at 05:52:05PM +0100, Andre Przywara wrote: >> Hi Christopher, >> >> On 10/04/15 16:29, Christopher Covington wrote: >>> Hi Andre, >>> >>> On 04/10/2015 11:17 AM, Andre Przywara wrote: >>>> When userland injects a SPI via the KVM_IRQ_LINE ioctl we currently >>>> only check it against a fixed limit, which historically is set >>>> to 127. With the new dynamic IRQ allocation the effective limit may >>>> actually be smaller (64). >>>> So when now a malicious or buggy userland injects a SPI in that >>>> range, we spill over on our VGIC bitmaps and bytemaps memory. >>>> I could trigger a host kernel NULL pointer dereference with current >>>> mainline by injecting some bogus IRQ number from a hacked kvmtool: >>> >>>> --- a/arch/arm/include/uapi/asm/kvm.h >>>> +++ b/arch/arm/include/uapi/asm/kvm.h >>>> @@ -195,7 +195,11 @@ struct kvm_arch_memory_slot { >>>> #define KVM_ARM_IRQ_CPU_IRQ 0 >>>> #define KVM_ARM_IRQ_CPU_FIQ 1 >>>> >>>> -/* Highest supported SPI, from VGIC_NR_IRQS */ >>>> +/* >>>> + * This used to hold the highest supported SPI, but it is now obsolete >>>> + * and only here to provide source code level compatibility with older >>>> + * userland. The highest SPI number can be set via KVM_DEV_ARM_VGIC_GRP_NR_IRQS. >>>> + */ >>>> #define KVM_ARM_IRQ_GIC_MAX 127 >>> >>> If that's the case should it maybe only defined when __KERNEL__ is not defined? >> >> Mmmh, I am not sure it's really worth the hassle. Actually it seems like >> that neither kvmtool nor QEMU use this definition, so it's more or less >> orphaned by now. I am confident we can avoid it sneaking in in the >> kernel again. >> > TBH, I wouldn't object against Marc enclosing the definition in an > #ifdef __KERNEL__. So just to clarify: Isn't this definition part of the userspace API that we have to maintain forever? That was my understanding when Christopher suggested to enclose it in #ifndef __KERNEL__ (if I got this correctly). So we would at least avoid any future kernel code to (ab)use it again. But putting it inside #ifdef __KERNEL__ does contradict this, doesn't it? If it is not an API, I'd rather remove it all: a) it is not used by the kernel anymore after this patch b) it became meaningless with the introduction of the dynamic NR_IRQS allocation c) none of the two KVM userspace tools (I know of) is using it So I guess any userspace tool trying to make use of it deserves to not compile anymore. But if we somehow promised that any userspace program will compile forever against even the newest kernel headers, then we have to keep it in, haven't we? Or am I totally wrong here? Cheers, Andre.
On Mon, Apr 13, 2015 at 11:21:20AM +0100, Marc Zyngier wrote: > On 13/04/15 11:04, Christoffer Dall wrote: > > On Fri, Apr 10, 2015 at 05:52:05PM +0100, Andre Przywara wrote: > >> Hi Christopher, > >> > >> On 10/04/15 16:29, Christopher Covington wrote: > >>> Hi Andre, > >>> > >>> On 04/10/2015 11:17 AM, Andre Przywara wrote: > >>>> When userland injects a SPI via the KVM_IRQ_LINE ioctl we currently > >>>> only check it against a fixed limit, which historically is set > >>>> to 127. With the new dynamic IRQ allocation the effective limit may > >>>> actually be smaller (64). > >>>> So when now a malicious or buggy userland injects a SPI in that > >>>> range, we spill over on our VGIC bitmaps and bytemaps memory. > >>>> I could trigger a host kernel NULL pointer dereference with current > >>>> mainline by injecting some bogus IRQ number from a hacked kvmtool: > >>> > >>>> --- a/arch/arm/include/uapi/asm/kvm.h > >>>> +++ b/arch/arm/include/uapi/asm/kvm.h > >>>> @@ -195,7 +195,11 @@ struct kvm_arch_memory_slot { > >>>> #define KVM_ARM_IRQ_CPU_IRQ 0 > >>>> #define KVM_ARM_IRQ_CPU_FIQ 1 > >>>> > >>>> -/* Highest supported SPI, from VGIC_NR_IRQS */ > >>>> +/* > >>>> + * This used to hold the highest supported SPI, but it is now obsolete > >>>> + * and only here to provide source code level compatibility with older > >>>> + * userland. The highest SPI number can be set via KVM_DEV_ARM_VGIC_GRP_NR_IRQS. > >>>> + */ > >>>> #define KVM_ARM_IRQ_GIC_MAX 127 > >>> > >>> If that's the case should it maybe only defined when __KERNEL__ is not defined? > >> > >> Mmmh, I am not sure it's really worth the hassle. Actually it seems like > >> that neither kvmtool nor QEMU use this definition, so it's more or less > >> orphaned by now. I am confident we can avoid it sneaking in in the > >> kernel again. > >> > > TBH, I wouldn't object against Marc enclosing the definition in an > > #ifdef __KERNEL__. > > Yeah, I'll fix that up (assuming you mean #ifndef rather than #ifdef). > Yes, Monday morning ;) -Christoffer
diff --git a/arch/arm/include/uapi/asm/kvm.h b/arch/arm/include/uapi/asm/kvm.h index 0db25bc..9d5ed83 100644 --- a/arch/arm/include/uapi/asm/kvm.h +++ b/arch/arm/include/uapi/asm/kvm.h @@ -195,7 +195,11 @@ struct kvm_arch_memory_slot { #define KVM_ARM_IRQ_CPU_IRQ 0 #define KVM_ARM_IRQ_CPU_FIQ 1 -/* Highest supported SPI, from VGIC_NR_IRQS */ +/* + * This used to hold the highest supported SPI, but it is now obsolete + * and only here to provide source code level compatibility with older + * userland. The highest SPI number can be set via KVM_DEV_ARM_VGIC_GRP_NR_IRQS. + */ #define KVM_ARM_IRQ_GIC_MAX 127 /* PSCI interface */ diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c index 4f0afeb..0bbc0aa 100644 --- a/arch/arm/kvm/arm.c +++ b/arch/arm/kvm/arm.c @@ -653,8 +653,7 @@ int kvm_vm_ioctl_irq_line(struct kvm *kvm, struct kvm_irq_level *irq_level, if (!irqchip_in_kernel(kvm)) return -ENXIO; - if (irq_num < VGIC_NR_PRIVATE_IRQS || - irq_num > KVM_ARM_IRQ_GIC_MAX) + if (irq_num < VGIC_NR_PRIVATE_IRQS) return -EINVAL; return kvm_vgic_inject_irq(kvm, 0, irq_num, level); diff --git a/arch/arm64/include/uapi/asm/kvm.h b/arch/arm64/include/uapi/asm/kvm.h index 3ef77a4..cc4affe 100644 --- a/arch/arm64/include/uapi/asm/kvm.h +++ b/arch/arm64/include/uapi/asm/kvm.h @@ -188,7 +188,11 @@ struct kvm_arch_memory_slot { #define KVM_ARM_IRQ_CPU_IRQ 0 #define KVM_ARM_IRQ_CPU_FIQ 1 -/* Highest supported SPI, from VGIC_NR_IRQS */ +/* + * This used to hold the highest supported SPI, but it is now obsolete + * and only here to provide source code level compatibility with older + * userland. The highest SPI number can be set via KVM_DEV_ARM_VGIC_GRP_NR_IRQS. + */ #define KVM_ARM_IRQ_GIC_MAX 127 /* PSCI interface */ diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c index 1b4c344..b0dd2d9 100644 --- a/virt/kvm/arm/vgic.c +++ b/virt/kvm/arm/vgic.c @@ -1372,6 +1372,9 @@ int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int irq_num, goto out; } + if (irq_num >= kvm->arch.vgic.nr_irqs) + return -EINVAL; + vcpu_id = vgic_update_irq_pending(kvm, cpuid, irq_num, level); if (vcpu_id >= 0) { /* kick the specified vcpu */
When userland injects a SPI via the KVM_IRQ_LINE ioctl we currently only check it against a fixed limit, which historically is set to 127. With the new dynamic IRQ allocation the effective limit may actually be smaller (64). So when now a malicious or buggy userland injects a SPI in that range, we spill over on our VGIC bitmaps and bytemaps memory. I could trigger a host kernel NULL pointer dereference with current mainline by injecting some bogus IRQ number from a hacked kvmtool: ----------------- .... DEBUG: kvm_vgic_inject_irq(kvm, cpu=0, irq=114, level=1) DEBUG: vgic_update_irq_pending(kvm, cpu=0, irq=114, level=1) DEBUG: IRQ #114 still in the game, writing to bytemap now... Unable to handle kernel NULL pointer dereference at virtual address 00000000 pgd = ffffffc07652e000 [00000000] *pgd=00000000f658b003, *pud=00000000f658b003, *pmd=0000000000000000 Internal error: Oops: 96000006 [#1] PREEMPT SMP Modules linked in: CPU: 1 PID: 1053 Comm: lkvm-msi-irqinj Not tainted 4.0.0-rc7+ #3027 Hardware name: FVP Base (DT) task: ffffffc0774e9680 ti: ffffffc0765a8000 task.ti: ffffffc0765a8000 PC is at kvm_vgic_inject_irq+0x234/0x310 LR is at kvm_vgic_inject_irq+0x30c/0x310 pc : [<ffffffc0000ae0a8>] lr : [<ffffffc0000ae180>] pstate: 80000145 ..... So this patch fixes this by checking the SPI number against the actual limit. Also we remove the former legacy hard limit of 127 in the ioctl code. Signed-off-by: Andre Przywara <andre.przywara@arm.com> CC: <stable@vger.kernel.org> # 4.0, 3.19, 3.18 --- arch/arm/include/uapi/asm/kvm.h | 6 +++++- arch/arm/kvm/arm.c | 3 +-- arch/arm64/include/uapi/asm/kvm.h | 6 +++++- virt/kvm/arm/vgic.c | 3 +++ 4 files changed, 14 insertions(+), 4 deletions(-)