From: ebiederm@xmission.com (Eric W. Biederman)
To: "Jürg Billeter" <j@bitron.ch>
Cc: Andrew Morton <akpm@linux-foundation.org>,
Oleg Nesterov <oleg@redhat.com>,
Linus Torvalds <torvalds@linux-foundation.org>,
Michael Kerrisk <mtk.manpages@gmail.com>,
Filipe Brandenburger <filbranden@google.com>,
David Wilcox <davidvsthegiant@gmail.com>,
hansecke@gmail.com, linux-kernel@vger.kernel.org
Subject: Re: [RESEND PATCH] prctl: add PR_[GS]ET_PDEATHSIG_PROC
Date: Tue, 03 Oct 2017 09:46:18 -0500 [thread overview]
Message-ID: <878tgse1c5.fsf@xmission.com> (raw)
In-Reply-To: <1507013157.2304.48.camel@bitron.ch> ("Jürg Billeter"'s message of "Tue, 03 Oct 2017 08:45:57 +0200")
Jürg Billeter <j@bitron.ch> writes:
> On Mon, 2017-10-02 at 22:25 -0500, Eric W. Biederman wrote:
>> The code where it calls group_send_sig_info is buggy for pdeath_signal.
>> And it no less buggy for this new case. There is no point to check
>> permissions when sending a signal to yourself. Especially this signal
>> gets cleared during exec with a change of permissions.
>>
>>
>> I would recommend using:
>> do_send_sig_info(p->signal->pdeath_signal_proc, SEND_SIG_NOINFO, p, true);
>>
>> Perhaps with a comment saying that no permission check is needed when
>> sending a signal to yourself.
>
> Depending on how you look at it, one could also argue that the dying
> parent sends the signal. However, I'm fine with dropping the permission
> check in v2. I'll also send a patch to change this for the existing
> pdeath_signal.
The process that requests the signal be sent is the process that is
receiving the signal. I can see a theoretical need for a permission
check in there somewhere (especially as this persists over fork).
That is what matters for a permission check.
I tried fixing this once along with a bunch of other changes, and
apparently I did not do it obviously enough. So I think this needs to
happen but let's make it very clear.
>> I don't know what I think about inherit over fork, and the whole tree
>> killing thing. Except when the signal is SIGKILL I don't know if that
>> code does what is intended. So I am a little leary of it.
>
> I agree that inheritance across fork is mainly useful for SIGKILL.
> While non-SIGKILL users could clear the setting after fork(), another
> option would be to allow the caller to specify whether the setting
> should be inherited using prctl arg3.
>
> This would allow both, the exact process-based equivalent to
> pdeath_signal (no inheritance) as well as the interesting SIGKILL case
> for killing a process tree. Does this sound sensible? I'd be happy to
> add this to v2.
Possibly.
There is a general need to find out about the death of other processes,
if you are not the parent of the process. I would be inclined to call
it waitfd. Something that you give a pid. It performs a permission
check and the pid becomes readable when the process dies. With poll
working on the fd, and the fd returning wstatus of the dead child.
Support SIGIO on the fd and you have a signal delivery mechanism,
if you want it.
For the kill all children when the parent dies the mechanism you are
proposing is escapable. We already have an inescapable version of it
with init in a pid namespace. We already have an escapable version of
it with orphaned process groups and SIGHUP.
So I would really appreciate a very clear use case for what we are
building here. As it appears the killing of children can already be
done another way, and that the waiting for the parent can be done better
another way.
At the end of the day I would prefer to support something that has a
good solid definition and a clear use case.
I may be missing something and there is a good reason to extend pdeath
signal, but I don't see it right now.
Eric
next prev parent reply other threads:[~2017-10-03 14:46 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-09-09 9:40 [PATCH] prctl: add PR_[GS]ET_PDEATHSIG_PROC Jürg Billeter
2017-09-12 17:05 ` Oleg Nesterov
2017-09-12 18:54 ` Jürg Billeter
2017-09-13 17:11 ` Oleg Nesterov
2017-09-13 17:26 ` Jürg Billeter
2017-09-13 17:48 ` Oleg Nesterov
2017-09-29 12:30 ` [RESEND PATCH] " Jürg Billeter
2017-10-02 23:20 ` Andrew Morton
2017-10-03 3:25 ` Eric W. Biederman
2017-10-03 6:45 ` Jürg Billeter
2017-10-03 14:46 ` Eric W. Biederman [this message]
2017-10-03 16:10 ` Linus Torvalds
2017-10-03 16:36 ` Eric W. Biederman
2017-10-03 17:02 ` Linus Torvalds
2017-10-03 19:30 ` Eric W. Biederman
2017-10-03 20:02 ` Linus Torvalds
2017-10-03 20:32 ` Eric W. Biederman
2017-10-03 17:00 ` Jürg Billeter
2017-10-03 17:40 ` Eric W. Biederman
2017-10-03 17:47 ` Jürg Billeter
2017-10-03 19:05 ` Eric W. Biederman
2017-10-05 16:27 ` Oleg Nesterov
2017-10-08 17:47 ` Jürg Billeter
2017-10-09 16:32 ` Eric W. Biederman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=878tgse1c5.fsf@xmission.com \
--to=ebiederm@xmission.com \
--cc=akpm@linux-foundation.org \
--cc=davidvsthegiant@gmail.com \
--cc=filbranden@google.com \
--cc=hansecke@gmail.com \
--cc=j@bitron.ch \
--cc=linux-kernel@vger.kernel.org \
--cc=mtk.manpages@gmail.com \
--cc=oleg@redhat.com \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox