[RFC PATCH] uprobes: copy to user-space xol page with proper cache flushing
Oleg Nesterov
oleg at redhat.com
Tue Apr 15 10:43:30 PDT 2014
On 04/15, David Long wrote:
>
> On 04/15/14 11:46, Oleg Nesterov wrote:
> >
> > But. Please do not add copy_to_user_page() into copy_to_page() (as your patch
> > did). This is certainly not what uprobe_write_opcode() wants, we do not want
> > or need "flush" in this case. The same for __create_xol_area().
> >
>
> It looked me like a call to a new __copy_to_user_page(current->mm, ...) in xol_get_insn_slot()
> would be in line with David Miller's suggestion and would cure the problem on ARM (and hopefuly
> be more philosophically correct for all architectures):
David, let me repeat. I am not going to argue even if I obviously like
the Linus's suggestion more.
I only argued with the suggestions to follow the __access_remote_vm()
logic.
> @@ -1297,13 +1298,11 @@ static unsigned long xol_get_insn_slot(struct uprobe *uprobe)
> return 0;
>
> /* Initialize the slot */
> - copy_to_page(area->page, xol_vaddr,
> - &uprobe->arch.ixol, sizeof(uprobe->arch.ixol));
> - /*
> - * We probably need flush_icache_user_range() but it needs vma.
> - * This should work on supported architectures too.
> - */
> - flush_dcache_page(area->page);
> + kaddr = kmap_atomic(area->page);
> + __copy_to_user_page(current->mm, area->page, xol_vaddr,
> + kaddr + (xol_vaddr & ~PAGE_MASK),
> + &uprobe->arch.ixol, sizeof(uprobe->arch.ixol), true);
> + kunmap_atomic(kaddr);
This all is fine, but you need to introduce __copy_to_user_page() first.
And you need to do this for every arch. And you need to verify that it
does not need mmap_sem. It seems that at least alpha needs this lock.
And as Russel pointed out (again, again, I could easily misunderstood him)
you need preempt_disable() around memcpy + flush, so down_read(mmap_sem)
should probably go into __copy_to_user_page(). And I am not sure this
helper needs "struct mm_struct *mm" argument, but this is minor.
Finally, let me repeat, you should verify that this
__copy_to_user_page(page, uaddr, kaddr) will not something bad if uaddr
is not mmapped, or its mapping do not match area->page.
Good luck ;)
Oleg.
More information about the linux-arm-kernel
mailing list