Re: [PATCH 1/3] perf: Remove redundant dev_err()/dev_err_probe()

[sashiko-bot@kernel.org]

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

New issues:
- [Low] The `memrc` variable is no longer used after the removal of the `dev_err()` call, leaving an unused local variable.

Pre-existing issues:
- [High] The IRQ handler context is freed via kfree() before the devm_request_irq() resource is released, creating a use-after-free vulnerability.
- [High] Missing cleanup of CPU hotplug instance registration when perf_pmu_register() fails, leaving a freed pointer in the global hotplug list.
--

--- Patch [1]: [PATCH 1/3] perf: Remove redundant dev_err()/dev_err_probe() ---
commit 83d88878cd6553e7af185d501c3dfdb2c01816e8
Author: Pan Chuang <panchuang@vivo.com>

perf: Remove redundant dev_err()/dev_err_probe()

Since commit 55b48e23f5c4, devm_request_irq() automatically logs
detailed error messages on failure. This patch removes the now-redundant
driver-specific dev_err() and dev_err_probe() calls.

> diff --git a/drivers/perf/alibaba_uncore_drw_pmu.c b/drivers/perf/alibaba_uncore_drw_pmu.c
> index 74786a5dd6a23..fc934bccf4a64 100644
> --- a/drivers/perf/alibaba_uncore_drw_pmu.c
> +++ b/drivers/perf/alibaba_uncore_drw_pmu.c
> @@ -449,11 +449,8 @@ static struct ali_drw_pmu_irq *__ali_drw_pmu_init_irq(struct platform_device
>  	 */
>  	ret = devm_request_irq(&pdev->dev, irq_num, ali_drw_pmu_isr,
>  			       IRQF_SHARED, dev_name(&pdev->dev), irq);
> -	if (ret < 0) {
> -		dev_err(&pdev->dev,
> -			"Fail to request IRQ:%d ret:%d\n", irq_num, ret);
> +	if (ret < 0)
>  		goto out_free;

[Severity: High]
This is a pre-existing issue, but since we are modifying the error path
here, can this lead to a use-after-free if a subsequent step fails?

In __ali_drw_pmu_init_irq(), if devm_request_irq() succeeds but a later
function like irq_set_affinity_hint() fails, the code jumps to out_free:

out_free:
        kfree(irq);
        return ERR_PTR(ret);

Since devm_request_irq() uses the IRQF_SHARED flag, another device sharing
this IRQ could trigger an interrupt immediately. If ali_drw_pmu_isr()
executes before the devres cleanup deregisters the handler, will it
dereference the already freed irq pointer?

> -	}
>  
>  	ret = irq_set_affinity_hint(irq_num, cpumask_of(irq->cpu));
>  	if (ret)

[ ... ]

> diff --git a/drivers/perf/qcom_l3_pmu.c b/drivers/perf/qcom_l3_pmu.c
> index c8d259dd1f807..e18d0f94e8d86 100644
> --- a/drivers/perf/qcom_l3_pmu.c
> +++ b/drivers/perf/qcom_l3_pmu.c
> @@ -767,11 +767,8 @@ static int qcom_l3_cache_pmu_probe(struct platform_device *pdev)
>  
>  	ret = devm_request_irq(&pdev->dev, ret, qcom_l3_cache__handle_irq, 0,
>  			       name, l3pmu);
> -	if (ret) {
> -		dev_err(&pdev->dev, "Request for IRQ failed for slice @%pa\n",
> -			&memrc->start);
> +	if (ret)
>  		return ret;

[Severity: Low]
The patch removes the dev_err() call which was the only reader of the
memrc variable in qcom_l3_cache_pmu_probe(). Does this leave memrc
populated but entirely unused?

Could the code be cleaned up to use devm_platform_ioremap_resource() 
instead of devm_platform_get_and_ioremap_resource() and remove memrc 
completely?

> -	}
>  
>  	/* Add this instance to the list used by the offline callback */
>  	ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE, &l3pmu->node);

[Severity: High]
This isn't a bug introduced by this patch, but does this function leak the
CPU hotplug instance if perf_pmu_register() fails later in the probe?

qcom_l3_cache_pmu_probe() registers the instance here:

    ret = cpuhp_state_add_instance(CPUHP_AP_PERF_ARM_QCOM_L3_ONLINE,
                                   &l3pmu->node);

But if perf_pmu_register() fails right after, it returns immediately:

    ret = perf_pmu_register(&l3pmu->pmu, name, -1);
    if (ret < 0) {
        dev_err(&pdev->dev, "Failed to register L3 cache PMU (%d)\n", ret);
        return ret;
    }

Since devres will automatically free the l3pmu struct on probe failure,
will the freed l3pmu->node remain linked in the global list and cause a
use-after-free during subsequent CPU hotplug events?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260717103123.34887-1-panchuang@vivo.com?part=1