linux-kernel - Re: [REVIEW][PATCH] pid_ns: Sleep in TASK_INTERRUPTIBLE in zap_pid_ns

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: <20170512193351.GB14843@roeck-us.net>
Date:   Fri, 12 May 2017 12:33:51 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     "Eric W. Biederman" <ebiederm@...ssion.com>
Cc:     Vovo Yang <vovoy@...gle.com>, Ingo Molnar <mingo@...nel.org>,
        linux-kernel@...r.kernel.org,
        Linux Containers <containers@...ts.linux-foundation.org>
Subject: Re: [REVIEW][PATCH] pid_ns: Sleep in TASK_INTERRUPTIBLE in
 zap_pid_ns_processes

On Fri, May 12, 2017 at 12:55:22PM -0500, Eric W. Biederman wrote:
> Date: Thu, 11 May 2017 18:21:01 -0500
> 
> The code can potentially sleep for an indefinite amount of time in
> zap_pid_ns_processes triggering the hung task timeout, and increasing
> the system average.  This is undesirable.  Sleep with a task state of
> TASK_INTERRUPTIBLE instead of TASK_UNINTERRUPTIBLE to remove these
> undesirable side effects.
> 
> Apparently under heavy load this has been allowing Chrome to trigger
> the hung time task timeout error and cause ChromeOS to reboot.
> 
> Reported-by: Vovo Yang <vovoy@...gle.com>
> Reported-by: Guenter Roeck <linux@...ck-us.net>

Tested-by: Guenter Roeck <linux@...ck-us.net>

> Fixes: 6347e9009104 ("pidns: guarantee that the pidns init will be the last pidns process reaped")
> Cc: stable@...r.kernel.org
> Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
> ---
> 
> This is what I have queued up posting so it is public knowledge and so
> that if anyone can see a flaw it can get fixed.
> 
>  kernel/pid_namespace.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c
> index de461aa0bf9a..6e51b8820495 100644
> --- a/kernel/pid_namespace.c
> +++ b/kernel/pid_namespace.c
> @@ -277,7 +277,7 @@ void zap_pid_ns_processes(struct pid_namespace *pid_ns)
>  	 * if reparented.
>  	 */
>  	for (;;) {
> -		set_current_state(TASK_UNINTERRUPTIBLE);
> +		set_current_state(TASK_INTERRUPTIBLE);
>  		if (pid_ns->nr_hashed == init_pids)
>  			break;
>  		schedule();
> -- 
> 2.10.1
>