Jani Hakala wrote:
>
> I got the Oops when I shut down the xfs server in debian (woody). I guess
> the following piece of initscript triggers the oops
>
> stillrunning () {
> if [ "$DAEMON" = "$(cat /proc/$DAEMONPID/cmdline 2> /dev/null)" ]; then
>>EIP; c011ea83 <access_process_vm+73/160> <=====
Yes, I found that bug a few days ago:
if you execute "cat /proc/<PID>/cmdline" while the process calls exit(),
the kernel will oops.
Linus,
did you check the attached patch?
AFAICS we have 2 options to fix the bug:
* add locking around functions that changes current->mm.
There are only 3 functions that do that, and we could use the mmap_sem
semaphore.
* switch current->mm [ my patch does that]
-- Manfred
// $Header$ // Kernel Version: // VERSION = 2 // PATCHLEVEL = 3 // SUBLEVEL = 99 // EXTRAVERSION = -pre5 --- 2.3/kernel/fork.c Wed Apr 12 15:00:33 2000 +++ build-2.3/kernel/fork.c Fri Apr 21 16:52:10 2000 @@ -329,6 +329,9 @@ /* * Decrement the use count and release all resources for an mm. + * + * ptrace assumes that this function is only called with the + * kernel lock held. */ void mmput(struct mm_struct *mm) { --- 2.3/kernel/ptrace.c Fri Mar 24 11:10:18 2000 +++ build-2.3/kernel/ptrace.c Fri Apr 21 16:53:54 2000 @@ -14,6 +14,7 @@ #include <asm/pgtable.h> #include <asm/uaccess.h> +#include <asm/mmu_context.h> /* * Access another process' address space, one page at a time. @@ -80,16 +81,27 @@ int access_process_vm(struct task_struct *tsk, unsigned long addr, void *buf, int len, int write) { - int copied; - struct vm_area_struct * vma; + int copied = 0; + struct mm_struct *mm, *old_mm, *old_activemm; + struct vm_area_struct *vma; + + lock_kernel(); + mm=tsk->mm; + if(!mm) + goto out; + /* concurrent mmput()'s are prevented by the kernel lock */ + atomic_inc(&mm->mm_users); + old_mm = current->mm; + old_activemm = current->active_mm; + current->mm=current->active_mm=mm; + activate_mm(old_activemm,mm); + down(&mm->mmap_sem); - down(&tsk->mm->mmap_sem); - vma = find_extend_vma(tsk, addr); - if (!vma) { - up(&tsk->mm->mmap_sem); - return 0; - } copied = 0; + vma = find_extend_vma(current, addr); + if (!vma) + goto out_restoremm; + for (;;) { unsigned long offset = addr & ~PAGE_MASK; int this_len = PAGE_SIZE - offset; @@ -97,7 +109,7 @@ if (this_len > len) this_len = len; - retval = access_one_page(tsk, vma, addr, buf, this_len, write); + retval = access_one_page(current, vma, addr, buf, this_len, write); copied += retval; if (retval != this_len) break; @@ -118,7 +130,14 @@ vma = vma->vm_next; } - up(&tsk->mm->mmap_sem); +out_restoremm: + up(&mm->mmap_sem); + current->mm = old_mm; + current->active_mm = old_activemm; + activate_mm(mm, old_activemm); + mmput(mm); +out: + unlock_kernel(); return copied; } --- 2.3/Documentation/vm/locking Fri Jan 21 13:08:06 2000 +++ build-2.3/Documentation/vm/locking Fri Apr 21 16:52:10 2000 @@ -136,3 +136,44 @@ Swap device deletion code currently breaks all the scache assumptions, since it grabs neither mmap_sem nor page_table_lock. + +Locking requirements for ptrace +------------------------------- + +ptrace(2) accesses another process' address space, and the target process +could be in the middle of functions such as exec_mmap(), daemonize() or +do_exit(). + +The locking sequence must be (error handling ommitted): + + read_lock(&task_list_lock); + tsk=find_task_by_pid(pid); + get_task_struct(tsk); + read_unlock(&task_list_lock); + task_lock(tsk); + + lock_kernel(); + mm=current->mm; + atomic_inc(&mm->m_users); + unlock_kernel(); + +If you only need the mm pointer without the task struct: + + lock_kernel(); + read_lock(&task_list_lock); + tsk=find_task_by_pid(pid); + mm=tsk->mm; + atomic_inc(&mm->m_users); + read_unlock(&task_list_lock); + unlock_kernel(); + +Both versions assume that mmput() is only called with the kernel +lock acquired. + +Locking requirements for handle_mm_fault() +------------------------------------------ + +mm->mmap_sem must be downed, and tsk must be current. +We could support calls with tsk!=current, but then all functions +that change "current->mm" from a non-NULL value (start_lazy_tlb(), +exec_mmap(), __exit_mm()) must acquire the mmap semaphore.
- 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/
This archive was generated by hypermail 2b29 : Sun Apr 23 2000 - 21:00:20 EST