uprobes: mmap_region() corrupts mm->mm_rb if uprobe_mmap() fails
Get series with b4
b4 am [email protected]Versions newest first
- v1 Latest0 of 1 patch 1 replyIn review
Patches in v1 No applies recorded yet
The patch the cover letter announces is not in this archive.
Reviews trailers per patch
| Patch | William Cohen | Srikar Dronamraju |
|---|---|---|
| Series | Reported-and-tested-by | A-b |
Trailers given in replies are filled; those carried in the patches from earlier versions are outlined with dashes.
Cover letter [PATCH 0/1] uprobes: mmap_region() corrupts mm->mm_rb if uprobe_mmap() fails
https://bugzilla.redhat.com/show_bug.cgi?id=843640
If mmap_region()->uprobe_mmap() fails, unmap_and_free_vma path does unmap_region() but does not remove the soon-to-be-freed vma from rb tree. Actually there are more problems but this is how William noticed this bug.
Perhaps we could do do_munmap() + return in this case, but in fact it is simply wrong to abort if uprobe_mmap() fails. Until at least we move the !UPROBE_COPY_INSN code from install_breakpoint() to uprobe_register().
For example, uprobe_mmap()->install_breakpoint() can fail if the probed insn is not supported (remember, uprobe_register() succeeds if nobody mmaps inode/offset), mmap() should not fail in this case.
dup_mmap()->uprobe_mmap() is wrong too by the same reason, fork() can race with uprobe_register() and fail for no reason if it wins the race and does install_breakpoint() first.
And, if nothing else, both mmap_region() and dup_mmap() return success if uprobe_mmap() fails. Change them to ignore the error code from uprobe_mmap().
--- kernel/fork.c | 4 ++-- mm/mmap.c | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-)
kernel/fork.c +2 -2
@@ -454,8 +454,8 @@ static int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm)if (retval) goto out; -if (file &&uprobe_mmap (tmp)) - goto out; +if (file) +uprobe_mmap (tmp); }/* a new mm has just been created */ arch_dup_mmap (oldmm, mm);
mm/mmap.c +2 -3
@@ -1355,9 +1355,8 @@ out: }else if ((flags &MAP_POPULATE ) && !(flags &MAP_NONBLOCK ))make_pages_present (addr, addr + len); -if (file &&uprobe_mmap (vma)) -/* matching probes but cannot insert */ - goto unmap_and_free_vma; +if (file) +uprobe_mmap (vma);return addr;
Signature · 1 line
1.5.5.1