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