do page fault in atomic bug on arm

From: mhiramat@kernel.org (Masami Hiramatsu)
To: linux-arm-kernel@lists.infradead.org
Subject: do page fault in atomic bug on arm
Date: Tue, 28 Nov 2017 14:52:21 +0900	[thread overview]
Message-ID: <20171128145221.facdf1b4d74dd029f1edcb83@kernel.org> (raw)
In-Reply-To: <20171127135523.GE31757@n2100.armlinux.org.uk>

On Mon, 27 Nov 2017 13:55:23 +0000
Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

> On Mon, Nov 27, 2017 at 02:36:31PM +0100, Andrew Lunn wrote:
> > On Mon, Nov 27, 2017 at 10:40:34AM +0900, Masami Hiramatsu wrote:
> > > On Sun, 26 Nov 2017 22:59:58 +0800
> > > Alex Shi <alex.shi@linaro.org> wrote:
> > > 
> > > > cc mhiramat at kernel.org
> > > > 
> > > > Thanks a lot for look into this! :)
> > > > 
> > > > Regards
> > > > Alex
> > > > 
> > > > On 11/25/2017 04:25 AM, Russell King - ARM Linux wrote:
> > > > > On Fri, Nov 24, 2017 at 07:27:00PM +0000, Russell King - ARM Linux wrote:
> > > > >> Adding Tixy, as he's more knowledgable in this area - I suggest
> > > > >> someone knowledgable in this area runs
> > > > >>
> > > > >> 	ftracetest test.d/kprobe/multiple_kprobes.tc
> > > > >>
> > > > >> and fixes these bugs... also running the entire ftracetest suite
> > > > >> would probably also be a very good idea.
> > > > > 
> > > > > There's some other stupidities as well:
> > > > > 
> > > > > trace_kprobe: Inserting kprobe at __error_lpae+0
> > > > > trace_kprobe: Inserting kprobe at str_lpae+0
> > > > > trace_kprobe: Inserting kprobe at __error_p+0
> > > > > trace_kprobe: Inserting kprobe at str_p1+0
> > > > > trace_kprobe: Inserting kprobe at str_p2+0
> > > > > trace_kprobe: Could not insert probe at str_p2+0: -22
> > > > > 
> > > > > str_lpae: .asciz "\nError: Kernel with LPAE support, but CPU does not support LPAE.\n"
> > > > > str_p1: .asciz  "\nError: unrecognized/unsupported processor variant (0x"
> > > > > str_p2: .asciz  ").\n"
> > > > > 
> > > > > So we successfully placed a kprobe on some ASCII strings, which are
> > > > > used prior to the kernel being properly up and running, which means
> > > > > we have to use relative references to these strings, and relative
> > > > > references to strings in other sections is not simple.
> > > 
> > > Oops, that's my mistake. It should pick only text symbols.
> > 
> > Hi Masami, Russell
> > 
> > Does the fact you are allowed to put a kprobe on an ASCII string from
> > userspace indicate a real problem? I would of thought the kernel core
> > kprobe could would be looking at the type of a symbol and rejecting
> > such requests. So fix the core, keep this test and make sure you get
> > an EINVAL back?
> 
> As far as I'm aware, the kernel doesn't know whether the address that
> userspace wants to set a kprobe is code or any kind of data.  All that
> it can do is:
> 
> 1. Translate the address to a ksym and offset, looking it up in
>    blacklists/blacklisted sections.
> 2. Look at the "instruction" and reject if it thinks the instruction
>    is unsuitable.
> 
> Making the kernel reject placing a kprobe on a local function ("t" in
> nm's or /proc/kallsyms output) would severely restrict the usefulness
> of kprobes - that would mean you couldn't ever set a kprobe on a static
> function.

Right, and anyway kprobes can put on a specified address via userspace.

In general, kprobe itself checks probe point is in the .text section,
so most of cases are safe as Russell explained.

> 
> So no, I don't agree with you.
> 
> Normally, there won't be strings in the .text section, but we have some
> exceptions in ARM code where we have to have them to make early kernel
> entry code sane without having to jump through excessive hoops.
> 

It's arch-specific reason, so it should be filtered out in arch
specific kprobe code.

> As I've already said, we should not be placing kprobes even on this
> code.  Think about a kprobe placed on the secondary CPU entry point,
> where the CPU is running with the MMU off and there's no exception
> tuable in place.  The kprobe instruction is a CPU undefined instruction,
> so the CPU will try and take the undefined instruction vector, which
> won't be present - the result will be an instant crash.

OK.

> 
> The same is true of the identity-mapped code - which again can run
> with the MMU off, and suffer from exactly the same issue.
> 
> Then we have the exception code itself.  Consider the implications of
> placing the kprobe undefined instruction somewhere in the undefined
> instruction exception handling code - that would result in recursive
> faulting if placed on the SVC paths.
> 
> So no, this problem has nothing to do with symbol types - it's about
> what code is safe for kprobes to be placed.

So there is already NOKPROBE_SYMBOL() macro to make a blacklist so that
we can avoid such recursive faults. We need to identify such place and
put the symbols in it as I did on x86.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>