All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4 0/3] kallsyms base relative series
@ 2016-01-25 14:19 Ard Biesheuvel
  2016-01-25 14:19 ` [PATCH v4 1/3] x86: kallsyms: disable absolute percpu symbols on !SMP Ard Biesheuvel
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2016-01-25 14:19 UTC (permalink / raw)
  To: linux-kernel, linux-s390, linuxppc-dev, x86, keescook, akpm,
	mingo, hpa, heiko.carstens, benh, mpe, mmarek, rusty, arnd,
	linux-arch, linux
  Cc: Ard Biesheuvel

This v4 of my kallsyms base relative implementation is now a 3 piece series,
since it caused some problems due to the way absolute symbols are handled
by the absolute per cpu code. As it turns out, that code was probably wrong
in the sense that it caused non-relocated symbol addresses to be emitted
for values that are in fact relative to the address of the kernel text.

Patch #1 fixes the scripts/kallsyms invocation to only pass the x86_64
specific --absolute-percpu option if CONFIG_SMP is also set.

Patch #2 reworks the absolute percpu code to only emit those percpu symbols
as absolute, and not symbols that have been classified as 'A' (absolute) by
the linker, since that does not mean quite the same thing.

Patch #3 is the original kallsyms base relative patch, with some
modifications:
- folded the s/ULLONG_MAX/-1ULL? change made by Andrew
- ensured that the kallsyms_relative_base value itself is relocated as
  required.
- dropped all of the acks and other tags, as they have become outdated with
  the recent changes to this patch.

Ard Biesheuvel (3):
  x86: kallsyms: disable absolute percpu symbols on !SMP
  kallsyms: don't overload absolute symbol type for percpu symbols
  kallsyms: add support for relative offsets in kallsyms address table

 init/Kconfig            |  16 +++
 kernel/kallsyms.c       |  38 ++++++--
 scripts/kallsyms.c      | 102 +++++++++++++++++---
 scripts/link-vmlinux.sh |   6 +-
 scripts/namespace.pl    |   2 +
 5 files changed, 142 insertions(+), 22 deletions(-)

-- 
2.5.0

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v4 1/3] x86: kallsyms: disable absolute percpu symbols on !SMP
  2016-01-25 14:19 [PATCH v4 0/3] kallsyms base relative series Ard Biesheuvel
@ 2016-01-25 14:19 ` Ard Biesheuvel
  2016-01-25 14:19 ` [PATCH v4 2/3] kallsyms: don't overload absolute symbol type for percpu symbols Ard Biesheuvel
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2016-01-25 14:19 UTC (permalink / raw)
  To: linux-kernel, linux-s390, linuxppc-dev, x86, keescook, akpm,
	mingo, hpa, heiko.carstens, benh, mpe, mmarek, rusty, arnd,
	linux-arch, linux
  Cc: Ard Biesheuvel

scripts/kallsyms.c has a special --absolute-percpu command line
option which deals with the zero based per cpu offsets that are
used when building for SMP on x86_64. This means that the option
should only be passed in that case, so add a check for CONFIG_SMP.
Otherwise, per cpu variables are recorded as absolute quantities,
(which they are not under !CONFIG_SMP) and they are not relocated
under CONFIG_RELOCATABLE=y.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 scripts/link-vmlinux.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index ba6c34ea5429..b541c21072f2 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -86,7 +86,7 @@ kallsyms()
 		kallsymopt="${kallsymopt} --page-offset=$CONFIG_PAGE_OFFSET"
 	fi
 
-	if [ -n "${CONFIG_X86_64}" ]; then
+	if [ -n "${CONFIG_X86_64}" ] && [ -n "${CONFIG_SMP}" ]; then
 		kallsymopt="${kallsymopt} --absolute-percpu"
 	fi
 
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 2/3] kallsyms: don't overload absolute symbol type for percpu symbols
  2016-01-25 14:19 [PATCH v4 0/3] kallsyms base relative series Ard Biesheuvel
  2016-01-25 14:19 ` [PATCH v4 1/3] x86: kallsyms: disable absolute percpu symbols on !SMP Ard Biesheuvel
