Re: [PATCH v5 1/2] perf/arm-cmn: Move DTM index data out of hw_perf_event

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] Type confusion in `arm_cmn_validate_group` allows arbitrary kernel memory read and out-of-bounds heap write via mixed PMU event groups.
--

commit 5cf0bca45fce2ef5c66d58f875b8d4b3f7c015e0
Author: Robin Murphy <robin.murphy@arm.com>

perf/arm-cmn: Move DTM index data out of hw_perf_event

This commit moves the DTM index arrays and watchpoint tracking out of
hw_perf_event into separate dynamically allocated memory structures. This
change is intended to accommodate growing CMN configurations without exceeding
the limits of hw_perf_event.

> diff --git a/drivers/perf/arm-cmn.c b/drivers/perf/arm-cmn.c
> index f5305c8fdca43..f1978a53d1c19 100644
> --- a/drivers/perf/arm-cmn.c
> +++ b/drivers/perf/arm-cmn.c

[ ... ]

> @@ -1787,6 +1809,11 @@ static int arm_cmn_event_init(struct perf_event *event)
>  	if (type == CMN_TYPE_DTC)
>  		return arm_cmn_validate_group(cmn, event);
>  

[Severity: Critical]
This isn't a bug introduced by this patch, but could this lead to a type
confusion and out-of-bounds write when validating groups with mixed PMU events?

When arm_cmn_validate_group() iterates over the group's siblings, it doesn't
verify that sibling->pmu == event->pmu. If a sibling is a hardware event from
a different PMU, it passes it to arm_cmn_val_add_event():

drivers/perf/arm-cmn.c:arm_cmn_validate_group() {
...
    for_each_sibling_event(sibling, leader)
        arm_cmn_val_add_event(cmn, val, sibling);
...
}

Inside arm_cmn_val_add_event(), it casts the sibling's hw_perf_event to an
arm_cmn_hw_event. Since userspace controls attr.config, it can overlap with
the hw->dn pointer, allowing arbitrary kernel memory reads:

drivers/perf/arm-cmn.c:arm_cmn_val_add_event() {
...
    for_each_hw_dn(hw, dn, i) {
        int wp_idx, dtm = dn->dtm, sel = hw->filter_sel;

        val->dtm_count[dtm]++;
...
}

Because the read byte dtm can be up to 255 and the array bounds are
CMN_MAX_DTMS (188), does this enable a controllable out-of-bounds write on
the kernel heap?

> +	event->destroy = arm_cmn_event_destroy;
> +	hw->dtm_idx = arm_cmn_alloc_dtm_idx();
> +	if (!hw->dtm_idx)
> +		return -ENOMEM;
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260603150025.30980-1-avivb@amazon.com?part=1