From mboxrd@z Thu Jan 1 00:00:00 1970 Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.remote-ip=100.103.45.18 ARC-Seal: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; cv=none; t=1794308386; b=rQz+gTBYJlewzrUwsRqA4A76mOiZEy24a58HCQzLnplTS6dIxfO0AqJcRrMsoBCabfpe nuSmoz+l77Zi2h3WcT65aoyALVRNHy8JodFe04mx/IecnOLBkClw+Vr0XgU3WFjFp3giI LR+CqMOLZBNFPSpPGlr6V4kYesOdUzUPksXAYDjL5jX3SNk6BE/hzESgb1eBIi6841dvx KVRkyrgSq5um/onYT2WD59r64HhwtzBPEl4VkixVZWODxSlkt45MmuwU2+MomHnZdb1MH IdwW67/lsrg0zHDv7V2L0h5g8DHBbHIiY13GEEoVcuAkVsbE/7XqadXGYBHauEClXSw== ARC-Message-Signature: i=1; d=subspace.kernel.org; s=arc20261015; a=rsa-sha256; c=relaxed/relaxed; t=1794308386; h=Received:DKIM-Signature:Received:Date:Message-ID:From:To:Cc:Subject: In-Reply-To:References:User-Agent:MIME-Version:Content-Type: X-SA-Exim-Connect-IP:X-SA-Exim-Rcpt-To:X-SA-Exim-Mail-From: X-SA-Exim-Scanned; bh=ZRAi0FDh12qnpfYeLj6iY5LoEiQS1o5jjozIS5Vb968=; b=DwAB1qQcwl7+Wd2e4W/6HRqb9is/oknm8z83CMutk+fDDhE8hIxQemTOjoYDpsMBxRqO RtX7Wj0I0APNfFy8rFAScI7jxWFJBDPtXu9o26QW+YT+Qof5DnaKE9bqlWIUgiFfKfmXa L/pGL8eK2+FaAYSDcMIzTLwzx2Bfd6W31kCA43/hfvIEnTpYIcNzyFRHZfTFRHhiSJJOO mbNmvUw5AuFlU7K5DWD7VGyWJditOJzWEZvCHrwlByelsOAwrTHfei3isHasS4kiUPD3q Wzs4S75tjW1S9V0Wz8lrPOYU9J41VrCfBVPUqbvo6iQyALUBiMLgG4QdoJQ8FMjIlzQ== ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b="C+/uVY1F"; arc=none smtp.remote-ip=100.103.45.18 Received: from smtp.kernel.org (quasi.space.kernel.org [100.103.45.18]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature ECDSA (prime256v1)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id DD3F960A2607; Tue, 10 Nov 2026 10:59:45 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256 header.s=k20260515 header.b=C+/uVY1F Received: by smtp.kernel.org (Postfix) with ESMTPSA id 819E91F008A1; Tue, 10 Nov 2026 10:59:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1794308385; bh=ZRAi0FDh12qnpfYeLj6iY5LoEiQS1o5jjozIS5Vb968=; h=Date:From:To:Cc:Subject:In-Reply-To:References; b=C+/uVY1FCOgYsFoFwCEx5osZbgdLma+t0+9g5me4CrJMuL+HLP/7Z7OsESx/10pdW DzJCwsL5eD8t6XOYzYMwGhuTHZj+ljTG5gKDCC4kzmQjFL7ZhubrEqKkq8P/i0u6Ui z863mwPRMCtjTuqD6pf04KzhMxy8aCPEbxdRNSLHJCjlxF8gj/CXgBTuhU8/lEz1s3 9TO7+/YexWZENcDST0DCx54bj75AUU68pVNL3zowaLe39c/FgMEaWixVqeuVY44QhC SKwzRIycA2bTZIn7yyKouFkduj7xgYk1n0qN4EfgmqaHVAMxwNKlzfgx4abK8EMrM7 QEgAYnOl4Q2LQ== Received: from sofa.misterjones.org ([185.219.108.64] helo=goblin-girl.misterjones.org) by disco-boy.misterjones.org with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.98.2) (envelope-from ) id 1xQjZn-0000000DiSU-0wAO; Tue, 10 Nov 2026 10:59:43 +0000 Date: Tue, 10 Nov 2026 10:59:42 +0000 Message-ID: <86wlql0x5t.wl-maz@kernel.org> From: Marc Zyngier To: Dan Carpenter Cc: Eric Auger , Oliver Upton , Fuad Tabba , Joey Gouly , Steffen Eiden , Suzuki K Poulose , Zenghui Yu , Catalin Marinas , Will Deacon , Mark Rutland , Carlos =?UTF-8?B?TMOzcGV6?= , Sascha Bischoff , Jackie Liu , Christoffer Dall , linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, Karthik Bhargavan , =?UTF-8?B?Q2zDqW1lbnQ=?= Blaudeau Subject: Re: [PATCH] KVM: arm64: vgic-its: Reject ITS_REGS offsets above U32_MAX In-Reply-To: References: User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI-EPG/1.14.7 (Harue) FLIM-LB/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL-LB/10.8 EasyPG/1.0.0 Emacs/30.1 (aarch64-unknown-linux-gnu) MULE/6.0 (HANACHIRUSATO) X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Post: List-Help: List-Subscribe: List-Unsubscribe: List-Owner: Precedence: list MIME-Version: 1.0 (generated by SEMI-EPG 1.14.7 - "Harue") Content-Type: text/plain; charset=US-ASCII X-SA-Exim-Connect-IP: 185.219.108.64 X-SA-Exim-Rcpt-To: dan@cryspen.com, eric.auger@redhat.com, oupton@kernel.org, fuad.tabba@linux.dev, joey.gouly@arm.com, seiden@linux.ibm.com, suzuki.poulose@arm.com, yuzenghui@huawei.com, catalin.marinas@arm.com, will@kernel.org, mark.rutland@arm.com, clopez@suse.de, sascha.bischoff@arm.com, liuyun01@kylinos.cn, cdall@linaro.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.linux.dev, linux-kernel@vger.kernel.org, karthik@pereq.eu, clement@cryspen.com X-SA-Exim-Mail-From: maz@kernel.org X-SA-Exim-Scanned: No (on disco-boy.misterjones.org); SAEximRunCond expanded to false On Tue, 10 Nov 2026 05:07:22 +0000, Dan Carpenter wrote: > > The "offset" variable is a u64 that the user controls through the > KVM_SET_DEVICE_ATTR, KVM_GET_DEVICE_ATTR and KVM_HAS_DEVICE_ATTR > ioctls. The problem is vgic_find_mmio_region() truncates the u64 to a > u32, so it could return a valid region instead of failing. Then we > use the original incorrect value of "offset" to calculate "addr". > > addr = its->vgic_its_base + offset; > > The impact of this bug is relatively minor. A user in the kvm group > could potentially crash the VMM which they can already do without using > a bug. > > Fixes: 8331c23c28e8 ("KVM: arm64: vgic-its: Implement vgic_its_has_attr_regs and attr_regs_access") > Cc: stable@vger.kernel.org > Signed-off-by: Dan Carpenter > --- > arch/arm64/kvm/vgic/vgic-its.c | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/arch/arm64/kvm/vgic/vgic-its.c b/arch/arm64/kvm/vgic/vgic-its.c > index 5ce485485a11..532a82bed24c 100644 > --- a/arch/arm64/kvm/vgic/vgic-its.c > +++ b/arch/arm64/kvm/vgic/vgic-its.c > @@ -2018,6 +2018,9 @@ static int vgic_its_has_attr_regs(struct kvm_device *dev, > gpa_t offset = attr->attr; > int align; > > + if (offset > U32_MAX) > + return -ENXIO; > + > align = (offset < GITS_TYPER) || (offset >= GITS_PIDR4) ? 0x3 : 0x7; > > if (offset & align) > @@ -2045,6 +2048,9 @@ static int vgic_its_attr_regs_access(struct kvm_device *dev, > its = dev->private; > offset = attr->attr; > > + if (offset > U32_MAX) > + return -ENXIO; > + > /* > * Although the spec supports upper/lower 32-bit accesses to > * 64-bit ITS registers, the userspace ABI requires 64-bit I think this is going in the wrong direction. Instead of capping the offset at an arbitrary value, the infrastructure should be able to deal with arbitrary offsets. Could you try the following (untested) hack? I also don't think this is worth a Cc stable (the worse case is that the VMM accesses an emulated register and sets something nonsensical), which it can always do. Thanks, M. diff --git a/arch/arm64/kvm/vgic/vgic-mmio.c b/arch/arm64/kvm/vgic/vgic-mmio.c index fddb9da0403d5..8f9ebe241e9e4 100644 --- a/arch/arm64/kvm/vgic/vgic-mmio.c +++ b/arch/arm64/kvm/vgic/vgic-mmio.c @@ -820,13 +820,13 @@ void vgic_write_irq_line_level_info(struct kvm_vcpu *vcpu, u32 intid, static int match_region(const void *key, const void *elt) { - const unsigned int offset = (unsigned long)key; + const u64 offset = (unsigned long)key; const struct vgic_register_region *region = elt; - if (offset < region->reg_offset) + if (offset < (u64)region->reg_offset) return -1; - if (offset >= region->reg_offset + region->len) + if (offset >= (u64)region->reg_offset + region->len) return 1; return 0; @@ -834,9 +834,9 @@ static int match_region(const void *key, const void *elt) const struct vgic_register_region * vgic_find_mmio_region(const struct vgic_register_region *regions, - int nr_regions, unsigned int offset) + int nr_regions, u64 offset) { - return bsearch((void *)(uintptr_t)offset, regions, nr_regions, + return bsearch((void *)offset, regions, nr_regions, sizeof(regions[0]), match_region); } diff --git a/arch/arm64/kvm/vgic/vgic-mmio.h b/arch/arm64/kvm/vgic/vgic-mmio.h index 38ed730d68ac3..6c3c4de6883ef 100644 --- a/arch/arm64/kvm/vgic/vgic-mmio.h +++ b/arch/arm64/kvm/vgic/vgic-mmio.h @@ -228,6 +228,6 @@ u64 vgic_sanitise_field(u64 reg, u64 field_mask, int field_shift, /* Find the proper register handler entry given a certain address offset */ const struct vgic_register_region * vgic_find_mmio_region(const struct vgic_register_region *regions, - int nr_regions, unsigned int offset); + int nr_regions, u64 offset); #endif -- Without deviation from the norm, progress is not possible.