@ 2016-01-25 14:19 ` Ard Biesheuvel
  2016-01-25 14:19 ` [PATCH v4 3/3] kallsyms: add support for relative offsets in kallsyms address table Ard Biesheuvel
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2016-01-25 14:19 UTC (permalink / raw)
  To: linux-kernel, linux-s390, linuxppc-dev, x86, keescook, akpm,
	mingo, hpa, heiko.carstens, benh, mpe, mmarek, rusty, arnd,
	linux-arch, linux
  Cc: Ard Biesheuvel

Commit c6bda7c988a5 ("kallsyms: fix percpu vars on x86-64 with relocation")
overloaded the 'A' (absolute) symbol type to signify that a symbol is not
subject to dynamic relocation. However, the original A type does not imply
that at all, and depending on the version of the toolchain, many A type
symbols are emitted that are in fact relative to the kernel text, i.e., if
the kernel is relocated at runtime, these symbols should be updated as well.

For instance, on sparc32, the following symbols are emitted as absolute
(kindly provided by Guenter Roeck):

  f035a420 A _etext
  f03d9000 A _sdata
  f03de8c4 A jiffies
  f03f8860 A _edata
  f03fc000 A __init_begin
  f041bdc8 A __init_text_end
  f0423000 A __bss_start
  f0423000 A __init_end
  f044457d A __bss_stop
  f044457d A _end

On x86_64, similar behavior can be observed:

  ffffffff81a00000 A __end_rodata_hpage_align
  ffffffff81b19000 A __vvar_page
  ffffffff81d3d000 A _end

Even if only a couple of them pass the symbol range check that results in
them to be taken into account for the final kallsyms symbol table, it is
obvious that 'A' does not mean the symbol does not need to be updated at
relocation time, and overloading its meaning to signify that is perhaps not
a good idea.

So instead, add a new percpu_absolute member to struct sym_entry, and when
--absolute-percpu is in effect, use it to record symbols whose addresses
should be emitted as final values rather than values that still require
relocation at runtime. That way, we can drop the check against the 'A' type.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 scripts/kallsyms.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 8fa81e84e295..d39a1eeb080e 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -34,6 +34,7 @@ struct sym_entry {
 	unsigned int len;
 	unsigned int start_pos;
 	unsigned char *sym;
+	unsigned int percpu_absolute;
 };
 
 struct addr_range {
@@ -171,6 +172,8 @@ static int read_symbol(FILE *in, struct sym_entry *s)
 	strcpy((char *)s->sym + 1, str);
 	s->sym[0] = stype;
 
+	s->percpu_absolute = 0;
+
 	/* Record if we've found __per_cpu_start/end. */
 	check_symbol_range(sym, s->addr, &percpu_range, 1);
 
@@ -325,7 +328,7 @@ static int expand_symbol(unsigned char *data, int len, char *result)
 
 static int symbol_absolute(struct sym_entry *s)
 {
-	return toupper(s->sym[0]) == 'A';
+	return s->percpu_absolute;
 }
 
 static void write_src(void)
@@ -681,8 +684,15 @@ static void make_percpus_absolute(void)
 	unsigned int i;
 
 	for (i = 0; i < table_cnt; i++)
-		if (symbol_in_range(&table[i], &percpu_range, 1))
+		if (symbol_in_range(&table[i], &percpu_range, 1)) {
+			/*
+			 * Keep the 'A' override for percpu symbols to
+			 * ensure consistent behavior compared to older
+			 * versions of this tool.
+			 */
 			table[i].sym[0] = 'A';
+			table[i].percpu_absolute = 1;
+		}
 }
 
 int main(int argc, char **argv)
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v4 3/3] kallsyms: add support for relative offsets in kallsyms address table
  2016-01-25 14:19 [PATCH v4 0/3] kallsyms base relative series Ard Biesheuvel
  2016-01-25 14:19 ` [PATCH v4 1/3] x86: kallsyms: disable absolute percpu symbols on !SMP Ard Biesheuvel
  2016-01-25 14:19 ` [PATCH v4 2/3] kallsyms: don't overload absolute symbol type for percpu symbols Ard Biesheuvel
