[RFC PATCH] uprobes: copy to user-space xol page with proper cache flushing

[Linus Torvalds]

On Fri, Apr 11, 2014 at 10:24 AM, Oleg Nesterov <oleg at redhat.com> wrote:
> +static void arch_uprobe_copy_ixol(struct xol_area *area, unsigned long vaddr,
> +                                       struct arch_uprobe *auprobe)
> +{
> +#ifndef ARCH_UPROBE_XXX
> +       copy_to_page(area->page, vaddr, &auprobe->ixol, sizeof(&auprobe->ixol));
> +       /*
> +        * We probably need flush_icache_user_range() but it needs vma.
> +        * If this doesn't work define ARCH_UPROBE_XXX.
> +        */
> +       flush_dcache_page(area->page);
> +#else
> +       struct mm_struct *mm = current->mm;
> +       struct vm_area_struct *vma;
> +
> +       down_read(&mm->mmap_sem);
> +       vma = find_exact_vma(mm, area->vaddr, area->vaddr + PAGE_SIZE);
> +       if (vma) {
> +               void *kaddr = kmap_atomic(area->page);
> +               copy_to_user_page(vma, area->page,
> +                                       vaddr, kaddr + (vaddr & ~PAGE_MASK),
> +                                       &auprobe->ixol, sizeof(&auprobe->ixol));
> +               kunmap_atomic(kaddr);
> +       }
> +       up_read(&mm->mmap_sem);
> +#endif

Yeah, no, this is wrong.

the fact is, the *only* possible use for the whole "vma" argument is
the "can this be executable" optimization, afaik.

So I really think we should just have a fixed
"flush_icache_page(page,vaddr)" function. Maybe add a "len" argument,
to allow architectures that have to loop over cachelines to do just a
minimal loop.

Then, to do the vma optimization, let's introduce a new

    arch_needs_icache_flush(vma, page)

function, which on cache coherent architectures can just be zero (or
one, since the icache flush itself will be a no-op, so it doesn't
really matter), and on others it can do the "have we executed from
this page", which may involve just looking at the vma..

Then the uprobe case can just do

    copy_to_page()
    flush_dcache_page()
    flush_icache_page()

and be done with it.

Hmm?

                Linus