do page fault in atomic bug on arm

On Mon, 27 Nov 2017 13:55:23 +0000
Russell King - ARM Linux [off-list ref] wrote:

On Mon, Nov 27, 2017 at 02:36:31PM +0100, Andrew Lunn wrote:

quoted

On Mon, Nov 27, 2017 at 10:40:34AM +0900, Masami Hiramatsu wrote:

quoted

On Sun, 26 Nov 2017 22:59:58 +0800
Alex Shi [off-list ref] wrote:

quoted

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:

quoted

On Fri, Nov 24, 2017 at 07:27:00PM +0000, Russell King - ARM Linux wrote:

quoted

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 [off-list ref]