linux-kernel - Re: [PATCH 04/17] signal: In get_signal call do_exit when it is unnecessary to shoot down threads

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]

Date: Tue, 25 Jun 2024 14:35:58 +0200
From: Oleg Nesterov <oleg@...hat.com>
To: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Tejun Heo <tj@...nel.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 04/17] signal: In get_signal call do_exit when it is
 unnecessary to shoot down threads

On 06/18, Eric W. Biederman wrote:
>
> --- a/kernel/exit.c
> +++ b/kernel/exit.c
> @@ -1001,8 +1001,6 @@ do_group_exit(int exit_code)
>
>  	if (sig->flags & SIGNAL_GROUP_EXIT)
>  		exit_code = sig->group_exit_code;
> -	else if (sig->group_exec_task)
> -		exit_code = 0;

OK...

> @@ -1010,8 +1008,6 @@ do_group_exit(int exit_code)
>  		if (sig->flags & SIGNAL_GROUP_EXIT)
>  			/* Another thread got here before we took the lock.  */
>  			exit_code = sig->group_exit_code;
> -		else if (sig->group_exec_task)
> -			exit_code = 0;

Well... So with this change do_group_exit() always "wins" the race if it is
called when another thread has already started de_thread().

But de_thread() won't necessarily notice SIGKILL. Sure, the execing thread
can't return to user-space, but it can do a lot of things after de_thread().

Just for example, can it reach trace_sched_process_exec() ? If yes, then it
will look as if it exits with the exit_code provided by do_group_exit()
_after_ exec.

This differs from case when the execing thread is killed after de_thread(),
in this case exit_code = SIGKILL...

I do not see anything really wrong, just trying to understand the impact of
this change, it looks a bit subtle...

Oleg.