Re: [RFC PATCH] oom: Don't count on mm-less current process.

Michal Hocko
2014-12-22T20:25:15Z

On Sat 20-12-14 20:42:08, Tetsuo Handa wrote:
[...]
> >From a2ebb5b873ec5af45e0bea9ea6da2a93c0f06c35 Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> Date: Sat, 20 Dec 2014 20:05:14 +0900
> Subject: [PATCH] oom: Close race of setting TIF_MEMDIE to mm-less process.
>
> exit_mm() and oom_kill_process() could race with regard to handling of
> TIF_MEMDIE flag if sequence described below occurred.
>
> P1 calls out_of_memory(). out_of_memory() calls select_bad_process().
> select_bad_process() calls oom_scan_process_thread(P2). If P2->mm != NULL
> and task_will_free_mem(P2) == false, oom_scan_process_thread(P2) returns
> OOM_SCAN_OK. And if P2 is chosen as a victim task, select_bad_process()
> returns P2 after calling get_task_struct(P2). Then, P1 goes to sleep and
> P2 is woken up. P2 enters into do_exit() and gets PF_EXITING at exit_signals()
> and releases mm at exit_mm(). Then, P2 goes to sleep and P1 is woken up.
> P1 calls oom_kill_process(P2). oom_kill_process() sets TIF_MEMDIE on P2
> because task_will_free_mem(P2) == true due to PF_EXITING already set.
> Afterward, oom_scan_process_thread(P2) will return OOM_SCAN_ABORT because
> test_tsk_thread_flag(P2, TIF_MEMDIE) is checked before P2->mm is checked.
>
> If TIF_MEMDIE was again set to P2, the OOM killer will be blocked by P2
> sitting in the final schedule() waiting for P2's parent to reap P2.
> It will trigger an OOM livelock if P2's parent is unable to reap P2 due to
> doing an allocation and waiting for the OOM killer to kill P2.
>
> To close this race window, clear TIF_MEMDIE if P2->mm == NULL after
> set_tsk_thread_flag(P2, TIF_MEMDIE) is done.

I do not think this patch is sufficient. P2 could pass exit_mm() right
after task_unlock in oom_kill_process and we would set TIF_MEMDIE to
this task as well. Something like the following should work and it
doesn't add memory barriers trickery.
---