Re: [PATCH] proc: report eip and esp for all threads when coredumping

From: "Jan Lübbe" <jlu@pengutronix.de>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>,
	John Ogness <john.ogness@linutronix.de>,
	Andy Lutomirski <luto@kernel.org>,
	kernel@pengutronix.de, linux-kernel@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
Subject: Re: [PATCH] proc: report eip and esp for all threads when coredumping
Date: Thu, 23 May 2019 10:15:16 +0200	[thread overview]
Message-ID: <1558599316.4138.86.camel@pengutronix.de> (raw)
In-Reply-To: <20190522110047.6bc80ca511a1425d8a069110@linux-foundation.org>

On Wed, 2019-05-22 at 11:00 -0700, Andrew Morton wrote:
> On Wed, 22 May 2019 18:16:14 +0200 Jan Luebbe <jlu@pengutronix.de> wrote:
> 
> > Commit 0a1eb2d474ed ("fs/proc: Stop reporting eip and esp in
> > /proc/PID/stat") stopped reporting eip/esp and commit fd7d56270b52
> > ("fs/proc: Report eip/esp in /prod/PID/stat for coredumping")
> > reintroduced the feature to fix a regression with userspace core dump
> > handlers (such as minicoredumper).
> > 
> > Because PF_DUMPCORE is only set for the primary thread, this didn't fix
> > the original problem for secondary threads. This commit checks
> > mm->core_state instead, as already done for /proc/<pid>/status in
> > task_core_dumping(). As we have a mm_struct available here anyway, this
> > seems to be a clean solution.
> 
> Could we please have an explicit and complete description of the
> end-user visible effect of this change?

In current mainline, all threads except the main have the
/proc/[pid]/stat fields 'kstkesp' (29, current stack pointer) and
'kstkeip' (30, current instruction pointer) show as 0 even during
coredumping when read by the core dump handler.

minicoredumper for example tries to use this value to find each
thread's stack and tries to dump it, which fails as there is nothing
mapped at 0. The result is that the thread's stack data is missing from
the generated core dump.

With this patch, kstkesp and kstkeip are visible again to the core dump
handler, so the minified core dump contains all stacks again. For a
process running normally, the values are still reported as 0 (as
intended).

> It sounds like we should be backporting this into -stable but without
> the above info it's hard to determine this.

We've been using this patch on 4.19.x for some time, so I agree that
this should be back-ported (fd7d56270b52 is in 4.14).

Andrew, should I send a v2 with Alexey's fix squashed and an updated
commit message?

Regards,
Jan
-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |