linux: add linker tables

Linux makes extensive use of custom ELF header sections, documentation for these are well scatterred. Unify this documentation in a central place.

Quoted 3 lines · Luis R. Rodriguez

Linux makes extensive use of custom ELF header sections, documentation for these are well scatterred. Unify this documentation in a central place.

Minor questions:

Documentation/DocBook/sections.tmpl line 43

Quoted 6 lines

Documentation/DocBook/sections.tmpl new file +6 -0

+      <para>
+        You should have received a copy of the GNU General Public
+        License along with this documentation; if not, write to the Free
+        Software Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+        MA 02111-1307 USA
+      </para>

Unless you are willing to personally track the office moves of the FSF for the next 20+ years, just drop this paragraph please :)

Quoted 7 lines

+++ b/include/linux/sections.h

include/linux/sections.h new file +5 -0

@@ -0,0 +1,224 @@
+#ifndef _LINUX_SECTIONS_H
+#define _LINUX_SECTIONS_H
+/*
+ * Linux ELF sections
+ *

include/linux/sections.h line 11

Quoted 6 lines

include/linux/sections.h new file +6 -0

+ * Copyright (C) 2016 Luis R. Rodriguez <[email protected]>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.

Do you really mean "any later version"?

include/linux/sections.h line 16

Quoted 4 lines

include/linux/sections.h new file +4 -0

+ * Due to this file being licensed under the GPL there is controversy over
+ * whether this permits you to write a module that #includes this file
+ * without placing your module under the GPL.  Please consult a lawyer for
+ * advice before doing this.

Interesting wording,

I don't think there's any "controversy" here, so I'd drop that word if at all possible.

Quoted 7 lines · Greg KH

Quoted 4 lines · Luis R. Rodriguez

+ * Due to this file being licensed under the GPL there is controversy over
+ * whether this permits you to write a module that #includes this file
+ * without placing your module under the GPL. Please consult a lawyer for
+ * advice before doing this.

Interesting wording,

This was copied from security.h.

Quoted 7 lines · Luis R. Rodriguez

Quoted 6 lines · Greg KH

Quoted 4 lines · Luis R. Rodriguez

+ * Due to this file being licensed under the GPL there is controversy over
+ * whether this permits you to write a module that #includes this file
+ * without placing your module under the GPL. Please consult a lawyer for
+ * advice before doing this.

Quoted 3 lines

Quoted 1 line

Interesting wording,

This was copied from security.h.

Ah, and look, I wrote that, nevermind then :)

sorry for the noise,

include/linux/tables.h new file +12 -0

+
+/**
+ * DOC: Regular linker linker table constructors
+ *
+ * Regular constructors are expected to be used for valid linker table entries.
+ * Valid uses of weak entries other than the beginning and is currently
+ * untested but should in theory work.
+ */
+
+/**
+ * LINKTABLE_TEXT - Declares a linker table entry for execution
+ *

Quoted 16 lines · Luis R. Rodriguez

include/linux/tables.h new file +16 -0

+
+/**
+ * DOC: Regular linker linker table constructors
+ *
+ * Regular constructors are expected to be used for valid linker table entries.
+ * Valid uses of weak entries other than the beginning and is currently
+ * untested but should in theory work.
+ */
+
+/**
+ * LINKTABLE_TEXT - Declares a linker table entry for execution
+ *
+ * @name: linker table name
+ * @level: order level
+ *
+ * Declares a linker table to be used for execution.

include/linux/tables.h line 424

Quoted 6 lines

+ */

include/linux/tables.h new file +5 -0

+#define LINKTABLE_TEXT(name, level)					\
+	      __typeof__(name[0])					\
+	      __attribute__((used,					\
+			     __aligned__(LINKTABLE_ALIGNMENT(name)),	\
+			     section(SECTION_TBL(SECTION_TEXT, name, level))))

I'm really confused by this. Text should obviously be readonly,

but I'm not at all clear how this works here.

The issue with linktables for text is kind of confusing if nothing else; Russel is right about that. It doesn't prevent us from doing something similar, but perhaps it ought to have a different name.

For one thing, priority level is meaningless for text, since it is not a table that can be indexed into.

Quoted 17 lines · H. Peter Anvin

Quoted 16 lines · Luis R. Rodriguez

include/linux/tables.h new file +16 -0

+
+/**
+ * DOC: Regular linker linker table constructors
+ *
+ * Regular constructors are expected to be used for valid linker table entries.
+ * Valid uses of weak entries other than the beginning and is currently
+ * untested but should in theory work.
+ */
+
+/**
+ * LINKTABLE_TEXT - Declares a linker table entry for execution
+ *
+ * @name: linker table name
+ * @level: order level
+ *
+ * Declares a linker table to be used for execution.

Quoted 8 lines

include/linux/tables.h line 424

Quoted 6 lines

+ */

include/linux/tables.h new file +5 -0

+#define LINKTABLE_TEXT(name, level)                                        \
+         __typeof__(name[0])                                       \
+         __attribute__((used,                                      \
+                        __aligned__(LINKTABLE_ALIGNMENT(name)),    \
+                        section(SECTION_TBL(SECTION_TEXT, name, level))))

I'm really confused by this. Text should obviously be readonly,

So this uses SECTION_TEXT, so we just pegged the linker table entry right below the standard SECTION_TEXT:

OK yes I see the issue now. I've modified this to use const, and retested the kprobe patch and it works well still. kprobe would not use LINKTABLE_TEXT, instead it uses its own macro, however users of LINKTABLE_TEXT would then have const declared. The implications are that you *can* declare structs so long as everything is const.

Quoted 7 lines · Luis R. Rodriguez

OK yes I see the issue now. I've modified this to use const, and retested the kprobe patch and it works well still. kprobe would not use LINKTABLE_TEXT, instead it uses its own macro, however users of LINKTABLE_TEXT would then have const declared. The implications are that you *can* declare structs so long as everything is const.

No, such structures belong in .rodata not in .text.

I have been thinking about it somewhat, and for text we really have text "ranges" rather than tables. The big difference between the two are:

1. priority ordering doesn't make any sense for ranges.

2. ranges can be hierarchial, that is, range "bar" can be entirely inside range "foo".

3. ranges aren't typed (although in C, that pretty much means they are "char" or "unsigned char" as there really isn't any way to define an "array of void".)

4. the only useful operator on a range is "is address X inside this range"; this operator is likely *not* useful for a table, since if you have to ever invoke it you are probably doing something very wrong.

For this to work, we need strings such that they will always sort in the appropriate order with the bracket symbols around subranges. I need to think about this a few minutes before I actually say anything about it...

Quoted 5 lines

4. the only useful operator on a range is "is address X inside this range"; this operator is likely *not* useful for a table, since if you have to ever invoke it you are probably doing something very wrong.

kprobe uses it :P

Quoted 3 lines · Luis R. Rodriguez

Quoted 3 lines

4. the only useful operator on a range is "is address X inside this range"; this operator is likely *not* useful for a table, since

Quoted 5 lines

Quoted 2 lines

if you have to ever invoke it you are probably doing something very wrong.

kprobe uses it :P

Could you explain how?

include/linux/tables.h new file +9 -0

