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

["Michael S. Tsirkin" <mst@redhat.com>]

On Sun, Jul 03, 2016 at 03:47:19PM +0200, Oleg Nesterov wrote:
> On 07/01, Michal Hocko wrote:
> >
> > From: Michal Hocko <mhocko@suse.com>
> >
> > vhost driver relies on copy_from_user/get_user from a kernel thread.
> > This makes it impossible to reap the memory of an oom victim which
> > shares mm with the vhost kernel thread because it could see a zero
> > page unexpectedly and theoretically make an incorrect decision visible
> > outside of the killed task context.
> 
> And I still can't understand how, but let me repeat that I don't understand
> this code at all.
> 
> > To quote Michael S. Tsirkin:
> > : Getting an error from __get_user and friends is handled gracefully.
> > : Getting zero instead of a real value will cause userspace
> > : memory corruption.
> 
> Which userspace memory corruption? We are going to kill the dev->mm owner,
> the task which did ioctl(VHOST_SET_OWNER) and (at first glance) the task
> who communicates with the callbacks fired by vhost_worker().
> 
> Michael, could you please spell why should we care?

I am concerned that
- oom victim is sharing memory with another task
- getting incorrect value from ring read makes vhost
  change that shared memory


Also, I don't see where do we kill the task that communicates with the
callbacks.


Having said all that, how about we just add some kind of per-mm
notifier list, and let vhost know that owner is going away so
it should stop looking at memory?

Seems cleaner than looking at flags at each memory access,
since vhost has its own locking.


> > --- a/mm/oom_kill.c
> > +++ b/mm/oom_kill.c
> > @@ -492,6 +492,14 @@ static bool __oom_reap_task(struct task_struct *tsk)
> >  		goto unlock_oom;
> >  	}
> >
> > +	/*
> > +	 * Tell all users of get_user_mm/copy_from_user_mm that the content
> > +	 * is no longer stable. No barriers really needed because unmapping
> > +	 * should imply barriers already and the reader would hit a page fault
> > +	 * if it stumbled over a reaped memory.
> > +	 */
> > +	set_bit(MMF_UNSTABLE, &mm->flags);
> 
> And this is racy anyway.
> 
> 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>