From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0a-0031df01.pphosted.com (mx0a-0031df01.pphosted.com [205.220.168.131]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 3593E455168 for ; Fri, 7 Aug 2026 06:54:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=205.220.168.131 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786085672; cv=none; b=Kx7XSz32gL6qj6+lIltsQzav1fCyHf0px5gmJdHrg2r5bKsWOcmiNdcDbL71Evb1CA53ofbC+XDsIhGFHAhbk+RQaXqQpV+2Yk0IXSA4ddTNa7P8rwc1MqeGv0sZOg8bEFmCB9z1ape5m5/va2zv+CAyPO0TLq3UWLc+NawqBJQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786085672; c=relaxed/simple; bh=tug7sQswO90srQ+3Q3AllW5rofm8ksyrqn6GVbehczM=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=SkHS2QGzViCbsG/b1EfouPNDAMPGcyuO7FUmXln5SEBTGP/6JEDsVind2Y5rCvxOm7AY9zolBnlvk9XhT3r0cOWG7srnt/erZcMnVdohPMTczx5XG+4f8ze49UAG3ldU1sp3OhYmWK1saCpeTn9xaHQFdqowYbHvGRIrfzVOwLw= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=oss.qualcomm.com; spf=pass smtp.mailfrom=oss.qualcomm.com; dkim=pass (2048-bit key) header.d=qualcomm.com header.i=@qualcomm.com header.b=leP7pXFk; dkim=pass (2048-bit key) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com header.b=UUkomepa; arc=none smtp.client-ip=205.220.168.131 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=oss.qualcomm.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=oss.qualcomm.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=qualcomm.com header.i=@qualcomm.com header.b="leP7pXFk"; dkim=pass (2048-bit key) header.d=oss.qualcomm.com header.i=@oss.qualcomm.com header.b="UUkomepa" Received: from pps.filterd (m0279866.ppops.net [127.0.0.1]) by mx0a-0031df01.pphosted.com (8.18.1.11/8.18.1.11) with ESMTP id 6770GgZ23177110 for ; Fri, 7 Aug 2026 06:54:27 GMT DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=qualcomm.com; h= cc:content-transfer-encoding:content-type:date:from:in-reply-to :message-id:mime-version:references:subject:to; s=qcppdkim1; bh= ZeOBTbYHrkUTwW5imyA1ZIjKriCPkAvo/0a6LVyIW64=; b=leP7pXFkWUrClpiK Ickn3XtAgxSfXj1RcXk19Fme9576WrJTTDMaZSJU07Mgj2wZsNes11qS68wbdh2H P47d9fu8iq0YdAf9e4qVW6vKLxHu6g+ifk2a2KzRAQR0vcveEum2mcTMF96zr3RS 0JAa3Z8nram284q7ytcuHKTc3uPJv+lKSDqijUfwc0Mt1SH9RSA1R12J9xNFP5Us M56gzuo0/BA1UQEezcoezE0mTRMD7MslN1ldkO9ARVqN67vO+9UVWBQLshNkWpTV L9YB6ScdVgJWNHhK9Y0o8NHaQoaR4WcAabzRW+Fp60feDrNW3qEEwX1qZucKels+ E/pwog== Received: from mail-pf1-f200.google.com (mail-pf1-f200.google.com [209.85.210.200]) by mx0a-0031df01.pphosted.com (PPS) with ESMTPS id 4fvy6jjkqu-1 (version=TLSv1.3 cipher=TLS_AES_128_GCM_SHA256 bits=128 verify=NOT) for ; Fri, 07 Aug 2026 06:54:27 +0000 (GMT) Received: by mail-pf1-f200.google.com with SMTP id d2e1a72fcca58-84870e7f498so3345751b3a.3 for ; Thu, 06 Aug 2026 23:54:27 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=oss.qualcomm.com; s=google; t=1786085666; x=1786690466; darn=vger.kernel.org; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:from:to:cc:subject:date:message-id:reply-to :content-type; bh=ZeOBTbYHrkUTwW5imyA1ZIjKriCPkAvo/0a6LVyIW64=; b=UUkomepaZB4Ob93GQy6X5cwlBRTWt1fY2ENdgYW+Zj8+46oHwPc3cERPt4TugirdPv AtyWRZNx5RkFSTt7VYe3wAlusqnFH2RHpK08E3ruAuGWrOlS7aEsbyaEf9wIvAdWRM66 7bea/efbLbEoy+ho1AOnb7sX7sogtUaPvAARTQEbCS33T5LaS6quigOX1D8riIus7d06 uyontBqu5mwENm0JOgJsRcXOpeScHiXDIlAdIeblVR9HkDaFP592B6C/ZYCeQbjcrK+l TB2jt3v4BPhMRmkPwpaUfkjCeMMeKfrlIP48Qdvyp1kD0X6xBAc0m1ScfYQFLb9x/kv4 RAWw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1786085666; x=1786690466; h=content-transfer-encoding:content-type:in-reply-to:from :content-language:references:cc:to:subject:user-agent:mime-version :date:message-id:x-gm-gg:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to:content-type; bh=ZeOBTbYHrkUTwW5imyA1ZIjKriCPkAvo/0a6LVyIW64=; b=hDeHkqOl2RyLUpLz3jio3dXa7fNJYFbLrDWLGn7lSnZHA4HEVOK8wQmtM/RgRgD1MD yhuh4Il5Kl0fsCjIha7as3f5m3VF78LH3wwIO3VbL5lHxvNmtcMYUHuI+CVpSOxS6B4T iuMWDWjlpIpLBqrD9K5yAn04jQ672coeOou/a5h0gwwVBLKxibQ2PE56fdJSQm9rmdrd WrrUB26DXGre+eiZE3XVLuXBDRoZLgSSOtTeU4kQpSKngaRrsShf635Lc+Y/2KdUbart 9PXlpec0N1y+MTHqqUOfds0PTQ27dVD1C2TQ2dVErr8159PQWdiTqe5pW2mI2Pn4l+KS WJmw== X-Forwarded-Encrypted: i=1; AHgh+Rp4k1+Z8BZdQ1YIDQbFsnN+LwPSRU/pNhQAbjD6uwq8uk/JjA0evlDR7z73eHl4L+7gQJXfYluaW4Y2Eb8=@vger.kernel.org X-Gm-Message-State: AOJu0YxgiQrs+N05fSzvnetVAaNoGD47bcxVKEOQZBnMc1axas619LPB y1wXslyZEwUNacmN6qIlzMWnQOJiJN/5+SlWAAbQ3IfRFEx04+iaOof2lom5OwCZANICG9Gxaow oxIUe9+yFnuMk7MiIXt1LBx0RqcsPdyPgRAmWZi4NXy+USAmGoOpxXvE4O5I+6DtIw9I= X-Gm-Gg: AR+sD11Wf6t1/547oPkLILhZ0ePisp1qmHvBq8CUrrQM3KspXPe2Tg8dSYgH5TjQUr4 czNW+UFhJ40vcxiICyGL3RXqf0JidxkYVOiMfFQZjXEx0L9uAyzk273u49jbYsOL8GEdj1sji2B 5nYS0/oHBEfKZTbZap6H+kFwnNbZKON/MFxQpfwXOprMyNe9y4Y9pz6DetR0XiYiH25loXMHo+C iQfEWan7Tkh6i9bJ0PmGAjuCrEAFEX1QwxXpWU10Vp71zg52liu3EflKQUONQoUI8zBaDt/hgee vWInNY+Ego4FiTG7KAq/q0j85Uayqo5fCTzjsARbYmVFdOqErfUKAQBNvm8/L6/i2ctXfzdXhZ8 Ie0XKQl0LpH0yAJ4LKqhWbsdKh//t13PVzA== X-Received: by 2002:aa7:9303:0:b0:848:6b76:592f with SMTP id d2e1a72fcca58-84f2e021ecfmr25217915b3a.5.1786085666423; Thu, 06 Aug 2026 23:54:26 -0700 (PDT) X-Received: by 2002:aa7:9303:0:b0:848:6b76:592f with SMTP id d2e1a72fcca58-84f2e021ecfmr25217866b3a.5.1786085665952; Thu, 06 Aug 2026 23:54:25 -0700 (PDT) Received: from [10.218.24.187] ([202.46.22.19]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-84f5a3a188esm510948b3a.5.2026.08.06.23.54.21 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 06 Aug 2026 23:54:25 -0700 (PDT) Message-ID: <9a965b40-c15d-4802-8d55-aacb9ad33987@oss.qualcomm.com> Date: Fri, 7 Aug 2026 12:24:20 +0530 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [PATCH v2] perf/core: Fix group leader use-after-free after sibling detach To: Peter Zijlstra Cc: Ingo Molnar , Arnaldo Carvalho de Melo , Namhyung Kim , Mark Rutland , Alexander Shishkin , Jiri Olsa , Ian Rogers , Adrian Hunter , James Clark , Ingo Molnar , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org References: <20260630-fix-group-leader-uaf-v2-1-9349121835ee@oss.qualcomm.com> <20260806195020.GR48970@noisy.programming.kicks-ass.net> Content-Language: en-US From: Aditya Chillara In-Reply-To: <20260806195020.GR48970@noisy.programming.kicks-ass.net> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Proofpoint-Spam-Details-Enc: AW1haW4tMjYwODA3MDA1MCBTYWx0ZWRfX8m0Ep56Ob3TP V65NaP4qunvh9+BwUG0S0DWYg6O21Amt7wc8MQFiv3NV7VqpvISoHbjtXhTo+MOTchVmCVQ0zPL AIRmcwdW7AYGZDa3/uO1YcHBFHiDZMzgbtXPMPm06N2p1fkbOpbwBCXAbMREqc8GZnEfPqt+yR1 JIsYC5W0CPkHJRRl9Qei/a7jZiGYqzQDelhIAIoADj/+z/ULiXB+rMMbsJwXhqjN1he+Gun6fKI cpa60Q/zYU9pDHegUlBokEKy5Slg54aLKqBr878juTzRZC1MbDpJtzGjXummBT8T3c6L2uFM8fo GTVbuNjHkQ0bTHpjwh4i9UPrqbsSTU36FcwgN3z9TcDYVmX0fb3YesMf+ZNFTaG4VtA12rB8XKs jypkZvP4CZPWVfm6dsCr7m4m7IxpnaGYpdThMNGImrrXHLNF0ZuG6pJwjG9YuPh1CfdbIp8yhpq fbddfw/T2dnASHzZASQ== X-Proofpoint-Spam-Info: AW1haW4tMjYwODA3MDA1MCBTYWx0ZWRfX+PyZAPBHe2Su RVQ2xVaxqw/iRz6SPeEIJs0o8grG1OOIajOV0cSKylN9jo+9A1hSKi+90Hp4PxPBSPe34QmMWbH AJQ2iN0no/qKcvjl2Yy9hGpue3PE9cE= X-Proofpoint-ORIG-GUID: QXEWcPoPU_SKKSqmXK_PvHlBfvVRUhSX X-Proofpoint-GUID: QXEWcPoPU_SKKSqmXK_PvHlBfvVRUhSX X-Authority-Analysis: v=2.4 cv=e5Y2j6p/ c=1 sm=1 tr=0 ts=6a758123 cx=c_pps a=mDZGXZTwRPZaeRUbqKGCBw==:117 a=fChuTYTh2wq5r3m49p7fHw==:17 a=IkcTkHD0fZMA:10 a=Sv0fKeRqtYgA:10 a=s4-Qcg_JpJYA:10 a=VkNPw1HP01LnGYTKEx00:22 a=u7WPNUs3qKkmUXheDGA7:22 a=YMgV9FUhrdKAYTUUvYB2:22 a=6HyQyo9fOVBPkGHd9gEA:9 a=QEXdDO2ut3YA:10 a=zc0IvFSfCIW2DFIPzwfm:22 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.293,Aquarius:18.0.1176,Hydra:6.1.134,FMLib:17.12.100.49 definitions=2026-08-07_01,2026-08-06_01,2025-10-01_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 clxscore=1015 adultscore=0 spamscore=0 priorityscore=1501 impostorscore=0 bulkscore=0 lowpriorityscore=0 phishscore=0 suspectscore=0 malwarescore=0 classifier=typeunknown authscore=0 authtc= authcc= route=outbound adjust=0 reason=mlx scancount=1 engine=8.22.0-2606150000 definitions=main-2608070050 On 8/7/2026 1:20 AM, Peter Zijlstra wrote: > > Finally got to look at this. In principle this seems okay, but while > staring at it, I had a few questions, see below. > > On Tue, Jun 30, 2026 at 12:42:12AM +0530, Aditya Chillara wrote: >> --- >> kernel/events/core.c | 62 +++++++++++++++++++++++++++++++++++----------------- >> 1 file changed, 42 insertions(+), 20 deletions(-) >> >> diff --git a/kernel/events/core.c b/kernel/events/core.c >> index 954c36e28101..744643ada948 100644 >> --- a/kernel/events/core.c >> +++ b/kernel/events/core.c >> @@ -2253,6 +2253,8 @@ static void put_event(struct perf_event *event); >> static void __event_disable(struct perf_event *event, >> struct perf_event_context *ctx, >> enum perf_event_state state); >> +static void event_sched_out(struct perf_event *event, >> + struct perf_event_context *ctx); >> >> static void perf_put_aux_event(struct perf_event *event) >> { >> @@ -2343,6 +2345,44 @@ static inline struct list_head *get_event_list(struct perf_event *event) >> &event->pmu_ctx->flexible_active; >> } >> >> +/* @sibling must already be unlinked from its old leader's sibling_list. */ >> +static void perf_promote_sibling_to_leader(struct perf_event *sibling, >> + struct perf_event_context *ctx, >> + int group_caps) >> +{ >> + /* >> + * Events that have PERF_EV_CAP_SIBLING require being part of >> + * a group and cannot exist on their own, schedule them out >> + * and move them into the ERROR state. Also see >> + * _perf_event_enable(), it will not be able to recover this >> + * ERROR state. >> + */ >> + if (sibling->event_caps & PERF_EV_CAP_SIBLING) { >> + event_sched_out(sibling, ctx); >> + >> + /* >> + * The guards keep this correct even when @sibling is already >> + * disabled (see __perf_remove_from_context()). >> + */ >> + if (sibling->state > PERF_EVENT_STATE_OFF) >> + perf_cgroup_event_disable(sibling, ctx); >> + if (sibling->state > PERF_EVENT_STATE_ERROR) >> + perf_event_set_state(sibling, PERF_EVENT_STATE_ERROR); >> + } > > The below code used __event_disable(); and this change is not > mentioned in the Changelog. Why was this changed? I should've mentioned this in the Changelog, I'll update it. In case of __perf_remove_from_context(event, ...) where the event is part of a group but is not the leader; by the time we get to perf_group_detach(): 1. perf_cgroup_event_disable(event, ctx) would have been called 2. The sibling's state may have been set to EXIT/REVOKED/DEAD if not OFF So we cannot use __event_disable() because it would unconditionally call 1. perf_cgroup_event_disable() that would decrement nr_cgroups twice 2. perf_event_set_state(event, PERF_EVENT_STATE_ERROR) that could bring back the state from EXIT/REVOKED/DEAD/OFF to ERROR. In previous code, in case of __perf_remove_from_context(event, ...) where the event is a leader; __event_disable() was called for siblings unconditionally but I do not see a reason why the added guards don't hold. Should we move the guards to __event_disable() instead? That would simplify the code: static void __event_disable(struct perf_event *event, struct perf_event_context *ctx, enum perf_event_state state) { event_sched_out(event, ctx); if (event->state > PERF_EVENT_STATE_OFF) perf_cgroup_event_disable(event, ctx); perf_event_set_state(event, min(event->state, state)); } Also, event_sched_out() in case of pending_disable calls perf_cgroup_event_disable() and sets the state to OFF, but __event_disable() immediately calls perf_cgroup_event_disable() again. > >> + >> + sibling->group_leader = sibling; >> + sibling->group_caps = group_caps; >> + >> + if (sibling->attach_state & PERF_ATTACH_CONTEXT) { >> + add_event_to_groups(sibling, ctx); >> + >> + if (sibling->state == PERF_EVENT_STATE_ACTIVE) >> + list_add_tail(&sibling->active_list, get_event_list(sibling)); >> + } >> + >> + perf_event__header_size(sibling); >> +} >> + >> static void perf_group_detach(struct perf_event *event) >> { >> struct perf_event *leader = event->group_leader; >> @@ -2368,6 +2408,7 @@ static void perf_group_detach(struct perf_event *event) >> list_del_init(&event->sibling_list); >> event->group_leader->nr_siblings--; >> event->group_leader->group_generation++; > > Here we can do 's/event->group_//' I'll update this. > > Also, this case 'leader != event' we remove one sibling from a group and > decrement leader->nr_siblings... > >> + perf_promote_sibling_to_leader(event, ctx, event->event_caps); >> goto out; >> } >> >> @@ -2377,29 +2418,10 @@ static void perf_group_detach(struct perf_event *event) >> * to whatever list we are on. >> */ >> list_for_each_entry_safe(sibling, tmp, &event->sibling_list, sibling_list) { >> - >> - /* >> - * Events that have PERF_EV_CAP_SIBLING require being part of >> - * a group and cannot exist on their own, schedule them out >> - * and move them into the ERROR state. Also see >> - * _perf_event_enable(), it will not be able to recover this >> - * ERROR state. >> - */ >> - if (sibling->event_caps & PERF_EV_CAP_SIBLING) >> - __event_disable(sibling, ctx, PERF_EVENT_STATE_ERROR); >> - >> - sibling->group_leader = sibling; >> list_del_init(&sibling->sibling_list); >> >> /* Inherit group flags from the previous leader */ >> - sibling->group_caps = event->group_caps; >> - >> - if (sibling->attach_state & PERF_ATTACH_CONTEXT) { >> - add_event_to_groups(sibling, event->ctx); >> - >> - if (sibling->state == PERF_EVENT_STATE_ACTIVE) >> - list_add_tail(&sibling->active_list, get_event_list(sibling)); >> - } >> + perf_promote_sibling_to_leader(sibling, ctx, event->group_caps); >> >> WARN_ON_ONCE(sibling->ctx != event->ctx); >> } > > This is the case 'leader == event' (per not being the other case), and > this we remove all siblings, but then do not set leader->nr_siblings = > 0, should we ? Yes, I'll add this. Thank you, Aditya