Re: [PATCH v6 10/13] gpio: mxc: use devres functions simplify probe code

[Peng Fan <peng.fan@oss.nxp.com>]

On Sun, Oct 11, 2026 at 12:05:31PM +0000, sashiko-bot@kernel.org wrote:
>Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>- [High] gpio: use-after-free and premature exposure of irq_domain
>--
>
>commit e70e13ad3287f8fd0b2357d428d23270ccfc9522
>Author: Peng Fan <peng.fan@nxp.com>
>
>gpio: mxc: use devres functions simplify probe code
>
>This commit replaces legacy IRQ domain creation and manual PM runtime error
>handling with devres equivalents in the MXC GPIO driver probe. It removes
>error path labels by relying on device-managed allocations to unwind resources.
>
>> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
>> index 8dcfd05e658a6..09acb186c84c1 100644
>> --- a/drivers/gpio/gpio-mxc.c
>> +++ b/drivers/gpio/gpio-mxc.c
>
>[ ... ]
>
>> @@ -534,27 +536,31 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>>  
>>  	err = devm_gpiochip_add_data(dev, &port->gen_gc.gc, port);
>>  	if (err)
>> -		goto out_bgio;
>> +		return err;
>
>[Severity: High]
>Does this premature exposure of the GPIO chip cause a use-after-free risk?
>
>By calling devm_gpiochip_add_data() before devm_irq_domain_instantiate(), the
>GPIO chip is registered and immediately exposed to kernel consumers and
>userspace.

This is a pre-existing issue. Seems I need to add another fix patch
before doing the cleanup.

Regards
Peng

>
>If a consumer requests an IRQ during this window, mxc_gpio_to_irq() will
>execute:
>
>drivers/gpio/gpio-mxc.c:mxc_gpio_to_irq() {
>	struct mxc_gpio_port *port = gpiochip_get_data(gc);
>
>	return irq_find_mapping(port->domain, offset);
>}
>
>This passes a NULL port->domain to irq_find_mapping(), failing to map the IRQ.
>
>Additionally, if a subsequent probe step like mxc_gpio_init_gc() fails, devres
>unwinds allocations in reverse order (LIFO). Devres will first destroy the IRQ
>domain, leaving port->domain as a dangling pointer to freed memory.
>
>The GPIO chip remains fully registered and exposed until its own devres release
>action runs later. A concurrent gpiod_to_irq() call during this unwinding
>window will dereference the freed port->domain, causing a use-after-free.
>
>Should devm_irq_domain_instantiate() be called before
>devm_gpiochip_add_data() to ensure the domain is fully prepared before exposure
>and destroyed after the chip is unregistered?
>
>>  
>>  	irq_base = devm_irq_alloc_descs(dev, -1, 0, 32, numa_node_id());
>> -	if (irq_base < 0) {
>> -		err = irq_base;
>> -		goto out_bgio;
>> -	}
>> +	if (irq_base < 0)
>> +		return irq_base;
>> +
>> +	d_info = (struct irq_domain_info) {
>> +		.fwnode		= dev_fwnode(dev),
>> +		.size		= 32,
>> +		.hwirq_max	= 32,
>> +		.virq_base	= irq_base,
>> +		.ops		= &irq_domain_simple_ops,
>> +		.dev		= dev,
>> +	};
>>  
>> -	port->domain = irq_domain_create_legacy(dev_fwnode(dev), 32, irq_base, 0,
>> -						&irq_domain_simple_ops, NULL);
>> -	if (!port->domain) {
>> -		err = -ENODEV;
>> -		goto out_bgio;
>> -	}
>> +	port->domain = devm_irq_domain_instantiate(dev, &d_info);
>> +	if (IS_ERR(port->domain))
>> +		return PTR_ERR(port->domain);
>>  
>>  	irq_domain_set_pm_device(port->domain, dev);
>>  
>>  	/* gpio-mxc can be a generic irq chip */
>>  	err = mxc_gpio_init_gc(port, irq_base);
>>  	if (err < 0)
>> -		goto out_irqdomain_remove;
>> +		return err;
>
>-- 
>Sashiko AI review · https://sashiko.dev/#/patchset/20261011-gpio-mxc-cleanup-v6-0-cb47ce2ad758@nxp.com?part=10
>