Hi James, On Tue, 8 Jan 2019 17:13:36 +0000 James Morse <james.mo...@arm.com> wrote:
> Hi! > > On 08/01/2019 02:39, Masami Hiramatsu wrote: > > On Thu, 3 Jan 2019 17:05:18 +0000 > > James Morse <james.mo...@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. > > As I pointed, the exception_table contains some range of code which inside > > functions, must be smaller than function. > > Since those instructions are expected to cause exception (that is main > > reason > > why it can not be probed on arm64), I thought such situation was similar to > > the limitation of instruction. > > > > So I think below will be better. > > ---- > > Please do not blacklisting instructions on exception_table, > > since those are smaller than one function. > > ---- > > I keep tripping over this because the exception_table lists addresses that are > allowed to fault. Nothing looks at the instruction, and we happily kprobe the > same instruction elsewhere. Thanks! > > (based on my assumptions about where you are going next!,), How about: > | The blacklist is exposed via debugfs as a list of symbols. extable entries > are > | smaller, so must be filtered out by arch_prepare_kprobe(). This looks much better for me too :) Should I resend with the description? Thank you! > > (only we currently have more than one blacklist...) > > > Thanks, > > James -- Masami Hiramatsu <mhira...@kernel.org>
