linux-kernel - Re: + mm-oom-killer-kills-more-than-needed.patch added to -mm tree

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

Message-ID: <20080923124035.GA253@tv-sign.ru>
Date:	Tue, 23 Sep 2008 16:40:35 +0400
From:	Oleg Nesterov <oleg@...sign.ru>
To:	akpm@...ux-foundation.org
Cc:	linux-kernel@...r.kernel.org, chad.zanonie@...il.com,
	npiggin@...e.de, rientjes@...gle.com
Subject: Re: + mm-oom-killer-kills-more-than-needed.patch added to -mm tree

On 09/22, Andrew Morton wrote:
>
> ------------------------------------------------------
> Subject: mm: oom-killer kills more than needed
> From: "Chad Zanonie" <chad.zanonie@...il.com>
>
> Possibility exists for an exiting application to be in between marking its
> mm NULL and calling mmput when out_of_memory is invoked. 
> select_bad_process() will continue past this process as opposed to
> returning -1UL due to its mm being NULL.  This causes the oom killer in
> certain scenarios to not only kill the memory culprit, but also kill the
> runner up.
>
> EXIT_DEAD seems to be the only flag that guarantees that mmput() has
> finished.

I don't think this is right.

Let's suppose we have a single zombie. Now select_bad_process() always
returns -1 ? IOW, doesn't this means that, say,

	$ perl -e 'fork && sleep'

disables oom-kill completely and forever?

Hmm. But please see below. This doesn't happen because the usage
of EXIT_DEAD is not right.

> Checking for PF_KTHREAD should replace p->mm regardless.

Yes, almost every check for ->mm in oom_kill.c is not right.

> Adding EXIT_DEAD to the check seems to prevent unnecessary kills in local
> testing.

This is strange, could you re-test? Because

> @@ -216,7 +216,7 @@ static struct task_struct *select_bad_pr
>  		 * skip kernel threads and tasks which have already released
>  		 * their mm.
>  		 */
> -		if (!p->mm)
> +		if (p->flags & PF_KTHREAD || p->flags & EXIT_DEAD)
                                                ^^^^^^^^^^^^^^^^^

this is not possible. EXIT_DEAD lives in ->exit_state, not in ->flags.

Oleg.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/