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 v4] ARM: uprobes xol write directly to userspace
Date: Mon, 21 Apr 2014 12:16:42 -0400	[thread overview]
Message-ID: <5355446A.9080302@linaro.org> (raw)
In-Reply-To: <20140416222548.GL24070@n2100.arm.linux.org.uk>

On 04/16/14 18:25, Russell King - ARM Linux wrote:

> Our I-caches don't snoop/see the D-cache at all - so writes need to be
> pushed out to what we call the "point of unification" where the I and D
> streams meet.  For anything we care about, that's normally the L2 cache -
> L1 cache is harvard, L2 cache is unified.
> 
> Hence, we don't care which D-alias (if any) the data is written, so long
> as it's pushed out of the L1 data cache so that it's visible to the L1
> instruction cache.
> 
> If we're writing via a different mapping to that which is being executed,
> I think the safest thing to do is to flush it out of the L1 D-cache at
> the address it was written, and then flush any stale line from the L1
> I-cache using the user address.  This is quite a unique requirement, and
> we don't have anything which covers it.  The closest you could get is
> to that using existing calls is:
> 
> 1. write the new instruction
> 2. flush_dcache_page()
> 3. flush_cache_user_range() using the user address
> 
> and I think that should be safe on all the above cache types.
> 

It doesn't feel to me like we yet have a clear consensus on the appropriate
near or long-term fix for this problem.  I'm worried time is short to get a
fix in for v3.15.  I'm not sure how elegant that fix needs to be.  I've gotten
good test runs using a modified/simplified version of Victor's arch callback
and a slight variation of Russell's sequence of operations from above:

void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
			const void *src, int len)
{
	void *kaddr = kmap_atomic(page);

#ifdef CONFIG_SMP
	preempt_disable();
#endif
	memcpy(kaddr + (vaddr & ~PAGE_MASK), src, len);
	clean_dcache_area(kaddr, len);
	flush_cache_user_range(vaddr, vaddr + len);
#ifdef CONFIG_SMP
	preempt_enable();
#endif
	kunmap_atomic(kaddr);
}


I have to say using clean_dcache_area() to write back the two words that changed
(and rest of the cache line of course) seems more appropriate than flushing a
whole page.  Are there implications in doing that which makes this a bad idea
though?

At any rate, for v3.15 do we want to persue the more complex solutions with
"congruent" mappings and use of copy_to_user(), or just something like the above
(plus the rest of Victors v3 patch)?  I'm sure Oleg is even less happy than me
about yet another arch_ callback but we can hold out the hope that a more elegant
solution can follow in the next release.  One that might introduce risk we can't
accept in v3.15 right now (e.g.: mapping the xol area writeable for all
architectures).

I have also tested (somewhat) both Victor's unmodified v3 and v4 patches on
exynos 5250 and found them to work.

Thanks,
-dl

  parent reply	other threads:[~2014-04-21 16:16 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-16  5:31 [RFC PATCH v4] ARM: uprobes xol write directly to userspace Victor Kamensky
2014-04-16  5:31 ` Victor Kamensky
2014-04-16 14:51   ` Oleg Nesterov
2014-04-16 15:00     ` David Miller
2014-04-16 16:43       ` Oleg Nesterov
2014-04-16 17:38         ` David Miller
2014-04-16 19:18           ` Oleg Nesterov
2014-04-16 19:37             ` David Miller
2014-04-16 20:24               ` David Long
2014-04-16 21:21                 ` David Miller
2014-04-16 22:01                   ` Victor Kamensky
2014-04-16 22:25                   ` Russell King - ARM Linux
2014-04-16 23:19                     ` David Long
2014-04-21 16:16                     ` David Long [this message]
2014-04-21 16:41                       ` Linus Torvalds
2014-04-21 17:56                       ` Victor Kamensky
2014-04-16 19:53             ` Russell King - ARM Linux
2014-04-16 20:23               ` Oleg Nesterov

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=5355446A.9080302@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).