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

[Andi Kleen]

On Fri, Jan 21, 2005 at 05:51:07PM +0300, Oleg Nesterov wrote:
> Hello.
> 
> sys_get_mempolicy() accesses user memory with mmap_sem held.
> If I understand correctly, this can cause deadlock:
> 
> sys_get_mempolicy:            Another thread, same mm:
> 
> down_read(mmap_sem);
>                               down_write(mmap_sem);
> put_user();
> do_page_fault:
> down_read(mmap_sem);

Hrm. Normal Linux policy seems to ignore this potential
and rare deadlock and using nesting safely (e.g. it's been 
known for a long time for the tasklist rw spinlock, but
nobody really cares and it doesn't seem to be hit by users). 
Do you really think it is likely to happen?
> 
> Compile tested only, I have no NUMA machine.

It's hard to figure out what your patch actually does because
of all the gratious white space changes.

I suppose this simpler patch has the same effect (also untested).


diff -u linux-2.6.11-rc1-bk4/mm/mempolicy.c-o 
linux-2.6.11-rc1-bk4/mm/mempolicy.c
--- linux-2.6.11-rc1-bk4/mm/mempolicy.c-o       2005-01-14 10:12:27.000000000 
+0100
+++ linux-2.6.11-rc1-bk4/mm/mempolicy.c 2005-01-21 15:26:12.000000000 +0100
@@ -485,7 +485,7 @@
        int err, pval;
        struct mm_struct *mm = current->mm;
        struct vm_area_struct *vma = NULL;
-       struct mempolicy *pol = current->mempolicy;
+       struct mempolicy *pol = current->mempolicy, *pol2 = NULL;
 
        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);
        } else if (addr)
                return -EINVAL;
 
@@ -536,8 +540,7 @@
        }
 
  out:
-       if (vma)
-               up_read(&current->mm->mmap_sem);
+       mpol_free(pol2);
        return err;
 }
 

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/