+/**
+ * LINKTABLE_RUN_ERR - run each linker table entry func and return error if any
+ *
+ * @tbl: linker table
+ * @func: structure name for the function name we want to call.
+ * @args...: arguments to pass to func
+ *
+ * Example usage:
+ *

Quoted 9 lines · Luis R. Rodriguez

include/linux/tables.h new file +9 -0

+/**
+ * LINKTABLE_RUN_ERR - run each linker table entry func and return error if any
+ *
+ * @tbl: linker table
+ * @func: structure name for the function name we want to call.
+ * @args...: arguments to pass to func
+ *
+ * Example usage:
+ *

include/linux/tables.h line 694

Quoted 10 lines

include/linux/tables.h new file +10 -0

+ *   unsigned int err = LINKTABLE_RUN_ERR(frobnicator_fns, some_run,);
+ */
+#define LINKTABLE_RUN_ERR(tbl, func, args...)				\
+({									\
+	size_t i;							\
+	int err = 0;							\
+	for (i = 0; !err && i < LINKTABLE_SIZE(tbl); i++)		\
+		err = (tbl[i]).func (args);				\
+		err; \
+})

This is wrong and pointless. As written it returns the error code of the last instance.

What I suggested for this macro was that we ought to exit the loop on error.

Furthermore:

1. Using an advancing pointer would make more sense than a counter.

2. .func doesn't make any sense -- it ought to be "func"; otherwise you

    can't call into either a table containing pure function pointers,
    nor into a field of table *pointed to* by the table; which is
    likely to be quite common.

    So the idea is to use something like:

    /* Array of function pointers */
    LINKTABLE_RUN_ALL(frobnicator_fns, , arg1, arg2);

    /* Array of structures */
    LINKTABLE_RUN_ALL(frobnicator_fns, .some_run, arg1, arg2);

    /* Array of structure pointers */
    LINKTABLE_RUN_ALL(frobnicator_fns, ->some_run, arg1, arg2);

This ports built-in firmware to use linker tables, this replaces the custom section solution with a generic solution.

This also demos the use of the .rodata (SECTION_RO) linker tables.

Tested with 0 built-in firmware, 1 and 2 built-in
firmwares successfully.

Quoted 7 lines · Luis R. Rodriguez

This ports built-in firmware to use linker tables, this replaces the custom section solution with a generic solution.

This also demos the use of the .rodata (SECTION_RO) linker tables.

Quoted 2 lines

Tested with 0 built-in firmware, 1 and 2 built-in
firmwares successfully.

I think we'd do better to rip this support out entirely. It just isn't needed; firmware can live in an initramfs and don't even need *any* actual running userspace support to load it from there these days, do we?

Quoted 11 lines · David Woodhouse

Quoted 9 lines · Luis R. Rodriguez

This ports built-in firmware to use linker tables, this replaces the custom section solution with a generic solution.

This also demos the use of the .rodata (SECTION_RO) linker tables.

Tested with 0 built-in firmware, 1 and 2 built-in
firmwares successfully.

Quoted 4 lines

I think we'd do better to rip this support out entirely. It just isn't needed; firmware can live in an initramfs and don't even need *any* actual running userspace support to load it from there these days, do we?

I think this is reasonable if and only if we really don't know of anyone out there not able to use initramfs. I'm happy to rip it out.

Quoted 12 lines · Luis R. Rodriguez

Quoted 11 lines · David Woodhouse

Quoted 9 lines · Luis R. Rodriguez

This ports built-in firmware to use linker tables, this replaces the custom section solution with a generic solution.

This also demos the use of the .rodata (SECTION_RO) linker tables.

Tested with 0 built-in firmware, 1 and 2 built-in
firmwares successfully.

Quoted 7 lines

Quoted 4 lines

I think we'd do better to rip this support out entirely. It just isn't needed; firmware can live in an initramfs and don't even need *any* actual running userspace support to load it from there these days, do we?

I think this is reasonable if and only if we really don't know of anyone out there not able to use initramfs. I'm happy to rip it out.

