linux-kernel - Re: [PATCH] irqchip/imx-irqsteer: Fix irq handling if an error occurs in imx_irqsteer_irq

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <864ivhacys.wl-maz@kernel.org>
Date: Sat, 12 Jul 2025 12:02:51 +0100
From: Marc Zyngier <maz@...nel.org>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc: Thomas Gleixner <tglx@...utronix.de>,
	Shawn Guo <shawnguo@...nel.org>,
	Sascha Hauer <s.hauer@...gutronix.de>,
	Pengutronix Kernel Team <kernel@...gutronix.de>,
	Fabio Estevam <festevam@...il.com>,
	Lucas Stach <l.stach@...gutronix.de>,
	Aisheng Dong <aisheng.dong@....com>,
	linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org,
	imx@...ts.linux.dev,
	linux-arm-kernel@...ts.infradead.org
Subject: Re: [PATCH] irqchip/imx-irqsteer: Fix irq handling if an error occurs in imx_irqsteer_irq_handler()

On Sat, 12 Jul 2025 10:58:35 +0100,
Christophe JAILLET <christophe.jaillet@...adoo.fr> wrote:
> 
> Le 17/11/2024 à 12:21, Christophe JAILLET a écrit :
> > chained_irq_enter(() should be paired with a corresponding
> > chained_irq_exit().
> > 
> > Here, if (hwirq < 0), a early return occurs and chained_irq_exit() is not
> > called.
> 
> After several month without any feedback, this is a polite ping.
> Is this patch correct?

An untested patch is unlikely to make a lot of forward progress, to
be honest.

> 
> CJ
> 
> 
> > 
> > Add a new label and a goto for fix it.
> > 
> > Fixes: 28528fca4908 ("irqchip/imx-irqsteer: Add multi output interrupts support")
> > Signed-off-by: Christophe JAILLET <christophe.jaillet@...adoo.fr>
> > ---
> > Compile tested only.
> > 
> > Review with care, irq handling is sometimes tricky...
> > ---
> >   drivers/irqchip/irq-imx-irqsteer.c | 3 ++-
> >   1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/irqchip/irq-imx-irqsteer.c b/drivers/irqchip/irq-imx-irqsteer.c
> > index 75a0e980ff35..59abe5a8beb8 100644
> > --- a/drivers/irqchip/irq-imx-irqsteer.c
> > +++ b/drivers/irqchip/irq-imx-irqsteer.c
> > @@ -135,7 +135,7 @@ static void imx_irqsteer_irq_handler(struct irq_desc *desc)
> >   	if (hwirq < 0) {
> >   		pr_warn("%s: unable to get hwirq base for irq %d\n",
> >   			__func__, irq);
> > -		return;
> > +		goto out;
> >   	}
> >     	for (i = 0; i < 2; i++, hwirq += 32) {
> > @@ -153,6 +153,7 @@ static void imx_irqsteer_irq_handler(struct irq_desc *desc)
> >   			generic_handle_domain_irq(data->domain, pos + hwirq);
> >   	}
> >   +out:
> >   	chained_irq_exit(irq_desc_get_chip(desc), desc);
> >   }

The real question is *how* do you end-up in this situation.

To trigger this case, you need a mux interrupt that is handled by
imx_irqsteer_irq_handler(), but for which you haven't got a
translation from DT the first place. Do you see the chicken and egg
problem?

In summary, this driver is checking for conditions that can't possibly
happen, and this check should simply be deleted instead of being
blindly "fixed".

	M.

-- 
Without deviation from the norm, progress is not possible.