[PATCH -tip 0/4] Add section address checking helper

[PATCH -tip 0/4] Add section address checking helper

[Masami Hiramatsu]

Hi,

This series adds some address checking helpers in sections.h
so that each one doesn't need to implement it individually.
This series contains basically cleanup patches. Each patch
replaces some checking code with helper function but changes
no behavior.

 - [1/4]: x86: add in_entry_text() helper to cleanup kprobes
	  and unwind_frame code.
 - [2/4]: arm: move in_exception_text() to asm/sections.h and
	  cleanup using memory_contains().
 - [3/4]: arm64: move in_exception_text() to asm/sections.h and
	  cleanup using memory_contains().
 - [4/4]: add in_init_text() and in_core_text() and use it in
	  kernel/extable.c and kernel/kallsyms.c, also this
	  cleanup core_kernel_data using memory_contains().

By the way, I found core_kernel_text() and is_kernel_text()
has different behavior, is_kernel_text() checks arch dependent
text area and in_gate_area_no_mm(), but core_kernel_text()
doesn't. (and core_kernel_text() checks inittext section
while booting up)
Can we ignore this difference or better to merge it?

Thank you,
---

Masami Hiramatsu (4):
      x86: Add in_entry_text() helper function
      arm: Cleanup in_exception_text() and move it in asm/sections.h
      arm64: Cleanup in_exception_text() and move it in asm/sections.h
      extable: kallsyms: Add in_init_text() and in_core_text() helper


 arch/arm/include/asm/sections.h   |   17 +++++++++++++++++
 arch/arm/include/asm/traps.h      |   22 +---------------------
 arch/arm64/include/asm/sections.h |   16 ++++++++++++++++
 arch/arm64/include/asm/traps.h    |   16 ----------------
 arch/x86/include/asm/sections.h   |   15 +++++++++++++++
 arch/x86/kernel/kprobes/opt.c     |    5 +----
 arch/x86/kernel/unwind_frame.c    |   15 +--------------
 include/asm-generic/sections.h    |   24 ++++++++++++++++++++++++
 kernel/extable.c                  |   18 ++++--------------
 kernel/kallsyms.c                 |   14 +++-----------
 10 files changed, 82 insertions(+), 80 deletions(-)

--
Masami Hiramatsu (Linaro)

[PATCH -tip 1/4] x86: Add in_entry_text() helper function

[Masami Hiramatsu]

Add in_entry_text() helper function to cleanup
entry/irqentry section checking code in kprobes and
unwind_frame.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/x86/include/asm/sections.h |   15 +++++++++++++++
 arch/x86/kernel/kprobes/opt.c   |    5 +----
 arch/x86/kernel/unwind_frame.c  |   15 +--------------
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/arch/x86/include/asm/sections.h b/arch/x86/include/asm/sections.h
index 2f75f30cb2f6..bb72ea0f4367 100644
--- a/arch/x86/include/asm/sections.h
+++ b/arch/x86/include/asm/sections.h
@@ -11,4 +11,19 @@ extern struct exception_table_entry __stop___ex_table[];
 extern char __end_rodata_hpage_align[];
 #endif
 
+/**
+ * in_entry_text - check if an address is in entry_text or irqentry_text
+ * @addr: virtual address to be checked
+ *
+ * Returns: true if the address specified by @addr is in the entry_text or
+ * irqentry_text, false otherwise.
+ */
+static inline bool in_entry_text(unsigned long addr)
+{
+	return memory_contains(__entry_text_start, __entry_text_end,
+			       (void *)addr, 0) ||
+		memory_contains(__irqentry_text_start, __irqentry_text_end,
+				(void *)addr, 0);
+}
+
 #endif	/* _ASM_X86_SECTIONS_H */
diff --git a/arch/x86/kernel/kprobes/opt.c b/arch/x86/kernel/kprobes/opt.c
index 4f98aad38237..74c5ad55ba6a 100644
--- a/arch/x86/kernel/kprobes/opt.c
+++ b/arch/x86/kernel/kprobes/opt.c
@@ -254,10 +254,7 @@ static int can_optimize(unsigned long paddr)
 	 * Do not optimize in the entry code due to the unstable
 	 * stack handling and registers setup.
 	 */
