From patchwork Thu Oct 19 08:23:59 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: tip-bot2 for Thomas Gleixner X-Patchwork-Id: 155365 Return-Path: Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:612c:2010:b0:403:3b70:6f57 with SMTP id fe16csp234955vqb; Thu, 19 Oct 2023 01:24:41 -0700 (PDT) X-Google-Smtp-Source: AGHT+IHU6TdjrzNeWnk/XB8KC5S+yS7ZTA2JGhoUsd3dXcB3VrZGSylMiCimkOZZ9eFpriSS0o8C X-Received: by 2002:a05:6a20:7da2:b0:17b:2df4:87ed with SMTP id v34-20020a056a207da200b0017b2df487edmr1582038pzj.17.1697703880894; Thu, 19 Oct 2023 01:24:40 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1697703880; cv=none; d=google.com; s=arc-20160816; b=dule99yZDyaEP03JuM0xfFzzdHTzIs/SDFdjbmRpAndvaajezpQp9Pd2r7Rm6ZiWf3 /uXevi5PJZpsDvxcyzdSBdQT9P62XU9TwXv04kPdtgRIwcioEgoNZsHE6Yvbs50E0XIu jOEtIcwukJSkZdI/eHYLpKdGcfataZ3Y56vCWQC6Lqnngnck+Dxsg8vBBHCUV4OBXIjk k361eBt6yu/zyKmJb+TlGBYw9zyeHtzn6lNLI5ZJC94W/0EsE/C4MdaylgWzWee9KVeR Km9gxXjuM3jj9BWwEv5BCb46uYjt1Mo+75m26dmAK9i4KV11YtgCJ+ceoUFfMOeCIGIi tk0w== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:robot-unsubscribe :robot-id:message-id:mime-version:references:in-reply-to:cc:subject :to:reply-to:sender:from:dkim-signature:dkim-signature:date; bh=ATmV+shENxojKtOgLX8bX5Ko5VODxesMS6F+xBHm3TA=; fh=hdBu+3HWTdONyPLGZY80QUy8Q+ogP7e5CEdVNQZ0sQQ=; b=c7XBQZgiLS4ldCfdliekEIGCVmQnp90qLXtSPNirUFFrqcLIgHFkme/JeMXkvSqIh6 Uo9lvTvv1vsKTv1ENMdnD+27VIwweGaNt3e6dByWAruwwTzLkDnUmKkTtw3Qcx4hrJzm 5ELyV1eAb8IFh6SajK/y+6dJtjCKpnMZxR2MQam11ZVBOZ5i6QKDg+2+yEGcoU2rxoIU botCUt9JtYSTOdwS/017sUEuelRlL/ZvUwVWGlY7U0nEqpNda7fkLMHcrVq+dRsWglQi WLqigLCwJZKWxOq8R2ooKuoX273Sk66NXlDWAAcheqqbjMwVlDKIk+cd0sp/8cGviRk5 k5WQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=IYCjluzw; dkim=neutral (no key) header.i=@linutronix.de header.b=HHMawuVs; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: from howler.vger.email (howler.vger.email. [23.128.96.34]) by mx.google.com with ESMTPS id ay9-20020a17090b030900b00276df8c5b83si1530539pjb.143.2023.10.19.01.24.39 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 19 Oct 2023 01:24:40 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) client-ip=23.128.96.34; Authentication-Results: mx.google.com; dkim=pass header.i=@linutronix.de header.s=2020 header.b=IYCjluzw; dkim=neutral (no key) header.i=@linutronix.de header.b=HHMawuVs; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.34 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=QUARANTINE dis=NONE) header.from=linutronix.de Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by howler.vger.email (Postfix) with ESMTP id 56AF28246949; Thu, 19 Oct 2023 01:24:37 -0700 (PDT) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.10 at howler.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1344904AbjJSIYL (ORCPT + 24 others); Thu, 19 Oct 2023 04:24:11 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:59244 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235308AbjJSIYF (ORCPT ); Thu, 19 Oct 2023 04:24:05 -0400 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 15014182; Thu, 19 Oct 2023 01:24:03 -0700 (PDT) Date: Thu, 19 Oct 2023 08:23:59 -0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1697703840; h=from:from:sender:sender:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ATmV+shENxojKtOgLX8bX5Ko5VODxesMS6F+xBHm3TA=; b=IYCjluzwcIrVVHp6/It2Clz1236LclJy3xsuqgHEfrN1dWRCv/kqQFJAlLWQ9gXwQvogmG c2l19LdpXDT4L4MtKJ9WlezYgKV9q3ocCG9dBQSuXV1/FOdO7v6NHiNO92pKhbEMQGi6c3 iCbDN9e3cGG0GiAt8ujYnljzVDx1LC07OtzTDy437bUmFwdurDVNxEymCBRY3K67/P6OYs tNcoaGPaDg3hc0pO91sbayQ/cxgpgT25P14vyeBcFq0ak0g8KtS0MyAtzg7iI4bbxaL9vh DXBPb6Vrjkpco3/ePc20HwVntP56rasFUlL0yVbFh5e2kJSvgOSNF9+q+8YCKQ== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1697703840; h=from:from:sender:sender:reply-to:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ATmV+shENxojKtOgLX8bX5Ko5VODxesMS6F+xBHm3TA=; b=HHMawuVsLa5nSxJr+aRXzbbpTudgOMa5mvW5AigGl424YCceLV00ZfI+FFax9oQ3uKup0m vSFUQS/cgGfUweAQ== From: "tip-bot2 for Peter Zijlstra" Sender: tip-bot2@linutronix.de Reply-to: linux-kernel@vger.kernel.org To: linux-tip-commits@vger.kernel.org Subject: [tip: perf/urgent] perf: Disallow mis-matched inherited group reads Cc: Budimir Markovic , "Peter Zijlstra (Intel)" , x86@kernel.org, linux-kernel@vger.kernel.org In-Reply-To: <20231018115654.GK33217@noisy.programming.kicks-ass.net> References: <20231018115654.GK33217@noisy.programming.kicks-ass.net> MIME-Version: 1.0 Message-ID: <169770383951.3135.17771457264387517954.tip-bot2@tip-bot2> Robot-ID: Robot-Unsubscribe: Contact to get blacklisted from these emails X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on howler.vger.email Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (howler.vger.email [0.0.0.0]); Thu, 19 Oct 2023 01:24:37 -0700 (PDT) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1780171544772766916 X-GMAIL-MSGID: 1780171544772766916 The following commit has been merged into the perf/urgent branch of tip: Commit-ID: 32671e3799ca2e4590773fd0e63aaa4229e50c06 Gitweb: https://git.kernel.org/tip/32671e3799ca2e4590773fd0e63aaa4229e50c06 Author: Peter Zijlstra AuthorDate: Wed, 18 Oct 2023 13:56:54 +02:00 Committer: Peter Zijlstra CommitterDate: Thu, 19 Oct 2023 10:09:42 +02:00 perf: Disallow mis-matched inherited group reads 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 --- include/linux/perf_event.h | 1 +- kernel/events/core.c | 39 +++++++++++++++++++++++++++++++------ 2 files changed, 34 insertions(+), 6 deletions(-) diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h index e85cd1c..7b5406e 100644 --- a/include/linux/perf_event.h +++ b/include/linux/perf_event.h @@ -704,6 +704,7 @@ struct perf_event { /* The cumulative AND of all event_caps for events in this group. */ int group_caps; + unsigned int group_generation; struct perf_event *group_leader; /* * event->pmu will always point to pmu in which this event belongs. diff --git a/kernel/events/core.c b/kernel/events/core.c index 4c72a41..d0663b9 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -1954,6 +1954,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); @@ -2144,6 +2145,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; } @@ -5440,7 +5442,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; @@ -5450,6 +5452,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 @@ -5483,8 +5512,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, @@ -5503,10 +5533,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); @@ -13346,6 +13372,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; }