RE: [PATCH v5 3/9] irqchip/imx-irqsteer: Validate IRQ count before creating domain

"Zhipeng Wang (OSS)" <[email protected]>
Newsgroups org.infradead.lists.linux-arm-kernel,dev.linux.lists.imx,org.kernel.vger.linux-kernel
Message-ID <AMBPR04MB12334D1C91BFC42AEDB26E7F2EBA02@AMBPR04MB12334.eurprd04.prod.outlook.com>
> On Fri, Aug 21, 2026 at 07:10:33PM +0900, [email protected]
> wrote:
> > From: Fabio Estevam <[email protected]>
> >
> > The IRQ count is validated after creating the IRQ domain. If it is
> > invalid, probe returns without removing the domain, leaving its host
> > data pointing at devm-managed memory that is freed on probe failure.
> >
> > Validate the count before allocating resources to avoid the leak and
> > dangling pointer.
> >
> > Fixes: 28528fca4908 ("irqchip/imx-irqsteer: Add multi output
> > interrupts support")
> > Signed-off-by: Fabio Estevam <[email protected]>
> > Signed-off-by: Zhipeng Wang <[email protected]>
> > ---
> 
> This patch should be second patch.  After move to second patch, some error
> handle will be nice to have.
> 
> your 3 patch needn't because no error after it. change to devm_ version will be
> improvement patch and put after new API added.
> 
> Frank
> 
Hi Frank,

Makes sense, I'll restructure it that way for the next version:

 - Move Fabio's IRQ-count validation up so the count is validated before
   the domain is created, and add proper error handling there.
 - Since the invalid count is then rejected before the domain exists,
   the domain leak can no longer happen, so managing the domain with
   devm is no longer a fix. I'll drop its Fixes tag and reframe it as an
   improvement.
 - Move that devm conversion after the new devm_irq_domain_create_linear()
   helper, together with the other cleanup.

Thanks for the review.

Zhipeng
> 
> >  drivers/irqchip/irq-imx-irqsteer.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/irqchip/irq-imx-irqsteer.c
> > b/drivers/irqchip/irq-imx-irqsteer.c
> > index c1df84551efa..571324de8bd1 100644
> > --- a/drivers/irqchip/irq-imx-irqsteer.c
> > +++ b/drivers/irqchip/irq-imx-irqsteer.c
> > @@ -218,6 +218,8 @@ static int imx_irqsteer_probe(struct platform_device
> *pdev)
> >  	 */
> >  	data->irq_count = DIV_ROUND_UP(irqs_num, 64);
> >  	data->reg_num = irqs_num / 32;
> > +	if (!data->irq_count || data->irq_count > CHAN_MAX_OUTPUT_INT)
> > +		return -EINVAL;
> >
> >  	if (IS_ENABLED(CONFIG_PM)) {
> >  		data->saved_reg = devm_kzalloc(&pdev->dev, @@ -253,11 +255,6
> @@
> > static int imx_irqsteer_probe(struct platform_device *pdev)
> >  	}
> >  	irq_domain_set_pm_device(data->domain, &pdev->dev);
> >
> > -	if (!data->irq_count || data->irq_count > CHAN_MAX_OUTPUT_INT) {
> > -		ret = -EINVAL;
> > -		goto out;
> > -	}
> > -
> >  	for (i = 0; i < data->irq_count; i++) {
> >  		data->irq[i] = irq_of_parse_and_map(np, i);
> >  		if (!data->irq[i])
> > --
> > 2.34.1
> >
> >
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.