linux-kernel - Re: [PATCH 07/10] mm, oom: fortify task_will_free

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:	Thu, 9 Jun 2016 22:18:28 +0900
From:	Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:	mhocko@...nel.org, linux-mm@...ck.org
Cc:	rientjes@...gle.com, oleg@...hat.com, vdavydov@...allels.com,
	akpm@...ux-foundation.org, linux-kernel@...r.kernel.org,
	mhocko@...e.com
Subject: Re: [PATCH 07/10] mm, oom: fortify task_will_free_mem

Michal Hocko wrote:
> @@ -766,15 +797,12 @@ void oom_kill_process(struct oom_control *oc, struct task_struct *p,
>  	 * If the task is already exiting, don't alarm the sysadmin or kill
>  	 * its children or threads, just set TIF_MEMDIE so it can die quickly
>  	 */
> -	task_lock(p);
> -	if (p->mm && task_will_free_mem(p)) {
> +	if (task_will_free_mem(p)) {

I think it is possible that p->mm becomes NULL here.

Also, I think setting TIF_MEMDIE on p when find_lock_task_mm(p) != p is
wrong. While oom_reap_task() will anyway clear TIF_MEMDIE even if we set
TIF_MEMDIE on p when p->mm == NULL, it is not true for CONFIG_MMU=n case.

>  		mark_oom_victim(p);
> -		try_oom_reaper(p);
> -		task_unlock(p);
> +		wake_oom_reaper(p);
>  		put_task_struct(p);
>  		return;
>  	}
> -	task_unlock(p);
>  
>  	if (__ratelimit(&oom_rs))
>  		dump_header(oc, p);
> @@ -940,14 +968,10 @@ bool out_of_memory(struct oom_control *oc)
>  	 * If current has a pending SIGKILL or is exiting, then automatically
>  	 * select it.  The goal is to allow it to allocate so that it may
>  	 * quickly exit and free its memory.
> -	 *
> -	 * But don't select if current has already released its mm and cleared
> -	 * TIF_MEMDIE flag at exit_mm(), otherwise an OOM livelock may occur.
>  	 */
> -	if (current->mm &&
> -	    (fatal_signal_pending(current) || task_will_free_mem(current))) {
> +	if (task_will_free_mem(current)) {

Setting TIF_MEMDIE on current when current->mm == NULL and
find_lock_task_mm(current) != NULL is wrong.

>  		mark_oom_victim(current);
> -		try_oom_reaper(current);
> +		wake_oom_reaper(current);
>  		return true;
>  	}