[PATCH v2 0/4] kallsyms: remove special handling for CONFIG_ARM

[PATCH v2 0/4] kallsyms: remove special handling for CONFIG_ARM

[Ard Biesheuvel]

This series applies on top of today's -next, and addresses an issue with
the new kallsyms code that is queued there, that enables base relative
kallsyms tables for all architectures (except IA-64)

Two issues have surfaced on ARM with the new kallsyms code:
a) CONFIG_HAVE_TCM creates a virtual region that is too far away for the
   relative kallsyms code to reach it;
b) CONFIG_XIP_KERNEL=y kernel symbols are not filtered against PAGE_OFFSET,
   as is the case for kernels that execute from RAM, resulting in symbols
   that are out of range.

Since the way kallsyms deals with XIP kernels on ARM leaves some room for
improvement regardless of the base relative changes, this series proposes
a fix that allows the special case to be removed from the kallsyms handling
entirely.

Changes since v1:
- new patch #4 (optional, RFC)
- added Nico's ack to #1 - #3
- use PROVIDE() for vector_fiq_offset since vector_fiq itself is not always
  defined
- put __stubs_start/_end inside the section definition so that the value of the
  start symbol equals the start of the section after alignment (fixes an issue
  on XIP spotted by Chris)

Patch #1 moves the .stubs and .vectors section back into the kernel VMA, while
preserving the guaranteed virtual offset of 4 KB. This results in all symbols
that kallsyms sees to be in a reasonable interval.

Patch #2 removes the special case for CONFIG_ARM && !CONFIG_XIP_KERNEL in the
invocation of scripts/kallsyms

Patch #3 removes the now unused --page-offset command line argument handling
from scripts/kallsyms.c

Patch #4 is included as an RFC, it removes the magic constant 0x1000 which is
the offset between the start of the .vectors section and the start of the
.stubs section.

Note that we may still need to remove ARM from the list of architectures that
support base relative kallsyms tables if we cannot fix issue a) above, but
removing this special case seemed like an obvious improvement to me. Note the
a) is a pathological case where VMSPLIT_1G, KALLSYMS_ALL and HAVE_TCM are all
set.

Ard Biesheuvel (4):
  ARM: move .vectors and .stubs sections back into the kernel VMA
  kallsyms: remove special lower address limit for CONFIG_ARM
  kallsyms: remove --page-offset command line option
  ARM: use single definition for vectors-to-stubs section offset

 arch/arm/kernel/entry-armv.S  | 20 ++++++++++-----
 arch/arm/kernel/traps.c       |  4 ++-
 arch/arm/kernel/vmlinux.lds.S | 22 +++++++++++-----
 scripts/kallsyms.c            | 27 ++++----------------
 scripts/link-vmlinux.sh       |  4 ---
 5 files changed, 37 insertions(+), 40 deletions(-)

-- 
2.5.0

[PATCH v2 1/4] ARM: move .vectors and .stubs sections back into the kernel VMA

[Ard Biesheuvel]

Commit b9b32bf70f2f ("ARM: use linker magic for vectors and vector stubs")
updated the linker script to emit the .vectors and .stubs sections into a
VMA range that is zero based and disjoint from the normal static kernel
region. The reason for that was that this way, the sections can be placed
exactly 4 KB apart, while the payload of the .vectors section is only 32
bytes.

Since the symbols that are part of the .stubs section are emitted into the
kallsyms table, they appear with zero based addresses as well, e.g.,

  00000000 t __vectors_start
  00001000 t __stubs_start
  00001004 t vector_rst
  00001020 t vector_irq
  000010a0 t vector_dabt
  00001120 t vector_pabt
  000011a0 t vector_und
  00001220 t vector_addrexcptn
  00001240 t vector_fiq
  00001240 T vector_fiq_offset

