]>
Commit | Line | Data |
---|---|---|
5b0b9568 GKH |
1 | From 3e536e222f2930534c252c1cc7ae799c725c5ff9 Mon Sep 17 00:00:00 2001 |
2 | From: Snild Dolkow <snild@sony.com> | |
3 | Date: Thu, 26 Jul 2018 09:15:39 +0200 | |
4 | Subject: kthread, tracing: Don't expose half-written comm when creating kthreads | |
5 | ||
6 | From: Snild Dolkow <snild@sony.com> | |
7 | ||
8 | commit 3e536e222f2930534c252c1cc7ae799c725c5ff9 upstream. | |
9 | ||
10 | There is a window for racing when printing directly to task->comm, | |
11 | allowing other threads to see a non-terminated string. The vsnprintf | |
12 | function fills the buffer, counts the truncated chars, then finally | |
13 | writes the \0 at the end. | |
14 | ||
15 | creator other | |
16 | vsnprintf: | |
17 | fill (not terminated) | |
18 | count the rest trace_sched_waking(p): | |
19 | ... memcpy(comm, p->comm, TASK_COMM_LEN) | |
20 | write \0 | |
21 | ||
22 | The consequences depend on how 'other' uses the string. In our case, | |
23 | it was copied into the tracing system's saved cmdlines, a buffer of | |
24 | adjacent TASK_COMM_LEN-byte buffers (note the 'n' where 0 should be): | |
25 | ||
26 | crash-arm64> x/1024s savedcmd->saved_cmdlines | grep 'evenk' | |
27 | 0xffffffd5b3818640: "irq/497-pwr_evenkworker/u16:12" | |
28 | ||
29 | ...and a strcpy out of there would cause stack corruption: | |
30 | ||
31 | [224761.522292] Kernel panic - not syncing: stack-protector: | |
32 | Kernel stack is corrupted in: ffffff9bf9783c78 | |
33 | ||
34 | crash-arm64> kbt | grep 'comm\|trace_print_context' | |
35 | #6 0xffffff9bf9783c78 in trace_print_context+0x18c(+396) | |
36 | comm (char [16]) = "irq/497-pwr_even" | |
37 | ||
38 | crash-arm64> rd 0xffffffd4d0e17d14 8 | |
39 | ffffffd4d0e17d14: 2f71726900000000 5f7277702d373934 ....irq/497-pwr_ | |
40 | ffffffd4d0e17d24: 726f776b6e657665 3a3631752f72656b evenkworker/u16: | |
41 | ffffffd4d0e17d34: f9780248ff003231 cede60e0ffffff9b 12..H.x......`.. | |
42 | ffffffd4d0e17d44: cede60c8ffffffd4 00000fffffffffd4 .....`.......... | |
43 | ||
44 | The workaround in e09e28671 (use strlcpy in __trace_find_cmdline) was | |
45 | likely needed because of this same bug. | |
46 | ||
47 | Solved by vsnprintf:ing to a local buffer, then using set_task_comm(). | |
48 | This way, there won't be a window where comm is not terminated. | |
49 | ||
50 | Link: http://lkml.kernel.org/r/20180726071539.188015-1-snild@sony.com | |
51 | ||
52 | Cc: stable@vger.kernel.org | |
53 | Fixes: bc0c38d139ec7 ("ftrace: latency tracer infrastructure") | |
54 | Reviewed-by: Steven Rostedt (VMware) <rostedt@goodmis.org> | |
55 | Signed-off-by: Snild Dolkow <snild@sony.com> | |
56 | Signed-off-by: Steven Rostedt (VMware) <rostedt@goodmis.org> | |
57 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
58 | ||
59 | --- | |
60 | kernel/kthread.c | 8 +++++++- | |
61 | 1 file changed, 7 insertions(+), 1 deletion(-) | |
62 | ||
63 | --- a/kernel/kthread.c | |
64 | +++ b/kernel/kthread.c | |
65 | @@ -311,8 +311,14 @@ struct task_struct *__kthread_create_on_ | |
66 | task = create->result; | |
67 | if (!IS_ERR(task)) { | |
68 | static const struct sched_param param = { .sched_priority = 0 }; | |
69 | + char name[TASK_COMM_LEN]; | |
70 | ||
71 | - vsnprintf(task->comm, sizeof(task->comm), namefmt, args); | |
72 | + /* | |
73 | + * task is already visible to other tasks, so updating | |
74 | + * COMM must be protected. | |
75 | + */ | |
76 | + vsnprintf(name, sizeof(name), namefmt, args); | |
77 | + set_task_comm(task, name); | |
78 | /* | |
79 | * root may have changed our (kthreadd's) priority or CPU mask. | |
80 | * The kernel thread should not inherit these properties. |