From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4927E3822A8 for ; Wed, 17 Jun 2026 19:11:12 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781723477; cv=none; b=Ea5FUtdA6552u4diy+sphOfUDlc3gzNt2DIa9lZPrlxIG7czjLanOy5lvNDJNkB1NIAfj06P0TVHNtr+pxDsKgXu2apVrKKZSGIgym/bc+zDeydbGZtODeRq4bos1SLc32iUnIG88vgoAsKBUb8xCBn9BmGVt2gj4iyd80D9BFo= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781723477; c=relaxed/simple; bh=dOWRQg16tak2YtuEPlk8ynufGgzsmyJ5rELyoslaE2A=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=fHYGXT6eXxwgK82uTf31TYk0fCSUwzFE+/Yw0qyia6ch7rmA0sJ2dQYJ4GsZ3cHeCDE9p6YFn8nMWMZdKOs+UT+ZXRvLcVKWKoTybXlTiZxzGI2rD5o3mRK/LwkT5vPfF/x9i8PaPYe/Bf2BPfBFeKOVBm5CrBbYd6jcTFIR0qY= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=TQuSY3kz; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="TQuSY3kz" Received: by smtp.kernel.org (Postfix) with ESMTPSA id E62DA1F000E9; Wed, 17 Jun 2026 19:11:11 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1781723472; bh=y55GaD91gTCtS76Es0CzbqTAveHY8+1xGzGUFk39Yuw=; h=From:To:Cc:Subject:In-Reply-To:References:Date; b=TQuSY3kzLsexzVGwNMS1hK25fHsyl4cwGpmaSYWe0fe8YmA3Nv15fNDSoOo/Jrqvk AyT2kwU+8TiHRkwmpt659KvJVMZ24WONNIT2uMAiwgCa4/4Lc6QnQKTXyaupniDUTm 9dYC/iZy7yfdgTUKwRlhsw/3IhpAxjg7rOki7k+jQE7Md768+DSfyIqe6u+5AZQM8w SR/52x5Fp61JzVqx5gD4dZQ4Pi4cmlSbQE5U55V4wHuC+IQuByvTYVYM3OkyVExmc0 6xVdlJ6Dx99hLWCIWouQ/abpCRAIQHOkHft+dw1NHoNV8KKmXfA6lBnrQkidQmglsB iBodt2eGA3bGg== From: Thomas Gleixner To: Bhargav Joshi , Tony Lindgren , Jason Cooper , Marc Zyngier Cc: linux-kernel@vger.kernel.org, goledhruva@gmail.com, m-chawdhry@ti.com, daniel.baluta@gmail.com, simona.toaca@nxp.com, j.bhargav.u@gmail.com Subject: Re: [PATCH 1/2] irqchip: crossbar: Fix out-of-bounds access in crossbar_domain_free() In-Reply-To: <20260610-irq-crossbar-fix-v1-1-26797369ff6a@gmail.com> References: <20260610-irq-crossbar-fix-v1-0-26797369ff6a@gmail.com> <20260610-irq-crossbar-fix-v1-1-26797369ff6a@gmail.com> Date: Wed, 17 Jun 2026 21:11:09 +0200 Message-ID: <87ik7hc78y.ffs@fw13> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain On Wed, Jun 10 2026 at 02:26, Bhargav Joshi wrote: $Subject: irqchip/crossbar: .... > crossbar_domain_free() uses 'd->hwirq' (crossbar source index which can > go up to 0 to 399) as the index for cb->irq_map and cb->write(), rather > than the GIC SPI index. This can cause out of out-of-bounds write. but > irq_domain_reset_irq_data() which zeros d->hwirq is called before > d->hwirq is read. subsequent accesses use hwirq=0 which is always > in-bounds but writes to the wrong slot. So the subject line is misleading as there is no out of bounds access at all. It's not helpful to make claims which are wrong and then not explaining what the consequences are. Something like this: irqchip/crossbar: Use correct index in crossbar_domain_free() crossbar_domain_free() resets the domain data and then uses the nulled out data::hwirq member as index to reset the irq_map[] entry and to write the relevant crossbar register with a safe entry. That means it never frees the correct index and keeps the crossbar register connection to the source interrupt active. If it would not reset the domain data, then this would be even worse as data::hwirq holds the source interrupt number, but both the map and register index need the corresponding GIC SPI number and not the source interrupt number. This might even result in an out of bounds access as the source interrupt number can be higher than the maximal index space. > Fix this by using the GIC SPI index from the parent domain's irq_data, > moving the reset after cleanup. The ordering of the reset is not relevant at all once the proper index is used. > Fixes: 783d31863fb82 ("irqchip: crossbar: Convert dra7 crossbar to stacked domains") > Pointless newline. > Signed-off-by: Bhargav Joshi > --- > drivers/irqchip/irq-crossbar.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/irqchip/irq-crossbar.c b/drivers/irqchip/irq-crossbar.c > index cd1134101ace..6a4718be0c58 100644 > --- a/drivers/irqchip/irq-crossbar.c > +++ b/drivers/irqchip/irq-crossbar.c > @@ -158,9 +158,9 @@ static void crossbar_domain_free(struct irq_domain *domain, unsigned int virq, > for (i = 0; i < nr_irqs; i++) { > struct irq_data *d = irq_domain_get_irq_data(domain, virq + i); > > + cb->irq_map[d->parent_data->hwirq - GIC_IRQ_START] = IRQ_FREE; > + cb->write(d->parent_data->hwirq - GIC_IRQ_START, cb->safe_map); This lacks a comment explaining why this needs to access parent_data->hwirq and what that contains. > irq_domain_reset_irq_data(d); > - cb->irq_map[d->hwirq] = IRQ_FREE; > - cb->write(d->hwirq, cb->safe_map); > } > raw_spin_unlock(&cb->lock); > }