linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: dave.long@linaro.org (David Long)
To: linux-arm-kernel@lists.infradead.org
Subject: [RFC PATCH] uprobes: copy to user-space xol page with proper cache flushing
Date: Tue, 15 Apr 2014 13:19:27 -0400	[thread overview]
Message-ID: <534D6A1F.70102@linaro.org> (raw)
In-Reply-To: <20140415154637.GA3560@redhat.com>

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):


diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 04709b6..b418626 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -1287,6 +1287,7 @@ static unsigned long xol_get_insn_slot(struct uprobe *uprobe)
 {
        struct xol_area *area;
        unsigned long xol_vaddr;
+       void *kaddr;
 
        area = get_xol_area();
        if (!area)
@@ -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);
 
        return xol_vaddr;
 }


Opinions?  It's possible this approach isn't good enough.  Cache operations and VM
are not my strong suit.

-dl

  parent reply	other threads:[~2014-04-15 17:19 UTC|newest]

Thread overview: 66+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-09  5:58 [PATCH v2] ARM: uprobes need icache flush after xol write Victor Kamensky
2014-04-09  5:58 ` Victor Kamensky
2014-04-09 18:23   ` David Long
2014-04-09 18:45   ` Oleg Nesterov
2014-04-09 19:13     ` Victor Kamensky
2014-04-09 19:19       ` Russell King - ARM Linux
2014-04-11  3:42       ` [RFC PATCH] uprobes: copy to user-space xol page with proper cache flushing David Long
2014-04-11  3:45       ` David Long
2014-04-11  4:36         ` David Miller
2014-04-11 14:26           ` Victor Kamensky
2014-04-11 14:35             ` Oleg Nesterov
2014-04-11 14:55             ` Victor Kamensky
2014-04-11 14:56           ` Oleg Nesterov
2014-04-11 15:22             ` Oleg Nesterov
2014-04-11 15:30               ` Russell King - ARM Linux
2014-04-11 17:24                 ` Oleg Nesterov
2014-04-11 17:38                   ` Oleg Nesterov
2014-04-11 18:00                     ` David Miller
2014-04-11 18:25                       ` Oleg Nesterov
2014-04-11 17:50                   ` Linus Torvalds
2014-04-11 18:02                     ` David Miller
2014-04-11 18:11                       ` Linus Torvalds
2014-04-11 18:19                         ` David Miller
2014-04-11 18:24                           ` Linus Torvalds
2014-04-11 18:58                             ` David Miller
2014-04-11 19:24                               ` Linus Torvalds
2014-04-11 18:13                       ` Victor Kamensky
2014-04-11 18:36                         ` Oleg Nesterov
2014-04-14 18:59                     ` Oleg Nesterov
2014-04-14 20:05                       ` Victor Kamensky
2014-04-14 21:40                         ` Victor Kamensky
2014-04-15 16:26                           ` Oleg Nesterov
2014-04-15 15:46                         ` Oleg Nesterov
2014-04-15 16:46                           ` Victor Kamensky
2014-04-15 17:19                           ` David Long [this message]
2014-04-15 17:38                             ` David Miller
2014-04-15 17:49                               ` Oleg Nesterov
2014-04-15 17:50                                 ` David Miller
2014-04-15 18:07                                   ` Oleg Nesterov
2014-04-15 18:27                                     ` David Miller
2014-04-15 18:46                                       ` Oleg Nesterov
2014-04-15 17:43                             ` Oleg Nesterov
2014-04-15 17:46                               ` David Miller
2014-04-15 18:03                                 ` Oleg Nesterov
2014-04-15 18:30                                   ` David Miller
2014-04-15 18:47                                     ` Russell King - ARM Linux
2014-04-15 18:53                                       ` David Miller
2014-04-15 18:50                                     ` David Miller
2014-04-15 19:29                                       ` Russell King - ARM Linux
2014-04-15 19:51                                         ` David Miller
2014-04-15 19:39                               ` David Long
2014-04-15 19:53                                 ` David Miller
2014-04-16  1:42                                   ` Victor Kamensky
2014-04-16  2:22                                     ` David Miller
2014-04-16  2:24                                     ` David Miller
2014-04-16  3:06                                       ` Victor Kamensky
2014-04-16  3:17                                         ` David Miller
2014-04-11 17:43                 ` David Miller
2014-04-11 15:32               ` Peter Zijlstra
2014-04-11 16:00                 ` Russell King - ARM Linux
2014-04-11 18:39                   ` Peter Zijlstra
2014-04-11 15:37             ` Victor Kamensky
2014-04-11 16:22               ` Oleg Nesterov
2014-04-11 15:42             ` Linus Torvalds
2014-04-11 13:08         ` Oleg Nesterov
2014-04-23 10:45         ` Catalin Marinas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=534D6A1F.70102@linaro.org \
    --to=dave.long@linaro.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).