Re: [PATCH v4 4/6] mfd: qcom-pm8xxx: register PM8921 USB ID extcon

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [High] The dynamically registered child platform device 'usb_extcon' is unregistered twice during driver removal, resulting in a use-after-free and double free.

Pre-existing issues:
- [High] The IRQ domain is freed explicitly while the devm-managed parent IRQ handler is still active, leading to a potential use-after-free if the interrupt fires before devres cleanup finishes.
--

commit c974699adaea81fa53001bda7b03087f8ea67b97
Author: Alexandre MINETTE <contact@alex-min.fr>
Date:   Tue Jun 9 08:13:47 2026 +0000

mfd: qcom-pm8xxx: register PM8921 USB ID extcon

This commit registers a child platform device for the USB ID interrupt on
PM8921 to act as an extcon provider. It shares the PMIC's devicetree node
and cleans up the child device upon driver removal or probe failure.

> diff --git a/drivers/mfd/qcom-pm8xxx.c b/drivers/mfd/qcom-pm8xxx.c
> index 0cf374c015ce7..884fc99a1488f 100644
> --- a/drivers/mfd/qcom-pm8xxx.c
> +++ b/drivers/mfd/qcom-pm8xxx.c
[ ... ]
> @@ -570,9 +630,22 @@ static int pm8xxx_probe(struct platform_device *pdev)
>  
>  	irq_set_irq_wake(irq, 1);
>  
> +	if (data->usb_id_irq) {
> +		rc = pm8xxx_add_usb_extcon(pdev, chip, data->usb_id_irq);
> +		if (rc)
> +			goto err_domain;
> +	}
> +
>  	rc = of_platform_populate(pdev->dev.of_node, NULL, NULL, &pdev->dev);
>  	if (rc)
> -		irq_domain_remove(chip->irqdomain);
> +		goto err_extcon;
> +
> +	return 0;
> +
> +err_extcon:
> +	pm8xxx_remove_usb_extcon(chip);
> +err_domain:
> +	irq_domain_remove(chip->irqdomain);
>  
>  	return rc;
>  }
> @@ -582,6 +655,7 @@ static void pm8xxx_remove(struct platform_device *pdev)
>  	struct pm_irq_chip *chip = platform_get_drvdata(pdev);
>  
>  	of_platform_depopulate(&pdev->dev);
> +	pm8xxx_remove_usb_extcon(chip);

[Severity: High]
Does this cause a double-free of the usb_extcon platform device?

When of_platform_depopulate() runs, it iterates over all children and invokes
of_platform_device_destroy(). Because usb_extcon is registered with
pdevinfo.of_node_reused = true and shares the parent's of_node (which has the
OF_POPULATED flag set), of_platform_device_destroy() will assume it created
the device and unregister usb_extcon.

Calling pm8xxx_remove_usb_extcon() immediately afterward will call
platform_device_unregister() on the already freed usb_extcon pointer,
resulting in a use-after-free.

>  	irq_domain_remove(chip->irqdomain);

[Severity: High]
This is a pre-existing issue, but does this explicitly free the IRQ domain
while the parent interrupt handler is still active?

In pm8xxx_probe(), the parent interrupt is requested via devm_request_irq().
This means the devres cleanup mechanism won't free the IRQ until
devres_release_all() runs, which happens after pm8xxx_remove() completes.

If a hardware interrupt triggers during the window between this
irq_domain_remove() call and devres cleanup, will pm8xxx_irq_handler() call
generic_handle_domain_irq() and dereference the freed chip->irqdomain?

>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609-mainline-send-v1-sending-v4-0-83768fbf404d@alex-min.fr?part=4