The changelog for this doesn't say anything about _why_ the change is being made? (and what about other architectures.) Also, Chrome OS doesn't use an initramfs (and plenty of other things don't too). Being able to build monolithic kernels (e.g. Android and Brillo) with builtin firmware is very handy. Please don't remove built-in firmware support.

Quoted 21 lines · Kees Cook

Quoted 12 lines · Luis R. Rodriguez

Quoted 11 lines · David Woodhouse

Quoted 9 lines · Luis R. Rodriguez

This ports built-in firmware to use linker tables, this replaces the custom section solution with a generic solution.

This also demos the use of the .rodata (SECTION_RO) linker tables.

Tested with 0 built-in firmware, 1 and 2 built-in
firmwares successfully.

Quoted 7 lines

Quoted 4 lines

I think we'd do better to rip this support out entirely. It just isn't needed; firmware can live in an initramfs and don't even need *any* actual running userspace support to load it from there these days, do we?

I think this is reasonable if and only if we really don't know of anyone out there not able to use initramfs. I'm happy to rip it out.

Quoted 6 lines

The changelog for this doesn't say anything about _why_ the change is being made? (and what about other architectures.) Also, Chrome OS doesn't use an initramfs (and plenty of other things don't too). Being able to build monolithic kernels (e.g. Android and Brillo) with builtin firmware is very handy. Please don't remove built-in firmware support.

I second this, we can't break existing systems at all. I thought we were going to keep built-in firmware, right Luis?

Quoted 17 lines · Luis R. Rodriguez

Quoted 15 lines · David Woodhouse

Quoted 9 lines · Luis R. Rodriguez

This ports built-in firmware to use linker tables, this replaces the custom section solution with a generic solution.

This also demos the use of the .rodata (SECTION_RO) linker tables.

Tested with 0 built-in firmware, 1 and 2 built-in
firmwares successfully.

I think we'd do better to rip this support out entirely. It just isn't needed; firmware can live in an initramfs and don't even need *any* actual running userspace support to load it from there these days, do we?

Quoted 2 lines

I think this is reasonable if and only if we really don't know of anyone out there not able to use initramfs. I'm happy to rip it out.

The changelog for this doesn't say anything about _why_ the change is being made? (and what about other architectures.)

Also, Chrome OS
doesn't use an initramfs (and plenty of other things don't too). Being able to build monolithic kernels (e.g. Android and Brillo) with builtin firmware is very handy. Please don't remove built-in firmware support.

Thanks! Can you confirm if any Android or Brillo builds are already using it?

or it was decided

Quoted 1 line · Luis R. Rodriguez

Thanks! Can you confirm if any Android or Brillo builds are already using it?

Also more importantly, any chance you can provide any technical reasons why initramfs cannot be used, or it was decided to not use it on these systems? It should help others in the future as well.

Quoted 1 line · Luis R. Rodriguez

Thanks! Can you confirm if any Android or Brillo builds are already using it?

Also more importantly, any chance you can provide any technical reasons why initramfs cannot be used, or it was decided to not use it on these systems? It should help others in the future as well.

Quoted 7 lines · Luis R. Rodriguez

This ports built-in firmware to use linker tables, this replaces the custom section solution with a generic solution.

This also demos the use of the .rodata (SECTION_RO) linker tables.

Quoted 2 lines

Tested with 0 built-in firmware, 1 and 2 built-in
firmwares successfully.

I think we'd do better to rip this support out entirely. It just isn't needed; firmware can live in an initramfs and don't even need *any* actual running userspace support to load it from there these days, do we?

Quoted 8 lines · David Woodhouse

Quoted 7 lines · Luis R. Rodriguez

This ports built-in firmware to use linker tables, this replaces the custom section solution with a generic solution.

This also demos the use of the .rodata (SECTION_RO) linker tables.

Quoted 7 lines

Quoted 2 lines

Tested with 0 built-in firmware, 1 and 2 built-in
firmwares successfully.

I think we'd do better to rip this support out entirely. It just isn't needed; firmware can live in an initramfs and don't even need *any* actual running userspace support to load it from there these days, do we?

We have lots of SCSI drivers with built in firmware. The obvious examples are 53c700, aic7xxx and aic79xx. For them, we actually have the firmware compilers in tree. The firmware model they use just isn't amenable to the firmware loader: they're not monolithic blobs, it's a set of firmware scripts we use to handle particular operations before giving control back to the host, so the firmware and the driver are very much symbiotic.

On the other hand, I don't think any of them uses firmware sections, so it's not an argument for not ripping out this type.

Quoted 17 lines · James Bottomley

Quoted 8 lines · David Woodhouse

Quoted 7 lines · Luis R. Rodriguez

This ports built-in firmware to use linker tables, this replaces the custom section solution with a generic solution.

This also demos the use of the .rodata (SECTION_RO) linker tables.

Quoted 7 lines

Quoted 2 lines

Tested with 0 built-in firmware, 1 and 2 built-in
firmwares successfully.

I think we'd do better to rip this support out entirely. It just isn't needed; firmware can live in an initramfs and don't even need *any* actual running userspace support to load it from there these days, do we?

Quoted 7 lines

We have lots of SCSI drivers with built in firmware. The obvious examples are 53c700, aic7xxx and aic79xx. For them, we actually have the firmware compilers in tree. The firmware model they use just isn't amenable to the firmware loader: they're not monolithic blobs, it's a set of firmware scripts we use to handle particular operations before giving control back to the host, so the firmware and the driver are very much symbiotic.

I'm in the process of doing some other cleanups with the firmware_class stuff so that odd requirements get supported but clean interfaces are also not hampered by these odd requirements, so I'll take a look at this later. If you have other oddball firmware requirements please let me know so I can also keep in mind.

Quoted 2 lines

On the other hand, I don't think any of them uses firmware sections, so it's not an argument for not ripping out this type.

Thanks for confirming. I'll rip this out.

With a generic linker tables solution in place we need a general asm solution for declaring entries with asm. The first easy target is to cover the C asm declarations, guard the header file for now and define a first generic entry push_section_tbl() to be used later for custom linker table annotations.

Signed-off-by Luis R. Rodriguez <[email protected]>

---
 include/asm-generic/sections.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

include/asm-generic/sections.h +2 -0

index af0254c09424..f5ea98bd85d2 100644
@@ -3,8 +3,10 @@
 
 /* References to section boundaries */
 
+#ifndef __ASSEMBLY__
 #include <linux/compiler.h>
 #include <linux/types.h>
+#include <linux/sections.h>
 
 /*
  * Usage guidelines:

@@ -128,4 +130,12 @@ static inline bool init_section_intersects(void *virt, size_t size)
 	return memory_intersects(__init_begin, __init_end, virt, size);

Quoted 31 lines · Luis R. Rodriguez

With a generic linker tables solution in place we need a general asm solution for declaring entries with asm. The first easy target is to cover the C asm declarations, guard the header file for now and define a first generic entry push_section_tbl() to be used later for custom linker table annotations.

Signed-off-by Luis R. Rodriguez <[email protected]>

---
 include/asm-generic/sections.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

include/asm-generic/sections.h +2 -0

index af0254c09424..f5ea98bd85d2 100644
@@ -3,8 +3,10 @@
 
 /* References to section boundaries */
 
+#ifndef __ASSEMBLY__
 #include <linux/compiler.h>
 #include <linux/types.h>
+#include <linux/sections.h>
 
 /*
  * Usage guidelines:

@@ -128,4 +130,12 @@ static inline bool init_section_intersects(void *virt, size_t size)
 	return memory_intersects(__init_begin, __init_end, virt, size);
 }
 

include/asm-generic/sections.h static inline bool init_section_intersects(void *virt, size_t size) · line 140

Quoted 8 lines

include/asm-generic/sections.h +8 -0

+/*
+ * Some architectures do not like the "\t" at the end (s39), we should be
+ * able to generalize this further, but so far this covers most architectures.
+ */
+#define push_section_tbl(section, name, level, flags)			\
+	".pushsection " SECTION_TBL(section,name,level) ",  \"" #flags "\"\n\t"
+#endif
+

I think the \n\t is unnecessary.

Quoted 2 lines

I think the \n\t is unnecessary.

Super! I wonder if we we can just use this on s390 as well without it pooping? I ask as this would set a precedent.

Quoted 6 lines · Luis R. Rodriguez

Quoted 2 lines

I think the \n\t is unnecessary.

Super! I wonder if we we can just use this on s390 as well without it pooping? I ask as this would set a precedent.

Ask Heike, but I think just ; or \n ought be be fine. I do not know of *any* case where \t at the end of a string would ever be necessary, and it would *always* be possible to replace it with a space in a pinch.

Quoted 8 lines · H. Peter Anvin

Quoted 6 lines · Luis R. Rodriguez

Quoted 2 lines

I think the \n\t is unnecessary.

Super! I wonder if we we can just use this on s390 as well without it pooping? I ask as this would set a precedent.

Quoted 3 lines

Ask Heike, but I think just ; or \n ought be be fine. I do not know of *any* case where \t at the end of a string would ever be necessary, and it would *always* be possible to replace it with a space in a pinch.

\n should be fine on s390.

kprobe makes use of two custom sections:

type name begin end
init.data _kprobe_blacklist __start_kprobe_blacklist __stop_kprobe_blacklist
text .kprobes.text __kprobes_text_start __kprobes_text_end

Port these to the linker table generic solution. This lets us remove all the custom kprobe section declarations on the linker script.

Quoted 6 lines · Luis R. Rodriguez

kprobe makes use of two custom sections:

type name begin end
init.data _kprobe_blacklist __start_kprobe_blacklist __stop_kprobe_blacklist
text .kprobes.text __kprobes_text_start __kprobes_text_end

Quoted 3 lines

Port these to the linker table generic solution. This lets us remove all the custom kprobe section declarations on the linker script.

I don't like this. You create this "table" thing, which you use for tables, and then you go and use it for something else - for kprobes text which is not a table. The kprobes text section is a section for text which is not kprobe-able.

So, it seems to be a complete mis-nomer to me. Maybe you can explain the rationale here?

From: Luis R. Rodriguez [mailto:[email protected]]

kprobe makes use of two custom sections:

type name begin end
init.data _kprobe_blacklist __start_kprobe_blacklist __stop_kprobe_blacklist
text .kprobes.text __kprobes_text_start __kprobes_text_end

Port these to the linker table generic solution. This lets us remove all the custom kprobe section declarations on the linker script.

Tested with CONFIG_KPROBES_SANITY_TEST, it passes with:

Kprobe smoke test: started
Kprobe smoke test: passed successfully

Then tested CONFIG_SAMPLE_KPROBES on do_fork, and the kprobe bites and kicks as expected. Lastly tried registering a kprobe on a kprobe blacklisted symbol (NOKPROBE_SYMBOL()), and confirms that fails to work.

Signed-off-by Luis R. Rodriguez <[email protected]>

---

arch/arc/kernel/vmlinux.lds.S        |  1 -
arch/arm/kernel/entry-armv.S         |  2 +-
arch/arm/kernel/vmlinux.lds.S        |  1 -
arch/avr32/kernel/entry-avr32b.S     |  4 ++--
arch/avr32/kernel/vmlinux.lds.S      |  1 -
arch/blackfin/kernel/vmlinux.lds.S   |  1 -
arch/c6x/kernel/vmlinux.lds.S        |  1 -
arch/hexagon/kernel/vmlinux.lds.S    |  1 -
arch/ia64/kernel/jprobes.S           |  2 +-
arch/ia64/kernel/vmlinux.lds.S       |  1 -
arch/ia64/lib/flush.S                |  4 ++--
arch/metag/kernel/vmlinux.lds.S      |  1 -
arch/microblaze/kernel/vmlinux.lds.S |  1 -
arch/mips/kernel/vmlinux.lds.S       |  1 -
arch/mn10300/kernel/vmlinux.lds.S    |  1 -
arch/nios2/kernel/vmlinux.lds.S      |  1 -
arch/openrisc/kernel/vmlinux.lds.S   |  1 -
arch/parisc/kernel/vmlinux.lds.S     |  1 -
arch/powerpc/include/asm/ppc_asm.h   |  6 +++---
arch/powerpc/kernel/vmlinux.lds.S    |  1 -
arch/s390/kernel/entry.S             |  4 ++--
arch/s390/kernel/kprobes.c           |  2 +-
arch/s390/kernel/mcount.S            |  2 +-
arch/s390/kernel/vmlinux.lds.S       |  1 -
arch/score/kernel/vmlinux.lds.S      |  1 -
arch/sh/kernel/vmlinux.lds.S         |  1 -
arch/sparc/kernel/vmlinux.lds.S      |  1 -
arch/sparc/mm/ultra.S                |  2 +-
arch/tile/kernel/vmlinux.lds.S       |  1 -
arch/x86/kernel/kprobes/core.c       | 10 ++++++----
arch/x86/kernel/vmlinux.lds.S        |  1 -
include/asm-generic/sections.h       |  4 ++--
include/asm-generic/vmlinux.lds.h    | 16 ----------------
include/linux/compiler.h             |  3 ++-
include/linux/kprobes.h              |  7 +++++--
kernel/kprobes.c                     | 16 +++++++---------
scripts/mod/modpost.c                |  3 ++-
scripts/recordmcount.c               |  3 ++-
scripts/recordmcount.pl              |  2 +-
39 files changed, 41 insertions(+), 72 deletions(-)

arch/arc/kernel/vmlinux.lds.S +0 -1

index 894e696bddaa..52f23df2b9b6 100644
@@ -98,7 +98,6 @@ SECTIONS
 		TEXT_TEXT
 		SCHED_TEXT
 		LOCK_TEXT
-		KPROBES_TEXT
 		*(.fixup)
 		*(.gnu.warning)
 	}

