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

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

[Nam Cao]

The field "eip" (instruction pointer) and "esp" (stack pointer) of a task
can be read from /proc/PID/stat. These fields can be interesting for
coredump.

However, these fields were disabled by commit 0a1eb2d474ed ("fs/proc: Stop
reporting eip and esp in /proc/PID/stat"), because it is generally unsafe
to do so. But it is safe for a coredumping process, and therefore
exceptions were made:

  - for a coredumping thread by commit fd7d56270b52 ("fs/proc: Report
    eip/esp in /prod/PID/stat for coredumping").

  - for all other threads in a coredumping process by commit cb8f381f1613
    ("fs/proc/array.c: allow reporting eip/esp for all coredumping
    threads").

The above two commits check the PF_DUMPCORE flag to determine a coredump thread
and the PF_EXITING flag for the other threads.

Unfortunately, commit 92307383082d ("coredump:  Don't perform any cleanups
before dumping core") moved coredump to happen earlier and before PF_EXITING is
set. Thus, checking PF_EXITING is no longer the correct way to determine
threads in a coredumping process.

Instead of PF_EXITING, use PF_POSTCOREDUMP to determine the other threads.

Checking of PF_EXITING was added for coredumping, so it probably can now be
removed. But it doesn't hurt to keep.

Fixes: 92307383082d ("coredump:  Don't perform any cleanups before dumping core")
Cc: stable@vger.kernel.org
Cc: Eric W. Biederman <ebiederm@xmission.com>
Acked-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Nam Cao <namcao@linutronix.de>
---
 fs/proc/array.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 55ed3510d2bb..d6a0369caa93 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -500,7 +500,7 @@ static int do_task_stat(struct seq_file *m, struct pid_namespace *ns,
 		 * 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 (permitted && (task->flags & (PF_EXITING|PF_DUMPCORE|PF_POSTCOREDUMP))) {
 			if (try_get_task_stack(task)) {
 				eip = KSTK_EIP(task);
 				esp = KSTK_ESP(task);
-- 
2.39.5

Re: [PATCH v2 1/2] fs/proc: do_task_stat: Fix ESP not readable during coredump

[Kees Cook]

On Mon, Dec 30, 2024 at 11:17:30AM +0100, Nam Cao wrote:
> The field "eip" (instruction pointer) and "esp" (stack pointer) of a task
> can be read from /proc/PID/stat. These fields can be interesting for
> coredump.
> 
> However, these fields were disabled by commit 0a1eb2d474ed ("fs/proc: Stop
> reporting eip and esp in /proc/PID/stat"), because it is generally unsafe
> to do so. But it is safe for a coredumping process, and therefore
> exceptions were made:
> 
>   - for a coredumping thread by commit fd7d56270b52 ("fs/proc: Report
>     eip/esp in /prod/PID/stat for coredumping").
> 
>   - for all other threads in a coredumping process by commit cb8f381f1613
>     ("fs/proc/array.c: allow reporting eip/esp for all coredumping
>     threads").
> 
> The above two commits check the PF_DUMPCORE flag to determine a coredump thread
> and the PF_EXITING flag for the other threads.
> 
> Unfortunately, commit 92307383082d ("coredump:  Don't perform any cleanups
> before dumping core") moved coredump to happen earlier and before PF_EXITING is
> set. Thus, checking PF_EXITING is no longer the correct way to determine
> threads in a coredumping process.
> 
> Instead of PF_EXITING, use PF_POSTCOREDUMP to determine the other threads.
> 
> Checking of PF_EXITING was added for coredumping, so it probably can now be
> removed. But it doesn't hurt to keep.
> 
> Fixes: 92307383082d ("coredump:  Don't perform any cleanups before dumping core")
> Cc: stable@vger.kernel.org
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Acked-by: Oleg Nesterov <oleg@redhat.com>
> Signed-off-by: Nam Cao <namcao@linutronix.de>

Thanks for fixing this!

Acked-by: Kees Cook <kees@kernel.org>

-- 
Kees Cook