lkml.org 
[lkml]   [2019]   [Jan]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] arm64: kprobes: Move extable address check into arch_prepare_kprobe()
On Fri, 11 Jan 2019 18:22:38 +0000
James Morse <james.morse@arm.com> wrote:

> Hi,
>
> On 09/01/2019 02:05, Masami Hiramatsu wrote:
> > On Tue, 8 Jan 2019 17:13:36 +0000
> > James Morse <james.morse@arm.com> wrote:
> >> On 08/01/2019 02:39, Masami Hiramatsu wrote:
> >>> On Thu, 3 Jan 2019 17:05:18 +0000
> >>> James Morse <james.morse@arm.com> wrote:
> >>>> On 17/12/2018 06:40, Masami Hiramatsu wrote:
> >>>>> Move extable address check into arch_prepare_kprobe() from
> >>>>> arch_within_kprobe_blacklist().
> >>>>
> >>>> I'm trying to work out the pattern for what should go in the blacklist, and what
> >>>> should be rejected by the arch code.
> >>>>
> >>>> It seems address-ranges should be blacklisted as the contents don't matter.
> >>>> easy-example: the idmap text.
> >>>
> >>> Yes, more precisely, the code smaller than a function (symbol), it must be
> >>> rejected by arch_prepare_kprobe(), since blacklist is poplated based on
> >>> kallsyms.
> >>
> >> Ah, okay, so the pattern is the blacklist should only be for whole symbols,
> >> (which explains why its usually based on sections).
> >
> > Correct. Actually, the blacklist is generated based on the symbol info
> > from symbol address.
> >
> >> I see kprobe_add_ksym_blacklist() would go wrong if you give it something like:
> >> platform_drv_probe+0x50/0xb0, as it will log platform_drv_probe+0x50 as the
> >> start_addr and platform_drv_probe+0x50+0xb0 as the end.
> >
> > Yes, it expects given address is the entry of a symbol.
>
> >> But how does anything from the arch code's blacklist get into the
> >> kprobe_blacklist list?
> >
> > It should be done via arch_populate_kprobe_blacklist().
>
> >> We don't have an arch_populate_kprobe_blacklist(), so rely on
> >> within_kprobe_blacklist() calling arch_within_kprobe_blacklist() with the
> >> address, as well as walking kprobe_blacklist.
> >>
> >> Is this cleanup ahead of a series that does away with
> >> arch_within_kprobe_blacklist() so that debugfs list is always complete?
> >
> > Right, after this cleanup, I will send arch_populate_kprobe_blacklist()
> > patch for arm64 and others. My plan is to move all arch_within_kprobe_blacklist()
> > to arch_populate_kprobe_blacklist() so that user can get more precise blacklist
> > via debugfs.
>
> Thanks, now it all makes sense!
>
> Reviewed-by: James Morse <james.morse@arm.com>

Thanks!

>
>
> Could you include a paragraph like that in the cover-letter or commit-message?
> The 'fix' in the cover-letter subject had me looking for the bug!

Ok, I'll update commit message with your reviewed-by.

Thank you!

>
>
> Thanks,
>
> James


--
Masami Hiramatsu <mhiramat@kernel.org>

\
 
 \ /
  Last update: 2019-01-15 06:50    [W:0.053 / U:0.876 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site