linux-kernel - Re: [PATCH] irqchip/gic: Fix UBSAN shift-out-of-bounds in GIC helpers

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <86ldjlp0qi.wl-maz@kernel.org>
Date: Tue, 02 Dec 2025 11:44:05 +0000
From: Marc Zyngier <maz@...nel.org>
To: zhuling0805 <zhuling0805@...ton.me>
Cc: Thomas Gleixner <tglx@...utronix.de>,
	"linux-arm-kernel@...ts.infradead.org" <linux-arm-kernel@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] irqchip/gic: Fix UBSAN shift-out-of-bounds in GIC helpers

On Tue, 02 Dec 2025 10:53:24 +0000,
zhuling0805 <zhuling0805@...ton.me> 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 <zhuling0805@...ton.me>
> 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.

> 
> Signed-off-by: Zhu Ling <zhuling0805@...ton.me>
> ---
>  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);

>  	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).

>  
>  	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.