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

[Oleg Nesterov <oleg@tv-sign.ru>]

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.

> 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.

	if (flags & MPOL_F_ADDR) {
		struct mm_struct *mm = current->mm;
		struct vm_area_struct *vma;

		err = 0;
		down_read(&mm->mmap_sem);
		vma = find_vma_intersection(mm, addr, addr+1);
		if (!vma)
			err = -EFAULT;
		else {
			if (vma->vm_ops && vma->vm_ops->get_policy)
				pol = vma->vm_ops->get_policy(vma, addr);
			else
				pol = vma->vm_policy;

			if (flags & MPOL_F_NODE) {
				pval = lookup_node(mm, addr);
				if (pval < 0)
					err = pval;
			}
		}
		up_read(&mm->mmap_sem);
		if (err)
			goto out;
	} else if (addr)
		return -EINVAL;

	if (!pol)
		pol = &default_policy;

	if (flags & MPOL_F_NODE) {
		if (!(flags & MPOL_F_ADDR)) {
			if (pol == current->mempolicy &&
					pol->policy == MPOL_INTERLEAVE) {
				pval = current->il_next;
			} else {
				err = -EINVAL;
				goto out;
			}
		}
	} else
		pval = pol->policy;