lkml.org 
[lkml]   [2019]   [Jan]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
From
SubjectRe: [PATCH v2 4/4] arm64: kprobes: Use arch_populate_kprobe_blacklist()
Date
Hello,

On 15/01/2019 06:25, Masami Hiramatsu wrote:
> Use arch_populate_kprobe_blacklist() instead of
> arch_within_kprobe_blacklist() so that we can see the full
> blacklisted symbols under the debugfs.
> diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c
> index b9e9758b6534..6c066c34c8a4 100644
> --- a/arch/arm64/kernel/probes/kprobes.c
> +++ b/arch/arm64/kernel/probes/kprobes.c
> @@ -465,26 +465,30 @@ kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr)
> return DBG_HOOK_HANDLED;
> }
>
> -bool arch_within_kprobe_blacklist(unsigned long addr)
> +int __init arch_populate_kprobe_blacklist(void)
> {
> - if ((addr >= (unsigned long)__kprobes_text_start &&
> - addr < (unsigned long)__kprobes_text_end) ||
> - (addr >= (unsigned long)__entry_text_start &&
> - addr < (unsigned long)__entry_text_end) ||
> - (addr >= (unsigned long)__idmap_text_start &&
> - addr < (unsigned long)__idmap_text_end) ||

> - in_exception_text(addr))

You added this one in the previous patch, but it disappears here.


> - return true;
> -
> - if (!is_kernel_in_hyp_mode()) {
> - if ((addr >= (unsigned long)__hyp_text_start &&
> - addr < (unsigned long)__hyp_text_end) ||
> - (addr >= (unsigned long)__hyp_idmap_text_start &&
> - addr < (unsigned long)__hyp_idmap_text_end))
> - return true;
> - }
> -
> - return false;
> + int ret;


> + ret = kprobe_add_area_blacklist((unsigned long)__kprobes_text_start,
> + (unsigned long)__kprobes_text_end);
> + if (ret)
> + return ret;

Now that we have arch_populate_kprobe_blacklist(), does the arch-code need to
blacklist the kprobes section itself?

The weak arch_within_kprobe_blacklist() will test it at kprobe-load time, and
populate_kprobe_blacklist() adds it to the list before it calls
arch_populate_kprobe_blacklist().

Won't this result in duplicate entries?


> + ret = kprobe_add_area_blacklist((unsigned long)__entry_text_start,
> + (unsigned long)__entry_text_end);
> + if (ret)
> + return ret;

> + ret = kprobe_add_area_blacklist((unsigned long)__idmap_text_start,
> + (unsigned long)__idmap_text_end);

> + if (ret || is_kernel_in_hyp_mode())
> + return ret;


Hmmm, I think we have a bug here today.

This is saying we can kprobe KVM when we have VHE, because all of KVMs code runs
at the same exception-level as the kernel. Which is true...
But KVM switches VBAR_EL1, so if we run over one of kprobes BRK instructions,
we're going to hyp-panic, because KVM doesn't handle synchronous exceptions from
EL2.

The __hyp_text also contains the guest entry/exit code, which we mustn't probe,
even on VHE.

I think we should always blacklist the __hyp_text, and KVM should mark its
vhe-only functions with __kprobes. I'll post patches for this.


Thanks,

James


> + ret = kprobe_add_area_blacklist((unsigned long)__hyp_text_start,
> + (unsigned long)__hyp_text_end);
> + if (ret)
> + return ret;
> + ret = kprobe_add_area_blacklist((unsigned long)__hyp_idmap_text_start,
> + (unsigned long)__hyp_idmap_text_end);
> + return ret;
> }
>
> void __kprobes __used *trampoline_probe_handler(struct pt_regs *regs)
>

\
 
 \ /
  Last update: 2019-01-21 13:21    [W:0.136 / U:11.112 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site