From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH] KVM: arm/arm64: check IRQ number on userland injection Date: Mon, 13 Apr 2015 12:04:00 +0200 Message-ID: <20150413100400.GJ6186@cbox> References: <1428679079-16499-1-git-send-email-andre.przywara@arm.com> <5527EC52.4050500@codeaurora.org> <5527FFB5.9020103@arm.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Cc: Christopher Covington , Marc Zyngier , "kvmarm@lists.cs.columbia.edu" , "linux-arm-kernel@lists.infradead.org" , "kvm@vger.kernel.org" To: Andre Przywara Return-path: Received: from mail-lb0-f180.google.com ([209.85.217.180]:33510 "EHLO mail-lb0-f180.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753478AbbDMKDz (ORCPT ); Mon, 13 Apr 2015 06:03:55 -0400 Received: by lbbzk7 with SMTP id zk7so54625358lbb.0 for ; Mon, 13 Apr 2015 03:03:54 -0700 (PDT) Content-Disposition: inline In-Reply-To: <5527FFB5.9020103@arm.com> Sender: kvm-owner@vger.kernel.org List-ID: 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