linux-kernel - Re: [Patch] x86,mm: check freeze request in page fault handler

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 12 Aug 2014 17:47:02 -0700
From:	Cong Wang <xiyou.wangcong@...il.com>
To:	Michal Hocko <mhocko@...e.cz>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	David Rientjes <rientjes@...gle.com>,
	"Rafael J. Wysocki" <rjw@...ysocki.net>, Tejun Heo <tj@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>
Subject: Re: [Patch] x86,mm: check freeze request in page fault handler

On Tue, Aug 12, 2014 at 5:09 AM, Michal Hocko <mhocko@...e.cz> wrote:
> On Mon 11-08-14 17:53:55, Cong Wang wrote:
>> When a process triggers a page fault and kernel keeps
>> trying to retry the fault, there is no chance for this process
>> to be frozen, so the freeze request will always be pending.
>
> The retry cannot happen indefinitely, no?
>
> Besides that the patch is broken in at least 2 ways. You are not
> releasing mmap_sem and this will break memcg OOM killer handling.

Ah, it's my bad to miss the up_read().

>
> If a memcg is under OOM (because of hard limit) then try_charge
> calls mem_cgroup_oom which marks the current task with OOM
> information. Notably takes a reference to memcg->css. The charge fail
> will then gets up the pagefault stack until we get to mm_fault_error
> where you put the task into freezer and then returns without
> pagefault_out_of_memory which would handle memcg specific parts in
> mem_cgroup_oom_synchronize. If the task wakes up and the page fault
> retry succeeds (because some charges were released in the meantime) then
> you leak a reference to memcg->css.

OK, I thought skipping pagefault_out_of_memory() is safe, you are right here,
thanks for explanation.

>
> Besides that the whole change would need a better justification. Why
> other archs do not need this?
>

They probably need this as well, just that I only have x86 to care and test. :)

Does the following updated patch make any sense to you? If not, I will just
drop it.

diff --git a/arch/x86/mm/fault.c b/arch/x86/mm/fault.c
index a241946..8c0a7d8 100644
--- a/arch/x86/mm/fault.c
+++ b/arch/x86/mm/fault.c
@@ -14,6 +14,7 @@
 #include <linux/hugetlb.h>             /* hstate_index_to_shift        */
 #include <linux/prefetch.h>            /* prefetchw                    */
 #include <linux/context_tracking.h>    /* exception_enter(), ...       */
+#include <linux/freezer.h>             /* try_to_freeze()              */

 #include <asm/traps.h>                 /* dotraplinkage, ...           */
 #include <asm/pgalloc.h>               /* pgd_*(), ...                 */
@@ -904,6 +905,9 @@ mm_fault_error(struct pt_regs *regs, unsigned long
error_code,
                 * oom-killed):
                 */
                pagefault_out_of_memory();
+
+               if (signal_pending(current))
+                       try_to_freeze();
        } else {
                if (fault & (VM_FAULT_SIGBUS|VM_FAULT_HWPOISON|
                             VM_FAULT_HWPOISON_LARGE))
--
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/