[PATCH 1/1] drivers/perf: Fix kernel panic due to the invalid mon_ctx pointer
Robin Murphy
robin.murphy at arm.com
Thu Oct 26 09:39:22 PDT 2023
[ Since I made the mistake of looking... ]
On 26/10/2023 3:29 pm, Shanker Donthineni wrote:
> The return pointer from the resctrl_arch_mon_ctx_alloc_no_wait() function
> is saved in a 32-bit variable 'hwc->idx,' which results in the loss of
> the upper 32 bits. This, in turn, triggers a kernel panic when attempting
> to access a corrupted pointer.
>
> This patch addresses the issue by utilizing the IDR data structure to
> keep track of a 32-bit value associated with a pointer (64-bit value).
This seems unnecessaraily complicated - there's plenty of space in that
anonymous hw_perf_event "driver data" union (I know I cram quite a lot
in there for arm-cmn) so it should just be a case of picking a different
appropriately-sized field to (ab)use.
Thanks,
Robin.
> Signed-off-by: Shanker Donthineni <sdonthineni at nvidia.com>
> ---
> drivers/perf/resctrl_pmu.c | 27 ++++++++++++++++++++++++---
> 1 file changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/perf/resctrl_pmu.c b/drivers/perf/resctrl_pmu.c
> index 99a2b90b5d83..68cda619da48 100644
> --- a/drivers/perf/resctrl_pmu.c
> +++ b/drivers/perf/resctrl_pmu.c
> @@ -6,6 +6,8 @@
> #include <linux/perf_event.h>
> #include <linux/platform_device.h>
> #include <linux/resctrl.h>
> +#include <linux/mutex.h>
> +#include <linux/idr.h>
>
> struct resctrl_pmu {
> struct pmu pmu;
> @@ -25,6 +27,9 @@ RESCTRL_PMU_EVENT_ATTR_EXTRACTOR(event, config, 0, 7);
> RESCTRL_PMU_EVENT_ATTR_EXTRACTOR(domain, config, 16, 23);
> RESCTRL_PMU_EVENT_ATTR_EXTRACTOR(resctrl_id, config1, 0, 63);
>
> +static DEFINE_MUTEX(resctrl_pmu_lock);
> +static DEFINE_IDR(resctrl_pmu_idr);
> +
> static void resctrl_pmu_do_nothing(struct pmu *pmu)
> {
> }
> @@ -69,12 +74,17 @@ static void resctrl_pmu_event_destroy(struct perf_event *event)
> struct hw_perf_event *hwc = &event->hw;
> u16 event_num = get_event(event);
> struct rdt_resource *r;
> + void *mon_ctx;
>
> r = resctrl_event_get_resource(event_num);
> if (!r)
> return;
>
> - resctrl_arch_mon_ctx_free(r, event_num, hwc->idx);
> + mutex_lock(&resctrl_pmu_lock);
> + mon_ctx = idr_remove(&resctrl_pmu_idr, hwc->idx);
> + mutex_unlock(&resctrl_pmu_lock);
> +
> + resctrl_arch_mon_ctx_free(r, event_num, mon_ctx);
> }
>
> static int resctrl_pmu_event_init(struct perf_event *event)
> @@ -87,6 +97,7 @@ static int resctrl_pmu_event_init(struct perf_event *event)
> struct rdt_domain *d;
> u16 event_num, domain_id;
> u32 closid, rmid;
> + void *mon_ctx;
> int err;
> u64 id;
>
> @@ -144,7 +155,12 @@ static int resctrl_pmu_event_init(struct perf_event *event)
> return -EINVAL;
> }
>
> - hwc->idx = resctrl_arch_mon_ctx_alloc_no_wait(r, event_num);
> + mon_ctx = resctrl_arch_mon_ctx_alloc_no_wait(r, event_num);
> +
> + mutex_lock(&resctrl_pmu_lock);
> + hwc->idx = idr_alloc(&resctrl_pmu_idr, mon_ctx, 0, INT_MAX, GFP_KERNEL);
> + mutex_unlock(&resctrl_pmu_lock);
> +
> if (hwc->idx == -ENOSPC)
> return -ENOSPC;
> event->destroy = resctrl_pmu_event_destroy;
> @@ -162,6 +178,7 @@ static void resctrl_pmu_event_update(struct perf_event *event)
> struct rdt_resource *r;
> struct rdt_domain *d;
> u32 closid, rmid;
> + void *mon_ctx;
> int err;
>
> event_num = get_event(event);
> @@ -179,11 +196,15 @@ static void resctrl_pmu_event_update(struct perf_event *event)
> if (!cpumask_test_cpu(smp_processor_id(), &d->cpu_mask))
> return;
>
> + mutex_lock(&resctrl_pmu_lock);
> + mon_ctx = idr_find(&resctrl_pmu_idr, hwc->idx);
> + mutex_unlock(&resctrl_pmu_lock);
> +
> do {
> prev = local64_read(&hwc->prev_count);
>
> err = resctrl_arch_rmid_read(r, d, closid, rmid,
> - event_num, &now, hwc->idx);
> + event_num, &now, mon_ctx);
> if (err)
> return;
> } while (local64_cmpxchg(&hwc->prev_count, prev, now) != prev);
More information about the linux-arm-kernel
mailing list