| Message ID | 20200320180032.799569116@linutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | x86/entry: Consolidation part II (syscalls) | expand |
Hi Thomas, On Fri, 20 Mar 2020 19:00:00 +0100 Thomas Gleixner <tglx@linutronix.de> wrote: > Instrumentation is forbidden in the .noinstr.text section. Make kprobes > respect this. > > This lacks support for .noinstr.text sections in modules, which is required > to handle VMX and SVM. > Would you have any plan to list or mark the noinstr symbols on some debugfs interface? I need a blacklist of those symbols so that user (and perf-probe) can check which function can not be probed. It is just calling kprobe_add_area_blacklist() like below. diff --git a/kernel/kprobes.c b/kernel/kprobes.c index 2625c241ac00..4835b644bd2b 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -2212,6 +2212,10 @@ static int __init populate_kprobe_blacklist(unsigned long *start, ret = kprobe_add_area_blacklist((unsigned long)__kprobes_text_start, (unsigned long)__kprobes_text_end); + /* Symbols in noinstr section are blacklisted */ + ret = kprobe_add_area_blacklist((unsigned long)__noinstr_text_start, + (unsigned long)__noinstr_text_end); + return ret ? : arch_populate_kprobe_blacklist(); } Thank you, > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > kernel/kprobes.c | 11 +++++++++++ > 1 file changed, 11 insertions(+) > > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -1443,10 +1443,21 @@ static bool __within_kprobe_blacklist(un > return false; > } > > +/* Functions in .noinstr.text must not be probed */ > +static bool within_noinstr_text(unsigned long addr) > +{ > + /* FIXME: Handle module .noinstr.text */ > + return addr >= (unsigned long)__noinstr_text_start && > + addr < (unsigned long)__noinstr_text_end; > +} > + > bool within_kprobe_blacklist(unsigned long addr) > { > char symname[KSYM_NAME_LEN], *p; > > + if (within_noinstr_text(addr)) > + return true; > + > if (__within_kprobe_blacklist(addr)) > return true; > >
Masami, Masami Hiramatsu <mhiramat@kernel.org> writes: > On Fri, 20 Mar 2020 19:00:00 +0100 > Thomas Gleixner <tglx@linutronix.de> wrote: > >> Instrumentation is forbidden in the .noinstr.text section. Make kprobes >> respect this. >> >> This lacks support for .noinstr.text sections in modules, which is required >> to handle VMX and SVM. >> > > Would you have any plan to list or mark the noinstr symbols on > some debugfs interface? I need a blacklist of those symbols so that > user (and perf-probe) can check which function can not be probed. > > It is just calling kprobe_add_area_blacklist() like below. > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index 2625c241ac00..4835b644bd2b 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -2212,6 +2212,10 @@ static int __init populate_kprobe_blacklist(unsigned long *start, > ret = kprobe_add_area_blacklist((unsigned long)__kprobes_text_start, > (unsigned long)__kprobes_text_end); > > + /* Symbols in noinstr section are blacklisted */ > + ret = kprobe_add_area_blacklist((unsigned long)__noinstr_text_start, > + (unsigned long)__noinstr_text_end); > + > return ret ? : arch_populate_kprobe_blacklist(); > } So that extra function is not required when adding that, right? >> +/* Functions in .noinstr.text must not be probed */ >> +static bool within_noinstr_text(unsigned long addr) >> +{ >> + /* FIXME: Handle module .noinstr.text */ >> + return addr >= (unsigned long)__noinstr_text_start && >> + addr < (unsigned long)__noinstr_text_end; >> +} >> + >> bool within_kprobe_blacklist(unsigned long addr) >> { >> char symname[KSYM_NAME_LEN], *p; >> >> + if (within_noinstr_text(addr)) >> + return true; >> + >> if (__within_kprobe_blacklist(addr)) >> return true;
On Mon, 23 Mar 2020 17:03:24 +0100 Thomas Gleixner <tglx@linutronix.de> wrote: > Masami, > > Masami Hiramatsu <mhiramat@kernel.org> writes: > > On Fri, 20 Mar 2020 19:00:00 +0100 > > Thomas Gleixner <tglx@linutronix.de> wrote: > > > >> Instrumentation is forbidden in the .noinstr.text section. Make kprobes > >> respect this. > >> > >> This lacks support for .noinstr.text sections in modules, which is required > >> to handle VMX and SVM. > >> > > > > Would you have any plan to list or mark the noinstr symbols on > > some debugfs interface? I need a blacklist of those symbols so that > > user (and perf-probe) can check which function can not be probed. > > > > It is just calling kprobe_add_area_blacklist() like below. > > > > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > > index 2625c241ac00..4835b644bd2b 100644 > > --- a/kernel/kprobes.c > > +++ b/kernel/kprobes.c > > @@ -2212,6 +2212,10 @@ static int __init populate_kprobe_blacklist(unsigned long *start, > > ret = kprobe_add_area_blacklist((unsigned long)__kprobes_text_start, > > (unsigned long)__kprobes_text_end); > > > > + /* Symbols in noinstr section are blacklisted */ > > + ret = kprobe_add_area_blacklist((unsigned long)__noinstr_text_start, > > + (unsigned long)__noinstr_text_end); > > + > > return ret ? : arch_populate_kprobe_blacklist(); > > } > > So that extra function is not required when adding that, right? That's right :) > > >> +/* Functions in .noinstr.text must not be probed */ > >> +static bool within_noinstr_text(unsigned long addr) > >> +{ > >> + /* FIXME: Handle module .noinstr.text */ And this reminds me that the module .kprobes.text is not handled yet :(. Thank you, > >> + return addr >= (unsigned long)__noinstr_text_start && > >> + addr < (unsigned long)__noinstr_text_end; > >> +} > >> + > >> bool within_kprobe_blacklist(unsigned long addr) > >> { > >> char symname[KSYM_NAME_LEN], *p; > >> > >> + if (within_noinstr_text(addr)) > >> + return true; > >> + > >> if (__within_kprobe_blacklist(addr)) > >> return true;
Masami Hiramatsu <mhiramat@kernel.org> writes: > On Mon, 23 Mar 2020 17:03:24 +0100 > Thomas Gleixner <tglx@linutronix.de> wrote: >> > @@ -2212,6 +2212,10 @@ static int __init populate_kprobe_blacklist(unsigned long *start, >> > ret = kprobe_add_area_blacklist((unsigned long)__kprobes_text_start, >> > (unsigned long)__kprobes_text_end); >> > >> > + /* Symbols in noinstr section are blacklisted */ >> > + ret = kprobe_add_area_blacklist((unsigned long)__noinstr_text_start, >> > + (unsigned long)__noinstr_text_end); >> > + >> > return ret ? : arch_populate_kprobe_blacklist(); >> > } >> >> So that extra function is not required when adding that, right? > > That's right :) > >> >> >> +/* Functions in .noinstr.text must not be probed */ >> >> +static bool within_noinstr_text(unsigned long addr) >> >> +{ >> >> + /* FIXME: Handle module .noinstr.text */ > > And this reminds me that the module .kprobes.text is not handled yet :(. Correct. Any idea how to do that with a simple oneliner like the above? Thanks, tglx
On Tue, 24 Mar 2020 10:47:30 +0100 Thomas Gleixner <tglx@linutronix.de> wrote: > Masami Hiramatsu <mhiramat@kernel.org> writes: > > On Mon, 23 Mar 2020 17:03:24 +0100 > > Thomas Gleixner <tglx@linutronix.de> wrote: > >> > @@ -2212,6 +2212,10 @@ static int __init populate_kprobe_blacklist(unsigned long *start, > >> > ret = kprobe_add_area_blacklist((unsigned long)__kprobes_text_start, > >> > (unsigned long)__kprobes_text_end); > >> > > >> > + /* Symbols in noinstr section are blacklisted */ > >> > + ret = kprobe_add_area_blacklist((unsigned long)__noinstr_text_start, > >> > + (unsigned long)__noinstr_text_end); > >> > + > >> > return ret ? : arch_populate_kprobe_blacklist(); > >> > } > >> > >> So that extra function is not required when adding that, right? > > > > That's right :) > > > >> > >> >> +/* Functions in .noinstr.text must not be probed */ > >> >> +static bool within_noinstr_text(unsigned long addr) > >> >> +{ > >> >> + /* FIXME: Handle module .noinstr.text */ > > > > And this reminds me that the module .kprobes.text is not handled yet :(. > > Correct. Any idea how to do that with a simple oneliner like the above? Hmm, we can store the .kprobes.text and .noinstr.text section info in the struct module and register it in module callback. But before that, I have to introduce a remove function. Currently, the blacklist can only add symbols. So that will not be a oneliner. Let me try. Thank you,
--- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -1443,10 +1443,21 @@ static bool __within_kprobe_blacklist(un return false; } +/* Functions in .noinstr.text must not be probed */ +static bool within_noinstr_text(unsigned long addr) +{ + /* FIXME: Handle module .noinstr.text */ + return addr >= (unsigned long)__noinstr_text_start && + addr < (unsigned long)__noinstr_text_end; +} + bool within_kprobe_blacklist(unsigned long addr) { char symname[KSYM_NAME_LEN], *p; + if (within_noinstr_text(addr)) + return true; + if (__within_kprobe_blacklist(addr)) return true;
Instrumentation is forbidden in the .noinstr.text section. Make kprobes respect this. This lacks support for .noinstr.text sections in modules, which is required to handle VMX and SVM. Signed-off-by: Thomas Gleixner <tglx@linutronix.de> --- kernel/kprobes.c | 11 +++++++++++ 1 file changed, 11 insertions(+)