arch/arm/kernel/entry-armv.S +1 -1

index 3ce377f7251f..4959a83833fd 100644
@@ -83,7 +83,7 @@
 	.endm
 
 #ifdef CONFIG_KPROBES
-	.section	.kprobes.text,"ax",%progbits
+	.section	.text.tbl.kprobes.all,"ax",%progbits
 #else
 	.text
 #endif

arch/arm/kernel/vmlinux.lds.S +0 -1

index 8b60fde5ce48..8de7921ac331 100644
@@ -108,7 +108,6 @@ SECTIONS
 			TEXT_TEXT
 			SCHED_TEXT
 			LOCK_TEXT
-			KPROBES_TEXT
 			*(.gnu.warning)
 			*(.glue_7)
 			*(.glue_7t)

arch/avr32/kernel/entry-avr32b.S +1 -1

index 7301f4806bbe..c0fe699d0a21 100644
@@ -605,7 +605,7 @@ fault_exit_work:
 	brcc	fault_resume_user
 	rjmp	enter_monitor_mode
 
-	.section .kprobes.text, "ax", @progbits
+	.section .text.tbl.kprobes.all, "ax", @progbits
 	.type	handle_debug, @function

handle_debug:

	sub	sp, 4		/* r12_orig */

arch/avr32/kernel/entry-avr32b.S +1 -1

@@ -826,7 +826,7 @@ irq_level\level:
 	IRQ_LEVEL 2
 	IRQ_LEVEL 3
 
-	.section .kprobes.text, "ax", @progbits
+	.section .text.tbl.kprobes.all, "ax", @progbits
 	.type	enter_monitor_mode, @function

enter_monitor_mode:

	/*

arch/avr32/kernel/vmlinux.lds.S +0 -1

index a4589176bed5..bf4f3f1f9dbb 100644
@@ -49,7 +49,6 @@ SECTIONS
 		_stext = .;
 		*(.ex.text)
 		*(.irq.text)
-		KPROBES_TEXT
 		TEXT_TEXT
 		SCHED_TEXT
 		LOCK_TEXT

arch/blackfin/kernel/vmlinux.lds.S +0 -1

index c9eec84aa258..8a193fad4c5d 100644
@@ -35,7 +35,6 @@ SECTIONS
 #endif
 		LOCK_TEXT
 		IRQENTRY_TEXT
-		KPROBES_TEXT
 #ifdef CONFIG_ROMKERNEL
 		__sinittext = .;
 		INIT_TEXT

arch/c6x/kernel/vmlinux.lds.S +0 -1

index 5a6e141d1641..a23bed62cec3 100644
@@ -72,7 +72,6 @@ SECTIONS
 		SCHED_TEXT
 		LOCK_TEXT
 		IRQENTRY_TEXT
-		KPROBES_TEXT
 		*(.fixup)
 		*(.gnu.warning)
 	}

arch/hexagon/kernel/vmlinux.lds.S +0 -1

index 5f268c1071b3..a358b699ec64 100644
@@ -51,7 +51,6 @@ SECTIONS
 		TEXT_TEXT
 		SCHED_TEXT
 		LOCK_TEXT
-		KPROBES_TEXT
 		*(.fixup)
 	}
 	_etext = .;

arch/ia64/kernel/jprobes.S +1 -1

index f69389c7be1d..c6c23a3e92fb 100644
@@ -50,7 +50,7 @@
 	/*
 	 * void jprobe_break(void)
 	 */
