Re: [PATCH RFC v2 02/10] arm_mpam: Fix MSC MMIO window size off-by-one with resource_size()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The per-cpu memory allocated for msc->error_dev_id is never freed, resulting in a memory leak.
- [High] Premature devm_kfree of the msc structure leads to a Use-After-Free during devres cleanup.
- [High] Virtual memory leak in the NVIDIA T241 quirk due to unconditional ioremap.
- [High] Hardware NRDY wait delay loop may be skipped entirely due to usecs_to_jiffies truncation.
--

commit 730cddf02b20e6f247fdb58e9bc95501ec54e318
Author: Yin Li <yin.li@oss.qualcomm.com>

    arm_mpam: Fix MSC MMIO window size off-by-one with resource_size()

    struct resource uses an inclusive end address, so the window size is
    end - start + 1. do_mpam_msc_drv_probe() computed it as end - start,
    which is one byte too small.

Closes: https://lore.kernel.org/oe-kbuild-all/202609030809.ObirDhR3-lkp@intel.com/

> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index 6da217abf6894..c68135ee0ffcb 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
> @@ -2095,7 +2095,7 @@ static struct mpam_msc *do_mpam_msc_drv_probe(struct platform_device *pdev)
>  			dev_err_once(dev, "Failed to map MSC base address\n");
>  			return ERR_CAST(io);
>  		}
> -		msc->mapped_hwpage_sz = msc_res->end - msc_res->start;
> +		msc->mapped_hwpage_sz = resource_size(msc_res);
>  		msc->mapped_hwpage = io;
>  	} else {
>  		return ERR_PTR(-EINVAL);

[Severity: High]
This is a pre-existing issue, but does do_mpam_msc_drv_probe() leak
the per-cpu memory allocated for msc->error_dev_id?

When setting up the MSC, __setup_ppi() allocates per-cpu memory:

__setup_ppi()
	msc->error_dev_id = alloc_percpu(struct mpam_msc *);

If the probe later fails, or during normal device teardown via
mpam_msc_destroy(), is free_percpu() ever called to release this memory?

[Severity: High]
This isn't a bug introduced by this patch, but does the devres cleanup
for msc->probe_lock lead to a use-after-free?

In do_mpam_msc_drv_probe(), the msc structure is allocated via devm_kzalloc(),
and then several mutexes embedded within it are registered with
devm_mutex_init():

do_mpam_msc_drv_probe()
	msc = devm_kzalloc(&pdev->dev, sizeof(*msc), GFP_KERNEL);
	err = devm_mutex_init(dev, &msc->probe_lock);

During removal, the driver manually deletes the allocation:

mpam_free_garbage()
	devm_kfree(&iter->pdev->dev, iter->to_free);

This frees the msc memory immediately, but the devm_mutex_release actions
remain on the devres list.

When the driver core cleans up the remaining devres actions, will it call
mutex_destroy(&msc->probe_lock) on the already-freed msc structure?

[Severity: High]
This is a pre-existing issue, but does mpam_enable_quirk_nvidia_t241_1()
leak the virtual memory mapping for the scratch registers?

mpam_enable_quirk_nvidia_t241_1()
	t241_scratch_regs[msc->t241_id] = ioremap(phys, SZ_8M);

This maps an 8MB physical region and stores the pointer unconditionally.

If multiple MSCs exist on the same chip, or if the driver is unbound and
rebound, will this overwrite the pointer and permanently leak the previous
VMA, since there is no corresponding iounmap() during device removal?

[Severity: High]
This is a pre-existing issue, but can the hardware NRDY wait delay loop
be skipped entirely due to truncation?

In mpam_msmon_read(), the back-off wait is calculated as:

mpam_msmon_read()
	wait_jiffies = usecs_to_jiffies(class->nrdy_usec);

	while (wait_jiffies)
		wait_jiffies = schedule_timeout_uninterruptible(wait_jiffies);

If nrdy_usec is smaller than one jiffy tick, usecs_to_jiffies() truncates
to 0.

Does this cause the while loop to be skipped entirely, forcing the code to
immediately retry without giving the hardware the required physical time to
exit the NRDY state?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914-mpam-resctrl-dt-knp-support-v2-0-bf6645bb2f65@oss.qualcomm.com?part=2