linux-hardening.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] proc: Disable /proc/$pid/wchan
@ 2021-09-23 23:31 Kees Cook
  2021-09-23 23:38 ` Kees Cook
                   ` (4 more replies)
  0 siblings, 5 replies; 26+ messages in thread
From: Kees Cook @ 2021-09-23 23:31 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: Kees Cook, Josh Poimboeuf, Vito Caputo, Ingo Molnar,
	Borislav Petkov, H. Peter Anvin, Jens Axboe, Mark Rutland,
	Peter Zijlstra, Stefan Metzmacher, Andy Lutomirski,
	Lai Jiangshan, Christian Brauner, Andrew Morton, Kenta.Tada,
	Daniel Bristot de Oliveira, Michael Weiß,
	Anand K Mistry, Alexey Gladkov, Michal Hocko, Helge Deller,
	Dave Hansen, Andrea Righi, Ohhoon Kwon, Kalesh Singh, YiFei Zhu,
	Eric W. Biederman, linux-kernel, x86, linux-fsdevel,
	linux-hardening

The /proc/$pid/wchan file has been broken by default on x86_64 for 4
years now[1]. As this remains a potential leak of either kernel
addresses (when symbolization fails) or limited observation of kernel
function progress, just remove the contents for good.

Unconditionally set the contents to "0" and also mark the wchan
field in /proc/$pid/stat with 0.

This leaves kernel/sched/fair.c as the only user of get_wchan(). But
again, since this was broken for 4 years, was this profiling logic
actually doing anything useful?

[1] https://lore.kernel.org/lkml/20210922001537.4ktg3r2ky3b3r6yp@treble/

Cc: Josh Poimboeuf <jpoimboe@redhat.com>
Cc: Vito Caputo <vcaputo@pengaru.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
 arch/x86/kernel/process.c |  2 +-
 fs/proc/array.c           | 16 +++++-----------
 fs/proc/base.c            | 16 +---------------
 3 files changed, 7 insertions(+), 27 deletions(-)

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 1d9463e3096b..84a4f9f3f0c2 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -937,7 +937,7 @@ unsigned long arch_randomize_brk(struct mm_struct *mm)
 }
 
 /*
- * Called from fs/proc with a reference on @p to find the function
+ * Called from scheduler with a reference on @p to find the function
  * which called into schedule(). This needs to be done carefully
  * because the task might wake up and we might look at a stack
  * changing under us.
diff --git a/fs/proc/array.c b/fs/proc/array.c
index 49be8c8ef555..8a4ecfd901b8 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -452,7 +452,7 @@ int proc_pid_status(struct seq_file *m, struct pid_namespace *ns,
 static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 			struct pid *pid, struct task_struct *task, int whole)
 {
-	unsigned long vsize, eip, esp, wchan = 0;
+	unsigned long vsize, eip, esp;
 	int priority, nice;
 	int tty_pgrp = -1, tty_nr = 0;
 	sigset_t sigign, sigcatch;
@@ -540,8 +540,6 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 		unlock_task_sighand(task, &flags);
 	}
 
-	if (permitted && (!whole || num_threads < 2))
-		wchan = get_wchan(task);
 	if (!whole) {
 		min_flt = task->min_flt;
 		maj_flt = task->maj_flt;
@@ -600,16 +598,12 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 	seq_put_decimal_ull(m, " ", sigcatch.sig[0] & 0x7fffffffUL);
 
 	/*
-	 * We used to output the absolute kernel address, but that's an
-	 * information leak - so instead we show a 0/1 flag here, to signal
-	 * to user-space whether there's a wchan field in /proc/PID/wchan.
-	 *
+	 * We used to output the absolute kernel address, and then just
+	 * a symbol. But both are information leaks, so just report 0
+	 * to indicate there is no wchan field in /proc/$PID/wchan.
 	 * This works with older implementations of procps as well.
 	 */
-	if (wchan)
-		seq_puts(m, " 1");
-	else
-		seq_puts(m, " 0");
+	seq_puts(m, " 0");
 
 	seq_put_decimal_ull(m, " ", 0);
 	seq_put_decimal_ull(m, " ", 0);
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 533d5836eb9a..52484cd77f99 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -378,24 +378,10 @@ static const struct file_operations proc_pid_cmdline_ops = {
 };
 
 #ifdef CONFIG_KALLSYMS
-/*
- * Provides a wchan file via kallsyms in a proper one-value-per-file format.
- * Returns the resolved symbol.  If that fails, simply return the address.
- */
 static int proc_pid_wchan(struct seq_file *m, struct pid_namespace *ns,
 			  struct pid *pid, struct task_struct *task)
 {
-	unsigned long wchan;
-
-	if (ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS))
-		wchan = get_wchan(task);
-	else
-		wchan = 0;
-
-	if (wchan)
-		seq_printf(m, "%ps", (void *) wchan);
-	else
-		seq_putc(m, '0');
+	seq_putc(m, '0');
 
 	return 0;
 }
-- 
2.30.2


^ permalink raw reply related	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2021-09-30 18:12 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-23 23:31 [PATCH] proc: Disable /proc/$pid/wchan Kees Cook
2021-09-23 23:38 ` Kees Cook
2021-09-24 13:31   ` Peter Zijlstra
2021-09-23 23:49 ` Vito Caputo
2021-09-24  0:08   ` Jann Horn
2021-09-24  0:22     ` Vito Caputo
2021-09-24  1:16       ` Kees Cook
2021-09-24  1:34         ` Vito Caputo
2021-09-24  1:42           ` Kees Cook
2021-09-24 13:54         ` Mark Rutland
2021-09-24 14:26           ` Kees Cook
2021-09-27  9:03             ` Mark Rutland
2021-09-27 18:07               ` Kees Cook
2021-09-27 20:50                 ` Josh Poimboeuf
2021-09-29 18:54                   ` Kees Cook
2021-09-29 19:00                     ` Mark Brown
2021-09-29 19:26                       ` Kees Cook
2021-09-29 19:40                     ` Peter Zijlstra
2021-09-29 21:10                       ` Josh Poimboeuf
2021-09-27  9:16           ` David Laight
2021-09-29  8:48           ` Peter Zijlstra
2021-09-24  2:13 ` Andrew Morton
2021-09-24  6:04   ` Kees Cook
2021-09-24 13:29 ` Peter Zijlstra
2021-09-30 18:05 ` Stephen Brennan
2021-09-30 18:12   ` Kees Cook

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).