| Date | Sun, 16 Oct 2011 18:13:59 +0200 | From | Oleg Nesterov <> | Subject | [PATCH 6/X] uprobes: reimplement xol_add_vma() via install_special_mapping() |
| |
I apologize in advance if this was already discussed, but I just can't understand why xol_add_vma() does not use install_special_mapping(). Unless I missed something this should work and this has the following advantages:
- we can avoid override_creds() hacks, install_special_mapping() fools security_file_mmap() passing prot/flags = 0
- no need to play with vma after do_mmap_pgoff()
- no need for get_user_pages(FOLL_WRITE/FOLL_FORCE) hack
- no need for do_munmap() if get_user_pages() fails
- this protects us from mprotect(READ/WRITE)
- this protects from MADV_DONTNEED, the page will be correctly re-instantiated from area->page
- this makes xol_vma more "cheap", swapper can't see this page and we avoid the meaningless add_to_swap/pageout.
Note that, before this patch, area->page can't be removed from the swap cache anyway (we have the reference). And it must not, uprobes modifies this page directly.
Note on vm_flags:
- we do not use VM_DONTEXPAND, install_special_mapping() adds it
- VM_IO protects from MADV_DOFORK
- I am not sure, may be some archs need VM_READ along with EXEC?
Anything else I have missed? ---
kernel/uprobes.c | 42 +++++++++++++++++++----------------------- 1 files changed, 19 insertions(+), 23 deletions(-)
diff --git a/kernel/uprobes.c b/kernel/uprobes.c index b59af3b..038f21c 100644 --- a/kernel/uprobes.c +++ b/kernel/uprobes.c @@ -1045,53 +1045,49 @@ void munmap_uprobe(struct vm_area_struct *vma) /* Slot allocation for XOL */ static int xol_add_vma(struct uprobes_xol_area *area) { - const struct cred *curr_cred; struct vm_area_struct *vma; struct mm_struct *mm; - unsigned long addr; + unsigned long addr_hint; int ret; + area->page = alloc_page(GFP_HIGHUSER); + if (!area->page) + return -ENOMEM; + mm = current->mm; down_write(&mm->mmap_sem); ret = -EALREADY; if (mm->uprobes_xol_area) goto fail; - - ret = -ENOMEM; /* * Find the end of the top mapping and skip a page. - * If there is no space for PAGE_SIZE above - * that, mmap will ignore our address hint. - * - * override credentials otherwise anonymous memory might - * not be granted execute permission when the selinux - * security hooks have their way. + * If there is no space for PAGE_SIZE above that, + * this hint will be ignored. */ vma = rb_entry(rb_last(&mm->mm_rb), struct vm_area_struct, vm_rb); - addr = vma->vm_end + PAGE_SIZE; - curr_cred = override_creds(&init_cred); - addr = do_mmap_pgoff(NULL, addr, PAGE_SIZE, PROT_EXEC, MAP_PRIVATE, 0); - revert_creds(curr_cred); + addr_hint = vma->vm_end + PAGE_SIZE; - if (IS_ERR_VALUE(addr)) + area->vaddr = get_unmapped_area(NULL, addr_hint, PAGE_SIZE, 0, 0); + if (IS_ERR_VALUE(area->vaddr)) { + ret = area->vaddr; goto fail; + } - vma = find_vma(mm, addr); - /* Don't expand vma on mremap(). */ - vma->vm_flags |= VM_DONTEXPAND | VM_DONTCOPY; - if (get_user_pages(current, mm, addr, 1, 1, 1, - &area->page, NULL) != 1) { - do_munmap(mm, addr, PAGE_SIZE); + ret = install_special_mapping(mm, area->vaddr, PAGE_SIZE, + VM_EXEC|VM_MAYEXEC | VM_DONTCOPY|VM_IO, + &area->page); + if (ret) goto fail; - } - area->vaddr = addr; smp_wmb(); /* pairs with get_uprobes_xol_area() */ mm->uprobes_xol_area = area; ret = 0; fail: up_write(&mm->mmap_sem); + if (ret) + __free_page(area->page); + return ret; }
|