Messages in this thread Patch in this message |  | | | Date | Thu, 31 May 2012 20:53:39 +0200 | | From | Oleg Nesterov <> | | Subject | Re: [PATCH 1/3] uprobes: install_breakpoint() should fail if is_swbp_insn() == T |
| |
On 05/30, Peter Zijlstra wrote: > > register's vma > iteration is very careful not to have the same vma twice,
Hmm. I am wondering if it is careful enough...
Just in case, I think your patch is great. But it seems to me there is another problem.
__find_next_vma_info() checks tmpvi->mm == vma->vm_mm to detect the already visited mm/vma. However, afaics this can be false positive?
The caller, register_for_each_vma(), does mmput() and after that this memory can be freed and re-used as another mm_struct.
I'll recheck this, but perhaps we need something like below?
Oleg.
--- x/kernel/events/uprobes.c +++ x/kernel/events/uprobes.c @@ -851,7 +851,6 @@ static int register_for_each_vma(struct list_del(&vi->probe_list); kfree(vi); up_write(&mm->mmap_sem); - mmput(mm); continue; } vaddr = vma_address(vma, uprobe->offset); @@ -860,7 +859,6 @@ static int register_for_each_vma(struct list_del(&vi->probe_list); kfree(vi); up_write(&mm->mmap_sem); - mmput(mm); continue; } @@ -870,7 +868,6 @@ static int register_for_each_vma(struct remove_breakpoint(uprobe, mm, vi->vaddr); up_write(&mm->mmap_sem); - mmput(mm); if (is_register) { if (ret && ret == -EEXIST) ret = 0; @@ -881,6 +878,7 @@ static int register_for_each_vma(struct list_for_each_entry_safe(vi, tmpvi, &try_list, probe_list) { list_del(&vi->probe_list); + mmput(vi->mm) kfree(vi); }
|  |