| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0ec73a83-4965-6f82-1775-1320b525a90f@huawei.com>
Date: Tue, 7 Mar 2023 11:50:59 +0800
From: "liuwenyu (D)" <liuwenyu7@...wei.com>
To: Arnaldo Carvalho de Melo <acme@...nel.org>
CC: <peterz@...radead.org>, <mingo@...hat.com>, <mark.rutland@....com>,
<alexander.shishkin@...ux.intel.com>, <jolsa@...nel.org>,
<namhyung@...nel.org>, <irogers@...gle.com>, <brauner@...nel.org>,
<adrian.hunter@...el.com>, <linux-perf-users@...r.kernel.org>,
<linux-kernel@...r.kernel.org>
Subject: [PATCH v2] perf top: Fix rare segfault in thread__comm_len()
>From f5bf6bd64f1888bbd529fe3e06063f31f2336abc Mon Sep 17 00:00:00 2001
From: Wenyu Liu <liuwenyu7@...wei.com>
Date: Thu, 2 Mar 2023 19:16:37 +0800
Subject: [PATCH] perf top: Fix rare segfault in thread__comm_len()
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:
(gdb) bt
#0 __strlen_evex () at ../sysdeps/x86_64/multiarch/strlen-evex.S:77
#1 0x000055ad15d31de5 in thread__comm_len (thread=0x7f627d20e300) at util/thread.c:320
#2 0x000055ad15d4fade in hists__calc_col_len (h=0x7f627d295940, hists=0x55ad1772bfe0)
at util/hist.c:103
#3 hists__calc_col_len (hists=0x55ad1772bfe0, h=0x7f627d295940) at util/hist.c:79
#4 0x000055ad15d52c8c in output_resort (hists=hists@...ry=0x55ad1772bfe0, prog=0x0,
use_callchain=false, cb=cb@...ry=0x0, cb_arg=0x0) at util/hist.c:1926
#5 0x000055ad15d530a4 in evsel__output_resort_cb (evsel=evsel@...ry=0x55ad1772bde0,
prog=prog@...ry=0x0, cb=cb@...ry=0x0, cb_arg=cb_arg@...ry=0x0) at util/hist.c:1945
#6 0x000055ad15d53110 in evsel__output_resort (evsel=evsel@...ry=0x55ad1772bde0,
prog=prog@...ry=0x0) at util/hist.c:1950
#7 0x000055ad15c6ae9a in perf_top__resort_hists (t=t@...ry=0x7ffcd9cbf4f0) at builtin-top.c:311
#8 0x000055ad15c6cc6d in perf_top__print_sym_table (top=0x7ffcd9cbf4f0) at builtin-top.c:346
#9 display_thread (arg=0x7ffcd9cbf4f0) at builtin-top.c:700
#10 0x00007f6282fab4fa in start_thread (arg=<optimized out>) at pthread_create.c:443
#11 0x00007f628302e200 in clone3 () at ../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 also freed.
In thread__comm_len(),it gets the pointer within the thread->comm_lock critical section,
but passed to strlen() outside of the thread->comm_lock critical section, and the perf
process_thread may called comm__free() concurrently, cause this segfault problem.
The process is as follows:
display_thread process_thread
-------------- --------------
thread__comm_len
-> thread__comm_str
# held the comm read lock
-> __thread__comm_str(thread)
# release the comm read lock
thread__delete
# held the comm write lock
-> comm__free
-> comm_str__put(comm->comm_str)
-> zfree(&cs->str)
# release the comm write lock
# The memory of the string pointed
to by comm has been free.
-> thread->comm_len = strlen(comm);
This patch expand the critical section range of thread->comm_lock in thread__comm_len(),
to make strlen() called safe.
Signed-off-by: Wenyu Liu <liuwenyu7@...wei.com>
---
v1 -> v2
- format the patch file correctly
tools/perf/util/thread.c | 25 +++++++++++++++++++------
1 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)
return str;
}
+static int __thread__comm_len(struct thread *thread, const char *comm)
+{
+ if (!comm)
+ return 0;
+ thread->comm_len = strlen(comm);
+
+ return thread->comm_len;
+}
+
/* CHECKME: it should probably better return the max comm len from its comm list */
int thread__comm_len(struct thread *thread)
{
- if (!thread->comm_len) {
- const char *comm = thread__comm_str(thread);
- if (!comm)
- return 0;
- thread->comm_len = strlen(comm);
+ int comm_len = thread->comm_len;
+
+ if (!comm_len) {
+ const char *comm;
+
+ down_read(&thread->comm_lock);
+ comm = __thread__comm_str(thread);
+ comm_len = __thread__comm_len(thread, comm);
+ up_read(&thread->comm_lock);
}
- return thread->comm_len;
+ return comm_len;
}
size_t thread__fprintf(struct thread *thread, FILE *fp)
--
2.36.1
Powered by blists - more mailing lists