Re: [PATCH] KVM: arm/arm64: check IRQ number on userland injection

[Marc Zyngier <marc.zyngier@arm.com>]

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.
-- 
Jazz is not dead. It just smells funny...