-	if (((paddr >= (unsigned long)__entry_text_start) &&
-	     (paddr <  (unsigned long)__entry_text_end)) ||
-	    ((paddr >= (unsigned long)__irqentry_text_start) &&
-	     (paddr <  (unsigned long)__irqentry_text_end)))
+	if (in_entry_text(paddr))
 		return 0;
 
 	/* Check there is enough space for a relative jump. */
diff --git a/arch/x86/kernel/unwind_frame.c b/arch/x86/kernel/unwind_frame.c
index d145a0b1f529..508cef8904f4 100644
--- a/arch/x86/kernel/unwind_frame.c
+++ b/arch/x86/kernel/unwind_frame.c
@@ -77,19 +77,6 @@ static size_t regs_size(struct pt_regs *regs)
 	return sizeof(*regs);
 }
 
-static bool in_entry_code(unsigned long ip)
-{
-	char *addr = (char *)ip;
-
-	if (addr >= __entry_text_start && addr < __entry_text_end)
-		return true;
-
-	if (addr >= __irqentry_text_start && addr < __irqentry_text_end)
-		return true;
-
-	return false;
-}
-
 static inline unsigned long *last_frame(struct unwind_state *state)
 {
 	return (unsigned long *)task_pt_regs(state->task) - 2;
@@ -321,7 +308,7 @@ bool unwind_next_frame(struct unwind_state *state)
 	 * Don't warn if the unwinder got lost due to an interrupt in entry
 	 * code or in the C handler before the first frame pointer got set up:
 	 */
-	if (state->got_irq && in_entry_code(state->ip))
+	if (state->got_irq && in_entry_text(state->ip))
 		goto the_end;
 	if (state->regs &&
 	    state->regs->sp >= (unsigned long)last_aligned_frame(state) &&

[PATCH -tip 2/4] arm: Cleanup in_exception_text() and move it in asm/sections.h

[Masami Hiramatsu]

Cleanup in_exception_text() using memory_contains() and
move it in asm/sections.h from asm/trap.h because
section symbols and memory_contains() are defined in
asm/sections.h.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/arm/include/asm/sections.h |   17 +++++++++++++++++
 arch/arm/include/asm/traps.h    |   22 +---------------------
 2 files changed, 18 insertions(+), 21 deletions(-)

diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
index 803bbf2b20b8..76791cfc7619 100644
--- a/arch/arm/include/asm/sections.h
+++ b/arch/arm/include/asm/sections.h
@@ -4,5 +4,22 @@
 #include <asm-generic/sections.h>
 
 extern char _exiprom[];
+extern char __exception_text_start[], __exception_text_end[];
+
+/**
+ * in_exception_text - check if an address is in exception_text or
+ *			irqentry_text
+ * @addr: virtual address to be checked
+ *
+ * Returns: true if the address specified by @addr is in the exception_text or
+ * irqentry_text, false otherwise.
+ */
+static inline bool in_exception_text(unsigned long addr)
+{
+	return memory_contains(__exception_text_start, __exception_text_end,
+			       (void *)addr, 0) ||
+		memory_contains(__irqentry_text_start, __irqentry_text_end,
+				(void *)addr, 0);
+}
 
 #endif	/* _ASM_ARM_SECTIONS_H */
diff --git a/arch/arm/include/asm/traps.h b/arch/arm/include/asm/traps.h
index 683d9230984a..0ee9a87b9dd4 100644
--- a/arch/arm/include/asm/traps.h
+++ b/arch/arm/include/asm/traps.h
@@ -2,6 +2,7 @@
 #define _ASMARM_TRAP_H
 
 #include <linux/list.h>
+#include <asm/sections.h>
 
 struct pt_regs;
 struct task_struct;
@@ -18,27 +19,6 @@ struct undef_hook {
 void register_undef_hook(struct undef_hook *hook);
 void unregister_undef_hook(struct undef_hook *hook);
 
-static inline int __in_irqentry_text(unsigned long ptr)
-{
-	extern char __irqentry_text_start[];
-	extern char __irqentry_text_end[];
-
-	return ptr >= (unsigned long)&__irqentry_text_start &&
-	       ptr < (unsigned long)&__irqentry_text_end;
-}
-
-static inline int in_exception_text(unsigned long ptr)
-{
-	extern char __exception_text_start[];
-	extern char __exception_text_end[];
-	int in;
-
-	in = ptr >= (unsigned long)&__exception_text_start &&
-	     ptr < (unsigned long)&__exception_text_end;
-
-	return in ? : __in_irqentry_text(ptr);
-}
-
 extern void __init early_trap_init(void *);
 extern void dump_backtrace_entry(unsigned long where, unsigned long from, unsigned long frame);
 extern void ptrace_break(struct task_struct *tsk, struct pt_regs *regs);

[PATCH -tip 2/4] arm: Cleanup in_exception_text() and move it in asm/sections.h

[Russell King - ARM Linux]

On Thu, Aug 17, 2017 at 04:15:00PM +0900, Masami Hiramatsu wrote:
> Cleanup in_exception_text() using memory_contains() and
> move it in asm/sections.h from asm/trap.h because
> section symbols and memory_contains() are defined in
> asm/sections.h.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> ---
>  arch/arm/include/asm/sections.h |   17 +++++++++++++++++
>  arch/arm/include/asm/traps.h    |   22 +---------------------
>  2 files changed, 18 insertions(+), 21 deletions(-)
> 
> diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
> index 803bbf2b20b8..76791cfc7619 100644
> --- a/arch/arm/include/asm/sections.h
> +++ b/arch/arm/include/asm/sections.h
> @@ -4,5 +4,22 @@
>  #include <asm-generic/sections.h>
>  
>  extern char _exiprom[];
> +extern char __exception_text_start[], __exception_text_end[];
> +
> +/**
> + * in_exception_text - check if an address is in exception_text or
> + *			irqentry_text
> + * @addr: virtual address to be checked
> + *
> + * Returns: true if the address specified by @addr is in the exception_text or
> + * irqentry_text, false otherwise.
> + */
> +static inline bool in_exception_text(unsigned long addr)
> +{
> +	return memory_contains(__exception_text_start, __exception_text_end,
> +			       (void *)addr, 0) ||
> +		memory_contains(__irqentry_text_start, __irqentry_text_end,
> +				(void *)addr, 0);

Patch looks buggy - where is __irqentry_text_start and __irqentry_text_end
declared?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up

[PATCH -tip 2/4] arm: Cleanup in_exception_text() and move it in asm/sections.h

[Masami Hiramatsu]

Hi Russell,

On Sun, 3 Sep 2017 23:21:52 +0100
Russell King - ARM Linux <linux@armlinux.org.uk> wrote:

> On Thu, Aug 17, 2017 at 04:15:00PM +0900, Masami Hiramatsu wrote:
> > Cleanup in_exception_text() using memory_contains() and
> > move it in asm/sections.h from asm/trap.h because
> > section symbols and memory_contains() are defined in
> > asm/sections.h.
> > 
> > Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> > ---
> >  arch/arm/include/asm/sections.h |   17 +++++++++++++++++
> >  arch/arm/include/asm/traps.h    |   22 +---------------------
> >  2 files changed, 18 insertions(+), 21 deletions(-)
> > 
> > diff --git a/arch/arm/include/asm/sections.h b/arch/arm/include/asm/sections.h
> > index 803bbf2b20b8..76791cfc7619 100644
> > --- a/arch/arm/include/asm/sections.h
> > +++ b/arch/arm/include/asm/sections.h
> > @@ -4,5 +4,22 @@
> >  #include <asm-generic/sections.h>
> >  
> >  extern char _exiprom[];
> > +extern char __exception_text_start[], __exception_text_end[];
> > +
> > +/**
> > + * in_exception_text - check if an address is in exception_text or
> > + *			irqentry_text
> > + * @addr: virtual address to be checked
> > + *
> > + * Returns: true if the address specified by @addr is in the exception_text or
> > + * irqentry_text, false otherwise.
> > + */
> > +static inline bool in_exception_text(unsigned long addr)
> > +{
> > +	return memory_contains(__exception_text_start, __exception_text_end,
> > +			       (void *)addr, 0) ||
> > +		memory_contains(__irqentry_text_start, __irqentry_text_end,
> > +				(void *)addr, 0);
> 
> Patch looks buggy - where is __irqentry_text_start and __irqentry_text_end
> declared?

Those are declared in asm-generic/sections.h by below patch (on -tip tree)

https://patchwork.kernel.org/patch/9878033/

So, I thougt it was a good time to clean up related code.

Thank you,

-- 
Masami Hiramatsu <mhiramat@kernel.org>

[PATCH -tip 3/4] arm64: Cleanup in_exception_text() and move it in asm/sections.h

[Masami Hiramatsu]

Cleanup in_exception_text() using memory_contains() and
move it in asm/sections.h from asm/trap.h because
section symbols and memory_contains() are defined in
asm/sections.h.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
 arch/arm64/include/asm/sections.h |   16 ++++++++++++++++
 arch/arm64/include/asm/traps.h    |   16 ----------------
 2 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/include/asm/sections.h b/arch/arm64/include/asm/sections.h
index 941267caa39c..abaa8ac79eaf 100644
--- a/arch/arm64/include/asm/sections.h
+++ b/arch/arm64/include/asm/sections.h
@@ -29,4 +29,20 @@ extern char __inittext_begin[], __inittext_end[];
 extern char __irqentry_text_start[], __irqentry_text_end[];
 extern char __mmuoff_data_start[], __mmuoff_data_end[];
 
+/**
+ * in_exception_text - check if an address is in exception_text or
+ *			irqentry_text
+ * @addr: virtual address to be checked
+ *
+ * Returns: true if the address specified by @addr is in the exception_text or
+ * irqentry_text, false otherwise.
+ */
+static inline bool in_exception_text(unsigned long addr)
+{
+	return memory_contains(__exception_text_start, __exception_text_end,
+			       (void *)addr, 0) ||
+		memory_contains(__irqentry_text_start, __irqentry_text_end,
+				(void *)addr, 0);
+}
+
 #endif /* __ASM_SECTIONS_H */
diff --git a/arch/arm64/include/asm/traps.h b/arch/arm64/include/asm/traps.h
index 47a9066f7c86..c3734c3e8115 100644
--- a/arch/arm64/include/asm/traps.h
+++ b/arch/arm64/include/asm/traps.h
@@ -37,20 +37,4 @@ void unregister_undef_hook(struct undef_hook *hook);
 
 void arm64_notify_segfault(struct pt_regs *regs, unsigned long addr);
 
-static inline int __in_irqentry_text(unsigned long ptr)
-{
-	return ptr >= (unsigned long)&__irqentry_text_start &&
-	       ptr < (unsigned long)&__irqentry_text_end;
-}
-
-static inline int in_exception_text(unsigned long ptr)
-{
-	int in;
-
-	in = ptr >= (unsigned long)&__exception_text_start &&
-	     ptr < (unsigned long)&__exception_text_end;
-
-	return in ? : __in_irqentry_text(ptr);
-}
-
 #endif

[PATCH -tip 4/4] extable: kallsyms: Add in_init_text() and in_core_text() helper

[Masami Hiramatsu]

Add in_init_text() and in_core_text() helper to check the
address is in between _sinittext and _einittext, and
in between _stext and _etext correspondingly.

This changes extable.c and kallsyms.c to use these helpers
and memory_contains() helper.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
---
  BTW, is_kernel_text() and core_kernel_text() seem different.
  is_kernel_text() checks arch dependent area and
  in_gate_area_no_mm(), but core_kernel_text() doesn't.
  Can we ignore this differences?
---
 include/asm-generic/sections.h |   24 ++++++++++++++++++++++++
 kernel/extable.c               |   18 ++++--------------
 kernel/kallsyms.c              |   14 +++-----------
 3 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index e5da44eddd2f..25ef53fc594c 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -135,4 +135,28 @@ static inline bool init_section_intersects(void *virt, size_t size)
 	return memory_intersects(__init_begin, __init_end, virt, size);
 }
 
+/**
+ * in_init_text - check if an address is in between _sinittext and _einittext.
+ * @addr: virtual address to be checked
+ *
+ * Returns: true if the address specified by @addr is in between _sinittext
+ * and _einittext, false otherwise.
+ */
+static inline bool in_init_text(unsigned long addr)
+{
+	return memory_contains(_sinittext, _einittext, (void *)addr, 0);
+}
+
+/**
+ * in_core_text - check if an address is in between _stext and _etext.
+ * @addr: virtual address to be checked
+ *
+ * Returns: true if the address specified by @addr is in between _stext
+ * and _etext, false otherwise.
+ */
+static inline bool in_core_text(unsigned long addr)
+{
+	return memory_contains(_stext, _etext, (void *)addr, 0);
+}
+
 #endif /* _ASM_GENERIC_SECTIONS_H_ */
diff --git a/kernel/extable.c b/kernel/extable.c
index 38c2412401a1..d8b365c8c916 100644
--- a/kernel/extable.c
+++ b/kernel/extable.c
@@ -62,22 +62,13 @@ const struct exception_table_entry *search_exception_tables(unsigned long addr)
 	return e;
 }
 
-static inline int init_kernel_text(unsigned long addr)
-{
-	if (addr >= (unsigned long)_sinittext &&
-	    addr < (unsigned long)_einittext)
-		return 1;
-	return 0;
-}
-
 int notrace core_kernel_text(unsigned long addr)
 {
-	if (addr >= (unsigned long)_stext &&
-	    addr < (unsigned long)_etext)
+	if (in_core_text(addr))
 		return 1;
 
 	if (system_state < SYSTEM_RUNNING &&
-	    init_kernel_text(addr))
+	    in_init_text(addr))
 		return 1;
 	return 0;
 }
@@ -94,8 +85,7 @@ int notrace core_kernel_text(unsigned long addr)
  */
 int core_kernel_data(unsigned long addr)
 {
-	if (addr >= (unsigned long)_sdata &&
-	    addr < (unsigned long)_edata)
+	if (memory_contains(_sdata, _edata, (void *)addr, 0))
 		return 1;
 	return 0;
 }
@@ -120,7 +110,7 @@ int __kernel_text_address(unsigned long addr)
 	 * Since we are after the module-symbols check, there's
 	 * no danger of address overlap:
 	 */
-	if (init_kernel_text(addr))
+	if (in_init_text(addr))
 		return 1;
 	return 0;
 }
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 127e7cfafa55..b1443e845dbd 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -51,17 +51,9 @@ extern const u16 kallsyms_token_index[] __weak;
 
 extern const unsigned long kallsyms_markers[] __weak;
 
-static inline int is_kernel_inittext(unsigned long addr)
-{
-	if (addr >= (unsigned long)_sinittext
-	    && addr <= (unsigned long)_einittext)
-		return 1;
-	return 0;
-}
-
 static inline int is_kernel_text(unsigned long addr)
 {
-	if ((addr >= (unsigned long)_stext && addr <= (unsigned long)_etext) ||
+	if (in_core_text(addr) ||
 	    arch_is_kernel_text(addr))
 		return 1;
 	return in_gate_area_no_mm(addr);
@@ -79,7 +71,7 @@ static int is_ksym_addr(unsigned long addr)
 	if (IS_ENABLED(CONFIG_KALLSYMS_ALL))
 		return is_kernel(addr);
 
-	return is_kernel_text(addr) || is_kernel_inittext(addr);
+	return is_kernel_text(addr) || in_init_text(addr);
 }
 
 /*
@@ -272,7 +264,7 @@ static unsigned long get_symbol_pos(unsigned long addr,
 
 	/* If we found no next symbol, we use the end of the section. */
 	if (!symbol_end) {
-		if (is_kernel_inittext(addr))
+		if (in_init_text(addr))
 			symbol_end = (unsigned long)_einittext;
 		else if (IS_ENABLED(CONFIG_KALLSYMS_ALL))
 			symbol_end = (unsigned long)_end;