Re: [PATCH v2 02/11] iio: adc: change from %ld to %pe for PTR_ERR() printing

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Direct return on devm_thermal_of_zone_register failure skips critical error cleanup (PM leak and Use-After-Free).
- [High] Unbalanced disable_irq() on sun4i_prepare_for_irq() error path permanently disables the IRQ.
--

commit 4c81144898d1e91a3f1c0bfb6354a6f38c7251b4
Author: Vojtěch Krátký <vo.kratky@seznam.cz>

iio: adc: change from %ld to %pe for PTR_ERR() printing

This commit replaces numeric PTR_ERR() logging with the %pe format specifier.
The goal is to print error values in a more readable form.

> diff --git a/drivers/iio/adc/sun4i-gpadc-iio.c b/drivers/iio/adc/sun4i-gpadc-iio.c
> index 479115ea50bf9..5d7a61abcaeae 100644
> --- a/drivers/iio/adc/sun4i-gpadc-iio.c
> +++ b/drivers/iio/adc/sun4i-gpadc-iio.c

[ ... ]

> @@ -639,8 +639,8 @@ static int sun4i_gpadc_probe(struct platform_device *pdev)
>  		 */
>  		if (IS_ERR(info->tzd) && PTR_ERR(info->tzd) != -ENODEV) {
>  			dev_err(&pdev->dev,
> -				"could not register thermal sensor: %ld\n",
> -				PTR_ERR(info->tzd));
> +				"could not register thermal sensor: %pe\n",
> +				info->tzd);
>  			return PTR_ERR(info->tzd);

[Severity: High]
This isn't a bug introduced by this patch, but returning directly here
skips the err_map cleanup label.

If devm_thermal_of_zone_register() fails with an error other than -ENODEV,
this code returns directly. Does this leave the device's runtime PM enabled
because the pm_runtime_disable() call in the err_map block is skipped?

Also, if the device was probed via sun4i_gpadc_probe_mfd(),
iio_map_array_register() was successfully called earlier:

drivers/iio/adc/sun4i-gpadc-iio.c:sun4i_gpadc_probe_mfd() {
    ...
    ret = iio_map_array_register(indio_dev, sun4i_gpadc_hwmon_maps);
    ...
}

By skipping iio_map_array_unregister() at the err_map label, does this
leave a dangling pointer in the global IIO maps pointing to indio_dev?
Since indio_dev is managed by devres (devm_iio_device_alloc), its memory
will be freed immediately upon probe failure. Could this result in a
use-after-free when another subsystem attempts to read from the map?

>  		}
>  	}

While reviewing this file, I also noticed another pre-existing regression in
sun4i_gpadc_read().

[Severity: High]
This is a pre-existing issue, but does the unbalanced disable_irq() on the
sun4i_prepare_for_irq() error path permanently disable the IRQ?

drivers/iio/adc/sun4i-gpadc-iio.c:sun4i_gpadc_read() {
    ...
    ret = sun4i_prepare_for_irq(indio_dev, channel, irq);
    if (ret)
        goto err;

    enable_irq(irq);
    ...
err:
    pm_runtime_put_autosuspend(indio_dev->dev.parent);
    disable_irq(irq);
    mutex_unlock(&info->mutex);
}

If sun4i_prepare_for_irq() fails, the code jumps to the err label and
unconditionally executes disable_irq(irq). Because enable_irq(irq) was never
reached, this increases the IRQ disable depth. 

Since the IRQs are initially requested with IRQF_NO_AUTOEN
(initial depth = 1), the unbalanced disable_irq() bumps the depth to 2.
On subsequent successful reads, enable_irq() will only decrement the depth
back to 1. Does this leave the IRQ permanently masked, causing
wait_for_completion_timeout() to continually time out?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260604120201.116925-1-vo.kratky@seznam.cz?part=2