linux-kernel - Re: [RESEND PATCH] prctl: add PR_[GS]ET_PDEATHSIG

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <1507013157.2304.48.camel@bitron.ch>
Date:   Tue, 03 Oct 2017 08:45:57 +0200
From:   Jürg Billeter <j@...ron.ch>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>,
        Andrew Morton <akpm@...ux-foundation.org>
Cc:     Oleg Nesterov <oleg@...hat.com>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Michael Kerrisk <mtk.manpages@...il.com>,
        Filipe Brandenburger <filbranden@...gle.com>,
        David Wilcox <davidvsthegiant@...il.com>, hansecke@...il.com,
        linux-kernel@...r.kernel.org
Subject: Re: [RESEND PATCH] prctl: add PR_[GS]ET_PDEATHSIG_PROC

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.

> 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.

Jürg