lkml.org 
[lkml]   [1998]   [Dec]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH] /proc/pid/mem race fixes
Hallo,

The /proc/pid/mem code was so full of races that it was very easy to crash
a box with it. This patch fixes them. I also reenabled writing because it
should be safe now.

I would regard this patch critical enough for 2.2pre, because without it
every local user can crash the box.

--- linux/fs/proc/mem.c-linus Tue Dec 29 00:39:20 1998
+++ linux/fs/proc/mem.c Tue Dec 29 01:08:26 1998
@@ -2,6 +2,7 @@
* linux/fs/proc/mem.c
*
* Copyright (C) 1991, 1992 Linus Torvalds
+ * Copyright (C) 1998 Andi Kleen
*/

#include <linux/types.h>
@@ -16,15 +17,9 @@
#include <asm/io.h>
#include <asm/pgtable.h>

-/*
- * mem_write isn't really a good idea right now. It needs
- * to check a lot more: if the process we try to write to
- * dies in the middle right now, mem_write will overwrite
- * kernel memory.. This disables it altogether.
- */
-#define mem_write NULL
-
-static int check_range(struct mm_struct * mm, unsigned long addr, int count)
+/* Must be called with the mm semaphore held. */
+static int check_range(struct mm_struct * mm, unsigned long addr, int count,
+ int flags)
{
struct vm_area_struct *vma;
int retval;
@@ -34,15 +29,16 @@
return -EACCES;
if (vma->vm_start > addr)
return -EACCES;
- if (!(vma->vm_flags & VM_READ))
+ if ((vma->vm_flags & flags) != flags)
return -EACCES;
+
while ((retval = vma->vm_end - addr) < count) {
struct vm_area_struct *next = vma->vm_next;
if (!next)
break;
if (vma->vm_end != next->vm_start)
break;
- if (!(next->vm_flags & VM_READ))
+ if ((next->vm_flags & flags) != flags)
break;
vma = next;
}
@@ -53,24 +49,74 @@

static struct task_struct * get_task(int pid)
{
- struct task_struct * tsk = current;
+ struct task_struct * tsk;

- if (pid != tsk->pid) {
- tsk = find_task_by_pid(pid);
+ tsk = find_task_by_pid(pid);

- /* Allow accesses only under the same circumstances
- * that we would allow ptrace to work.
- */
- if (tsk) {
- if (!(tsk->flags & PF_PTRACED)
- || tsk->state != TASK_STOPPED
- || tsk->p_pptr != current)
- tsk = NULL;
- }
+ /*
+ * Allow accesses only under the same circumstances
+ * that we would allow ptrace to work.
+ */
+ if (tsk && tsk!=current) {
+ if (!(tsk->flags & PF_PTRACED)
+ || tsk->state != TASK_STOPPED
+ || tsk->p_pptr != current)
+ tsk = NULL;
}
return tsk;
}

+struct mm_struct *get_mm(int pid, int *err)
+{
+ struct task_struct *tsk;
+ struct mm_struct *mm = NULL;
+
+ read_lock(&tasklist_lock);
+ tsk = get_task(pid);
+ *err = -ESRCH;
+ if (tsk) {
+ *err = -EINVAL;
+ /*
+ * Access of current vm: it could be special cased,
+ * but is it worth it?
+ */
+ if ((mm = tsk->mm) == current->mm)
+ mm = NULL;
+ else {
+ mmget(mm);
+ *err = 0;
+ }
+ }
+ read_unlock(&tasklist_lock);
+ return mm;
+}
+
+static int make_page_present(int pid, unsigned long addr, struct mm_struct *mm,
+ int write_flag)
+{
+ struct task_struct *tsk;
+ struct vm_area_struct *vma;
+
+ /*
+ * Caller may have slept, thus recheck.
+ */
+ read_lock(&tasklist_lock);
+ tsk = get_task(pid);
+ read_unlock(&tasklist_lock);
+ if (!tsk || tsk->mm != mm)
+ return -ESRCH;
+
+ /*
+ * The kernel lock ensures that the task does not go away.
+ * find_vma should not fail here because the mm semaphore is held.
+ */
+ vma = find_vma(mm, addr);
+ if (!handle_mm_fault(tsk, vma, addr, write_flag))
+ return -ENOMEM;
+
+ return 0;
+}
+
static ssize_t mem_read(struct file * file, char * buf,
size_t count, loff_t *ppos)
{
@@ -79,25 +125,30 @@
pmd_t *page_middle;
pte_t pte;
char * page;
- struct task_struct * tsk;
unsigned long addr;
- char *tmp;
+ char *dst;
ssize_t scount, i;
+ int err;
+ struct mm_struct *mm;
+
+ mm = get_mm(inode->i_ino >> 16, &err);
+ if (!mm)
+ return err;
+ err = 0;

- read_lock(&tasklist_lock);
- tsk = get_task(inode->i_ino >> 16);
- read_unlock(&tasklist_lock); /* FIXME: This should really be done only afetr not using tsk any more!!! */
- if (!tsk)
- return -ESRCH;
addr = *ppos;
- scount = check_range(tsk->mm, addr, count);
- if (scount < 0)
- return scount;
- tmp = buf;
+ down(&mm->mmap_sem);
+ scount = check_range(mm, addr, count, VM_READ);
+ if (scount < 0)
+ err = scount;
+ dst = buf;
while (scount > 0) {
- if (signal_pending(current))
+ /* XXX Is a current->need_resched check needed too? */
+ if (signal_pending(current)) {
+ err = -ERESTARTSYS;
break;
- page_dir = pgd_offset(tsk->mm,addr);
+ }
+ page_dir = pgd_offset(mm,addr);
if (pgd_none(*page_dir))
break;
if (pgd_bad(*page_dir)) {
@@ -114,24 +165,42 @@
break;
}
pte = *pte_offset(page_middle,addr);
- if (!pte_present(pte))
+ if (!pte_present(pte)) {
+ err = make_page_present(inode->i_ino >> 16, addr, mm, 0);
+ if (err)
+ break;
+ continue;
+ }
+ /* check_range should have catched this, but be paranoid */
+ if (!pte_read(pte)) {
+ err = -EACCES;
break;
+ }
+
page = (char *) pte_page(pte) + (addr & ~PAGE_MASK);
+ if (MAP_NR(page) >= max_mapnr)
+ break;
i = PAGE_SIZE-(addr & ~PAGE_MASK);
if (i > scount)
i = scount;
- copy_to_user(tmp, page, i);
+ /* Assumes that current->mm != mm */
+ err = copy_to_user(dst, page, i);
+ if (err) {
+ err = -EFAULT;
+ break;
+ }
+
addr += i;
- tmp += i;
+ dst += i;
scount -= i;
}
+ up(&mm->mmap_sem);
+ mmput(mm);
*ppos = addr;
- return tmp-buf;
+ return err ? err : dst-buf;
}

-#ifndef mem_write
-
-static ssize_t mem_write(struct file * file, char * buf,
+static ssize_t mem_write(struct file * file, const char * buf,
size_t count, loff_t *ppos)
{
struct inode * inode = file->f_dentry->d_inode;
@@ -139,20 +208,29 @@
pmd_t *page_middle;
pte_t pte;
char * page;
- struct task_struct * tsk;
+ struct mm_struct *mm;
unsigned long addr;
- char *tmp;
+ const char *src;
long i;
+ int err;

addr = *ppos;
- tsk = get_task(inode->i_ino >> 16);
- if (!tsk)
- return -ESRCH;
- tmp = buf;
+ mm = get_mm(inode->i_ino >> 16, &err);
+ if (!mm)
+ return err;
+
+ down(&mm->mmap_sem);
+ count = check_range(mm, addr, count, VM_WRITE);
+ if (count < 0)
+ err = count;
+
+ src = buf;
while (count > 0) {
- if (signal_pending(current))
+ if (signal_pending(current)) {
+ err = -ERESTARTSYS;
break;
- page_dir = pgd_offset(tsk,addr);
+ }
+ page_dir = pgd_offset(mm,addr);
if (pgd_none(*page_dir))
break;
if (pgd_bad(*page_dir)) {
@@ -169,29 +247,40 @@
break;
}
pte = *pte_offset(page_middle,addr);
- if (!pte_present(pte))
- break;
- if (!pte_write(pte))
- break;
+ if (!pte_present(pte) || !pte_write(pte)) {
+ err = make_page_present(inode->i_ino>>16, addr, mm, 1);
+ if (err)
+ break;
+ continue;
+ }
+
page = (char *) pte_page(pte) + (addr & ~PAGE_MASK);
+ if (MAP_NR(page) > max_mapnr)
+ break;
+
i = PAGE_SIZE-(addr & ~PAGE_MASK);
if (i > count)
i = count;
- copy_from_user(page, tmp, i);
+ /* Assumes current->mm != mm */
+ if (copy_from_user(page, src, i)) {
+ err = -EFAULT;
+ break;
+ }
+ set_pte(pte_offset(page_middle,addr), pte_mkdirty(pte));
+ flush_tlb_mm(mm);
+
addr += i;
- tmp += i;
+ src += i;
count -= i;
}
+ up(&mm->mmap_sem);
+ mmput(mm);
*ppos = addr;
- if (tmp != buf)
- return tmp-buf;
- if (signal_pending(current))
- return -ERESTARTSYS;
- return 0;
+ if (err)
+ return err;
+ return src-buf;
}

-#endif
-
static long long mem_lseek(struct file * file, long long offset, int orig)
{
switch (orig) {
@@ -206,93 +295,124 @@
}
}

-/*
- * This isn't really reliable by any means..
- */
+static inline struct mm_struct *grab_mm(int pid, int *err)
+{
+ struct mm_struct *mm;
+ /*
+ * Mmaping its own memory is a highly dubious feature,
+ * it would be better to forbid it, because it violates some
+ * assumptions elsewhere in the MM system
+ */
+ if (pid == current->pid) {
+ mm = current->mm;
+ mmget(mm);
+ } else {
+ mm = get_mm(pid, &err);
+ }
+ return mm;
+}
+
int mem_mmap(struct file * file, struct vm_area_struct * vma)
{
- struct task_struct *tsk;
pgd_t *src_dir, *dest_dir;
pmd_t *src_middle, *dest_middle;
pte_t *src_table, *dest_table;
unsigned long stmp, dtmp, mapnr;
struct vm_area_struct *src_vma = NULL;
struct inode *inode = file->f_dentry->d_inode;
-
- /* Get the source's task information */
-
- tsk = get_task(inode->i_ino >> 16);
-
- if (!tsk)
- return -ESRCH;
+ struct mm_struct *mm;
+ int err;

- /* Ensure that we have a valid source area. (Has to be mmap'ed and
- have valid page information.) We can't map shared memory at the
- moment because working out the vm_area_struct & nattach stuff isn't
- worth it. */
+ mm = grab_mm(inode->i_ino >> 16, &err);
+ if (!mm)
+ return err;
+
+ /*
+ * This relies on the kernel lock to make sure that there are
+ * no races between the aquisition of the current mm lock in the caller
+ * and the locking here.
+ */
+ if (mm != current->mm)
+ down(&mm->mmap_sem);
+
+ /*
+ * Ensure that we have a valid source area: has to be mmap'ed and
+ * have valid page information. Mapping shared memory is not supported
+ * ATM because working out the vm_area_struct & nattach stuff isn't
+ * worth it.
+ */

- src_vma = tsk->mm->mmap;
+ src_vma = mm->mmap;
stmp = vma->vm_offset;
while (stmp < vma->vm_offset + (vma->vm_end - vma->vm_start)) {
while (src_vma && stmp > src_vma->vm_end)
src_vma = src_vma->vm_next;
+ err = -EINVAL;
if (!src_vma || (src_vma->vm_flags & VM_SHM))
- return -EINVAL;
+ goto out;

- src_dir = pgd_offset(tsk->mm, stmp);
+ src_dir = pgd_offset(mm, stmp);
if (pgd_none(*src_dir))
- return -EINVAL;
+ goto out;
if (pgd_bad(*src_dir)) {
printk("Bad source page dir entry %08lx\n", pgd_val(*src_dir));
- return -EINVAL;
+ goto out;
}
src_middle = pmd_offset(src_dir, stmp);
if (pmd_none(*src_middle))
- return -EINVAL;
+ goto out;
if (pmd_bad(*src_middle)) {
printk("Bad source page middle entry %08lx\n", pmd_val(*src_middle));
- return -EINVAL;
+ goto out;
}
src_table = pte_offset(src_middle, stmp);
if (pte_none(*src_table))
- return -EINVAL;
+ goto out;

if (stmp < src_vma->vm_start) {
if (!(src_vma->vm_flags & VM_GROWSDOWN))
- return -EINVAL;
+ goto out;
if (src_vma->vm_end - stmp > current->rlim[RLIMIT_STACK].rlim_cur)
- return -EINVAL;
+ goto out;
}
stmp += PAGE_SIZE;
}

- src_vma = tsk->mm->mmap;
+ src_vma = mm->mmap;
stmp = vma->vm_offset;
dtmp = vma->vm_start;

flush_cache_range(vma->vm_mm, vma->vm_start, vma->vm_end);
flush_cache_range(src_vma->vm_mm, src_vma->vm_start, src_vma->vm_end);
+
while (dtmp < vma->vm_end) {
while (src_vma && stmp > src_vma->vm_end)
src_vma = src_vma->vm_next;

- src_dir = pgd_offset(tsk->mm, stmp);
+ src_dir = pgd_offset(mm, stmp);
src_middle = pmd_offset(src_dir, stmp);
src_table = pte_offset(src_middle, stmp);

dest_dir = pgd_offset(current->mm, dtmp);
+
+ /* Next two might sleep */
dest_middle = pmd_alloc(dest_dir, dtmp);
+ err = -ENOMEM;
if (!dest_middle)
- return -ENOMEM;
+ break;
dest_table = pte_alloc(dest_middle, dtmp);
if (!dest_table)
- return -ENOMEM;
+ break;

- if (!pte_present(*src_table))
- handle_mm_fault(tsk, src_vma, stmp, 1);
+ if (!pte_present(*src_table) ||
+ ((vma->vm_flags & VM_WRITE) && !pte_write(*src_table)) ) {
+ err = make_page_present(inode->i_ino >> 16, stmp, mm,
+ vma->vm_flags & VM_WRITE);
+ if (err)
+ break;
+ }

- if ((vma->vm_flags & VM_WRITE) && !pte_write(*src_table))
- handle_mm_fault(tsk, src_vma, stmp, 1);
+ err = 0;

set_pte(src_table, pte_mkdirty(*src_table));
set_pte(dest_table, *src_table);
@@ -306,7 +426,12 @@

flush_tlb_range(vma->vm_mm, vma->vm_start, vma->vm_end);
flush_tlb_range(src_vma->vm_mm, src_vma->vm_start, src_vma->vm_end);
- return 0;
+
+out:
+ if (mm != current->mm)
+ up(&mm->mmap_sem);
+ mmput(mm);
+ return err;
}

static struct file_operations proc_mem_operations = {



-Andi


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2005-03-22 13:46    [W:0.032 / U:0.208 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site