Skip to content

Commit 6a4ef7a

Browse files
braunerbrauner
committed
Merge patch series "fix reading ESP during coredump"
Nam Cao <namcao@linutronix.de> says: In /proc/PID/stat, there is the kstkesp field which is the stack pointer of a thread. While the thread is active, this field reads zero. But during a coredump, it should have a valid value. However, at the moment, kstkesp is zero even during coredump. The first commit fixes this problem, and the second commit adds a selftest to detect if this problem appears again in the future. * patches from https://lore.kernel.org/r/cover.1735805772.git.namcao@linutronix.de: selftests: coredump: Add stackdump test fs/proc: do_task_stat: Fix ESP not readable during coredump Link: https://lore.kernel.org/r/cover.1735805772.git.namcao@linutronix.de Signed-off-by: Christian Brauner <brauner@kernel.org>
2 parents aaec5a9 + 15858da commit 6a4ef7a

5 files changed

Lines changed: 223 additions & 1 deletion

File tree

‎fs/proc/array.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -500,7 +500,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
500500
* a program is not able to use ptrace(2) in that case. It is
501501
* safe because the task has stopped executing permanently.
502502
*/
503-
if (permitted && (task->flags & (PF_EXITING|PF_DUMPCORE))) {
503+
if (permitted && (task->flags & (PF_EXITING|PF_DUMPCORE|PF_POSTCOREDUMP))) {
504504
if (try_get_task_stack(task)) {
505505
eip = KSTK_EIP(task);
506506
esp = KSTK_ESP(task);

‎tools/testing/selftests/coredump/Makefile‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
# SPDX-License-Identifier: GPL-2.0-only
2+
CFLAGS = $(KHDR_INCLUDES)
3+
4+
TEST_GEN_PROGS := stackdump_test
5+
TEST_FILES := stackdump
6+
7+
include ../lib.mk

‎tools/testing/selftests/coredump/README.rst‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
coredump selftest
2+
=================
3+
4+
Background context
5+
------------------
6+
7+
`coredump` is a feature which dumps a process's memory space when the process terminates
8+
unexpectedly (e.g. due to segmentation fault), which can be useful for debugging. By default,
9+
`coredump` dumps the memory to the file named `core`, but this behavior can be changed by writing a
10+
different file name to `/proc/sys/kernel/core_pattern`. Furthermore, `coredump` can be piped to a
11+
user-space program by writing the pipe symbol (`|`) followed by the command to be executed to
12+
`/proc/sys/kernel/core_pattern`. For the full description, see `man 5 core`.
13+
14+
The piped user program may be interested in reading the stack pointers of the crashed process. The
15+
crashed process's stack pointers can be read from `procfs`: it is the `kstkesp` field in
16+
`/proc/$PID/stat`. See `man 5 proc` for all the details.
17+
18+
The problem
19+
-----------
20+
While a thread is active, the stack pointer is unsafe to read and therefore the `kstkesp` field
21+
reads zero. But when the thread is dead (e.g. during a coredump), this field should have valid
22+
value.
23+
24+
However, this was broken in the past and `kstkesp` was zero even during coredump:
25+
26+
* commit 0a1eb2d474ed ("fs/proc: Stop reporting eip and esp in /proc/PID/stat") changed kstkesp to
27+
always be zero
28+
29+
* commit fd7d56270b52 ("fs/proc: Report eip/esp in /prod/PID/stat for coredumping") fixed it for the
30+
coredumping thread. However, other threads in a coredumping process still had the problem.
31+
32+
* commit cb8f381f1613 ("fs/proc/array.c: allow reporting eip/esp for all coredumping threads") fixed
33+
for all threads in a coredumping process.
34+
35+
* commit 92307383082d ("coredump: Don't perform any cleanups before dumping core") broke it again
36+
for the other threads in a coredumping process.
37+
38+
The problem has been fixed now, but considering the history, it may appear again in the future.
39+
40+
The goal of this test
41+
---------------------
42+
This test detects problem with reading `kstkesp` during coredump by doing the following:
43+
44+
#. Tell the kernel to execute the "stackdump" script when a coredump happens. This script
45+
reads the stack pointers of all threads of crashed processes.
46+
47+
#. Spawn a child process who creates some threads and then crashes.
48+
49+
#. Read the output from the "stackdump" script, and make sure all stack pointer values are
50+
non-zero.

‎tools/testing/selftests/coredump/stackdump‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
#!/bin/sh
2+
# SPDX-License-Identifier: GPL-2.0
3+
4+
CRASH_PROGRAM_ID=$1
5+
STACKDUMP_FILE=$2
6+
7+
TMP=$(mktemp)
8+
9+
for t in /proc/$CRASH_PROGRAM_ID/task/*; do
10+
tid=$(basename $t)
11+
cat /proc/$tid/stat | awk '{print $29}' >> $TMP
12+
done
13+
14+
mv $TMP $STACKDUMP_FILE

‎tools/testing/selftests/coredump/stackdump_test.c‎

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,151 @@
1+
// SPDX-License-Identifier: GPL-2.0
2+
3+
#include <fcntl.h>
4+
#include <libgen.h>
5+
#include <linux/limits.h>
6+
#include <pthread.h>
7+
#include <string.h>
8+
#include <sys/resource.h>
9+
#include <unistd.h>
10+
11+
#include "../kselftest_harness.h"
12+
13+
#define STACKDUMP_FILE "stack_values"
14+
#define STACKDUMP_SCRIPT "stackdump"
15+
#define NUM_THREAD_SPAWN 128
16+
17+
static void *do_nothing(void *)
18+
{
19+
while (1)
20+
pause();
21+
}
22+
23+
static void crashing_child(void)
24+
{
25+
pthread_t thread;
26+
int i;
27+
28+
for (i = 0; i < NUM_THREAD_SPAWN; ++i)
29+
pthread_create(&thread, NULL, do_nothing, NULL);
30+
31+
/* crash on purpose */
32+
i = *(int *)NULL;
33+
}
34+
35+
FIXTURE(coredump)
36+
{
37+
char original_core_pattern[256];
38+
};
39+
40+
FIXTURE_SETUP(coredump)
41+
{
42+
char buf[PATH_MAX];
43+
FILE *file;
44+
char *dir;
45+
int ret;
46+
47+
file = fopen("/proc/sys/kernel/core_pattern", "r");
48+
ASSERT_NE(NULL, file);
49+
50+
ret = fread(self->original_core_pattern, 1, sizeof(self->original_core_pattern), file);
51+
ASSERT_TRUE(ret || feof(file));
52+
ASSERT_LT(ret, sizeof(self->original_core_pattern));
53+
54+
self->original_core_pattern[ret] = '\0';
55+
56+
ret = fclose(file);
57+
ASSERT_EQ(0, ret);
58+
}
59+
60+
FIXTURE_TEARDOWN(coredump)
61+
{
62+
const char *reason;
63+
FILE *file;
64+
int ret;
65+
66+
unlink(STACKDUMP_FILE);
67+
68+
file = fopen("/proc/sys/kernel/core_pattern", "w");
69+
if (!file) {
70+
reason = "Unable to open core_pattern";
71+
goto fail;
72+
}
73+
74+
ret = fprintf(file, "%s", self->original_core_pattern);
75+
if (ret < 0) {
76+
reason = "Unable to write to core_pattern";
77+
goto fail;
78+
}
79+
80+
ret = fclose(file);
81+
if (ret) {
82+
reason = "Unable to close core_pattern";
83+
goto fail;
84+
}
85+
86+
return;
87+
fail:
88+
/* This should never happen */
89+
fprintf(stderr, "Failed to cleanup stackdump test: %s\n", reason);
90+
}
91+
92+
TEST_F(coredump, stackdump)
93+
{
94+
struct sigaction action = {};
95+
unsigned long long stack;
96+
char *test_dir, *line;
97+
size_t line_length;
98+
char buf[PATH_MAX];
99+
int ret, i;
100+
FILE *file;
101+
pid_t pid;
102+
103+
/*
104+
* Step 1: Setup core_pattern so that the stackdump script is executed when the child
105+
* process crashes
106+
*/
107+
ret = readlink("/proc/self/exe", buf, sizeof(buf));
108+
ASSERT_NE(-1, ret);
109+
ASSERT_LT(ret, sizeof(buf));
110+
buf[ret] = '\0';
111+
112+
test_dir = dirname(buf);
113+
114+
file = fopen("/proc/sys/kernel/core_pattern", "w");
115+
ASSERT_NE(NULL, file);
116+
117+
ret = fprintf(file, "|%1$s/%2$s %%P %1$s/%3$s", test_dir, STACKDUMP_SCRIPT, STACKDUMP_FILE);
118+
ASSERT_LT(0, ret);
119+
120+
ret = fclose(file);
121+
ASSERT_EQ(0, ret);
122+
123+
/* Step 2: Create a process who spawns some threads then crashes */
124+
pid = fork();
125+
ASSERT_TRUE(pid >= 0);
126+
if (pid == 0)
127+
crashing_child();
128+
129+
/*
130+
* Step 3: Wait for the stackdump script to write the stack pointers to the stackdump file
131+
*/
132+
for (i = 0; i < 10; ++i) {
133+
file = fopen(STACKDUMP_FILE, "r");
134+
if (file)
135+
break;
136+
sleep(1);
137+
}
138+
ASSERT_NE(file, NULL);
139+
140+
/* Step 4: Make sure all stack pointer values are non-zero */
141+
for (i = 0; -1 != getline(&line, &line_length, file); ++i) {
142+
stack = strtoull(line, NULL, 10);
143+
ASSERT_NE(stack, 0);
144+
}
145+
146+
ASSERT_EQ(i, 1 + NUM_THREAD_SPAWN);
147+
148+
fclose(file);
149+
}
150+
151+
TEST_HARNESS_MAIN

0 commit comments

Comments
 (0)