All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Alexandre Chartre <alexandre.chartre@oracle.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Nicolas Boichat <drinkcat@chromium.org>
Subject: [PATCH 4.19 05/43] vmlinux.lds.h: Create section for protection against instrumentation
Date: Mon, 22 Mar 2021 13:28:19 +0100	[thread overview]
Message-ID: <20210322121920.112066796@linuxfoundation.org> (raw)
In-Reply-To: <20210322121919.936671417@linuxfoundation.org>

From: Nicolas Boichat <drinkcat@chromium.org>

From: Thomas Gleixner <tglx@linutronix.de>

commit 6553896666433e7efec589838b400a2a652b3ffa upstream.

Some code pathes, especially the low level entry code, must be protected
against instrumentation for various reasons:

 - Low level entry code can be a fragile beast, especially on x86.

 - With NO_HZ_FULL RCU state needs to be established before using it.

Having a dedicated section for such code allows to validate with tooling
that no unsafe functions are invoked.

Add the .noinstr.text section and the noinstr attribute to mark
functions. noinstr implies notrace. Kprobes will gain a section check
later.

Provide also a set of markers: instrumentation_begin()/end()

These are used to mark code inside a noinstr function which calls
into regular instrumentable text section as safe.

The instrumentation markers are only active when CONFIG_DEBUG_ENTRY is
enabled as the end marker emits a NOP to prevent the compiler from merging
the annotation points. This means the objtool verification requires a
kernel compiled with this option.

Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
Reviewed-by: Alexandre Chartre <alexandre.chartre@oracle.com>
Acked-by: Peter Zijlstra <peterz@infradead.org>
Link: https://lkml.kernel.org/r/20200505134100.075416272@linutronix.de

