Re: [PATCH RFC v2 05/10] arm_mpam: Add device tree support for MSC probing

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The function `mpam_dt_parse_resources` leaks the error code from `of_property_read_reg` if a child node lacks a `reg` property, incorrectly failing the entire MSC probe.
- [Medium] Unbounded recursion in `__mpam_get_cpumask_from_cache_id` allows a maliciously crafted Device Tree to cause a kernel stack overflow.
--

commit d6b10a65ae0592bb7895e72b5e2dca5d7b5f1fe9
Author: James Morse <james.morse@arm.com>

arm_mpam: Add device tree support for MSC probing

This commit adds device tree discovery for Memory System Resource
Partitioning and Monitoring (MPAM) Memory System Components (MSCs). It parses
MSC nodes, computes cache-id and affinity, and creates RIS entries.

> diff --git a/drivers/resctrl/mpam_devices.c b/drivers/resctrl/mpam_devices.c
> index c68135ee0ffcb..7a5b27e877594 100644
> --- a/drivers/resctrl/mpam_devices.c
> +++ b/drivers/resctrl/mpam_devices.c
[ ... ]
> @@ -161,6 +164,171 @@ static void mpam_free_garbage(void)
[ ... ]
> +/* Called recursively to walk the list of caches from a particular CPU */
> +static void __mpam_get_cpumask_from_cache_id(int cpu, struct device_node *cache_node,
> +					     u32 cache_id,
> +					     u32 cache_level,
> +					     cpumask_t *affinity)
> +{
> +	int err;
> +	u32 iter_level;
> +	u32 iter_cache_id;
> +	struct device_node *iter_node __free(device_node) = of_find_next_cache_node(cache_node);
> +
> +	if (!iter_node)
> +		return;
> +
> +	err = of_property_read_u32(iter_node, "cache-level", &iter_level);
> +	if (err)
> +		return;
> +
> +	/*
> +	 * get_cpu_cacheinfo_id() isn't ready until sometime
> +	 * during device_initcall(). Use cache_of_calculate_id().
> +	 */
> +	iter_cache_id = cache_of_calculate_id(iter_node);
> +	if (iter_cache_id == ~0U)
> +		return;
> +
> +	if (iter_level == cache_level && iter_cache_id == cache_id)
> +		cpumask_set_cpu(cpu, affinity);
> +
> +	if (iter_level < cache_level)
> +		__mpam_get_cpumask_from_cache_id(cpu, iter_node, cache_id,
> +						 cache_level, affinity);

[Severity: Medium]
Could a maliciously crafted or cyclic device tree cause unbounded recursion
here? If the device tree contains a circular reference in the cache hierarchy
where the levels remain strictly below cache_level, it looks like this could
result in an infinite loop and a kernel stack overflow.

> +}
[ ... ]
> +static int mpam_dt_parse_resources(struct mpam_msc *msc, void *ignored)
> +{
> +	u64 ris_idx = 0;
> +	int err, num_ris = 0;
> +	struct device_node *np;
> +
> +	np = msc->pdev->dev.of_node;
> +	for_each_available_child_of_node_scoped(np, iter) {
> +		err = of_property_read_reg(iter, 0, &ris_idx, NULL);
> +		if (!err) {
> +			/*
> +			 * ris_idx is read as u64 but indexes a 4-bit RIS selector
> +			 * (0..MPAM_MSC_MAX_NUM_RIS). Reject out-of-range values here,
> +			 * before it is narrowed to u8, so a large value cannot be
> +			 * truncated into a valid-looking index.
> +			 */
> +			if (ris_idx >= MPAM_MSC_MAX_NUM_RIS)
> +				return -EINVAL;
> +			num_ris++;
> +			err = mpam_dt_parse_resource(msc, iter, ris_idx);
> +			if (err)
> +				return err;
> +		}
> +	}
> +
> +	if (!num_ris)
> +		err = mpam_dt_parse_resource(msc, np, 0);
> +
> +	return err;

[Severity: High]
Does this leak the error code from of_property_read_reg() if the last child
node processed lacks a reg property?

If the device tree contains a valid RIS node (with reg) followed by an
auxiliary node (like opp-table), err becomes non-zero (e.g., -EINVAL) and
leaks out of the loop block. The if (!num_ris) fallback is skipped because
num_ris > 0, and the function returns the leaked non-zero err. Could this
incorrectly fail the entire MSC probe?

> +}

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