linux-kernel - Re: + x86-mm-handle-mm_fault_error-in-kernel-space.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 for Android: free password hash cracker in your pocket

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

Message-ID: <4D7A2FED.3060200@gmail.com>
Date:	Fri, 11 Mar 2011 17:21:33 +0300
From:	Andrew Vagin <avagin@...il.com>
To:	Oleg Nesterov <oleg@...hat.com>
CC:	Pavel Emelyanov <xemul@...nvz.org>,
	Andrey Vagin <avagin@...nvz.org>, Ingo Molnar <mingo@...e.hu>,
	Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	David Rientjes <rientjes@...gle.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	linux-kernel@...r.kernel.org
Subject: Re: + x86-mm-handle-mm_fault_error-in-kernel-space.patch added to
 -mm tree

On 03/11/2011 02:19 PM, Oleg Nesterov wrote:
> On 03/10, Andrew Vagin wrote:
>> On 03/10/2011 05:28 PM, Oleg Nesterov wrote:
>>> (add cc's)
>>>
>>>> Subject: x86/mm: handle mm_fault_error() in kernel space
>>>> From: Andrey Vagin<avagin@...nvz.org>
>>>>
>>>> mm_fault_error() should not execute oom-killer, if page fault occurs in
>>>> kernel space.  E.g.  in copy_from_user/copy_to_user.
>>> Why? I don't understand this part.
>> I thought for a bit more...
>>
>> I think we should not execute out_of_memory() in this case at all,
> Why?
>
> Btw, this may be true, but this is irrelevant. If we shouldn't call
> out_of_memory() in this case, then we shouldn't call it at all, even
> if PF_USER.
Yes. We shouldn't call it at all, even if PF_USER.
> Andrew, I think you missed the point. Or I misunderstood. Or both ;)
>
>> because when we return from page fault, we execute the same command and
>> provoke the "same" page fault again
> Sure. And the same happens if the fault occurs in user-space and
> handle_mm_fault() returns VM_FAULT_OOM. This is correct.
When does handle_mm_fault() return VM_FAULT_OOM? It may be if current 
task is killed or a file system returns VM_FAULT_OOM. All allocations of 
memory (alloc_pages(), kmalloc()) calls out_of_memory() themselves, wait 
it and try allocate memory again.
>> Now pls think what is the
>> difference between these page faults?
> The difference is that oom-killer should free the memory in between.
> _OR_ it can decide to kill us, and _this_ case should be fixed.
We wait memory in __alloc_pages_may_oom(). I think now handle_mm_fault() 
returns VM_FAULT_OOM only if OOM-killer killed current task.
>> It has been generated from one
>> place and the program do nothing between those.
> The program does nothing, but the kernel does.
The kernel code may do it in one page fault...

The kernel should return from page fault back to userspace for the task 
to handle the signal or to continue execute userspace code.
>> If handle_mm_fault() returns
>> VM_FAULT_OOM and pagefault occurred from userspace, the current task
>> should be killed by SIGKILL,
> Why do you think the current task should be killed? In this case we
> do not need oom-killer at all, we could always kill the caller of
> alloc_page/etc.
You don't understand. alloc_page calls oom-killer himself, then try 
allocate memory again. Pls look at __alloc_pages_slowpath(). 
__alloc_pages_slowpat may fail if order > 3 || gfp_mask & __GFP_NOFAIL 
|| test_thread_flag(TIF_MEMDIE)

> Suppose that the innocent task (which doesn't use a lot of memory) calls,
> say, sys_read() into the unpopulated memory. Suppose that alloc_page()
> fails because we have a memory hog which tries to eat all memory.
alloc_page() calls oom-killer, which kill a memory hog.
> Do you think the innocent task should be punished in this case?
Probaly you think that oom-killer is called from mm_fault_error() only. 
It's incorrect.
> Assuming that mm/oom_kill.c:out_of_memory() is correct, it should find
> the memory hog and kill it, after that we can retry the fault in a hope
> we have more memory.
>
> PF_USER is not relevant. If the application does mmap() and then
> accesses this memory, memcpy() or copy_from_user() should follow the
> same logic wrt OOM.
>
>> If handle_mm_fault()
>> returns VM_FAULT_OOM and pagefault occurred in kernel space, we should
>> execute no_context() to return from syscall.
> Only if current was killed by oom-killer, that is why my patch checks
> fatal_signal_pending().
>
>> Also note that out_of_memory is usually called from handle_mm_fault() ->
>> ... ->  alloc_page()->...->out_of_memory().
> And note that pagefault_out_of_memory() checks TIF_MEMDIE and calls
> schedule_timeout_uninterruptible(). This is exactly because if we are
> _not_ killed by oom-killer, we are going to retry later once the killed
> task frees the memory.
>
> See?
>
> 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/