-	.section .kprobes.text, "ax"
+	.section .text.tbl.kprobes.all, "ax"
 ENTRY(jprobe_break)
 	break.m __IA64_BREAK_JPROBE
 END(jprobe_break)

arch/ia64/kernel/vmlinux.lds.S +0 -1

index dc506b05ffbd..884f36d38989 100644
@@ -47,7 +47,6 @@ SECTIONS {
 		TEXT_TEXT
 		SCHED_TEXT
 		LOCK_TEXT
-		KPROBES_TEXT
 		*(.gnu.linkonce.t*)
 	}
 

arch/ia64/lib/flush.S +2 -2

index 1d8c88860063..7f257810e858 100644
@@ -20,7 +20,7 @@
 	 *
 	 *	Note: "in0" and "in1" are preserved for debugging purposes.
 	 */
-	.section .kprobes.text,"ax"
+	.section .text.tbl.kprobes.all,"ax"
 GLOBAL_ENTRY(flush_icache_range)
 
 	.prologue

@@ -72,7 +72,7 @@ END(flush_icache_range)
 	 *
 	 *	Note: "in0" and "in1" are preserved for debugging purposes.
 	 */
-	.section .kprobes.text,"ax"
+	.section .text.tbl.kprobes.all,"ax"
 GLOBAL_ENTRY(clflush_cache_range)
 
 	.prologue

arch/metag/kernel/vmlinux.lds.S +0 -1

index e12055e88bfe..da1689c85bc8 100644
@@ -22,7 +22,6 @@ SECTIONS
 	TEXT_TEXT
 	SCHED_TEXT
 	LOCK_TEXT
-	KPROBES_TEXT
 	IRQENTRY_TEXT
 	*(.text.*)
 	*(.gnu.warning)

arch/microblaze/kernel/vmlinux.lds.S +0 -1

index be9488d69734..94c1c5574a77 100644
@@ -34,7 +34,6 @@ SECTIONS {
 		EXIT_CALL
 		SCHED_TEXT
 		LOCK_TEXT
-		KPROBES_TEXT
 		IRQENTRY_TEXT
 		. = ALIGN (4) ;
 		_etext = . ;

arch/mips/kernel/vmlinux.lds.S +0 -1

index 0a93e83cd014..9183e9235651 100644
@@ -56,7 +56,6 @@ SECTIONS
 		TEXT_TEXT
 		SCHED_TEXT
 		LOCK_TEXT
-		KPROBES_TEXT
 		IRQENTRY_TEXT
 		*(.text.*)
 		*(.fixup)

arch/mn10300/kernel/vmlinux.lds.S +0 -1

index 13c4814c29f8..ead5efc01802 100644
@@ -31,7 +31,6 @@ SECTIONS
 	TEXT_TEXT
 	SCHED_TEXT
 	LOCK_TEXT
-	KPROBES_TEXT
 	*(.fixup)
 	*(.gnu.warning)
 	} = 0xcb

arch/nios2/kernel/vmlinux.lds.S +0 -1

index 326fab40a9de..dc8a84183700 100644
@@ -39,7 +39,6 @@ SECTIONS
 		SCHED_TEXT
 		LOCK_TEXT
 		IRQENTRY_TEXT
-		KPROBES_TEXT
 	} =0
 	_etext = .;
 

arch/openrisc/kernel/vmlinux.lds.S +0 -1

index 2d69a853b742..b010cb75edc4 100644
@@ -48,7 +48,6 @@ SECTIONS
 	  TEXT_TEXT
 	  SCHED_TEXT
 	  LOCK_TEXT
-	  KPROBES_TEXT
 	  IRQENTRY_TEXT
 	  *(.fixup)
 	  *(.text.__*)

arch/parisc/kernel/vmlinux.lds.S +0 -1

index 308f29081d46..9d9dc4984465 100644
@@ -70,7 +70,6 @@ SECTIONS
 		TEXT_TEXT
 		SCHED_TEXT
 		LOCK_TEXT
-		KPROBES_TEXT
 		IRQENTRY_TEXT
 		*(.text.do_softirq)
 		*(.text.sys_exit)

arch/powerpc/include/asm/ppc_asm.h +3 -3

index 499d9f89435a..4e36e86338f3 100644
@@ -218,7 +218,7 @@ name: \
 	.localentry name,.-name
 
 #define _KPROBE(name) \
-	.section ".kprobes.text","a"; \
+	.section ".text.tbl.kprobes.all","a"; \
 	.align 2 ; \
 	.type name,@function; \
 	.globl name; \

@@ -248,7 +248,7 @@ GLUE(.,name):
 #define _GLOBAL_TOC(name) _GLOBAL(name)
 
 #define _KPROBE(name) \
-	.section ".kprobes.text","a"; \
+	.section ".text.tbl.kprobes.all","a"; \
 	.align 2 ; \
 	.globl name; \
 	.globl GLUE(.,name); \

@@ -280,7 +280,7 @@ n:
 #define _GLOBAL_TOC(name) _GLOBAL(name)
 
 #define _KPROBE(n)	\
-	.section ".kprobes.text","a";	\
+	.section ".text.tbl.kprobes.all","a";	\
 	.globl	n;	\

n:

arch/powerpc/kernel/vmlinux.lds.S +0 -1

index d41fd0af8980..8779ae180783 100644
@@ -53,7 +53,6 @@ SECTIONS
 		*(.text .fixup __ftr_alt_* .ref.text)
 		SCHED_TEXT
 		LOCK_TEXT
-		KPROBES_TEXT
 		IRQENTRY_TEXT
 
 #ifdef CONFIG_PPC32

arch/s390/kernel/entry.S +1 -1

index cd5a191381b9..b09aa9ab5a1f 100644
@@ -162,7 +162,7 @@ _PIF_WORK	= (_PIF_PER_TRAP)
 		tm	off+\addr, \mask
 	.endm
 
-	.section .kprobes.text, "ax"
+	.section .text.tbl.kprobes.all, "ax"
 
 /*
 * Scheduler resume function, called by switch_to

@@ -965,7 +965,7 @@ ENTRY(restart_int_handler)
 	brc	2,2b

3: j 3b

arch/s390/kernel/entry.S +1 -1

-	.section .kprobes.text, "ax"
+	.section .text.tbl.kprobes.all, "ax"

#ifdef CONFIG_CHECK_STACK
/*

arch/s390/kernel/kprobes.c +1 -1

index 250f5972536a..376d4bd487c5 100644
@@ -288,7 +288,7 @@ static void kprobe_reenter_check(struct kprobe_ctlblk *kcb, struct kprobe *p)
 	default:
 		/*
 		 * A kprobe on the code path to single step an instruction
-		 * is a BUG. The code path resides in the .kprobes.text
+		 * is a BUG. The code path resides in the .text.tbl.kprobes.all
 		 * section and is executed with interrupts disabled.
 		 */
 		printk(KERN_EMERG "Invalid kprobe detected at %p.\n", p->addr);

arch/s390/kernel/mcount.S +1 -1

index e499370fbccb..4572045b968e 100644
@@ -10,7 +10,7 @@
 #include <asm/ftrace.h>
 #include <asm/ptrace.h>
 
