[PATCH v3 0/3] kallsyms: remove special handling for CONFIG_ARM

[PATCH v3 0/3] kallsyms: remove special handling for CONFIG_ARM

[Ard Biesheuvel]

The kallsyms symbol address table generation contains a workaround for
CONFIG_ARM to address an issue with perf when it sees symbols whose
addresses are close to 0x0. The workaround is only enabled if XIP_KERNEL=n,
not because the issue itself only exists in that case, but because the
chosen workaround cannot deal with [_stext,_etext] living below PAGE_OFFSET.

So let's fix this properly, by emitting the stubs symbols at a more reasonable
offset, which is more likely to be the actual location of the vectors and stubs
sections at runtime, and will prevent the symbols from being mistaken for
userland symbols by perf.

Changes since v2:
- Move back to the original arrangement, where .stubs follows .vectors, and both
  are emitted outside of the ordinary kernel VMA space
- Move .vectors and .stubs sections to VMA 0xffff0000/0xffff1000, respectively.
  As Russell pointed out, this is the most reasonable value, since modern CPUs
  actually execute the vectors and stubs from there, and it fixes the kallsyms
  perf issue all the same.
- Rebased onto latest -next, which contains related ARM patch 8514/1, and
  updated kallsyms base relative patches (queued via akpm's tree)

Changes since v1:
- added Nico's ack
- 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.

Ard Biesheuvel (3):
  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

 arch/arm/kernel/entry-armv.S  | 3 +--
 arch/arm/kernel/vmlinux.lds.S | 6 ++++--
 scripts/kallsyms.c            | 8 --------
 scripts/link-vmlinux.sh       | 4 ----
 4 files changed, 5 insertions(+), 16 deletions(-)

-- 
2.5.0

[PATCH v3 1/3] 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.,

  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
addresses are 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 not entirely straightforward.

Since the .vectors and .stubs sections contain position independent code
that is never executed in place, we can emit it at its most likely runtime
VMA (for more recent CPUs), which is 0xffff0000 for the vector table and
0xffff1000 for the stubs. Not only does this fix the perf issue with
kallsyms, allowing us to drop the special case in scripts/kallsyms
entirely, it also gives debuggers a more realistic view of the address
space, and setting breakpoints or single stepping through code in the
vector table or the stubs is more likely to work as expected on CPUs that
use a high vector address. E.g.,

  00001240 A vector_fiq_offset
  ...
  c0c35000 T __init_begin
  c0c35000 T __vectors_start
  c0c35020 T __stubs_start
  c0c35020 T __vectors_end
  c0c352e0 T _sinittext
  c0c352e0 T __stubs_end
  ...
  ffff1004 t vector_rst
  ffff1020 t vector_irq
  ffff10a0 t vector_dabt
  ffff1120 t vector_pabt
  ffff11a0 t vector_und
  ffff1220 t vector_addrexcptn
  ffff1240 T vector_fiq

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

The LMA footprint is identical with or without this change, only the VMAs
are different:

  Before:
  Idx Name          Size      VMA       LMA       File off  Algn
   ...
   14 .notes        00000024  c0c34020  c0c34020  00a34020  2**2
                    CONTENTS, ALLOC, LOAD, READONLY, CODE
   15 .vectors      00000020  00000000  c0c35000  00a40000  2**1
                    CONTENTS, ALLOC, LOAD, READONLY, CODE
   16 .stubs        000002c0  00001000  c0c35020  00a41000  2**5
                    CONTENTS, ALLOC, LOAD, READONLY, CODE
   17 .init.text    0006b1b8  c0c352e0  c0c352e0  00a452e0  2**5
                    CONTENTS, ALLOC, LOAD, READONLY, CODE
   ...

  After:
  Idx Name          Size      VMA       LMA       File off  Algn
   ...
   14 .notes        00000024  c0c34020  c0c34020  00a34020  2**2
                    CONTENTS, ALLOC, LOAD, READONLY, CODE
   15 .vectors      00000020  ffff0000  c0c35000  00a40000  2**1
                    CONTENTS, ALLOC, LOAD, READONLY, CODE
   16 .stubs        000002c0  ffff1000  c0c35020  00a41000  2**5
                    CONTENTS, ALLOC, LOAD, READONLY, CODE
   17 .init.text    0006b1b8  c0c352e0  c0c352e0  00a452e0  2**5
                    CONTENTS, ALLOC, LOAD, READONLY, CODE
   ...

Acked-by: Nicolas Pitre <nico@linaro.org>
Acked-by: Chris Brandt <chris.brandt@renesas.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/kernel/entry-armv.S  | 3 +--
 arch/arm/kernel/vmlinux.lds.S | 6 ++++--
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 788e40c1254f..e2550500486d 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -1201,8 +1201,7 @@ 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
 .L__vectors_start:
diff --git a/arch/arm/kernel/vmlinux.lds.S b/arch/arm/kernel/vmlinux.lds.S
index 8b60fde5ce48..844187a45aa3 100644
--- a/arch/arm/kernel/vmlinux.lds.S
+++ b/arch/arm/kernel/vmlinux.lds.S
@@ -165,19 +165,21 @@ SECTIONS
 	 * only thing that matters is their relative offsets
 	 */
 	__vectors_start = .;
-	.vectors 0 : AT(__vectors_start) {
+	.vectors 0xffff0000 : AT(__vectors_start) {
 		*(.vectors)
 	}
 	. = __vectors_start + SIZEOF(.vectors);
 	__vectors_end = .;
 
 	__stubs_start = .;
-	.stubs 0x1000 : AT(__stubs_start) {
+	.stubs ADDR(.vectors) + 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 : {
 		ARM_EXIT_KEEP(EXIT_TEXT)
-- 
2.5.0

[PATCH v3 2/3] 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>
Acked-by: Chris Brandt <chris.brandt@renesas.com>
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 49d61ade9425..f0f6d9d75435 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_KALLSYMS_ABSOLUTE_PERCPU}" ]; then
 		kallsymopt="${kallsymopt} --absolute-percpu"
 	fi
-- 
2.5.0

[PATCH v3 3/3] 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>
Acked-by: Chris Brandt <chris.brandt@renesas.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 scripts/kallsyms.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 638b143ee60f..de9b74b083c2 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];
@@ -228,10 +227,6 @@ static int symbol_valid(struct sym_entry *s)
 	int i;
 	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++;
@@ -765,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 v3 0/3] kallsyms: remove special handling for CONFIG_ARM

[Chris Brandt]

On 9 Feb 2016, Ard Biesheuvel wrote:

>Ard Biesheuvel (3):
>  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
>
> arch/arm/kernel/entry-armv.S  | 3 +--
> arch/arm/kernel/vmlinux.lds.S | 6 ++++--
> scripts/kallsyms.c            | 8 --------
> scripts/link-vmlinux.sh       | 4 ----
> 4 files changed, 5 insertions(+), 16 deletions(-)


The linker script split (XIP vs non-XIP) has been applied, so now there's a vmlinux-xip.lds.S. So, you'd think you would need to mirror these changes in there (since the breakage of XIP_KERNEL is what started this).

Chris