@ 2016-01-25 14:19 ` Ard Biesheuvel
  2016-01-25 17:44 ` [PATCH v4 0/3] kallsyms base relative series Guenter Roeck
  2016-01-25 18:53 ` Kees Cook
  4 siblings, 0 replies; 6+ messages in thread
From: Ard Biesheuvel @ 2016-01-25 14:19 UTC (permalink / raw)
  To: linux-kernel, linux-s390, linuxppc-dev, x86, keescook, akpm,
	mingo, hpa, heiko.carstens, benh, mpe, mmarek, rusty, arnd,
	linux-arch, linux
  Cc: Ard Biesheuvel

Similar to how relative extables are implemented, it is possible to emit
the kallsyms table in such a way that it contains offsets relative to some
anchor point in the kernel image rather than absolute addresses.

On 64-bit architectures, it cuts the size of the kallsyms address table in
half, since offsets between kernel symbols can typically be expressed in 32
bits. This saves several hundreds of kilobytes of permanent .rodata on
average. In addition, the kallsyms address table is no longer subject to
dynamic relocation when CONFIG_RELOCATABLE is in effect, so the relocation
work done after decompression now doesn't have to do relocation updates for
all these values. This saves up to 24 bytes (i.e., the size of a ELF64 RELA
relocation table entry) per value, which easily adds up to a couple of
megabytes of uncompressed __init data on ppc64 or arm64. Even if these
relocation entries typically compress well, the combined size reduction of
2.8 MB uncompressed for a ppc64_defconfig build (of which 2.4 MB is __init
data) results in a ~500 KB space saving in the compressed image.

Since it is useful for some architectures (like x86) to retain the ability
to emit absolute values as well, this patch adds support for both, by
emitting absolute addresses as positive 32-bit values, and addresses
relative to the lowest encountered relative symbol as negative values,
which are subtracted from the runtime address of this base symbol to
produce the actual address.

Support for the above is enabled by default for all architectures except
IA-64, whose symbols are too far apart to capture in this manner.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 init/Kconfig            | 16 ++++
 kernel/kallsyms.c       | 38 +++++++--
 scripts/kallsyms.c      | 88 +++++++++++++++++---
 scripts/link-vmlinux.sh |  4 +
 scripts/namespace.pl    |  2 +
 5 files changed, 129 insertions(+), 19 deletions(-)

diff --git a/init/Kconfig b/init/Kconfig
index 22320804fbaf..1cc72a068afc 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -1420,6 +1420,22 @@ config KALLSYMS_ALL
 
 	   Say N unless you really need all symbols.
 
+config KALLSYMS_BASE_RELATIVE
+	bool
+	depends on KALLSYMS
+	default !IA64
+	help
+	  Instead of emitting them as absolute values in the native word size,
+	  emit the symbol references in the kallsyms table as 32-bit entries,
+	  each containing either an absolute value in the range [0, S32_MAX] or
+	  a relative value in the range [base, base + S32_MAX], where base is
+	  the lowest relative symbol address encountered in the image.
+
+	  On 64-bit builds, this reduces the size of the address table by 50%,
+	  but more importantly, it results in entries whose values are build
+	  time constants, and no relocation pass is required at runtime to fix
+	  up the entries based on the runtime load address of the kernel.
+
 config PRINTK
 	default y
 	bool "Enable support for printk" if EXPERT
diff --git a/kernel/kallsyms.c b/kernel/kallsyms.c
index 5c5987f10819..10a8af9d5744 100644
--- a/kernel/kallsyms.c
+++ b/kernel/kallsyms.c
@@ -38,6 +38,7 @@
  * during the second link stage.
  */
 extern const unsigned long kallsyms_addresses[] __weak;
+extern const int kallsyms_offsets[] __weak;
 extern const u8 kallsyms_names[] __weak;
 
 /*
@@ -47,6 +48,9 @@ extern const u8 kallsyms_names[] __weak;
 extern const unsigned long kallsyms_num_syms
 __attribute__((weak, section(".rodata")));
 
+extern const unsigned long kallsyms_relative_base
+__attribute__((weak, section(".rodata")));
+
 extern const u8 kallsyms_token_table[] __weak;
 extern const u16 kallsyms_token_index[] __weak;
 
@@ -176,6 +180,19 @@ static unsigned int get_symbol_offset(unsigned long pos)
 	return name - kallsyms_names;
 }
 
+static unsigned long kallsyms_sym_address(int idx)
+{
+	if (!IS_ENABLED(CONFIG_KALLSYMS_BASE_RELATIVE))
+		return kallsyms_addresses[idx];
+
+	/* positive offsets are absolute values */
+	if (kallsyms_offsets[idx] >= 0)
+		return kallsyms_offsets[idx];
+
+	/* negative offsets are relative to kallsyms_relative_base - 1 */
+	return kallsyms_relative_base - 1 - kallsyms_offsets[idx];
+}
+
 /* Lookup the address for this symbol. Returns 0 if not found. */
 unsigned long kallsyms_lookup_name(const char *name)
 {
@@ -187,7 +204,7 @@ unsigned long kallsyms_lookup_name(const char *name)
 		off = kallsyms_expand_symbol(off, namebuf, ARRAY_SIZE(namebuf));
 
 		if (strcmp(namebuf, name) == 0)
-			return kallsyms_addresses[i];
+			return kallsyms_sym_address(i);
 	}
 	return module_kallsyms_lookup_name(name);
 }
@@ -204,7 +221,7 @@ int kallsyms_on_each_symbol(int (*fn)(void *, const char *, struct module *,
 
 	for (i = 0, off = 0; i < kallsyms_num_syms; i++) {
 		off = kallsyms_expand_symbol(off, namebuf, ARRAY_SIZE(namebuf));
-		ret = fn(data, namebuf, NULL, kallsyms_addresses[i]);
+		ret = fn(data, namebuf, NULL, kallsyms_sym_address(i));
 		if (ret != 0)
 			return ret;
 	}
@@ -220,7 +237,10 @@ static unsigned long get_symbol_pos(unsigned long addr,
 	unsigned long i, low, high, mid;
 
 	/* This kernel should never had been booted. */
-	BUG_ON(!kallsyms_addresses);
+	if (!IS_ENABLED(CONFIG_KALLSYMS_BASE_RELATIVE))
+		BUG_ON(!kallsyms_addresses);
+	else
+		BUG_ON(!kallsyms_offsets);
 
 	/* Do a binary search on the sorted kallsyms_addresses array. */
 	low = 0;
@@ -228,7 +248,7 @@ static unsigned long get_symbol_pos(unsigned long addr,
 
 	while (high - low > 1) {
 		mid = low + (high - low) / 2;
-		if (kallsyms_addresses[mid] <= addr)
+		if (kallsyms_sym_address(mid) <= addr)
 			low = mid;
 		else
 			high = mid;
@@ -238,15 +258,15 @@ static unsigned long get_symbol_pos(unsigned long addr,
 	 * Search for the first aliased symbol. Aliased
 	 * symbols are symbols with the same address.
 	 */
-	while (low && kallsyms_addresses[low-1] == kallsyms_addresses[low])
+	while (low && kallsyms_sym_address(low-1) == kallsyms_sym_address(low))
 		--low;
 
-	symbol_start = kallsyms_addresses[low];
+	symbol_start = kallsyms_sym_address(low);
 
 	/* Search for next non-aliased symbol. */
 	for (i = low + 1; i < kallsyms_num_syms; i++) {
-		if (kallsyms_addresses[i] > symbol_start) {
-			symbol_end = kallsyms_addresses[i];
+		if (kallsyms_sym_address(i) > symbol_start) {
+			symbol_end = kallsyms_sym_address(i);
 			break;
 		}
 	}
@@ -470,7 +490,7 @@ static unsigned long get_ksymbol_core(struct kallsym_iter *iter)
 	unsigned off = iter->nameoff;
 
 	iter->module_name[0] = '\0';
-	iter->value = kallsyms_addresses[iter->pos];
+	iter->value = kallsyms_sym_address(iter->pos);
 
 	iter->type = kallsyms_get_symbol_type(off);
 
diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index d39a1eeb080e..02473b71643b 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -22,6 +22,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <ctype.h>
+#include <limits.h>
 
 #ifndef ARRAY_SIZE
 #define ARRAY_SIZE(arr) (sizeof(arr) / sizeof(arr[0]))
@@ -43,6 +44,7 @@ struct addr_range {
 };
 
 static unsigned long long _text;
+static unsigned long long relative_base;
 static struct addr_range text_ranges[] = {
 	{ "_stext",     "_etext"     },
 	{ "_sinittext", "_einittext" },
@@ -62,6 +64,7 @@ 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];
 
@@ -75,7 +78,7 @@ static void usage(void)
 	fprintf(stderr, "Usage: kallsyms [--all-symbols] "
 			"[--symbol-prefix=<prefix char>] "
 			"[--page-offset=<CONFIG_PAGE_OFFSET>] "
-			"< in.map > out.S\n");
+			"[--base-relative] < in.map > out.S\n");
 	exit(1);
 }
 
@@ -205,6 +208,8 @@ static int symbol_valid(struct sym_entry *s)
 	 */
 	static char *special_symbols[] = {
 		"kallsyms_addresses",
+		"kallsyms_offsets",
+		"kallsyms_relative_base",
 		"kallsyms_num_syms",
 		"kallsyms_names",
 		"kallsyms_markers",
@@ -349,16 +354,47 @@ static void write_src(void)
 
 	printf("\t.section .rodata, \"a\"\n");
 
-	/* Provide proper symbols relocatability by their '_text'
-	 * relativeness.  The symbol names cannot be used to construct
-	 * normal symbol references as the list of symbols contains
-	 * symbols that are declared static and are private to their
-	 * .o files.  This prevents .tmp_kallsyms.o or any other
-	 * object from referencing them.
+	/* Provide proper symbols relocatability by their relativeness
+	 * to a fixed anchor point in the runtime image, either '_text'
+	 * for absolute address tables, in which case the linker will
+	 * emit the final addresses at build time. Otherwise, use the
+	 * offset relative to the lowest value encountered of all relative
+	 * symbols, and emit non-relocatable fixed offsets that will be fixed
+	 * up at runtime.
+	 *
+	 * The symbol names cannot be used to construct normal symbol
+	 * references as the list of symbols contains symbols that are
+	 * declared static and are private to their .o files.  This prevents
+	 * .tmp_kallsyms.o or any other object from referencing them.
 	 */
-	output_label("kallsyms_addresses");
+	if (!base_relative)
+		output_label("kallsyms_addresses");
+	else
+		output_label("kallsyms_offsets");
+
 	for (i = 0; i < table_cnt; i++) {
-		if (!symbol_absolute(&table[i])) {
+		if (base_relative) {
+			long long offset;
+
+			if (symbol_absolute(&table[i])) {
+				offset = table[i].addr;
+				if (offset < 0 || offset > INT_MAX) {
+					fprintf(stderr, "kallsyms failure: "
+						"absolute symbol value %#llx out of range in relative mode\n",
+						table[i].addr);
+					exit(EXIT_FAILURE);
+				}
+			} else {
+				offset = relative_base - table[i].addr - 1;
+				if (offset < INT_MIN || offset >= 0) {
+					fprintf(stderr, "kallsyms failure: "
+						"relative symbol value %#llx out of range in relative mode\n",
+						table[i].addr);
+					exit(EXIT_FAILURE);
+				}
+			}
+			printf("\t.long\t%#x\n", (int)offset);
+		} else if (!symbol_absolute(&table[i])) {
 			if (_text <= table[i].addr)
 				printf("\tPTR\t_text + %#llx\n",
 					table[i].addr - _text);
@@ -371,6 +407,12 @@ static void write_src(void)
 	}
 	printf("\n");
 
+	if (base_relative) {
+		output_label("kallsyms_relative_base");
+		printf("\tPTR\t_text - %#llx\n", _text - relative_base);
+		printf("\n");
+	}
+
 	output_label("kallsyms_num_syms");
 	printf("\tPTR\t%d\n", table_cnt);
 	printf("\n");
@@ -695,6 +737,28 @@ static void make_percpus_absolute(void)
 		}
 }
 
+/* find the minimum non-absolute symbol address */
+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;
+	}
+}
+
 int main(int argc, char **argv)
 {
 	if (argc >= 2) {
@@ -713,7 +777,9 @@ int main(int argc, char **argv)
 			} else if (strncmp(argv[i], "--page-offset=", 14) == 0) {
 				const char *p = &argv[i][14];
 				kernel_start_addr = strtoull(p, NULL, 16);
-			} else
+			} else if (strcmp(argv[i], "--base-relative") == 0)
+				base_relative = 1;
+			else
 				usage();
 		}
 	} else if (argc != 1)
@@ -722,6 +788,8 @@ int main(int argc, char **argv)
 	read_map(stdin);
 	if (absolute_percpu)
 		make_percpus_absolute();
+	if (base_relative)
+		record_relative_base();
 	sort_symbols();
 	optimize_token_table();
 	write_src();
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index b541c21072f2..aae2473301ea 100755
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -90,6 +90,10 @@ kallsyms()
 		kallsymopt="${kallsymopt} --absolute-percpu"
 	fi
 
+	if [ -n "${CONFIG_KALLSYMS_BASE_RELATIVE}" ]; then
+		kallsymopt="${kallsymopt} --base-relative"
+	fi
+
 	local aflags="${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL}               \
 		      ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS}"
 
diff --git a/scripts/namespace.pl b/scripts/namespace.pl
index a71be6b7cdec..9f3c9d47a4a5 100755
--- a/scripts/namespace.pl
+++ b/scripts/namespace.pl
@@ -117,6 +117,8 @@ my %nameexception = (
     'kallsyms_names'	=> 1,
     'kallsyms_num_syms'	=> 1,
     'kallsyms_addresses'=> 1,
+    'kallsyms_offsets'	=> 1,
+    'kallsyms_relative_base'=> 1,
     '__this_module'	=> 1,
     '_etext'		=> 1,
     '_edata'		=> 1,
-- 
2.5.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 0/3] kallsyms base relative series
  2016-01-25 14:19 [PATCH v4 0/3] kallsyms base relative series Ard Biesheuvel
                   ` (2 preceding siblings ...)
  2016-01-25 14:19 ` [PATCH v4 3/3] kallsyms: add support for relative offsets in kallsyms address table Ard Biesheuvel
@ 2016-01-25 17:44 ` Guenter Roeck
  2016-01-25 18:53 ` Kees Cook
  4 siblings, 0 replies; 6+ messages in thread
From: Guenter Roeck @ 2016-01-25 17:44 UTC (permalink / raw)
  To: Ard Biesheuvel, linux-kernel, linux-s390, linuxppc-dev, x86,
	keescook, akpm, mingo, hpa, heiko.carstens, benh, mpe, mmarek,
	rusty, arnd, linux-arch

On 01/25/2016 06:19 AM, Ard Biesheuvel wrote:
> This v4 of my kallsyms base relative implementation is now a 3 piece series,
> since it caused some problems due to the way absolute symbols are handled
> by the absolute per cpu code. As it turns out, that code was probably wrong
> in the sense that it caused non-relocated symbol addresses to be emitted
> for values that are in fact relative to the address of the kernel text.
>
> Patch #1 fixes the scripts/kallsyms invocation to only pass the x86_64
> specific --absolute-percpu option if CONFIG_SMP is also set.
>
> Patch #2 reworks the absolute percpu code to only emit those percpu symbols
> as absolute, and not symbols that have been classified as 'A' (absolute) by
> the linker, since that does not mean quite the same thing.
>
> Patch #3 is the original kallsyms base relative patch, with some
> modifications:
> - folded the s/ULLONG_MAX/-1ULL? change made by Andrew
> - ensured that the kallsyms_relative_base value itself is relocated as
>    required.
> - dropped all of the acks and other tags, as they have become outdated with
>    the recent changes to this patch.
>

I tested the series on top of mmotm (after reverting the original patches).
Qemu tests for mips, sparc, and x86 (both smp and nosmp) now pass.

Tested-by: Guenter Roeck <linux@roeck-us.net>

> Ard Biesheuvel (3):
>    x86: kallsyms: disable absolute percpu symbols on !SMP
>    kallsyms: don't overload absolute symbol type for percpu symbols
>    kallsyms: add support for relative offsets in kallsyms address table
>
>   init/Kconfig            |  16 +++
>   kernel/kallsyms.c       |  38 ++++++--
>   scripts/kallsyms.c      | 102 +++++++++++++++++---
>   scripts/link-vmlinux.sh |   6 +-
>   scripts/namespace.pl    |   2 +
>   5 files changed, 142 insertions(+), 22 deletions(-)
>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v4 0/3] kallsyms base relative series
  2016-01-25 14:19 [PATCH v4 0/3] kallsyms base relative series Ard Biesheuvel
                   ` (3 preceding siblings ...)
  2016-01-25 17:44 ` [PATCH v4 0/3] kallsyms base relative series Guenter Roeck
@ 2016-01-25 18:53 ` Kees Cook
  4 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2016-01-25 18:53 UTC (permalink / raw)
  To: Ard Biesheuvel
  Cc: LKML, linux-s390, linuxppc-dev, x86, Andrew Morton, Ingo Molnar,
	H. Peter Anvin, Heiko Carstens, benh, Michael Ellerman,
	Michal Marek, Rusty Russell, Arnd Bergmann, linux-arch,
	Guenter Roeck