[Nicolas:
Guard noinstr macro in include/linux/compiler_types.h in __KERNEL__
&& !__ASSEMBLY__, otherwise noinstr is expanded in the linker
script construct.

Upstream does not have this problem as many macros were moved by
commit 71391bdd2e9a ("include/linux/compiler_types.h: don't pollute
userspace with macro definitions"). We take the minimal approach here
and just guard the new macro.

Minor context conflicts in:
	arch/powerpc/kernel/vmlinux.lds.S
	include/asm-generic/vmlinux.lds.h
	include/linux/compiler.h]
Signed-off-by: Nicolas Boichat <drinkcat@chromium.org>

Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
Technically guarding with !__ASSEMBLY__ should be enough, but
there seems to be no reason to expose this new macro when
!__KERNEL__, so let's just match what upstream does.

Changes in v2:
 - Guard noinstr macro by __KERNEL__ && !__ASSEMBLY__ to prevent
   expansion in linker script and match upstream.

 arch/powerpc/kernel/vmlinux.lds.S |    1 
 include/asm-generic/sections.h    |    3 ++
 include/asm-generic/vmlinux.lds.h |   10 +++++++
 include/linux/compiler.h          |   54 ++++++++++++++++++++++++++++++++++++++
 include/linux/compiler_types.h    |    6 ++++
 scripts/mod/modpost.c             |    2 -
 6 files changed, 75 insertions(+), 1 deletion(-)

--- a/arch/powerpc/kernel/vmlinux.lds.S
+++ b/arch/powerpc/kernel/vmlinux.lds.S
@@ -99,6 +99,7 @@ SECTIONS
 #endif
 		/* careful! __ftr_alt_* sections need to be close to .text */
 		*(.text.hot TEXT_MAIN .text.fixup .text.unlikely .fixup __ftr_alt_* .ref.text);
+		NOINSTR_TEXT
 		SCHED_TEXT
 		CPUIDLE_TEXT
 		LOCK_TEXT
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -53,6 +53,9 @@ extern char __ctors_start[], __ctors_end
 /* Start and end of .opd section - used for function descriptors. */
 extern char __start_opd[], __end_opd[];
 
+/* Start and end of instrumentation protected text section */
+extern char __noinstr_text_start[], __noinstr_text_end[];
+
 extern __visible const void __nosave_begin, __nosave_end;
 
 /* Function descriptor handling (if any).  Override in asm/sections.h */
--- a/include/asm-generic/vmlinux.lds.h
+++ b/include/asm-generic/vmlinux.lds.h
@@ -483,6 +483,15 @@
 	}
 
 /*
+ * Non-instrumentable text section
+ */
+#define NOINSTR_TEXT							\
+		ALIGN_FUNCTION();					\
+		__noinstr_text_start = .;				\
+		*(.noinstr.text)					\
+		__noinstr_text_end = .;
+
+/*
  * .text section. Map to function alignment to avoid address changes
  * during second ld run in second ld pass when generating System.map
  *
@@ -496,6 +505,7 @@
 		*(TEXT_MAIN .text.fixup)				\
 		*(.text.unlikely .text.unlikely.*)			\
 		*(.text.unknown .text.unknown.*)			\
+		NOINSTR_TEXT						\
 		*(.text..refcount)					\
 		*(.ref.text)						\
 	MEM_KEEP(init.text*)						\
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -129,11 +129,65 @@ void ftrace_likely_update(struct ftrace_
 	".pushsection .discard.unreachable\n\t"				\
 	".long 999b - .\n\t"						\
 	".popsection\n\t"
+
+#ifdef CONFIG_DEBUG_ENTRY
+/* Begin/end of an instrumentation safe region */
+#define instrumentation_begin() ({					\
+	asm volatile("%c0:\n\t"						\
+		     ".pushsection .discard.instr_begin\n\t"		\
+		     ".long %c0b - .\n\t"				\
+		     ".popsection\n\t" : : "i" (__COUNTER__));		\
+})
+
+/*
+ * Because instrumentation_{begin,end}() can nest, objtool validation considers
+ * _begin() a +1 and _end() a -1 and computes a sum over the instructions.
+ * When the value is greater than 0, we consider instrumentation allowed.
+ *
+ * There is a problem with code like:
+ *
+ * noinstr void foo()
+ * {
+ *	instrumentation_begin();
+ *	...
+ *	if (cond) {
+ *		instrumentation_begin();
+ *		...
+ *		instrumentation_end();
+ *	}
+ *	bar();
+ *	instrumentation_end();
+ * }
+ *
+ * If instrumentation_end() would be an empty label, like all the other
+ * annotations, the inner _end(), which is at the end of a conditional block,
+ * would land on the instruction after the block.
+ *
+ * If we then consider the sum of the !cond path, we'll see that the call to
+ * bar() is with a 0-value, even though, we meant it to happen with a positive
+ * value.
+ *
+ * To avoid this, have _end() be a NOP instruction, this ensures it will be
+ * part of the condition block and does not escape.
+ */
+#define instrumentation_end() ({					\
+	asm volatile("%c0: nop\n\t"					\
+		     ".pushsection .discard.instr_end\n\t"		\
+		     ".long %c0b - .\n\t"				\
+		     ".popsection\n\t" : : "i" (__COUNTER__));		\
+})
+#endif /* CONFIG_DEBUG_ENTRY */
+
 #else
 #define annotate_reachable()
 #define annotate_unreachable()
 #endif
 
+#ifndef instrumentation_begin
+#define instrumentation_begin()		do { } while(0)
+#define instrumentation_end()		do { } while(0)
+#endif
+
 #ifndef ASM_UNREACHABLE
 # define ASM_UNREACHABLE
 #endif
