Re: [PATCH 7/9] MIPS: uprobes: Flush icache via kernel address

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 09/22/2016 02:15 PM, James Hogan wrote:
On Wed, Sep 21, 2016 at 11:15:55AM -0700, Leonid Yegoshin wrote:
On 09/21/2016 06:26 AM, Ralf Baechle wrote:
On Thu, Sep 01, 2016 at 05:30:13PM +0100, James Hogan wrote:

Update arch_uprobe_copy_ixol() to use the kmap_atomic() based kernel
address to flush the icache with flush_icache_range(), rather than the
user mapping. We have the kernel mapping available anyway and this
avoids having to switch to using the new __flush_icache_user_range() for
the sake of Enhanced Virtual Addressing (EVA) where flush_icache_range()
will become ineffective on user addresses.

Signed-off-by: James Hogan <james.hogan@xxxxxxxxxx>
Cc: Ralf Baechle <ralf@xxxxxxxxxxxxxx>
Cc: Leonid Yegoshin <leonid.yegoshin@xxxxxxxxxx>
Cc: linux-mips@xxxxxxxxxxxxxx
---
   arch/mips/kernel/uprobes.c | 13 ++++---------
   1 file changed, 4 insertions(+), 9 deletions(-)

diff --git a/arch/mips/kernel/uprobes.c b/arch/mips/kernel/uprobes.c
index 8452d933a645..9a507ab1ea38 100644
--- a/arch/mips/kernel/uprobes.c
+++ b/arch/mips/kernel/uprobes.c
@@ -301,19 +301,14 @@ int set_orig_insn(struct arch_uprobe *auprobe, struct mm_struct *mm,
   void __weak arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
   				  void *src, unsigned long len)
   {
-	void *kaddr;
+	void *kaddr, kstart;
/* Initialize the slot */
   	kaddr = kmap_atomic(page);
-	memcpy(kaddr + (vaddr & ~PAGE_MASK), src, len);
+	kstart = kaddr + (vaddr & ~PAGE_MASK);
+	memcpy(kstart, src, len);
+	flush_icache_range(kstart, kstart + len);
   	kunmap_atomic(kaddr);
-
-	/*
-	 * The MIPS version of flush_icache_range will operate safely on
-	 * user space addresses and more importantly, it doesn't require a
-	 * VMA argument.
-	 */
-	flush_icache_range(vaddr, vaddr + len);
I can't convince myself this is right wrt. to cache aliases ...

    Ralf

It is incorrect if there is I-cache aliasing (very rare) and there is no
HIGHMEM cache aliasing fix (not fixed). But top tree Linux doesn't work
with I-cache aliasing either.
Well its pretty trivial to just use the newly introduced
__flush_icache_user_range() on the user address instead of using
flush_icache_range().

It may not work - you should flush kernel Dcache but user Icache and __flush_icache_user_range() doesn't do it. Some CPU may accept an aliasing CACHE instruction and flush both colors and it can work in this case.

Besides that, I had no time to research - does uprobe keep data on the same page as code? If yes, then we may want to make a special flush sequence for cache aliasing systems here. (User-Dcache, Write-to-page, Kernel-Dcache, User-Icache).

- Leonid




[Index of Archives]     [Linux MIPS Home]     [LKML Archive]     [Linux ARM Kernel]     [Linux ARM]     [Linux]     [Git]     [Yosemite News]     [Linux SCSI]     [Linux Hams]

  Powered by Linux