Re: [PATCH] perf/marvell: cn10k_tad: Publish the OF module alias

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Modifying the device's shared memory resource start address in-place during probe corrupts it for future probe attempts, leading to a NULL pointer dereference.
--

commit bd6b6a4110e1310bd038c78cce7c34d20371595d
Author: hpp.iscas <hppiscas@163.com>

perf/marvell: cn10k_tad: Publish the OF module alias
    
The TAD PMU platform driver has ACPI and OF match tables, but only
the ACPI table is published. A Device Tree platform device emits an
OF modalias, so the ACPI alias does not select this module for it.
Publish the OF table inside CONFIG_OF and retain ACPI support.

> diff --git a/drivers/perf/marvell_cn10k_tad_pmu.c b/drivers/perf/marvell_cn10k_tad_pmu.c
> index 54909d0031b77..301be4d92b1bb 100644
> --- a/drivers/perf/marvell_cn10k_tad_pmu.c
> +++ b/drivers/perf/marvell_cn10k_tad_pmu.c
> @@ -424,6 +424,7 @@ static const struct of_device_id tad_pmu_of_match[] = {
>  	{ .compatible = "marvell,cn10k-tad-pmu", .data = &tad_pmu_data },
>  	{},
>  };
> +MODULE_DEVICE_TABLE(of, tad_pmu_of_match);
>  #endif

[Severity: High]
This isn't a bug introduced by this patch, but modifying the device's
shared memory resource start address in-place during probe corrupts it for
future probe attempts:

drivers/perf/marvell_cn10k_tad_pmu.c:tad_pmu_probe() {
	...
	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
	if (!res) {
		dev_err(&pdev->dev, "Mem resource not found\n");
		return -ENODEV;
	}
	...
	/* ioremap the distributed TAD pmu regions */
	for (i = 0; i < tad_cnt && res->start < res->end; i++) {
		regions[i].base = devm_ioremap(&pdev->dev,
					       res->start,
					       tad_pmu_page_size);
		if (!regions[i].base) {
			dev_err(&pdev->dev, "TAD%d ioremap fail\n", i);
			return -ENOMEM;
		}
		res->start += tad_page_size;
	}
	...
}

Because platform_get_resource() returns a pointer to the global platform
device resource, advancing res->start permanently alters it.

If the driver is unbound and rebound, the subsequent probe receives the
corrupted resource where res->start is already advanced past res->end. The
loop will not execute, leaving all regions[i].base pointers as NULL.

Any subsequent perf event execution will then call readq() on a NULL pointer
in tad_pmu_event_counter_read(), resulting in a kernel panic. 

Could we avoid modifying the global resource in-place and use a local
variable for the address calculation?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905134317.67317-1-hppiscas@163.com?part=1