--- a/include/linux/compiler_types.h
+++ b/include/linux/compiler_types.h
@@ -234,6 +234,12 @@ struct ftrace_likely_data {
 #define notrace			__attribute__((no_instrument_function))
 #endif
 
+#if defined(__KERNEL__) && !defined(__ASSEMBLY__)
+/* Section for code which can't be instrumented at all */
+#define noinstr								\
+	noinline notrace __attribute((__section__(".noinstr.text")))
+#endif
+
 /*
  * it doesn't make sense on ARM (currently the only user of __naked)
  * to trace naked functions because then mcount is called without
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -895,7 +895,7 @@ static void check_section(const char *mo
 
 #define DATA_SECTIONS ".data", ".data.rel"
 #define TEXT_SECTIONS ".text", ".text.unlikely", ".sched.text", \
-		".kprobes.text", ".cpuidle.text"
+		".kprobes.text", ".cpuidle.text", ".noinstr.text"
 #define OTHER_TEXT_SECTIONS ".ref.text", ".head.text", ".spinlock.text", \
 		".fixup", ".entry.text", ".exception.text", ".text.*", \
 		".coldtext"



  parent reply	other threads:[~2021-03-22 13:01 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-03-22 12:28 [PATCH 4.19 00/43] 4.19.183-rc1 review Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 01/43] ASoC: ak4458: Add MODULE_DEVICE_TABLE Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 02/43] ASoC: ak5558: " Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 03/43] ALSA: hda: generic: Fix the micmute led init state Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 04/43] Revert "PM: runtime: Update device status before letting suppliers suspend" Greg Kroah-Hartman
2021-03-22 12:28 ` Greg Kroah-Hartman [this message]
2021-03-22 12:28 ` [PATCH 4.19 06/43] lkdtm: dont move ctors to .rodata Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 07/43] perf tools: Use %define api.pure full instead of %pure-parser Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 08/43] tools build feature: Check if get_current_dir_name() is available Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 09/43] tools build feature: Check if eventfd() " Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 10/43] tools build: Check if gettid() is available before providing helper Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 11/43] btrfs: fix race when cloning extent buffer during rewind of an old root Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 12/43] btrfs: fix slab cache flags for free space tree bitmap Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 13/43] ASoC: fsl_ssi: Fix TDM slot setup for I2S mode Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 14/43] nvmet: dont check iosqes,iocqes for discovery controllers Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 15/43] NFSD: Repair misuse of sv_lock in 5.10.16-rt30 Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 16/43] svcrdma: disable timeouts on rdma backchannel Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 17/43] sunrpc: fix refcount leak for rpc auth modules Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 18/43] net/qrtr: fix __netdev_alloc_skb call Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 19/43] scsi: lpfc: Fix some error codes in debugfs Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 20/43] nvme-rdma: fix possible hang when failing to set io queues Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 21/43] powerpc: Force inlining of cpu_has_feature() to avoid build failure Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 22/43] usb-storage: Add quirk to defeat Kindles automatic unload Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 23/43] usbip: Fix incorrect double assignment to udc->ud.tcp_rx Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 24/43] USB: replace hardcode maximum usb string length by definition Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 25/43] usb: gadget: configfs: Fix KASAN use-after-free Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 26/43] iio:adc:stm32-adc: Add HAS_IOMEM dependency Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 27/43] iio:adc:qcom-spmi-vadc: add default scale to LR_MUX2_BAT_ID channel Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 28/43] iio: adis16400: Fix an error code in adis16400_initial_setup() Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 29/43] iio: gyro: mpu3050: Fix error handling in mpu3050_trigger_handler Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 30/43] iio: hid-sensor-humidity: Fix alignment issue of timestamp channel Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 31/43] iio: hid-sensor-prox: Fix scale not correct issue Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 32/43] iio: hid-sensor-temperature: Fix issues of timestamp channel Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 33/43] PCI: rpadlpar: Fix potential drc_name corruption in store functions Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 34/43] perf/x86/intel: Fix a crash caused by zero PEBS status Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 35/43] x86/ioapic: Ignore IRQ2 again Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 36/43] kernel, fs: Introduce and use set_restart_fn() and arch_set_restart_data() Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 37/43] x86: Move TS_COMPAT back to asm/thread_info.h Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 38/43] x86: Introduce TS_COMPAT_RESTART to fix get_nr_restart_syscall() Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 39/43] ext4: find old entry again if failed to rename whiteout Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 40/43] ext4: do not try to set xattr into ea_inode if value is empty Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 41/43] ext4: fix potential error in ext4_do_update_inode Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 42/43] genirq: Disable interrupts for force threaded handlers Greg Kroah-Hartman
2021-03-22 12:28 ` [PATCH 4.19 43/43] x86/apic/of: Fix CPU devicetree-node lookups Greg Kroah-Hartman
2021-03-22 14:35 ` [PATCH 4.19 00/43] 4.19.183-rc1 review Jon Hunter
2021-03-22 20:14 ` Pavel Machek
2021-03-22 21:53 ` Guenter Roeck
2021-03-23  7:17 ` Samuel Zou
2021-03-23 10:31 ` Naresh Kamboju

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210322121920.112066796@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=alexandre.chartre@oracle.com \
    --cc=drinkcat@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterz@infradead.org \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.