linux-kernel - Re: [PATCH v2 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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <202412301043.78340B2FD@keescook>
Date: Mon, 30 Dec 2024 10:43:58 -0800
From: Kees Cook <kees@...nel.org>
To: Nam Cao <namcao@...utronix.de>
Cc: 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>,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	linux-kselftest@...r.kernel.org, stable@...r.kernel.org
Subject: Re: [PATCH v2 1/2] fs/proc: do_task_stat: Fix ESP not readable
 during coredump

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@...r.kernel.org
> Cc: Eric W. Biederman <ebiederm@...ssion.com>
> Acked-by: Oleg Nesterov <oleg@...hat.com>
> Signed-off-by: Nam Cao <namcao@...utronix.de>

Thanks for fixing this!

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

-- 
Kees Cook