linux-kernel - Re: [PATCH] signal: Restore the stop PTRACE_EVENT

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: <20190213135557.GB9356@redhat.com>
Date:   Wed, 13 Feb 2019 14:55:57 +0100
From:   Oleg Nesterov <oleg@...hat.com>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     Dmitry Vyukov <dvyukov@...gle.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Arnaldo Carvalho de Melo <acme@...nel.org>,
        Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
        jolsa@...hat.com, Namhyung Kim <namhyung@...nel.org>,
        luca abeni <luca.abeni@...tannapisa.it>,
        syzkaller <syzkaller@...glegroups.com>,
        Ivan Delalande <colona@...sta.com>
Subject: Re: [PATCH] signal: Restore the stop PTRACE_EVENT_EXIT

On 02/12, Eric W. Biederman wrote:
>
> Oleg this looks like the most conservative regression fix I can manage.

This is what I tried to suggest.

Except I still think that __fatal_signal_pending() would look better.

Yes, yes, in the long term we can possibly even kill fatal_signal_pending()
by various reasons I am not going to discuss right now (although I'd prefer
to kill signal_group_exit() instead).

But then we will probably need to reconsider sigdelset(SIGKILL) added by
this patch too. And until then

	if (__fatal_signal_pending())
		sigdelset(SIGKILL);

looks much more clear and understandable to me.

But I am not going to argue, feel free to add my ack.

> 
>  kernel/signal.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 99fa8ff06fd9..57b7771e20d7 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -2436,9 +2436,12 @@ bool get_signal(struct ksignal *ksig)
>  	}
>  
>  	/* Has this task already been marked for death? */
> -	ksig->info.si_signo = signr = SIGKILL;
> -	if (signal_group_exit(signal))
> +	if (signal_group_exit(signal)) {
> +		ksig->info.si_signo = signr = SIGKILL;
> +		sigdelset(&current->pending.signal, SIGKILL);
> +		recalc_sigpending();
>  		goto fatal;
> +	}
>  
>  	for (;;) {
>  		struct k_sigaction *ka;
> -- 
> 2.17.1
>