kprobes updates for v5.8

Linus,

Please pull the latest core/kprobes git tree from:

git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git core-kprobes-2020-06-01

# HEAD: 66e9b0717102507e64f638790eaece88765cc9e5 kprobes: Prevent probes in .noinstr.text section

Various kprobes updates, mostly centered around cleaning up the no-instrumentation logic, instead of the current per debug facility blacklist, use the more generic .noinstr.text approach, combined with a 'noinstr' marker for functions.

Also add instrumentation_begin()/end() to better manage the exact place in entry code where instrumentation may be used.

Also add a kprobes blacklist for modules.

Thanks,

Ingo

------------------>
Masami Hiramatsu (4):
kprobes: Lock kprobe_mutex while showing kprobe_blacklist
kprobes: Support __kprobes blacklist in modules
kprobes: Support NOKPROBE_SYMBOL() in modules samples/kprobes: Add __kprobes and NOKPROBE_SYMBOL() for handlers.

Thomas Gleixner (2):
vmlinux.lds.h: Create section for protection against instrumentation
kprobes: Prevent probes in .noinstr.text section

 arch/powerpc/kernel/vmlinux.lds.S   |  1 +
 include/asm-generic/sections.h      |  3 ++
 include/asm-generic/vmlinux.lds.h   | 10 +++++
 include/linux/compiler.h            | 53 +++++++++++++++++++++++
 include/linux/compiler_types.h      |  4 ++
 include/linux/module.h              |  8 ++++
 kernel/kprobes.c                    | 85 ++++++++++++++++++++++++++++++++++++-
 kernel/module.c                     | 10 +++++
 samples/kprobes/kprobe_example.c    |  6 ++-
 samples/kprobes/kretprobe_example.c |  2 +
 scripts/mod/modpost.c               |  2 +-
 11 files changed, 180 insertions(+), 4 deletions(-)

I have pulled this, but do we really want to add this to a header file that is _so_ core that it gets included for basically every single file built?

I don't even see those instrumentation_begin/end() things used anywhere right now.

It seems excessive. That 53 lines is maybe not a lot, but it pushed that header file to over 12kB, and while it's mostly comments, it's extra IO and parsing basically for _every_ single file compiled in the kernel.

For what appears to be absolutely zero upside right now, and I really don't see why this should be in such a core header file!

I don't even see this as having anything at all to do with "compiler.h" in the first place.

I really think we should think twice about making core header files bigger like this. No, we're nowhere the disaster that C++ project headers are, but tokenization and parsing is actually a pretty big part of the build costs (which may surprise some people who think it's all the fancy optimizations that cost a lot of CPU time).

Linus

Fully agreed - and I made the attached patch to address this.

The code got cleaner and better structured, but it didn't help much in terms of inclusion count:

2616 total .o files

   2447  <linux/types.h>
   2436  <linux/compiler.h>
   2404  <linux/bug.h>

The reason is that <linux/bug.h> is included almost everywhere as well, and the instrumentation_begin()/end() annotations affect the BUG*() and WARN*() primitives as well.

At least non-x86 would have less instrumentation related noise, for now at least.

Thanks,

Ingo

==========================>
From: Ingo Molnar <[email protected]>
Date: Thu, 4 Jun 2020 08:36:22 +0200
Subject: [PATCH] compiler.h: Move instrumentation_begin()/end() into new <linux/instrumentation.h> header

Linus pointed out that compiler.h - which is a key header that gets included in every single of the 28,000+ kernel files files being built - was unnecessarily bloated in:

655389666643: ("vmlinux.lds.h: Create section for protection against instrumentation")

Move these primitives into a new header: <linux/instrumentation.h>, and include that header in context_tracking.h and x86/asm/bug.h, which makes use of it.

---
 arch/x86/include/asm/bug.h       |  1 +
 include/linux/compiler.h         | 53 -------------------------------------
 include/linux/context_tracking.h |  2 ++
 include/linux/instrumentation.h  | 57 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 60 insertions(+), 53 deletions(-)

index facba9bc30ca..37e4480dba75 100644
@@ -3,6 +3,7 @@
 #define _ASM_X86_BUG_H
 
 #include <linux/stringify.h>
+#include <linux/instrumentation.h>
 
 /*
  * Despite that some emulators terminate on UD2, we use it for WARN().

index 8cac62ee6add..ad6241c8003d 100644
@@ -5,6 +5,8 @@
 #include <linux/sched.h>
 #include <linux/vtime.h>
 #include <linux/context_tracking_state.h>
+#include <linux/instrumentation.h>
+
 #include <asm/ptrace.h>
 
 

The tested v2 version of the patch also needed the include in asm-generic/bug.h (see the fix attached below), because for completeness the generic version was annotated as well - even though only x86 has objtool support for now.

The readability improvement is real though.

Thanks,

Ingo

index 384b5c835ced..c43b5906e0dc 100644
@@ -3,6 +3,7 @@
 #define _ASM_GENERIC_BUG_H
 
 #include <linux/compiler.h>
+#include <linux/instrumentation.h>
 
 #define CUT_HERE		"------------[ cut here ]------------\n"
 

x86/asm/bug.h includes asm-generic/bug.h.

x86 uses the generic bug infrastructure.

Yes, indeed. No change to the patch required (other than the changelog), both asm-generic/bug.h and x86/asm/bug.h make use of instrumentation_begin()/end(), so they both need the #include.

Thanks,

Ingo