Re: [PATCH v2] kernel/signal: Signal-based pre-coredump notification

From: ebiederm@xmission.com (Eric W. Biederman)
To: Jann Horn <jannh@google.com>
Cc: enkechen@cisco.com, Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	"H . Peter Anvin" <hpa@zytor.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Arnd Bergmann <arnd@arndb.de>,
	Khalid Aziz <khalid.aziz@oracle.com>,
	Kate Stewart <kstewart@linuxfoundation.org>,
	deller@gmx.de, Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	christian@brauner.io, Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Dave.Martin@arm.com, mchehab+samsung@kernel.org,
	Michal Hocko <mhocko@kernel.org>, Rik van Riel <riel@surriel.com>,
	"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
	guro@fb.com, Marcos Souza <marcos.souza.org@gmail.com>,
	Oleg Nesterov <oleg@redhat.com>,
	linux@dominikbrodowski.net, Cyrill Gorcunov <gorcunov@openvz.org>,
	yang.shi@linux.alibaba.com, Kees Cook <keescook@chromium.org>,
	"the arch\/x86 maintainers" <x86@kernel.org>,
	kernel list <linux-kernel@vger.kernel.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	Victor Kamensky <kamensky@cisco.com>,
	xe-linux-external@cisco.com, sstrogin@cisco.com
Subject: Re: [PATCH v2] kernel/signal: Signal-based pre-coredump notification
Date: Thu, 25 Oct 2018 15:21:23 -0500	[thread overview]
Message-ID: <87va5plqv0.fsf@xmission.com> (raw)
In-Reply-To: <CAG48ez2aJD1wcz3UjhaQvgqhg5UMeQ_-iMZesA=SfhYwUdGGyA@mail.gmail.com> (Jann Horn's message of "Thu, 25 Oct 2018 15:45:29 +0200")

Jann Horn <jannh@google.com> writes:

> On Wed, Oct 24, 2018 at 3:30 PM Eric W. Biederman <ebiederm@xmission.com> wrote:
>> Enke Chen <enkechen@cisco.com> writes:
>> > For simplicity and consistency, this patch provides an implementation
>> > for signal-based fault notification prior to the coredump of a child
>> > process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can
>> > be used by an application to express its interest and to specify the
>> > signal (SIGCHLD or SIGUSR1 or SIGUSR2) for such a notification. A new
>> > signal code (si_code), CLD_PREDUMP, is also defined for SIGCHLD.
>> >
>> > Changes to prctl(2):
>> >
>> >    PR_SET_PREDUMP_SIG (since Linux 4.20.x)
>> >           Set the child pre-coredump signal of the calling process to
>> >           arg2 (either SIGUSR1, or SIUSR2, or SIGCHLD, or 0 to clear).
>> >           This is the signal that the calling process will get prior to
>> >           the coredump of a child process. This value is cleared across
>> >           execve(2), or for the child of a fork(2).
>> >
>> >           When SIGCHLD is specified, the signal code will be set to
>> >           CLD_PREDUMP in such an SIGCHLD signal.
> [...]
>> Ugh.  Your test case is even using signalfd.  So you don't even want
>> this signal to be delivered as a signal.
>
> Just to make sure everyone's on the same page: You're suggesting that
> it might make sense to deliver the pre-dump notification via a new
> type of file instead (along the lines of signalfd, timerfd, eventfd
> and so on)?

My real complaint was that the API was not being tested in the way it
is expected to be used.  Which makes a test pretty much useless as some
aspect userspace could regress and the test would not notice because it
is testing something different.

I do think that a file descriptor based API might be a good alternative
to a signal based API.  The proc connector and signals are not the only
API solution.

The common solution to this problem is that distributions defailt the
rlimit core file size to 0.

Eric