| Message ID | 154998628802.21869.4041705065539411318.stgit@devbox (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: kprobes: Update blacklist checking on arm64 | expand |
Hi Masami, On Wed, Feb 13, 2019 at 12:44:48AM +0900, 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. > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > --- > Changes in v3 > - Do not populate blacklist in __kprobe_text in > arch_populate_kprobe_blacklist(), since it is already > populated in populate_kprobe_blacklist(). > - Add exception entry text blacklist since those are rejected > by in_exception_text(). > --- > arch/arm64/kernel/probes/kprobes.c | 45 +++++++++++++++++++++--------------- > 1 file changed, 26 insertions(+), 19 deletions(-) > > diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c > index 194262fca5cd..37d913f33a89 100644 > --- a/arch/arm64/kernel/probes/kprobes.c > +++ b/arch/arm64/kernel/probes/kprobes.c > @@ -465,26 +465,33 @@ kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr) > return DBG_HOOK_HANDLED; > } > > -bool arch_within_kprobe_blacklist(unsigned long addr) I think it would be useful to add a comment here so that we remember why this code is the way it is: /* * Provide a blacklist of symbols identifying ranges which cannot be kprobed. * This blacklist is exposed to userspace via debugfs (kprobes/blacklist). */ With that, you can have my ack for the series: Acked-by: Will Deacon <will.deacon@arm.com> Catalin -- can you pick these up with that comment added, please? Cheers, Will
On Thu, 14 Feb 2019 15:55:22 +0000 Will Deacon <will.deacon@arm.com> wrote: > Hi Masami, > > On Wed, Feb 13, 2019 at 12:44:48AM +0900, 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. > > > > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> > > --- > > Changes in v3 > > - Do not populate blacklist in __kprobe_text in > > arch_populate_kprobe_blacklist(), since it is already > > populated in populate_kprobe_blacklist(). > > - Add exception entry text blacklist since those are rejected > > by in_exception_text(). > > --- > > arch/arm64/kernel/probes/kprobes.c | 45 +++++++++++++++++++++--------------- > > 1 file changed, 26 insertions(+), 19 deletions(-) > > > > diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c > > index 194262fca5cd..37d913f33a89 100644 > > --- a/arch/arm64/kernel/probes/kprobes.c > > +++ b/arch/arm64/kernel/probes/kprobes.c > > @@ -465,26 +465,33 @@ kprobe_breakpoint_handler(struct pt_regs *regs, unsigned int esr) > > return DBG_HOOK_HANDLED; > > } > > > > -bool arch_within_kprobe_blacklist(unsigned long addr) > > I think it would be useful to add a comment here so that we remember > why this code is the way it is: > > /* > * Provide a blacklist of symbols identifying ranges which cannot be kprobed. > * This blacklist is exposed to userspace via debugfs (kprobes/blacklist). > */ Agreed. This looks good to me too. > > With that, you can have my ack for the series: > > Acked-by: Will Deacon <will.deacon@arm.com> Thank you! > > Catalin -- can you pick these up with that comment added, please? > > Cheers, > > Will
diff --git a/arch/arm64/kernel/probes/kprobes.c b/arch/arm64/kernel/probes/kprobes.c index 194262fca5cd..37d913f33a89 100644 --- a/arch/arm64/kernel/probes/kprobes.c +++ b/arch/arm64/kernel/probes/kprobes.c @@ -465,26 +465,33 @@ 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) || - (addr >= (unsigned long)__hyp_text_start && - addr < (unsigned long)__hyp_text_end) || - in_exception_text(addr)) - return true; - - if (!is_kernel_in_hyp_mode()) { - if ((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)__entry_text_start, + (unsigned long)__entry_text_end); + if (ret) + return ret; + ret = kprobe_add_area_blacklist((unsigned long)__irqentry_text_start, + (unsigned long)__irqentry_text_end); + if (ret) + return ret; + ret = kprobe_add_area_blacklist((unsigned long)__exception_text_start, + (unsigned long)__exception_text_end); + if (ret) + return ret; + ret = kprobe_add_area_blacklist((unsigned long)__idmap_text_start, + (unsigned long)__idmap_text_end); + if (ret) + return ret; + ret = kprobe_add_area_blacklist((unsigned long)__hyp_text_start, + (unsigned long)__hyp_text_end); + if (ret || is_kernel_in_hyp_mode()) + 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)
Use arch_populate_kprobe_blacklist() instead of arch_within_kprobe_blacklist() so that we can see the full blacklisted symbols under the debugfs. Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org> --- Changes in v3 - Do not populate blacklist in __kprobe_text in arch_populate_kprobe_blacklist(), since it is already populated in populate_kprobe_blacklist(). - Add exception entry text blacklist since those are rejected by in_exception_text(). --- arch/arm64/kernel/probes/kprobes.c | 45 +++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 19 deletions(-)