Re: [PATCH] irqchip/gic: Fix UBSAN shift-out-of-bounds in GIC helpers
flat view
From: Marc Zyngier <maz@kernel.org>
Date: 2025-12-02 11:44:08
Also in:
lkml
On Tue, 02 Dec 2025 10:53:24 +0000, zhuling0805 [off-list ref] wrote:
Hi, When running a kernel built with UBSAN enabled, enabling a GPIO controller that uses a GIC interrupt as its parent triggers several shift-out-of-bounds warnings in the GIC helpers. This patch fixes the issue by using unsigned constants so that the left shifts are well-defined and UBSAN stays quiet.
Next time, please put additional notes *after* the commit message. In this case, you are simply repeating what is already in the commit message, so just sending the patch without anything else would have been better.
Thanks, Zhu Ling --- From: Zhu Ling <redacted> Date: Tue, 2 Dec 2025 17:54:10 +0800 Subject: [PATCH] irqchip/gic: Fix UBSAN shift-out-of-bounds in GIC helpers When running with UBSAN enabled, enabling a GPIO controller that uses a GIC interrupt as its parent triggers several shift-out-of-bounds warnings: UBSAN: shift-out-of-bounds in drivers/irqchip/irq-gic-common.c:50:21 left shift of 2 by 30 places cannot be represented in type 'int' and similar reports in gic_poke_irq() and gic_peek_irq() in drivers/irqchip/irq-gic-v3.c. These come from shifting signed integer constants. Use unsigned constants (0x2U and 1U) so that the behavior is well-defined and the UBSAN warnings go away.
I think a better approach would be to convert all of this to primitives that are designed for bit mask generation, rather than reinventing the wheel. See below for some (untested) suggestions.
quoted hunk ↗ jump to hunk
Signed-off-by: Zhu Ling <redacted> --- drivers/irqchip/irq-gic-common.c | 2 +- drivers/irqchip/irq-gic-v3.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)diff --git a/drivers/irqchip/irq-gic-common.c b/drivers/irqchip/irq-gic-common.c index c776f9142..d4f0afd9e 100644 --- a/drivers/irqchip/irq-gic-common.c +++ b/drivers/irqchip/irq-gic-common.c@@ -48,7 +48,7 @@ void gic_enable_quirks(u32 iidr, const struct gic_quirk *quirks, int gic_configure_irq(unsigned int irq, unsigned int type, void __iomem *base) { - u32 confmask = 0x2 << ((irq % 16) * 2); + u32 confmask = 0x2U << ((irq % 16) * 2);
This really should be written as: u32 confmask = BIT(((irq % 16) * 2) + 1);
quoted hunk ↗ jump to hunk
u32 confoff = (irq / 16) * 4; u32 val, oldval; int ret = 0;diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c index 3de351e66..f5226c03f 100644 --- a/drivers/irqchip/irq-gic-v3.c +++ b/drivers/irqchip/irq-gic-v3.c@@ -457,7 +457,7 @@ static int gic_peek_irq(struct irq_data *d, u32 offset) u32 index, mask; offset = convert_offset_index(d, offset, &index); - mask = 1 << (index % 32); + mask = 1U << (index % 32);
and this as BIT(index % 32).
quoted hunk ↗ jump to hunk
if (gic_irq_in_rdist(d)) base = gic_data_rdist_sgi_base();@@ -473,7 +473,7 @@ static void gic_poke_irq(struct irq_data *d, u32 offset) u32 index, mask; offset = convert_offset_index(d, offset, &index); - mask = 1 << (index % 32); + mask = 1U << (index % 32);
Same here.
if (gic_irq_in_rdist(d)) base = gic_data_rdist_sgi_base();
Thanks, M. -- Without deviation from the norm, progress is not possible.