Messages in this thread Patch in this message | | | Date | Thu, 27 May 2010 21:56:09 +0200 | From | Oleg Nesterov <> | Subject | [PATCH 1/2] blackfin: ptrace: fix the unsafe usage of mm/find_vma in is_user_addr_valid() |
| |
1. is_user_addr_valid() must not assume child->mm != NULL, we can race with SIGKILL. Use get_task_mm().
2. find_vma() needs mm->mmap_sem, we can race with another thread which shares ->mm with the tracee.
3. Move the simple FIXED_CODE_ checks up, before other potentially costly checks, and thus outside of mmap_sem.
4. Uninline it, it is fat and has 2 callers.
Note: we scan mm->context.sram_list under mmap_sem too. Currently this is unnecessary, but see the next patch.
Signed-off-by: Oleg Nesterov <oleg@redhat.com> ---
arch/blackfin/kernel/ptrace.c | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)
--- 34-rc1/arch/blackfin/kernel/ptrace.c~IUAV_1_GET_MM_TAKE_SEM 2010-05-27 17:52:36.000000000 +0200 +++ 34-rc1/arch/blackfin/kernel/ptrace.c 2010-05-27 20:07:10.000000000 +0200 @@ -113,29 +113,40 @@ put_reg(struct task_struct *task, long r /* * check that an address falls within the bounds of the target process's memory mappings */ -static inline int is_user_addr_valid(struct task_struct *child, +static int is_user_addr_valid(struct task_struct *child, unsigned long start, unsigned long len) { + struct mm_struct *mm; struct vm_area_struct *vma; struct sram_list_struct *sraml; + int ret = 0; /* overflow */ if (start + len < start) return -EIO; + if (start >= FIXED_CODE_START && start + len < FIXED_CODE_END) + return 0; - vma = find_vma(child->mm, start); + mm = get_task_mm(child); + if (!mm) + return -EIO; + + down_read(&mm->mmap_sem); + vma = find_vma(mm, start); if (vma && start >= vma->vm_start && start + len <= vma->vm_end) - return 0; + goto out; - for (sraml = child->mm->context.sram_list; sraml; sraml = sraml->next) + for (sraml = mm->context.sram_list; sraml; sraml = sraml->next) if (start >= (unsigned long)sraml->addr && start + len < (unsigned long)sraml->addr + sraml->length) - return 0; + goto out; - if (start >= FIXED_CODE_START && start + len < FIXED_CODE_END) - return 0; + ret = -EIO; +out: + up_read(&mm->mmap_sem); + mmput(mm); - return -EIO; + return ret; } /*
| |