Thread (47 messages) 47 messages, 5 authors, 2016-08-24

Re: [PATCH 09/10] vhost, mm: make sure that oom_reaper doesn't reap memory read by vhost

From: Paul E. McKenney <hidden>
Date: 2016-08-12 15:57:36

On Fri, Aug 12, 2016 at 03:21:41PM +0200, Oleg Nesterov wrote:
On 08/12, Michal Hocko wrote:
quoted
quoted
Let's CC Paul. Just to describe the situation. We have the following
situation:

#define __get_user_mm(mm, x, ptr)				\
({								\
	int ___gu_err = __get_user(x, ptr);			\
	if (!___gu_err && test_bit(MMF_UNSTABLE, &mm->flags))	\
		___gu_err = -EFAULT;				\
	___gu_err;						\
})

and the oom reaper doing:

	set_bit(MMF_UNSTABLE, &mm->flags);

	for (vma = mm->mmap ; vma; vma = vma->vm_next) {
		unmap_page_range

I assume that write memory barrier between set_bit and unmap_page_range
is not really needed because unmapping should already imply the memory
barrier.
Well, I leave this to Paul, but...

I think it is not needed because we can rely on pte locking. We do
not care if anything is re-ordered UNLESS __get_user() above actually
triggers a fault and re-populates the page which was already unmapped
by __oom_reap_task(), and in the latter case __get_user_mm() can't
miss MMF_UNSTABLE simply because __get_user() and unmap_page_range()
need to lock/unlock the same ptlock_ptr().

So we only need the compiler barrier to ensure that __get_user_mm()
won't read MMF_UNSTABLE before __get_user(). But since __get_user()
is function, it is not needed too.

There is a more interesting case when another 3rd thread can trigger
a fault and populate this page before __get_user_mm() calls _get_user().
But even in this case I think we are fine.
Hmmm...  What source tree are you guys looking at?  I am seeing some
of the above being macros rather than functions and others not being
present at all...

							Thanx, Paul
Whats really interesting is that I still fail to understand do we really
need this hack, iiuc you are not sure too, and Michael didn't bother to
explain why a bogus zero from anon memory is worse than other problems
caused by SIGKKILL from oom-kill.c.

Oleg.
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help