Commit 2f77d107 authored by Linus Torvalds's avatar Linus Torvalds

Fix incorrect user space access locking in mincore()

Doug Chapman noticed that mincore() will doa "copy_to_user()" of the
result while holding the mmap semaphore for reading, which is a big
no-no.  While a recursive read-lock on a semaphore in the case of a page
fault happens to work, we don't actually allow them due to deadlock
schenarios with writers due to fairness issues.

Doug and Marcel sent in a patch to fix it, but I decided to just rewrite
the mess instead - not just fixing the locking problem, but making the
code smaller and (imho) much easier to understand.

Cc: Doug Chapman <dchapman@redhat.com>
Cc: Marcel Holtmann <holtmann@redhat.com>
Cc: Hugh Dickins <hugh@veritas.com>
Cc: Andrew Morton <akpm@osdl.org>
Signed-off-by: default avatarLinus Torvalds <torvalds@osdl.org>
parent 0221872a
/* /*
* linux/mm/mincore.c * linux/mm/mincore.c
* *
* Copyright (C) 1994-1999 Linus Torvalds * Copyright (C) 1994-2006 Linus Torvalds
*/ */
/* /*
...@@ -38,46 +38,60 @@ static unsigned char mincore_page(struct vm_area_struct * vma, ...@@ -38,46 +38,60 @@ static unsigned char mincore_page(struct vm_area_struct * vma,
return present; return present;
} }
static long mincore_vma(struct vm_area_struct * vma, /*
unsigned long start, unsigned long end, unsigned char __user * vec) * Do a chunk of "sys_mincore()". We've already checked
* all the arguments, we hold the mmap semaphore: we should
* just return the amount of info we're asked for.
*/
static long do_mincore(unsigned long addr, unsigned char *vec, unsigned long pages)
{ {
long error, i, remaining; unsigned long i, nr, pgoff;
unsigned char * tmp; struct vm_area_struct *vma = find_vma(current->mm, addr);
error = -ENOMEM;
if (!vma->vm_file)
return error;
start = ((start - vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff; /*
if (end > vma->vm_end) * find_vma() didn't find anything: the address
end = vma->vm_end; * is above everything we have mapped.
end = ((end - vma->vm_start) >> PAGE_SHIFT) + vma->vm_pgoff; */
if (!vma) {
memset(vec, 0, pages);
return pages;
}
error = -EAGAIN; /*
tmp = (unsigned char *) __get_free_page(GFP_KERNEL); * find_vma() found something, but we might be
if (!tmp) * below it: check for that.
return error; */
if (addr < vma->vm_start) {
unsigned long gap = (vma->vm_start - addr) >> PAGE_SHIFT;
if (gap > pages)
gap = pages;
memset(vec, 0, gap);
return gap;
}
/* (end - start) is # of pages, and also # of bytes in "vec */ /*
remaining = (end - start), * Ok, got it. But check whether it's a segment we support
* mincore() on. Right now, we don't do any anonymous mappings.
*/
if (!vma->vm_file)
return -ENOMEM;
error = 0; /*
for (i = 0; remaining > 0; remaining -= PAGE_SIZE, i++) { * Calculate how many pages there are left in the vma, and
int j = 0; * what the pgoff is for our address.
long thispiece = (remaining < PAGE_SIZE) ? */
remaining : PAGE_SIZE; nr = (vma->vm_end - addr) >> PAGE_SHIFT;
if (nr > pages)
nr = pages;
while (j < thispiece) pgoff = (addr - vma->vm_start) >> PAGE_SHIFT;
tmp[j++] = mincore_page(vma, start++); pgoff += vma->vm_pgoff;
if (copy_to_user(vec + PAGE_SIZE * i, tmp, thispiece)) { /* And then we just fill the sucker in.. */
error = -EFAULT; for (i = 0 ; i < nr; i++, pgoff++)
break; vec[i] = mincore_page(vma, pgoff);
}
}
free_page((unsigned long) tmp); return nr;
return error;
} }
/* /*
...@@ -107,82 +121,50 @@ static long mincore_vma(struct vm_area_struct * vma, ...@@ -107,82 +121,50 @@ static long mincore_vma(struct vm_area_struct * vma,
asmlinkage long sys_mincore(unsigned long start, size_t len, asmlinkage long sys_mincore(unsigned long start, size_t len,
unsigned char __user * vec) unsigned char __user * vec)
{ {
int index = 0; long retval;
unsigned long end, limit; unsigned long pages;
struct vm_area_struct * vma; unsigned char *tmp;
size_t max;
int unmapped_error = 0;
long error;
/* check the arguments */
if (start & ~PAGE_CACHE_MASK)
goto einval;
limit = TASK_SIZE; /* Check the start address: needs to be page-aligned.. */
if (start >= limit) if (start & ~PAGE_CACHE_MASK)
goto enomem; return -EINVAL;
if (!len)
return 0;
max = limit - start; /* ..and we need to be passed a valid user-space range */
len = PAGE_CACHE_ALIGN(len); if (!access_ok(VERIFY_READ, (void __user *) start, len))
if (len > max || !len) return -ENOMEM;
goto enomem;
end = start + len; /* This also avoids any overflows on PAGE_CACHE_ALIGN */
pages = len >> PAGE_SHIFT;
pages += (len & ~PAGE_MASK) != 0;
/* check the output buffer whilst holding the lock */ if (!access_ok(VERIFY_WRITE, vec, pages))
error = -EFAULT; return -EFAULT;
down_read(&current->mm->mmap_sem);
if (!access_ok(VERIFY_WRITE, vec, len >> PAGE_SHIFT)) tmp = (void *) __get_free_page(GFP_USER);
goto out; if (!tmp)
return -ENOMEM;
retval = 0;
while (pages) {
/* /*
* If the interval [start,end) covers some unmapped address * Do at most PAGE_SIZE entries per iteration, due to
* ranges, just ignore them, but return -ENOMEM at the end. * the temporary buffer size.
*/ */
error = 0; down_read(&current->mm->mmap_sem);
retval = do_mincore(start, tmp, max(pages, PAGE_SIZE));
vma = find_vma(current->mm, start); up_read(&current->mm->mmap_sem);
while (vma) {
/* Here start < vma->vm_end. */
if (start < vma->vm_start) {
unmapped_error = -ENOMEM;
start = vma->vm_start;
}
/* Here vma->vm_start <= start < vma->vm_end. */ if (retval <= 0)
if (end <= vma->vm_end) { break;
if (start < end) { if (copy_to_user(vec, tmp, retval)) {
error = mincore_vma(vma, start, end, retval = -EFAULT;
&vec[index]); break;
if (error)
goto out;
}
error = unmapped_error;
goto out;
} }
pages -= retval;
/* Here vma->vm_start <= start < vma->vm_end < end. */ vec += retval;
error = mincore_vma(vma, start, vma->vm_end, &vec[index]); start += retval << PAGE_SHIFT;
if (error) retval = 0;
goto out;
index += (vma->vm_end - start) >> PAGE_CACHE_SHIFT;
start = vma->vm_end;
vma = vma->vm_next;
} }
free_page((unsigned long) tmp);
/* we found a hole in the area queried if we arrive here */ return retval;
error = -ENOMEM;
out:
up_read(&current->mm->mmap_sem);
return error;
einval:
return -EINVAL;
enomem:
return -ENOMEM;
} }
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment