Re: [PATCH] fix put_user under mmap_sem in sys_get_mempolicy()

Andi Kleen
2005-01-21T15:12:25Z

On Fri, Jan 21, 2005 at 07:01:55PM +0300, Oleg Nesterov wrote:
> Andi Kleen wrote:
> >
> > I suppose this simpler patch has the same effect (also untested).
> >
> > if (flags & ~(unsigned long)(MPOL_F_NODE|MPOL_F_ADDR))
> > return -EINVAL;
> >@@ -502,6 +502,10 @@
> > pol = vma->vm_ops->get_policy(vma, addr);
> > else
> > pol = vma->vm_policy;
> >+ pol2 = mpol_copy(pol);
> >+ up_read(&mm->mmap_sem);
> >+ if (IS_ERR(pol2))
> >+ return PTR_ERR(pol2);
> >
>
> I don't think so. With MPOL_F_ADDR|MPOL_F_NODE sys_get_mempolicy
> calls lookup_node()->get_user_pages() few lines below, so we can't
> up_read(&mm->mmap_sem) here.

True.

>
> > It's hard to figure out what your patch actually does because
> > of all the gratious white space changes.
>
> For your convenience here is the code with the patch applied.

Looks reasonable.

-Andi

P.S.: Again if you really care about these class of deadlocks take a look at
tasklist_lock.