Messages in this thread | | | Date | Tue, 15 Jan 2019 14:49:55 +0900 | From | Masami Hiramatsu <> | Subject | Re: [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>
| |