Messages in this thread Patch in this message | | | Date | Tue, 29 Dec 1998 01:14:04 +0100 | From | Andi Kleen <> | 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/
| |