As this confuses perf when it accesses the kallsyms tables, commit
7122c3e9154b ("scripts/link-vmlinux.sh: only filter kernel symbols for
arm") implemented a somewhat ugly special case for ARM, where the value
of CONFIG_PAGE_OFFSET is passed to scripts/kallsyms, and symbols whose
address is below it are filtered out. Note that this special case only
applies to CONFIG_XIP_KERNEL=n, not because the issue the patch addresses
exists only in that case, but because finding a limit below which to apply
the filtering is too difficult.

Since the constraint we are trying to meet here is that the .vectors
section lives exactly 4 KB before the .stubs section, regardless of the
absolute addresses of either (since relative branches are used to jump from
the vector table to the stubs), we can simply emit the .stubs section as
part of the kernel VMA, and place the .vectors section 4 KB before it using
an explicit VMA override. By doing that, and renaming the __vectors_start
symbol that is local to arch/arm/kernel/entry-armv.S (not the one in the
linker script), the kallsyms table looks somewhat sane, regardless of
whether CONFIG_XIP_KERNEL is set, and we can drop the special case in
scripts/kallsyms entirely. E.g.,

  00001240 A vector_fiq_offset
  ...
  c0c35000 T __init_begin
  c0c35000 t __stubs_start
  c0c35000 T __stubs_start
  c0c35004 t vector_rst
  c0c35020 t vector_irq
  c0c350a0 t vector_dabt
  c0c35120 t vector_pabt
  c0c351a0 t vector_und
  c0c35220 t vector_addrexcptn
  c0c35240 T vector_fiq
  c0c352c0 T __stubs_end
  c0c352c0 T __vectors_start
  c0c352e0 t __mmap_switched
  c0c352e0 T _sinittext
  c0c352e0 T __vectors_end

(Note that vector_fiq_offset is now an absolute symbol, which kallsyms
already ignores by default)

The sections themselves are emitted 4 KB apart, as required:
  ...
  [16] .stubs       PROGBITS   c0c35000 a35000 0002c0 00  AX  0   0 32
  [17] .vectors     PROGBITS   c0c34000 a44000 000020 00  AX  0   0  2
  ...

and the relative branches in the .vectors section still point to the
right place:

  c0c34000 <.vectors>:
  c0c34000:       f001 b800       b.w     c0c35004 <vector_rst>
  c0c34004:       f001 b8cc       b.w     c0c351a0 <vector_und>
  c0c34008:       f8df fff4       ldr.w   pc, [pc, #4084] ; c0c35000
  c0c3400c:       f001 b888       b.w     c0c35120 <vector_pabt>
  c0c34010:       f001 b846       b.w     c0c350a0 <vector_dabt>
  c0c34014:       f001 b904       b.w     c0c35220 <vector_addrexcptn>
  c0c34018:       f001 b802       b.w     c0c35020 <vector_irq>
  c0c3401c:       f001 b910       b.w     c0c35240 <vector_fiq>

Acked-by: Nicolas Pitre <nico@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/kernel/entry-armv.S  |  7 +++----
 arch/arm/kernel/vmlinux.lds.S | 15 ++++++++-------
 2 files changed, 11 insertions(+), 11 deletions(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 3ce377f7251f..8575ff42c0d4 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -1202,14 +1202,13 @@ vector_addrexcptn:
 	.long	__fiq_svc			@  e
 	.long	__fiq_svc			@  f
 
-	.globl	vector_fiq_offset
-	.equ	vector_fiq_offset, vector_fiq
+	.globl	vector_fiq
 
 	.section .vectors, "ax", %progbits
-__vectors_start:
+.L__vectors_start:
 	W(b)	vector_rst
 	W(b)	vector_und
-	W(ldr)	pc, __vectors_start + 0x1000
+	W(ldr)	pc, .L__vectors_start + 0x1000
 	W(b)	vector_pabt
 	W(b)	vector_dabt
 	W(b)	vector_addrexcptn
diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 8b60fde5ce48..9f96a54c7d90 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -164,19 +164,20 @@ SECTIONS
 	 * The vectors and stubs are relocatable code, and the
 	 * only thing that matters is their relative offsets
 	 */
+	.stubs : {
+		__stubs_start = .;
+		*(.stubs)
+		__stubs_end = .;
+	}
+
 	__vectors_start = .;
-	.vectors 0 : AT(__vectors_start) {
+	.vectors ADDR(.stubs) - 0x1000 : AT(__vectors_start) {
 		*(.vectors)
 	}
 	. = __vectors_start + SIZEOF(.vectors);
 	__vectors_end = .;
 
-	__stubs_start = .;
-	.stubs 0x1000 : AT(__stubs_start) {
-		*(.stubs)
-	}
-	. = __stubs_start + SIZEOF(.stubs);
-	__stubs_end = .;
+	PROVIDE(vector_fiq_offset = vector_fiq - ADDR(.vectors));
 
 	INIT_TEXT_SECTION(8)
 	.exit.text : {
-- 
2.5.0

[PATCH v2 2/4] kallsyms: remove special lower address limit for CONFIG_ARM

[Ard Biesheuvel]

Now that we no longer emit .stubs symbols into a section VMA loaded
at absolute address 0x1000, we can drop the ARM-specific override that
sets a lower limit based on CONFIG_PAGE_OFFSET, below which symbols are
filtered from the kallsyms output.

Acked-by: Nicolas Pitre <nico@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 scripts/link-vmlinux.sh | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index aae2473301ea..b83f918c7830 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -82,10 +82,6 @@ kallsyms()
 		kallsymopt="${kallsymopt} --all-symbols"
 	fi
 
-	if [ -n "${CONFIG_ARM}" ] && [ -z "${CONFIG_XIP_KERNEL}" ] && [ -n "${CONFIG_PAGE_OFFSET}" ]; then
-		kallsymopt="${kallsymopt} --page-offset=$CONFIG_PAGE_OFFSET"
-	fi
-
 	if [ -n "${CONFIG_X86_64}" ] && [ -n "${CONFIG_SMP}" ]; then
 		kallsymopt="${kallsymopt} --absolute-percpu"
 	fi
-- 
2.5.0

[PATCH v2 3/4] kallsyms: remove --page-offset command line option

[Ard Biesheuvel]

The --page-offset command line option was only used for ARM, to filter
symbol addresses below CONFIG_PAGE_OFFSET. This is no longer needed, so
remove the functionality altogether.

Acked-by: Nicolas Pitre <nico@linaro.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 scripts/kallsyms.c | 27 ++++----------------
 1 file changed, 5 insertions(+), 22 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 02473b71643b..ce36fc02fe8c 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -63,7 +63,6 @@ static unsigned int table_size, table_cnt;
 static int all_symbols = 0;
 static int absolute_percpu = 0;
 static char symbol_prefix_char = '\0';
-static unsigned long long kernel_start_addr = 0;
 static int base_relative = 0;
 
 int token_profit[0x10000];
@@ -229,9 +228,6 @@ static int symbol_valid(struct sym_entry *s)
 	char *sym_name = (char *)s->sym + 1;
 
 
-	if (s->addr < kernel_start_addr)
-		return 0;
-
 	/* skip prefix char */
 	if (symbol_prefix_char && *sym_name == symbol_prefix_char)
 		sym_name++;
@@ -742,21 +738,11 @@ static void record_relative_base(void)
 {
 	unsigned int i;
 
-	if (kernel_start_addr > 0) {
-		/*
-		 * If the kernel start address was specified, use that as
-		 * the relative base rather than going through the table,
-		 * since it should be a reasonable default, and values below
-		 * it will be ignored anyway.
-		 */
-		relative_base = kernel_start_addr;
-	} else {
-		relative_base = -1ULL;
-		for (i = 0; i < table_cnt; i++)
-			if (!symbol_absolute(&table[i]) &&
-			    table[i].addr < relative_base)
-				relative_base = table[i].addr;
-	}
+	relative_base = -1ULL;
+	for (i = 0; i < table_cnt; i++)
+		if (!symbol_absolute(&table[i]) &&
+		    table[i].addr < relative_base)
+			relative_base = table[i].addr;
 }
 
 int main(int argc, char **argv)
@@ -774,9 +760,6 @@ int main(int argc, char **argv)
 				if ((*p == '"' && *(p+2) == '"') || (*p == '\'' && *(p+2) == '\''))
 					p++;
 				symbol_prefix_char = *p;
-			} else if (strncmp(argv[i], "--page-offset=", 14) == 0) {
-				const char *p = &argv[i][14];
-				kernel_start_addr = strtoull(p, NULL, 16);
 			} else if (strcmp(argv[i], "--base-relative") == 0)
 				base_relative = 1;
 			else
-- 
2.5.0

[PATCH v2 4/4] ARM: use single definition for vectors-to-stubs section offset

[Ard Biesheuvel]

This replaces a couple of unannotated uses of the constant 0x1000 with
a single definition in the linker script, and updates the referring
code to use it.

Note that this involves explicitly emitting a relocation against the
literal in .stubs containing the address of vector_swi(), since the
assembler refuses to do so.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/kernel/entry-armv.S  | 17 +++++++++++++----
 arch/arm/kernel/traps.c       |  4 +++-
 arch/arm/kernel/vmlinux.lds.S |  9 ++++++++-
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 8575ff42c0d4..c19d3b82edb8 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -1064,8 +1064,7 @@ ENDPROC(vector_\name)
 	.endm
 
 	.section .stubs, "ax", %progbits
-__stubs_start:
-	@ This must be the first word
+.Lvector_swi:
 	.word	vector_swi
 
 vector_rst:
@@ -1205,16 +1204,26 @@ vector_addrexcptn:
 	.globl	vector_fiq
 
 	.section .vectors, "ax", %progbits
-.L__vectors_start:
 	W(b)	vector_rst
 	W(b)	vector_und
-	W(ldr)	pc, .L__vectors_start + 0x1000
+0:	W(ldr)	pc, .				@ vector_swi
 	W(b)	vector_pabt
 	W(b)	vector_dabt
 	W(b)	vector_addrexcptn
 	W(b)	vector_irq
 	W(b)	vector_fiq
 
+	/*
+	 * The assembler refuses to emit the correct relocation for the
+	 * cross-section reference to .Lvector_swi inside the ldr instruction
+	 * above, so we have to emit it manually. Note that the addend cannot
+	 * be set by the .reloc directive, and the initial offset recorded in
+	 * the instruction should compensate for the PC bias of the execution
+	 * mode, hence the use of '.' as the label.
+	 */
+ ARM(	.reloc	0b, R_ARM_LDR_PC_G0, .Lvector_swi	)
+ THUMB(	.reloc	0b, R_ARM_THM_PC12, .Lvector_swi	)
+
 	.data
 
 	.globl	cr_alignment
diff --git a/arch/arm/kernel/traps.c b/arch/arm/kernel/traps.c
index bc698383e822..d90a5b2c02a3 100644
--- a/arch/arm/kernel/traps.c
+++ b/arch/arm/kernel/traps.c
@@ -792,6 +792,8 @@ void __init early_trap_init(void *vectors_base)
 {
 #ifndef CONFIG_CPU_V7M
 	unsigned long vectors = (unsigned long)vectors_base;
+	extern char __stubs_offset[];
+	unsigned long stubs = vectors + (unsigned long)&__stubs_offset;
 	extern char __stubs_start[], __stubs_end[];
 	extern char __vectors_start[], __vectors_end[];
 	unsigned i;
@@ -813,7 +815,7 @@ void __init early_trap_init(void *vectors_base)
 	 * are visible to the instruction stream.
 	 */
 	memcpy((void *)vectors, __vectors_start, __vectors_end - __vectors_start);
-	memcpy((void *)vectors + 0x1000, __stubs_start, __stubs_end - __stubs_start);
+	memcpy((void *)stubs, __stubs_start, __stubs_end - __stubs_start);
 
 	kuser_init(vectors_base);
 
diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 9f96a54c7d90..8f270c8e9153 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -163,7 +163,14 @@ SECTIONS
 	/*
 	 * The vectors and stubs are relocatable code, and the
 	 * only thing that matters is their relative offsets
+	 * The .stubs section is virtually placed exactly 4 KB after
+	 * the .vectors section, so that it is in reach for indirect
+	 * jumps from the vector table ('ldr pc, <label>', where the
+	 * label itself is inside the .stubs section) but can still
+	 * be mapped with different permissions.
 	 */
+	__stubs_offset = 0x1000;
+
 	.stubs : {
 		__stubs_start = .;
 		*(.stubs)
@@ -171,7 +178,7 @@ SECTIONS
 	}
 
 	__vectors_start = .;
-	.vectors ADDR(.stubs) - 0x1000 : AT(__vectors_start) {
+	.vectors ADDR(.stubs) - __stubs_offset : AT(__vectors_start) {
 		*(.vectors)
 	}
 	. = __vectors_start + SIZEOF(.vectors);
-- 
2.5.0

[PATCH v2 0/4] kallsyms: remove special handling for CONFIG_ARM

[Chris Brandt]

On 3 Feb 2016, Ard Biesheuvel wrote:

> This series applies on top of today's -next, and addresses an issue
> with the new kallsyms code that is queued there, that enables base
> relative kallsyms tables for all architectures (except IA-64)
>
> Two issues have surfaced on ARM with the new kallsyms code:
> a) CONFIG_HAVE_TCM creates a virtual region that is too far away for the
>    relative kallsyms code to reach it;
> b) CONFIG_XIP_KERNEL=y kernel symbols are not filtered against PAGE_OFFSET,
>   as is the case for kernels that execute from RAM, resulting in symbols
>   that are out of range.
>
> Since the way kallsyms deals with XIP kernels on ARM leaves some room for
>  improvement regardless of the base relative changes, this series proposes
>  a fix that allows the special case to be removed from the kallsyms
>  handling entirely.
>
> Changes since v1:
> - new patch #4 (optional, RFC)
> - added Nico's ack to #1 - #3
> - use PROVIDE() for vector_fiq_offset since vector_fiq itself is not
>  always
>   defined
> - put __stubs_start/_end inside the section definition so that the
>  value of the
>   start symbol equals the start of the section after alignment
>  (fixes an issue
>   on XIP spotted by Chris)
> 
> Patch #1 moves the .stubs and .vectors section back into the kernel
> VMA, while preserving the guaranteed virtual offset of 4 KB. This
> results in all symbols that kallsyms sees to be in a reasonable interval.
> 
> Patch #2 removes the special case for CONFIG_ARM && !CONFIG_XIP_KERNEL
> in the invocation of scripts/kallsyms
> 
> Patch #3 removes the now unused --page-offset command line argument
> handling from scripts/kallsyms.c
> 
> Patch #4 is included as an RFC, it removes the magic constant 0x1000
> which is the offset between the start of the .vectors section and the
> start of the .stubs section.


For patches 1,2,3:

Tested with ARCH_R7S72100 with CONFIG_XIP_KERNEL=y


Acked-by: Chris Brandt <chris.brandt@renesas.com>