On Mon, Jan 25, 2016 at 6:19 AM, Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
> This v4 of my kallsyms base relative implementation is now a 3 piece series,
> since it caused some problems due to the way absolute symbols are handled
> by the absolute per cpu code. As it turns out, that code was probably wrong
> in the sense that it caused non-relocated symbol addresses to be emitted
> for values that are in fact relative to the address of the kernel text.
>
> Patch #1 fixes the scripts/kallsyms invocation to only pass the x86_64
> specific --absolute-percpu option if CONFIG_SMP is also set.
>
> Patch #2 reworks the absolute percpu code to only emit those percpu symbols
> as absolute, and not symbols that have been classified as 'A' (absolute) by
> the linker, since that does not mean quite the same thing.
>
> Patch #3 is the original kallsyms base relative patch, with some
> modifications:
> - folded the s/ULLONG_MAX/-1ULL? change made by Andrew
> - ensured that the kallsyms_relative_base value itself is relocated as
>   required.
> - dropped all of the acks and other tags, as they have become outdated with
>   the recent changes to this patch.
>
> Ard Biesheuvel (3):
>   x86: kallsyms: disable absolute percpu symbols on !SMP
>   kallsyms: don't overload absolute symbol type for percpu symbols
>   kallsyms: add support for relative offsets in kallsyms address table

