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@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.
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.
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 table 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.
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.