linux-kernel - [PATCH 1/2] fs/proc: do_task_stat: Fix ESP not readable during coredump

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <11e1777296b7d06085c9fd341bafc4b9d82e6e4e.1730883229.git.namcao@linutronix.de>
Date: Wed,  6 Nov 2024 10:22:15 +0100
From: Nam Cao <namcao@...utronix.de>
To: Shuah Khan <shuah@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Oleg Nesterov <oleg@...hat.com>,
	Dylan Hatch <dylanbhatch@...gle.com>,
	"Eric W . Biederman" <ebiederm@...ssion.com>,
	John Ogness <john.ogness@...utronix.de>,
	Kees Cook <kees@...nel.org>,
	linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org,
	linux-kselftest@...r.kernel.org
Cc: Nam Cao <namcao@...utronix.de>,
	stable@...r.kernel.org
Subject: [PATCH 1/2] fs/proc: do_task_stat: Fix ESP not readable during coredump

Commit 0a1eb2d474ed ("fs/proc: Stop reporting eip and esp in
/proc/PID/stat") disabled stack pointer reading, because it is generally
dangerous to do so.

Commit fd7d56270b52 ("fs/proc: Report eip/esp in /prod/PID/stat for
coredumping") made an exception for coredumping thread, because for this
case it is safe.

The exception was later extended to all threads in a coredumping process by
commit cb8f381f1613 ("fs/proc/array.c: allow reporting eip/esp for all
coredumping threads").

The above two commits determine if a task is core dumping by checking the
PF_EXITING and PF_DUMPCORE flags.

However, commit 92307383082d ("coredump:  Don't perform any cleanups before
dumping core") moved coredump to happen earlier and before PF_EXITING is
set. Thus, the check of the PF_EXITING flag no longer works.

Instead, use task->signal->core_state to determine if coredump is
happening. This pointer is set at the beginning of coredump and is cleared
once coredump is done. Thus, while this pointer is not NULL, it is safe to
read ESP.

Fixes: 92307383082d ("coredump:  Don't perform any cleanups before dumping core")
Signed-off-by: Nam Cao <namcao@...utronix.de>
Cc: <stable@...r.kernel.org>
Cc: Eric W. Biederman <ebiederm@...ssion.com>
---
 fs/proc/array.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 34a47fb0c57f..2f1dbfcf143d 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -489,25 +489,8 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 	vsize = eip = esp = 0;
 	permitted = ptrace_may_access(task, PTRACE_MODE_READ_FSCREDS | PTRACE_MODE_NOAUDIT);
 	mm = get_task_mm(task);
-	if (mm) {
+	if (mm)
 		vsize = task_vsize(mm);
-		/*
-		 * esp and eip are intentionally zeroed out.  There is no
-		 * non-racy way to read them without freezing the task.
-		 * Programs that need reliable values can use ptrace(2).
-		 *
-		 * The only exception is if the task is core dumping because
-		 * a program is not able to use ptrace(2) in that case. It is
-		 * safe because the task has stopped executing permanently.
-		 */
-		if (permitted && (task->flags & (PF_EXITING|PF_DUMPCORE))) {
-			if (try_get_task_stack(task)) {
-				eip = KSTK_EIP(task);
-				esp = KSTK_ESP(task);
-				put_task_stack(task);
-			}
-		}
-	}
 
 	sigemptyset(&sigign);
 	sigemptyset(&sigcatch);
@@ -534,6 +517,23 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 		ppid = task_tgid_nr_ns(task->real_parent, ns);
 		pgid = task_pgrp_nr_ns(task, ns);
 
+		/*
+		 * esp and eip are intentionally zeroed out.  There is no
+		 * non-racy way to read them without freezing the task.
+		 * Programs that need reliable values can use ptrace(2).
+		 *
+		 * The only exception is if the task is core dumping because
+		 * a program is not able to use ptrace(2) in that case. It is
+		 * safe because the task has stopped executing permanently.
+		 */
+		if (permitted && task->signal->core_state) {
+			if (try_get_task_stack(task)) {
+				eip = KSTK_EIP(task);
+				esp = KSTK_ESP(task);
+				put_task_stack(task);
+			}
+		}
+
 		unlock_task_sighand(task, &flags);
 	}
 
-- 
2.39.5