Still works for me!

Reviewed-by: Kees Cook <keescook@chromium.org>
Tested-by: Kees Cook <keescook@chromium.org>

-Kees

>
>  init/Kconfig            |  16 +++
>  kernel/kallsyms.c       |  38 ++++++--
>  scripts/kallsyms.c      | 102 +++++++++++++++++---
>  scripts/link-vmlinux.sh |   6 +-
>  scripts/namespace.pl    |   2 +
>  5 files changed, 142 insertions(+), 22 deletions(-)
>
> --
> 2.5.0
>



-- 
Kees Cook
Chrome OS & Brillo Security

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-01-25 18:53 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-25 14:19 [PATCH v4 0/3] kallsyms base relative series Ard Biesheuvel
2016-01-25 14:19 ` [PATCH v4 1/3] x86: kallsyms: disable absolute percpu symbols on !SMP Ard Biesheuvel
2016-01-25 14:19 ` [PATCH v4 2/3] kallsyms: don't overload absolute symbol type for percpu symbols Ard Biesheuvel
2016-01-25 14:19 ` [PATCH v4 3/3] kallsyms: add support for relative offsets in kallsyms address table Ard Biesheuvel
2016-01-25 17:44 ` [PATCH v4 0/3] kallsyms base relative series Guenter Roeck
2016-01-25 18:53 ` Kees Cook

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.