lkml.org 
[lkml]   [1999]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[FIX] oopsen in procfs (2.3.12-pre*)
    	Linus, procfs handling of task->mm had several bad races + ugly
    deadlock (my fault - down() is *not* a function to be used while holding
    the tasklist_lock). I think that patch below fixes most of the problems
    (at least simple tests are working fine). I also included the one-liner
    for arch/i386/kernel/smp.c - the thing forgot to call cpu_init() if it
    was built for SMP and found that it runs on UP box. Please, apply it.
    TIA,
    Al
    Patch follows:
    diff -urN linux-2.3.12-8/arch/i386/kernel/smp.c linux/arch/i386/kernel/smp.c
    --- linux-2.3.12-8/arch/i386/kernel/smp.c Wed Jul 28 02:28:12 1999
    +++ linux/arch/i386/kernel/smp.c Wed Jul 28 03:47:54 1999
    @@ -1229,6 +1229,7 @@
    io_apic_irqs = 0;
    #endif
    cpu_online_map = cpu_present_map;
    + cpu_init();
    goto smp_done;
    }

    diff -urN linux-2.3.12-8/fs/proc/array.c linux/fs/proc/array.c
    --- linux-2.3.12-8/fs/proc/array.c Wed Jul 28 02:28:19 1999
    +++ linux/fs/proc/array.c Wed Jul 28 06:07:17 1999
    @@ -42,6 +42,8 @@
    * Alan Cox : security fixes.
    * <Alan.Cox@linux.org>
    *
    + * Al Viro : safe handling of mm_struct
    + *
    */

    #include <linux/types.h>
    @@ -386,21 +388,15 @@
    return sprintf(buffer, "%s\n", saved_command_line);
    }

    -static unsigned long get_phys_addr(struct task_struct * p, unsigned long ptr)
    +static unsigned long get_phys_addr(struct mm_struct * mm, unsigned long ptr)
    {
    pgd_t *page_dir;
    pmd_t *page_middle;
    pte_t pte;

    - if (!p || !p->mm || ptr >= TASK_SIZE)
    - return 0;
    - /* Check for NULL pgd .. shouldn't happen! */
    - if (!p->mm->pgd) {
    - printk("get_phys_addr: pid %d has NULL pgd!\n", p->pid);
    + if (ptr >= TASK_SIZE)
    return 0;
    - }
    -
    - page_dir = pgd_offset(p->mm,ptr);
    + page_dir = pgd_offset(mm,ptr);
    if (pgd_none(*page_dir))
    return 0;
    if (pgd_bad(*page_dir)) {
    @@ -422,7 +418,7 @@
    return pte_page(pte) + (ptr & ~PAGE_MASK);
    }

    -static int get_array(struct task_struct *p, unsigned long start, unsigned long end, char * buffer)
    +static int get_array(struct mm_struct *mm, unsigned long start, unsigned long end, char * buffer)
    {
    unsigned long addr;
    int size = 0, result = 0;
    @@ -431,7 +427,7 @@
    if (start >= end)
    return result;
    for (;;) {
    - addr = get_phys_addr(p, start);
    + addr = get_phys_addr(mm, start);
    if (!addr)
    return result;
    do {
    @@ -451,29 +447,42 @@
    return result;
    }

    -static int get_env(int pid, char * buffer)
    +static struct mm_struct *get_mm(int pid)
    {
    struct task_struct *p;
    + struct mm_struct *mm = NULL;

    read_lock(&tasklist_lock);
    p = find_task_by_pid(pid);
    - read_unlock(&tasklist_lock); /* FIXME!! This should be done after the last use */
    + if (p)
    + mm = p->mm;
    + if (mm)
    + atomic_inc(&mm->mm_users);
    + read_unlock(&tasklist_lock);
    + return mm;
    +}

    - if (!p || !p->mm)
    - return 0;
    - return get_array(p, p->mm->env_start, p->mm->env_end, buffer);
    +
    +static int get_env(int pid, char * buffer)
    +{
    + struct mm_struct *mm = get_mm(pid);
    + int res = 0;
    + if (mm) {
    + res = get_array(mm, mm->env_start, mm->env_end, buffer);
    + mmput(mm);
    + }
    + return res;
    }

    static int get_arg(int pid, char * buffer)
    {
    - struct task_struct *p;
    -
    - read_lock(&tasklist_lock);
    - p = find_task_by_pid(pid);
    - read_unlock(&tasklist_lock); /* FIXME!! This should be done after the last use */
    - if (!p || !p->mm)
    - return 0;
    - return get_array(p, p->mm->arg_start, p->mm->arg_end, buffer);
    + struct mm_struct *mm = get_mm(pid);
    + int res = 0;
    + if (mm) {
    + res = get_array(mm, mm->arg_start, mm->arg_end, buffer);
    + mmput(mm);
    + }
    + return res;
    }

    /*
    @@ -740,48 +749,44 @@
    return buffer;
    }

    -static inline char * task_mem(struct task_struct *p, char *buffer)
    +static inline char * task_mem(struct mm_struct *mm, char *buffer)
    {
    - struct mm_struct * mm = p->mm;
    -
    - if (mm) {
    - struct vm_area_struct * vma;
    - unsigned long data = 0, stack = 0;
    - unsigned long exec = 0, lib = 0;
    -
    - down(&mm->mmap_sem);
    - for (vma = mm->mmap; vma; vma = vma->vm_next) {
    - unsigned long len = (vma->vm_end - vma->vm_start) >> 10;
    - if (!vma->vm_file) {
    - data += len;
    - if (vma->vm_flags & VM_GROWSDOWN)
    - stack += len;
    - continue;
    - }
    - if (vma->vm_flags & VM_WRITE)
    + struct vm_area_struct * vma;
    + unsigned long data = 0, stack = 0;
    + unsigned long exec = 0, lib = 0;
    +
    + down(&mm->mmap_sem);
    + for (vma = mm->mmap; vma; vma = vma->vm_next) {
    + unsigned long len = (vma->vm_end - vma->vm_start) >> 10;
    + if (!vma->vm_file) {
    + data += len;
    + if (vma->vm_flags & VM_GROWSDOWN)
    + stack += len;
    + continue;
    + }
    + if (vma->vm_flags & VM_WRITE)
    + continue;
    + if (vma->vm_flags & VM_EXEC) {
    + exec += len;
    + if (vma->vm_flags & VM_EXECUTABLE)
    continue;
    - if (vma->vm_flags & VM_EXEC) {
    - exec += len;
    - if (vma->vm_flags & VM_EXECUTABLE)
    - continue;
    - lib += len;
    - }
    + lib += len;
    }
    - buffer += sprintf(buffer,
    - "VmSize:\t%8lu kB\n"
    - "VmLck:\t%8lu kB\n"
    - "VmRSS:\t%8lu kB\n"
    - "VmData:\t%8lu kB\n"
    - "VmStk:\t%8lu kB\n"
    - "VmExe:\t%8lu kB\n"
    - "VmLib:\t%8lu kB\n",
    - mm->total_vm << (PAGE_SHIFT-10),
    - mm->locked_vm << (PAGE_SHIFT-10),
    - mm->rss << (PAGE_SHIFT-10),
    - data - stack, stack,
    - exec - lib, lib);
    - up(&mm->mmap_sem);
    }
    + buffer += sprintf(buffer,
    + "VmSize:\t%8lu kB\n"
    + "VmLck:\t%8lu kB\n"
    + "VmRSS:\t%8lu kB\n"
    + "VmData:\t%8lu kB\n"
    + "VmStk:\t%8lu kB\n"
    + "VmExe:\t%8lu kB\n"
    + "VmLib:\t%8lu kB\n",
    + mm->total_vm << (PAGE_SHIFT-10),
    + mm->locked_vm << (PAGE_SHIFT-10),
    + mm->rss << (PAGE_SHIFT-10),
    + data - stack, stack,
    + exec - lib, lib);
    + up(&mm->mmap_sem);
    return buffer;
    }

    @@ -842,44 +847,61 @@
    {
    char * orig = buffer;
    struct task_struct *tsk;
    + struct mm_struct *mm = NULL;

    read_lock(&tasklist_lock);
    tsk = find_task_by_pid(pid);
    + if (tsk)
    + mm = tsk->mm;
    + if (mm)
    + atomic_inc(&mm->mm_users);
    read_unlock(&tasklist_lock); /* FIXME!! This should be done after the last use */
    if (!tsk)
    return 0;
    buffer = task_name(tsk, buffer);
    buffer = task_state(tsk, buffer);
    - buffer = task_mem(tsk, buffer);
    + if (mm)
    + buffer = task_mem(mm, buffer);
    buffer = task_sig(tsk, buffer);
    buffer = task_cap(tsk, buffer);
    + if (mm)
    + mmput(mm);
    return buffer - orig;
    }

    static int get_stat(int pid, char * buffer)
    {
    struct task_struct *tsk;
    + struct mm_struct *mm = NULL;
    unsigned long vsize, eip, esp, wchan;
    long priority, nice;
    int tty_pgrp;
    sigset_t sigign, sigcatch;
    char state;
    + int res;

    read_lock(&tasklist_lock);
    tsk = find_task_by_pid(pid);
    + if (tsk)
    + mm = tsk->mm;
    + if (mm)
    + atomic_inc(&mm->mm_users);
    read_unlock(&tasklist_lock); /* FIXME!! This should be done after the last use */
    if (!tsk)
    return 0;
    state = *get_task_state(tsk);
    vsize = eip = esp = 0;
    - if (tsk->mm) {
    - struct vm_area_struct *vma = tsk->mm->mmap;
    + if (mm) {
    + struct vm_area_struct *vma;
    + down(&mm->mmap_sem);
    + vma = mm->mmap;
    while (vma) {
    vsize += vma->vm_end - vma->vm_start;
    vma = vma->vm_next;
    }
    eip = KSTK_EIP(tsk);
    esp = KSTK_ESP(tsk);
    + up(&mm->mmap_sem);
    }

    wchan = get_wchan(tsk);
    @@ -898,7 +920,7 @@
    nice = tsk->priority;
    nice = 20 - (nice * 20 + DEF_PRIORITY / 2) / DEF_PRIORITY;

    - return sprintf(buffer,"%d (%s) %c %d %d %d %d %d %lu %lu \
    + res = sprintf(buffer,"%d (%s) %c %d %d %d %d %d %lu %lu \
    %lu %lu %lu %lu %lu %ld %ld %ld %ld %ld %ld %lu %lu %ld %lu %lu %lu %lu %lu \
    %lu %lu %lu %lu %lu %lu %lu %lu %d %d\n",
    pid,
    @@ -924,11 +946,11 @@
    tsk->it_real_value,
    tsk->start_time,
    vsize,
    - tsk->mm ? tsk->mm->rss : 0, /* you might want to shift this left 3 */
    + mm ? mm->rss : 0, /* you might want to shift this left 3 */
    tsk->rlim ? tsk->rlim[RLIMIT_RSS].rlim_cur : 0,
    - tsk->mm ? tsk->mm->start_code : 0,
    - tsk->mm ? tsk->mm->end_code : 0,
    - tsk->mm ? tsk->mm->start_stack : 0,
    + mm ? mm->start_code : 0,
    + mm ? mm->end_code : 0,
    + mm ? mm->start_stack : 0,
    esp,
    eip,
    /* The signal information here is obsolete.
    @@ -944,6 +966,9 @@
    tsk->cnswap,
    tsk->exit_signal,
    tsk->processor);
    + if (mm)
    + mmput(mm);
    + return res;
    }

    static inline void statm_pte_range(pmd_t * pmd, unsigned long address, unsigned long size,
    @@ -1021,19 +1046,15 @@

    static int get_statm(int pid, char * buffer)
    {
    - struct task_struct *tsk;
    + struct mm_struct *mm = get_mm(pid);
    int size=0, resident=0, share=0, trs=0, lrs=0, drs=0, dt=0;

    - read_lock(&tasklist_lock);
    - tsk = find_task_by_pid(pid);
    - read_unlock(&tasklist_lock); /* FIXME!! This should be done after the last use */
    - if (!tsk)
    - return 0;
    - if (tsk->mm) {
    - struct vm_area_struct * vma = tsk->mm->mmap;
    -
    + if (mm) {
    + struct vm_area_struct * vma;
    + down(&mm->mmap_sem);
    + vma = mm->mmap;
    while (vma) {
    - pgd_t *pgd = pgd_offset(tsk->mm, vma->vm_start);
    + pgd_t *pgd = pgd_offset(mm, vma->vm_start);
    int pages = 0, shared = 0, dirty = 0, total = 0;

    statm_pgd_range(pgd, vma->vm_start, vma->vm_end, &pages, &shared, &dirty, &total);
    @@ -1051,6 +1072,8 @@
    drs += pages;
    vma = vma->vm_next;
    }
    + up(&mm->mmap_sem);
    + mmput(mm);
    }
    return sprintf(buffer,"%d %d %d %d %d %d %d\n",
    size, resident, share, trs, lrs, drs, dt);
    diff -urN linux-2.3.12-8/fs/proc/link.c linux/fs/proc/link.c
    --- linux-2.3.12-8/fs/proc/link.c Wed Jul 28 02:28:19 1999
    +++ linux/fs/proc/link.c Wed Jul 28 05:44:09 1999
    @@ -82,63 +82,70 @@
    ino &= 0x0000ffff;

    result = ERR_PTR(-ENOENT);
    - read_lock(&tasklist_lock);
    - p = find_task_by_pid(pid);
    - if (!p)
    - goto out_unlock;

    switch (ino) {
    case PROC_PID_CWD:
    - if (!p->fs || !p->fs->pwd)
    - goto out_unlock;
    - result = p->fs->pwd;
    - goto out_dget;
    + read_lock(&tasklist_lock);
    + p = find_task_by_pid(pid);
    + if (p && p->fs && p->fs->pwd)
    + result = dget(p->fs->pwd);
    + read_unlock(&tasklist_lock);
    + break;

    case PROC_PID_ROOT:
    - if (!p->fs || !p->fs->root)
    - goto out_unlock;
    - result = p->fs->root;
    - goto out_dget;
    + read_lock(&tasklist_lock);
    + p = find_task_by_pid(pid);
    + if (p && p->fs && p->fs->root)
    + result = dget(p->fs->root);
    + read_unlock(&tasklist_lock);
    + break;

    case PROC_PID_EXE: {
    + struct mm_struct *mm = NULL;
    struct vm_area_struct * vma;
    - if (!p->mm)
    - goto out_unlock;
    - down(&p->mm->mmap_sem);
    - vma = p->mm->mmap;
    + read_lock(&tasklist_lock);
    + p = find_task_by_pid(pid);
    + if (p)
    + mm = p->mm;
    + if (mm)
    + atomic_inc(&mm->mm_users);
    + read_unlock(&tasklist_lock);
    + if (!mm)
    + break;
    + down(&mm->mmap_sem);
    + vma = mm->mmap;
    while (vma) {
    if ((vma->vm_flags & VM_EXECUTABLE) &&
    vma->vm_file) {
    - result = vma->vm_file->f_dentry;
    - up(&p->mm->mmap_sem);
    - goto out_dget;
    + result = dget(vma->vm_file->f_dentry);
    + break;
    }
    vma = vma->vm_next;
    }
    - up(&p->mm->mmap_sem);
    - goto out_unlock;
    + up(&mm->mmap_sem);
    + mmput(mm);
    + break;
    }
    default:
    if (ino & PROC_PID_FD_DIR) {
    struct file * file;
    + struct files_struct *files = NULL;
    + read_lock(&tasklist_lock);
    + p = find_task_by_pid(pid);
    + if (p)
    + files = p->files;
    + read_unlock(&tasklist_lock);
    + if (!files)
    + break;
    ino &= 0x7fff;
    - if (!p->files) /* shouldn't happen here */
    - goto out_unlock;
    - read_lock(&p->files->file_lock);
    - file = fcheck_task(p, ino);
    - if (!file || !file->f_dentry)
    - goto out_unlock;
    - result = file->f_dentry;
    + read_lock(&files->file_lock);
    + /* XXXXXXXXXXXXXXXXXXXXXXXXXXXXXX */
    + if (ino < files->max_fds &&
    + (file = files->fd[ino]) && file->f_dentry)
    + result = dget(file->f_dentry);
    read_unlock(&p->files->file_lock);
    - goto out_dget;
    }
    }
    -out_dget:
    - result = dget(result);
    -
    -out_unlock:
    - read_unlock(&tasklist_lock);
    -
    out:
    return result;
    }

    -
    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:53    [W:5.622 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site