[PATCH 6/X] uprobes: reimplement xol_add_vma() viainstall_special_mapping()

[Oleg Nesterov]

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;
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/