From nobody Sat Apr 11 07:08:26 2026 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A9B22C678D4 for ; Fri, 3 Mar 2023 10:02:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S230282AbjCCKC3 (ORCPT ); Fri, 3 Mar 2023 05:02:29 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46266 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S230216AbjCCKC0 (ORCPT ); Fri, 3 Mar 2023 05:02:26 -0500 Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 0A0185CC32; Fri, 3 Mar 2023 02:02:22 -0800 (PST) Received: from kwepemm600006.china.huawei.com (unknown [172.30.72.54]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4PSk3F6YXhz16PB4; Fri, 3 Mar 2023 17:59:37 +0800 (CST) Received: from [10.174.177.30] (10.174.177.30) by kwepemm600006.china.huawei.com (7.193.23.105) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.21; Fri, 3 Mar 2023 18:02:18 +0800 Message-ID: Date: Fri, 3 Mar 2023 18:02:17 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.1.0 CC: , "hewenliang (C)" , , Wenyu Liu From: "liuwenyu (D)" Subject: [PATCH] perf top: Fix rare segfault in thread__comm_len() To: , , , , , , , , , , , Content-Type: text/plain; charset="utf-8"; format="flowed" Content-Transfer-Encoding: quoted-printable X-Originating-IP: [10.174.177.30] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To kwepemm600006.china.huawei.com (7.193.23.105) X-CFilter-Loop: Reflected Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org In thread__comm_len(),strlen() is called outside of the thread->comm_lock critical section,which may cause a UAF problems if comm__free() is called by the process_thread concurrently. backtrace of the core file is as follows: =C2=A0=C2=A0=C2=A0 (gdb) bt =C2=A0=C2=A0=C2=A0 #0=C2=A0 __strlen_evex () at ../sysdeps/x86_64/multiarc= h/strlen-evex.S:77 =C2=A0=C2=A0=C2=A0 #1=C2=A0 0x000055ad15d31de5 in thread__comm_len (thread= =3D0x7f627d20e300)=20 at util/thread.c:320 =C2=A0=C2=A0=C2=A0 #2=C2=A0 0x000055ad15d4fade in hists__calc_col_len (h= =3D0x7f627d295940,=20 hists=3D0x55ad1772bfe0) =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 at util/hist.c:103 =C2=A0=C2=A0=C2=A0 #3=C2=A0 hists__calc_col_len (hists=3D0x55ad1772bfe0, h= =3D0x7f627d295940) at=20 util/hist.c:79 =C2=A0=C2=A0=C2=A0 #4=C2=A0 0x000055ad15d52c8c in output_resort=20 (hists=3Dhists@entry=3D0x55ad1772bfe0, prog=3D0x0, =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 use_callchain=3Dfalse, cb=3Dcb@= entry=3D0x0, cb_arg=3D0x0) at=20 util/hist.c:1926 =C2=A0=C2=A0=C2=A0 #5=C2=A0 0x000055ad15d530a4 in evsel__output_resort_cb=20 (evsel=3Devsel@entry=3D0x55ad1772bde0, =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 prog=3Dprog@entry=3D0x0, cb=3Dc= b@entry=3D0x0, cb_arg=3Dcb_arg@entry=3D0x0)=20 at util/hist.c:1945 =C2=A0=C2=A0=C2=A0 #6=C2=A0 0x000055ad15d53110 in evsel__output_resort=20 (evsel=3Devsel@entry=3D0x55ad1772bde0, =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 prog=3Dprog@entry=3D0x0) at uti= l/hist.c:1950 =C2=A0=C2=A0=C2=A0 #7=C2=A0 0x000055ad15c6ae9a in perf_top__resort_hists=20 (t=3Dt@entry=3D0x7ffcd9cbf4f0) at builtin-top.c:311 =C2=A0=C2=A0=C2=A0 #8=C2=A0 0x000055ad15c6cc6d in perf_top__print_sym_tabl= e=20 (top=3D0x7ffcd9cbf4f0) at builtin-top.c:346 =C2=A0=C2=A0=C2=A0 #9=C2=A0 display_thread (arg=3D0x7ffcd9cbf4f0) at built= in-top.c:700 =C2=A0=C2=A0=C2=A0 #10 0x00007f6282fab4fa in start_thread (arg=3D) at=20 pthread_create.c:443 =C2=A0=C2=A0=C2=A0 #11 0x00007f628302e200 in clone3 () at=20 ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81 The reason is that strlen() get a pointer to a memory that has been freed. The string pointer is stored in the structure comm_str, which corresponds to a rb_tree node,when the node is erased, the memory of the string is=20 also freed. In thread__comm_len(),it gets the pointer within the thread->comm_lock=20 critical section, but passed to strlen() outside of the thread->comm_lock critical=20 section, and the perf process_thread may called comm__free() concurrently, cause this segfault=20 problem. The process is as follows: display_thread=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 process_thr= ead Acked-by: Namhyung Kim --------------=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 -----------= --- thread__comm_len =C2=A0 -> thread__comm_str =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 # held the comm read lock =C2=A0=C2=A0=C2=A0 -> __thread__comm_str(thread) =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 # release the comm read lock =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 thread__delete =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 # held the comm=20 write lock =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 ->= comm__free =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0 ->=20 comm_str__put(comm->comm_str) =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0 -> zfree(&cs->str) =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0= =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2= =A0=C2=A0=C2=A0 # release the comm=20 write lock =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 # The memory of the string pointed =C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0=C2=A0 to by comm has been free. =C2=A0=C2=A0=C2=A0 -> thread->comm_len =3D strlen(comm); This patch expand the critical section range of thread->comm_lock in=20 thread__comm_len(), to make strlen() called safe. Signed-off-by: Wenyu Liu=C2=A0 --- =C2=A0tools/perf/util/thread.c | 25 +++++++++++++++++++------ =C2=A01 file changed, 19 insertions(+), 6 deletions(-) diff --git a/tools/perf/util/thread.c b/tools/perf/util/thread.c index e3e5427e1c3c..a2490a20eb56 100644 --- a/tools/perf/util/thread.c +++ b/tools/perf/util/thread.c @@ -311,17 +311,30 @@ const char *thread__comm_str(struct thread *thread) =C2=A0=C2=A0=C2=A0=C2=A0 return str; =C2=A0} +static int __thread__comm_len(struct thread *thread, const char *comm) +{ +=C2=A0=C2=A0=C2=A0 if (!comm) +=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 return 0; +=C2=A0=C2=A0=C2=A0 thread->comm_len =3D strlen(comm); + +=C2=A0=C2=A0=C2=A0 return thread->comm_len; +} + =C2=A0/* CHECKME: it should probably better return the max comm len from i= ts=20 comm list */ =C2=A0int thread__comm_len(struct thread *thread) =C2=A0{ -=C2=A0=C2=A0=C2=A0 if (!thread->comm_len) { -=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 const char *comm =3D thread__comm_st= r(thread); -=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 if (!comm) -=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 return 0; -=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 thread->comm_len =3D strlen(comm); +=C2=A0=C2=A0=C2=A0 int comm_len =3D thread->comm_len; + +=C2=A0=C2=A0=C2=A0 if (!comm_len) { +=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 const char *comm; + +=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 down_read(&thread->comm_lock); +=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 comm =3D __thread__comm_str(thread); +=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 comm_len =3D __thread__comm_len(thre= ad, comm); +=C2=A0=C2=A0=C2=A0 =C2=A0=C2=A0=C2=A0 up_read(&thread->comm_lock); =C2=A0=C2=A0=C2=A0=C2=A0 } -=C2=A0=C2=A0=C2=A0 return thread->comm_len; +=C2=A0=C2=A0=C2=A0 return comm_len; =C2=A0} =C2=A0size_t thread__fprintf(struct thread *thread, FILE *fp) --=20 2.36.1