-	.section .kprobes.text, "ax"
+	.section .text.tbl.kprobes.all, "ax"
 
 ENTRY(ftrace_stub)
 	br	%r14

arch/s390/kernel/vmlinux.lds.S +0 -1

index 445657fe658c..4614b5bcab5e 100644
@@ -26,7 +26,6 @@ SECTIONS
 		TEXT_TEXT
 		SCHED_TEXT
 		LOCK_TEXT
-		KPROBES_TEXT
 		IRQENTRY_TEXT
 		*(.fixup)
 		*(.gnu.warning)

arch/score/kernel/vmlinux.lds.S +0 -1

index 7274b5c4287e..36ebcb3b1215 100644
@@ -41,7 +41,6 @@ SECTIONS
 		TEXT_TEXT
 		SCHED_TEXT
 		LOCK_TEXT
-		KPROBES_TEXT
 		*(.text.*)
 		*(.fixup)
 		. = ALIGN (4) ;

arch/sh/kernel/vmlinux.lds.S +0 -1

index db88cbf9eafd..af4f37375345 100644
@@ -37,7 +37,6 @@ SECTIONS
 		EXTRA_TEXT
 		SCHED_TEXT
 		LOCK_TEXT
-		KPROBES_TEXT
 		IRQENTRY_TEXT
 		*(.fixup)
 		*(.gnu.warning)

arch/sparc/kernel/vmlinux.lds.S +0 -1

index f1a2f688b28a..a1c8e3a3ebb6 100644
@@ -46,7 +46,6 @@ SECTIONS
 		TEXT_TEXT
 		SCHED_TEXT
 		LOCK_TEXT
-		KPROBES_TEXT
 		IRQENTRY_TEXT
 		*(.gnu.warning)
 	} = 0

arch/sparc/mm/ultra.S +1 -1

index b4f4733abc6e..a08fb40656ac 100644
@@ -148,7 +148,7 @@ __spitfire_flush_tlb_mm_slow:
 /*
 * The following code flushes one page_size worth.
 */
-	.section .kprobes.text, "ax"
+	.section .text.tbl.kprobes.all, "ax"
 	.align		32
 	.globl		__flush_icache_page

__flush_icache_page: /* %o0 = phys_page */

arch/tile/kernel/vmlinux.lds.S +0 -1

index 0e059a0101ea..0f04268fc394 100644
@@ -43,7 +43,6 @@ SECTIONS
    HEAD_TEXT
    SCHED_TEXT
    LOCK_TEXT
-    KPROBES_TEXT
    IRQENTRY_TEXT
    __fix_text_end = .;   /* tile-cpack won't rearrange before this */
    ALIGN_FUNCTION();

arch/x86/kernel/kprobes/core.c +6 -4

index 1deffe6cc873..dde4ec0e0185 100644
@@ -577,7 +577,10 @@ static int reenter_kprobe(struct kprobe *p, struct pt_regs *regs,
 	case KPROBE_REENTER:
 		/* A probe has been hit in the codepath leading up to, or just
 		 * after, single-stepping of a probed instruction. This entire
-		 * codepath should strictly reside in .kprobes.text section.
+		 * codepath should strictly reside in the kprobes section:
+		 *
+		 * SECTION_TBL(SECTION_TEXT, kprobes, all)
+		 *
 		 * Raise a BUG or we'll continue in an endless reentering loop
 		 * and eventually a stack overflow.
 		 */

@@ -1114,10 +1117,9 @@ NOKPROBE_SYMBOL(longjmp_break_handler);
 
 bool arch_within_kprobe_blacklist(unsigned long addr)
 {
-	return  (addr >= (unsigned long)__kprobes_text_start &&
-		 addr < (unsigned long)__kprobes_text_end) ||
+	return  (LINKTABLE_ADDR_WITHIN(kprobes, addr) ||
 		(addr >= (unsigned long)__entry_text_start &&
-		 addr < (unsigned long)__entry_text_end);
+		 addr < (unsigned long)__entry_text_end));
 }
 
 int __init arch_init_kprobes(void)

arch/x86/kernel/vmlinux.lds.S +0 -1

index 74e4bf11f562..80a9f9106d2a 100644
@@ -99,7 +99,6 @@ SECTIONS
 		TEXT_TEXT
 		SCHED_TEXT
 		LOCK_TEXT
-		KPROBES_TEXT
 		ENTRY_TEXT
 		IRQENTRY_TEXT
 		*(.fixup)

include/asm-generic/sections.h +2 -2

index f5ea98bd85d2..0438aab5cb58 100644
@@ -24,7 +24,7 @@
 * Following global variables are optional and may be unavailable on some
 * architectures and/or kernel configurations.
 *	_text, _data
- *	__kprobes_text_start, __kprobes_text_end
+ *	LINKTABLE_START(kprobes), LINKTABLE_END(kprobes)
 *	__entry_text_start, __entry_text_end
 *	__ctors_start, __ctors_end
 */

@@ -35,7 +35,7 @@ extern char __init_begin[], __init_end[];
 extern char _sinittext[], _einittext[];
 extern char _end[];
 extern char __per_cpu_load[], __per_cpu_start[], __per_cpu_end[];
-extern char __kprobes_text_start[], __kprobes_text_end[];
+DECLARE_LINKTABLE_TEXT(char, kprobes);
 extern char __entry_text_start[], __entry_text_end[];
 extern char __start_rodata[], __end_rodata[];
 

include/asm-generic/vmlinux.lds.h +0 -16

index c5fcac902cbe..f1420714a549 100644
@@ -112,15 +112,6 @@
 #define BRANCH_PROFILE()
 #endif
 
-#ifdef CONFIG_KPROBES
-#define KPROBE_BLACKLIST()	. = ALIGN(8);				      \
-				VMLINUX_SYMBOL(__start_kprobe_blacklist) = .; \
-				*(_kprobe_blacklist)			      \
-				VMLINUX_SYMBOL(__stop_kprobe_blacklist) = .;
-#else
-#define KPROBE_BLACKLIST()
-#endif
-
 #ifdef CONFIG_EVENT_TRACING
 #define FTRACE_EVENTS()	. = ALIGN(8);					\
 			VMLINUX_SYMBOL(__start_ftrace_events) = .;	\

@@ -433,12 +424,6 @@
 		*(.spinlock.text)					\
 		VMLINUX_SYMBOL(__lock_text_end) = .;
 
-#define KPROBES_TEXT							\
-		ALIGN_FUNCTION();					\
-		VMLINUX_SYMBOL(__kprobes_text_start) = .;		\
-		*(.kprobes.text)					\
-		VMLINUX_SYMBOL(__kprobes_text_end) = .;
-
 #define ENTRY_TEXT							\
 		ALIGN_FUNCTION();					\
 		VMLINUX_SYMBOL(__entry_text_start) = .;			\

@@ -504,7 +489,6 @@
 	*(.init.rodata)							\
 	FTRACE_EVENTS()							\
 	TRACE_SYSCALLS()						\
-	KPROBE_BLACKLIST()						\
 	MEM_DISCARD(init.rodata)					\
 	CLK_OF_TABLES()							\
 	RESERVEDMEM_OF_TABLES()						\

include/linux/compiler.h +2 -1

