From a307514d7c288a30cb99160df9554f64b97d1816 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Wcis=C5=82o?= Date: Mon, 4 Aug 2025 22:54:15 +0200 Subject: [PATCH 1/2] perf: Disallow mis-matched inherited group reads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit jira VULN-7623 cve CVE-2023-5717 commit-author Peter Zijlstra commit 32671e3799ca2e4590773fd0e63aaa4229e50c06 upstream-diff The mainline fix 32671e3799ca2e4590773fd0e63aaa4229e50c06 adds a new `group_generation' field to the `perf_event' struct. This breaks CBR 7.9 kABI. The new field was preserved, but moved to the end of the struct and wrapped in the `RH_KABI_EXTEND' macro. It can be assumed the kABI in this particular case is preserved based on the fact that there are already plenty of `RH_KABI_EXTEND(...)' fields at the end which could not have been added if the premise was false. Because group consistency is non-atomic between parent (filedesc) and children (inherited) events, it is possible for PERF_FORMAT_GROUP read() to try and sum non-matching counter groups -- with non-sensical results. Add group_generation to distinguish the case where a parent group removes and adds an event and thus has the same number, but a different configuration of events as inherited groups. This became a problem when commit fa8c269353d5 ("perf/core: Invert perf_read_group() loops") flipped the order of child_list and sibling_list. Previously it would iterate the group (sibling_list) first, and for each sibling traverse the child_list. In this order, only the group composition of the parent is relevant. By flipping the order the group composition of the child (inherited) events becomes an issue and the mis-match in group composition becomes evident. That said; even prior to this commit, while reading of a group that is not equally inherited was not broken, it still made no sense. (Ab)use ECHILD as error return to indicate issues with child process group composition. Fixes: fa8c269353d5 ("perf/core: Invert perf_read_group() loops") Reported-by: Budimir Markovic Signed-off-by: Peter Zijlstra (Intel) Link: https://lkml.kernel.org/r/20231018115654.GK33217@noisy.programming.kicks-ass.net (cherry picked from commit 32671e3799ca2e4590773fd0e63aaa4229e50c06) Signed-off-by: Marcin Wcisło --- include/linux/perf_event.h | 2 ++ kernel/events/core.c | 39 ++++++++++++++++++++++++++++++++------ 2 files changed, 35 insertions(+), 6 deletions(-) diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index 2faaa8b4dba8e..96cb6b86c161e 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -611,6 +611,8 @@ struct perf_event { #endif RH_KABI_EXTEND(unsigned long rcu_batches) RH_KABI_EXTEND(int rcu_pending) + + RH_KABI_EXTEND(unsigned int group_generation) #endif /* CONFIG_PERF_EVENTS */ }; diff --git a/kernel/events/core.c b/kernel/events/core.c index b829a386d1858..d3fbf948d811a 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -1773,6 +1773,7 @@ static void perf_group_attach(struct perf_event *event) list_add_tail(&event->group_entry, &group_leader->sibling_list); group_leader->nr_siblings++; + group_leader->group_generation++; perf_event__header_size(group_leader); @@ -1858,6 +1859,7 @@ static void perf_group_detach(struct perf_event *event) if (event->group_leader != event) { list_del_init(&event->group_entry); event->group_leader->nr_siblings--; + event->group_leader->group_generation++; goto out; } @@ -4561,7 +4563,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; @@ -4571,6 +4573,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 @@ -4600,8 +4629,9 @@ static int __perf_read_group_add(struct perf_event *leader, values[n++] = primary_event_id(sub); } +unlock: raw_spin_unlock_irqrestore(&ctx->lock, flags); - return 0; + return ret; } static int perf_read_group(struct perf_event *event, @@ -4620,10 +4650,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); @@ -11005,6 +11031,7 @@ static int inherit_group(struct perf_event *parent_event, if (IS_ERR(child_ctr)) return PTR_ERR(child_ctr); } + leader->group_generation = parent_event->group_generation; return 0; } From 9d05a62eca782bdd3e3d75556f80d25d3695de00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcin=20Wcis=C5=82o?= Date: Mon, 4 Aug 2025 22:54:45 +0200 Subject: [PATCH 2/2] perf/core: Fix potential NULL deref MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit jira VULN-7623 cve-bf CVE-2023-5717 commit-author Peter Zijlstra commit a71ef31485bb51b846e8db8b3a35e432cc15afb5 Smatch is awesome. Fixes: 32671e3799ca ("perf: Disallow mis-matched inherited group reads") Reported-by: Dan Carpenter Signed-off-by: Peter Zijlstra (Intel) Signed-off-by: Ingo Molnar (cherry picked from commit a71ef31485bb51b846e8db8b3a35e432cc15afb5) Signed-off-by: Marcin Wcisło --- kernel/events/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/kernel/events/core.c b/kernel/events/core.c index d3fbf948d811a..2d0ed33406bfd 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -11031,7 +11031,8 @@ static int inherit_group(struct perf_event *parent_event, if (IS_ERR(child_ctr)) return PTR_ERR(child_ctr); } - leader->group_generation = parent_event->group_generation; + if (leader) + leader->group_generation = parent_event->group_generation; return 0; }