From 7cbad58851d194c06be75425718d44d5ad0234c0 Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Wed, 31 Jan 2024 01:14:23 +0000 Subject: [PATCH] Reapply "perf: Disallow mis-matched inherited group reads" This reverts commit 63eafbb6b311cb2bb9432d685e4608d841b8c74b. Keeps the ABI stable by taking advantage of a hole in the structure! Bug: 307236803 Change-Id: Ic5f7ebeb3a9b13afdb3bfff7e54c4a93b863dab6 Signed-off-by: Greg Kroah-Hartman --- android/abi_gki_aarch64.stg | 7 +++++++ include/linux/perf_event.h | 3 +++ kernel/events/core.c | 39 +++++++++++++++++++++++++++++++------ 3 files changed, 43 insertions(+), 6 deletions(-) diff --git a/android/abi_gki_aarch64.stg b/android/abi_gki_aarch64.stg index b8f92c7bf9f9..c5b6b6cf0403 100644 --- a/android/abi_gki_aarch64.stg +++ b/android/abi_gki_aarch64.stg @@ -97923,6 +97923,12 @@ member { type_id: 0x6720d32f offset: 768 } +member { + id: 0x16cb0979 + name: "group_generation" + type_id: 0x4585663f + offset: 1120 +} member { id: 0x0f81c321 name: "group_index" @@ -246740,6 +246746,7 @@ struct_union { member_id: 0xb826ad15 member_id: 0xf0897d29 member_id: 0x26e58ea7 + member_id: 0x16cb0979 member_id: 0xc397b990 member_id: 0x933349d1 member_id: 0xbadfff7b diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 64eda2503892..19e7b77ef0cf 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -694,6 +694,9 @@ struct perf_event { /* The cumulative AND of all event_caps for events in this group. */ int group_caps; +#ifndef __GENKSYMS__ + unsigned int group_generation; +#endif struct perf_event *group_leader; struct pmu *pmu; void *pmu_private; diff --git a/kernel/events/core.c b/kernel/events/core.c index e8506aca1fc0..e82f21843b25 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -1977,6 +1977,7 @@ static void perf_group_attach(struct perf_event *event) list_add_tail(&event->sibling_list, &group_leader->sibling_list); group_leader->nr_siblings++; + group_leader->group_generation++; perf_event__header_size(group_leader); @@ -2171,6 +2172,7 @@ static void perf_group_detach(struct perf_event *event) if (leader != event) { list_del_init(&event->sibling_list); event->group_leader->nr_siblings--; + event->group_leader->group_generation++; goto out; } @@ -5295,7 +5297,7 @@ static int __perf_read_group_add(struct perf_event *leader, u64 read_format, u64 *values) { struct perf_event_context *ctx = leader->ctx; - struct perf_event *sub; + struct perf_event *sub, *parent; unsigned long flags; int n = 1; /* skip @nr */ int ret; @@ -5305,6 +5307,33 @@ static int __perf_read_group_add(struct perf_event *leader, return ret; raw_spin_lock_irqsave(&ctx->lock, flags); + /* + * Verify the grouping between the parent and child (inherited) + * events is still in tact. + * + * Specifically: + * - leader->ctx->lock pins leader->sibling_list + * - parent->child_mutex pins parent->child_list + * - parent->ctx->mutex pins parent->sibling_list + * + * Because parent->ctx != leader->ctx (and child_list nests inside + * ctx->mutex), group destruction is not atomic between children, also + * see perf_event_release_kernel(). Additionally, parent can grow the + * group. + * + * Therefore it is possible to have parent and child groups in a + * different configuration and summing over such a beast makes no sense + * what so ever. + * + * Reject this. + */ + parent = leader->parent; + if (parent && + (parent->group_generation != leader->group_generation || + parent->nr_siblings != leader->nr_siblings)) { + ret = -ECHILD; + goto unlock; + } /* * Since we co-schedule groups, {enabled,running} times of siblings @@ -5338,8 +5367,9 @@ static int __perf_read_group_add(struct perf_event *leader, values[n++] = atomic64_read(&sub->lost_samples); } +unlock: raw_spin_unlock_irqrestore(&ctx->lock, flags); - return 0; + return ret; } static int perf_read_group(struct perf_event *event, @@ -5358,10 +5388,6 @@ static int perf_read_group(struct perf_event *event, values[0] = 1 + leader->nr_siblings; - /* - * By locking the child_mutex of the leader we effectively - * lock the child list of all siblings.. XXX explain how. - */ mutex_lock(&leader->child_mutex); ret = __perf_read_group_add(leader, read_format, values); @@ -13268,6 +13294,7 @@ static int inherit_group(struct perf_event *parent_event, !perf_get_aux_event(child_ctr, leader)) return -EINVAL; } + leader->group_generation = parent_event->group_generation; return 0; }