From mboxrd@z Thu Jan 1 00:00:00 1970 From: ebiederm@xmission.com (Eric W. Biederman) Subject: Re: [PATCH v2] kernel/signal: Signal-based pre-coredump notification Date: Thu, 25 Oct 2018 15:21:23 -0500 Message-ID: <87va5plqv0.fsf@xmission.com> References: <458c04d8-d189-4a26-729a-bb1d1d751534@cisco.com> <87sh0vpj5q.fsf@xmission.com> Mime-Version: 1.0 Content-Type: text/plain Return-path: In-Reply-To: (Jann Horn's message of "Thu, 25 Oct 2018 15:45:29 +0200") Sender: linux-kernel-owner@vger.kernel.org To: Jann Horn Cc: enkechen@cisco.com, Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H . Peter Anvin" , Peter Zijlstra , Arnd Bergmann , Khalid Aziz , Kate Stewart , deller@gmx.de, Greg Kroah-Hartman , Al Viro , Andrew Morton , christian@brauner.io, Catalin Marinas , Will Deacon , Dave.Martin@arm.com, mchehab+samsung@kernel.org, Michal Hocko , Rik van Riel , "Kirill A . Shutemov" , guro@fb.com, Marcos Souza List-Id: linux-arch.vger.kernel.org Jann Horn writes: > On Wed, Oct 24, 2018 at 3:30 PM Eric W. Biederman wrote: >> Enke Chen 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 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out01.mta.xmission.com ([166.70.13.231]:53261 "EHLO out01.mta.xmission.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725817AbeJZE4Z (ORCPT ); Fri, 26 Oct 2018 00:56:25 -0400 From: ebiederm@xmission.com (Eric W. Biederman) References: <458c04d8-d189-4a26-729a-bb1d1d751534@cisco.com> <87sh0vpj5q.fsf@xmission.com> Date: Thu, 25 Oct 2018 15:21:23 -0500 In-Reply-To: (Jann Horn's message of "Thu, 25 Oct 2018 15:45:29 +0200") Message-ID: <87va5plqv0.fsf@xmission.com> MIME-Version: 1.0 Content-Type: text/plain Subject: Re: [PATCH v2] kernel/signal: Signal-based pre-coredump notification Sender: linux-arch-owner@vger.kernel.org List-ID: To: Jann Horn Cc: enkechen@cisco.com, Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H . Peter Anvin" , Peter Zijlstra , Arnd Bergmann , Khalid Aziz , Kate Stewart , deller@gmx.de, Greg Kroah-Hartman , Al Viro , Andrew Morton , christian@brauner.io, Catalin Marinas , Will Deacon , Dave.Martin@arm.com, mchehab+samsung@kernel.org, Michal Hocko , Rik van Riel , "Kirill A . Shutemov" , guro@fb.com, Marcos Souza , Oleg Nesterov , linux@dominikbrodowski.net, Cyrill Gorcunov , yang.shi@linux.alibaba.com, Kees Cook , the arch/x86 maintainers , kernel list , linux-arch , Victor Kamensky , xe-linux-external@cisco.com, sstrogin@cisco.com Message-ID: <20181025202123.KV5YsnJL_laP15nsRyxGIMGm_WiJ3lOjDhW4d4tRnLo@z> Jann Horn writes: > On Wed, Oct 24, 2018 at 3:30 PM Eric W. Biederman wrote: >> Enke Chen 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