index 00b042c49ccd..8e33fc1ffd3c 100644
@@ -546,7 +546,8 @@ static __always_inline void __write_once_size(volatile void *p, void *res, int s
 
 /* Ignore/forbid kprobes attach on very low level functions marked by this attribute: */
 #ifdef CONFIG_KPROBES
-# define __kprobes	__attribute__((__section__(".kprobes.text")))
+#include <linux/sections.h>
+# define __kprobes	__attribute__((__section__(SECTION_TBL(SECTION_TEXT, kprobes, all))))
 # define nokprobe_inline	__always_inline
 #else
 # define __kprobes

include/linux/kprobes.h +5 -2

index 8f6849084248..544eb59cdbb9 100644
@@ -42,8 +42,12 @@
 #include <linux/ftrace.h>
 
 #ifdef CONFIG_KPROBES
+#include <linux/tables.h>
 #include <asm/kprobes.h>
 
+DECLARE_LINKTABLE_TEXT(char, kprobes);
+DECLARE_LINKTABLE_INIT_DATA(unsigned long, _kprobe_blacklist);
+
 /* kprobe_status settings */
 #define KPROBE_HIT_ACTIVE	0x00000001
 #define KPROBE_HIT_SS		0x00000002

@@ -487,8 +491,7 @@ static inline int enable_jprobe(struct jprobe *jp)
 * by using this macro.
 */
 #define __NOKPROBE_SYMBOL(fname)			\
-static unsigned long __used				\
-	__attribute__((section("_kprobe_blacklist")))	\
+static LINKTABLE_INIT_DATA(_kprobe_blacklist, all)		\
 	_kbl_addr_##fname = (unsigned long)fname;
 #define NOKPROBE_SYMBOL(fname)	__NOKPROBE_SYMBOL(fname)
 #else

kernel/kprobes.c +7 -9

index d10ab6b9b5e0..d816c659f358 100644
@@ -1328,8 +1328,7 @@ out:
 bool __weak arch_within_kprobe_blacklist(unsigned long addr)
 {
 	/* The __kprobes marked functions and entry code must not be probed */
-	return addr >= (unsigned long)__kprobes_text_start &&
-	       addr < (unsigned long)__kprobes_text_end;
+	return LINKTABLE_ADDR_WITHIN(kprobes, addr);
 }
 
 bool within_kprobe_blacklist(unsigned long addr)

@@ -2054,14 +2053,13 @@ NOKPROBE_SYMBOL(dump_kprobe);
 * since a kprobe need not necessarily be at the beginning
 * of a function.
 */
-static int __init populate_kprobe_blacklist(unsigned long *start,
-					     unsigned long *end)
+static int __init populate_kprobe_blacklist(void)
 {
 	unsigned long *iter;
 	struct kprobe_blacklist_entry *ent;
 	unsigned long entry, offset = 0, size = 0;
 
-	for (iter = start; iter < end; iter++) {
+	LINKTABLE_FOR_EACH(iter, _kprobe_blacklist) {
 		entry = arch_deref_entry_point((void *)*iter);
 
 		if (!kernel_text_address(entry) ||

@@ -2126,8 +2124,9 @@ static struct notifier_block kprobe_module_nb = {
 };
 
 /* Markers of _kprobe_blacklist section */
-extern unsigned long __start_kprobe_blacklist[];
-extern unsigned long __stop_kprobe_blacklist[];
+DEFINE_LINKTABLE_INIT_DATA(unsigned long, _kprobe_blacklist);
+/* Actual kprobes linker table */
+DEFINE_LINKTABLE_TEXT(char, kprobes);
 
 static int __init init_kprobes(void)
 {

@@ -2141,8 +2140,7 @@ static int __init init_kprobes(void)
 		raw_spin_lock_init(&(kretprobe_table_locks[i].lock));
 	}
 
-	err = populate_kprobe_blacklist(__start_kprobe_blacklist,
-					__stop_kprobe_blacklist);
+	err = populate_kprobe_blacklist();
 	if (err) {
 		pr_err("kprobes: failed to populate blacklist: %d\n", err);
 		pr_err("Please take care of using kprobes.\n");

scripts/mod/modpost.c +2 -1

index 48958d3cec9e..b0a8f519efb8 100644
@@ -22,6 +22,7 @@
 #include "../../include/generated/autoconf.h"
 #include "../../include/linux/license.h"
 #include "../../include/linux/export.h"
+#include "../../include/linux/sections.h"
 
 /* Are we using CONFIG_MODVERSIONS? */
 static int modversions = 0;

@@ -888,7 +889,7 @@ static void check_section(const char *modname, struct elf_info *elf,
 
 #define DATA_SECTIONS ".data", ".data.rel"
 #define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \
-		".kprobes.text"
+		SECTION_TBL(SECTION_TEXT, kprobes, all)
 #define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \
 		".fixup", ".entry.text", ".exception.text", ".text.*", \
 		".coldtext"

scripts/recordmcount.c +2 -1

index e167592793a7..f37dac17f033 100644
@@ -32,6 +32,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include "../../include/linux/sections.h"
 
 #ifndef EM_METAG
 /* Remove this when these make it to the standard system elf.h. */

@@ -356,7 +357,7 @@ is_mcounted_section_name(char const *const txtname)
 		strcmp(".sched.text",    txtname) == 0 ||
 		strcmp(".spinlock.text", txtname) == 0 ||
 		strcmp(".irqentry.text", txtname) == 0 ||
-		strcmp(".kprobes.text", txtname) == 0 ||
+		strcmp(SECTION_TBL(SECTION_TEXT, kprobe, all), txtname) == 0 ||
 		strcmp(".text.unlikely", txtname) == 0;
 }
 

scripts/recordmcount.pl +1 -2

index 96e2486a6fc4..63e13009dda9 100755
@@ -134,7 +134,7 @@ my %text_sections = (
     ".sched.text" => 1,
     ".spinlock.text" => 1,
     ".irqentry.text" => 1,
-     ".kprobes.text" => 1,
+     ".text.tbl.kprobes.all" => 1,
     ".text.unlikely" => 1,
 );
 
--

2.7.0

Quoted 14 lines

From: Luis R. Rodriguez [mailto:[email protected]]

kprobe makes use of two custom sections:

type name begin end
init.data _kprobe_blacklist __start_kprobe_blacklist __stop_kprobe_blacklist
text .kprobes.text __kprobes_text_start __kprobes_text_end

Port these to the linker table generic solution. This lets us remove all the custom kprobe section declarations on the linker script.

Tested with CONFIG_KPROBES_SANITY_TEST, it passes with:

Quoted 7 lines

Kprobe smoke test: started
Kprobe smoke test: passed successfully

Then tested CONFIG_SAMPLE_KPROBES on do_fork, and the kprobe bites and kicks as expected. Lastly tried registering a kprobe on a kprobe blacklisted symbol (NOKPROBE_SYMBOL()), and confirms that fails to work.

Could you also check to run the testcases by using ftracetest as below?

$ cd tools/testing/selftests/ftrace/
$ sudo ./ftracetest

And I'm not sure about linker table.

Is that possible to support
__kprobes prefix, which moves the functions into kprobes.text?

Actually, I'm on the way to replacing __kprobes to NOKPROBE_SYMBOL macro, since NOKPROBE_SYMBOL() doesn't effect the kernel text itself. On x86, it is already replaced (see commit 820aede0209a), and same work should be done on other archs. So, could you hold this after that?

I think we should remove .kprobes.text first

and move to linker table.

Quoted 22 lines · 平松雅巳 / HIRAMATU，MASAMI

Quoted 14 lines

From: Luis R. Rodriguez [mailto:[email protected]]

kprobe makes use of two custom sections:

type name begin end
init.data _kprobe_blacklist __start_kprobe_blacklist __stop_kprobe_blacklist
text .kprobes.text __kprobes_text_start __kprobes_text_end

Port these to the linker table generic solution. This lets us remove all the custom kprobe section declarations on the linker script.

Tested with CONFIG_KPROBES_SANITY_TEST, it passes with:

Quoted 7 lines

Kprobe smoke test: started
Kprobe smoke test: passed successfully

Then tested CONFIG_SAMPLE_KPROBES on do_fork, and the kprobe bites and kicks as expected. Lastly tried registering a kprobe on a kprobe blacklisted symbol (NOKPROBE_SYMBOL()), and confirms that fails to work.

Quoted 4 lines

Could you also check to run the testcases by using ftracetest as below?

$ cd tools/testing/selftests/ftrace/
$ sudo ./ftracetest

Sure, it all passed:

$ sudo ./ftracetest
=== Ftrace unit tests ===
[1] Basic trace file check	[PASS]
[2] Basic test for tracers	[PASS]
[3] Basic trace clock test	[PASS]
[4] Basic event tracing check	[PASS]
[5] event tracing - enable/disable with event level files	[PASS]
[6] event tracing - enable/disable with subsystem level files	[PASS]
[7] event tracing - enable/disable with top level files	[PASS]
[8] ftrace - function graph filters with stack tracer	[PASS]
[9] ftrace - function graph filters	[PASS]
[10] ftrace - function profiler with function tracing	[PASS]
[11] Test creation and deletion of trace instances	[PASS]
[12] Kprobe dynamic event - adding and removing	[PASS]
[13] Kprobe dynamic event - busy event check	[PASS]
[14] Kprobe dynamic event with arguments	[PASS]
[15] Kprobe dynamic event with function tracer	[PASS]
[16] Kretprobe dynamic event with arguments	[PASS]

# of passed:  16
# of failed:  0
# of unresolved:  0
# of untested:  0
# of unsupported:  0
# of xfailed:  0
# of undefined(test bug):  0

Quoted 1 line

And I'm not sure about linker table.

So there's really a few parts to the linker table work, out of the ones that relate to kprobes:

* linker tables try to generalize our section use, and provide some helpers for common section use
* provides helpers to make it easier to make custom section, but by re-using standard sections
* when a custom section uses standard sections and helpers we also get a few gains:
- developers reviewing code can more easily get a quicker understanding and have expectations set of how the code feature using the custom section could be used
- people grep'ing on the kernel can more easily find specific types of custom section use by looking for the type of interest
- developers adding features do not need to modify
any linker scripts (vmlinux.lds.S) to make use of custom sections

In kprobe's case, since it uses two custom sections, we have only a small use for the first case: .kprobe.text is just used as a place holder for future developer annotated special cased executable code. It also makes use of the generic helpers:
LINKTABLE_ADDR_WITHIN(), LINKTABLE_START(), LINKTABLE_END().

The second use case, for the _kprobe_blacklist, makes much more use of the more advanced linker table helpers, for instance the iterator LINKTABLE_FOR_EACH().

For both though we now have each custom section's actual section clearly highlighted:

* kprobes: .text (SECTION_TEXT)
* kprobe blacklist: init.data (SECTION_INIT_DATA)

A reader / developer can more easily gain an understanding of how the above custom sections could be used just by its type.

Another feature of linker tables, but outside of the scope of how kprobe would use linker tables, is the ability to enable folks to avoid code bit rot by using table-$(CONFIG_FOO) instead of oby-$(CONFIG_FOO) on init paths of code but since this is outside of the scope of how kprobe would use I leave that just as a reference as another part of linker table. Unless of course you want to make people force compile all kprobe support code but only have it linked in when actually enabled. That would be outside of the scope and purpose of this patch though.

Quoted 2 lines

Is that possible to support
__kprobes prefix, which moves the functions into kprobes.text?

Absolutely, the respective change was just to annotate and make it clear the section kprobes were using:

include/linux/compiler.h +2 -1

-# define __kprobes     __attribute__((__section__(".kprobes.text")))
+#include <linux/sections.h>
+# define __kprobes     __attribute__((__section__(SECTION_TBL(SECTION_TEXT, kprobes, all))))

Quoted 6 lines

Actually, I'm on the way to replacing __kprobes to NOKPROBE_SYMBOL macro, since NOKPROBE_SYMBOL() doesn't effect the kernel text itself. On x86, it is already replaced (see commit 820aede0209a), and same work should be done on other archs. So, could you hold this after that?

Sure.

Quoted 1 line

I think we should remove .kprobes.text first

You mean just remove the incorrect users of .kprobes.text because as I read what you described above we have abuse of __kprobes use to protect against kprobes being introduced on those routines, and we should be using NOKPROBE_SYMBOL() instead. So from what I gather its not that you will remove .kprobes.text but rather clean our current abuse of __kprobes for protection to use NOKPROBE_SYMBOL() instead. Is that right?

Quoted 1 line

and move to linker table.

Sure, can you Cc me on your patches? I can follow up later.

This is my v2 of the original linker table work [0], now with six proof of concepts ports of existing code using custom section with custom linker script modifications:

* DEFINE_LINKTABLE_TEXT(char, kprobes);
* DEFINE_LINKTABLE_DATA(struct jump_entry, __jump_table);
* DEFINE_LINKTABLE_DATA(struct _ddebug, __verbose);
* DEFINE_LINKTABLE_RO(struct builtin_fw, builtin_fw);
* DEFINE_LINKTABLE_INIT(struct x86_init_fn, x86_init_fns);
* DEFINE_LINKTABLE_INIT_DATA(unsigned long, _kprobe_blacklist);

I've tested all except jump tables, I'd appreaciate some help with that.

Quoted 11 lines · Luis R. Rodriguez

This is my v2 of the original linker table work [0], now with six proof of concepts ports of existing code using custom section with custom linker script modifications:

* DEFINE_LINKTABLE_TEXT(char, kprobes);
* DEFINE_LINKTABLE_DATA(struct jump_entry, __jump_table);
* DEFINE_LINKTABLE_DATA(struct _ddebug, __verbose);
* DEFINE_LINKTABLE_RO(struct builtin_fw, builtin_fw);
* DEFINE_LINKTABLE_INIT(struct x86_init_fn, x86_init_fns);
* DEFINE_LINKTABLE_INIT_DATA(unsigned long, _kprobe_blacklist);

Quoted 2 lines

I've tested all except jump tables, I'd appreaciate some help with that.

We should add support for read-mostly, probably.

In fact, some of these probably *are* read-mostly.