[PATCH v2 0/2] kallsyms: handle special absolute symbols

[PATCH v2 0/2] kallsyms: handle special absolute symbols

[Kees Cook]

Handles 0-based per_cpu variables as being absolute so they are
not relocated under kASLR on x86_64.

-Kees

[PATCH v2 1/2] kallsyms: generalize address range checking

[Kees Cook]

This refactors the address range checks to be generalized instead of
specific to text range checks, in preparation for other range checks.
Also extracts logic for "is the symbol absolute" into a function.

Signed-off-by: Kees Cook <keescook@chromium.org>
---
v2:
 - return which range entry index an address was found in.
---
 scripts/kallsyms.c |   58 +++++++++++++++++++++++++++++++---------------------
 1 file changed, 35 insertions(+), 23 deletions(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 276e84b8a8e5..08f30ac5b07d 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -36,13 +36,13 @@ struct sym_entry {
 	unsigned char *sym;
 };
 
-struct text_range {
-	const char *stext, *etext;
+struct addr_range {
+	const char *start_sym, *end_sym;
 	unsigned long long start, end;
 };
 
 static unsigned long long _text;
-static struct text_range text_ranges[] = {
+static struct addr_range text_ranges[] = {
 	{ "_stext",     "_etext"     },
 	{ "_sinittext", "_einittext" },
 	{ "_stext_l1",  "_etext_l1"  },	/* Blackfin on-chip L1 inst SRAM */
@@ -83,19 +83,20 @@ static inline int is_arm_mapping_symbol(const char *str)
 	       && (str[2] == '\0' || str[2] == '.');
 }
 
-static int read_symbol_tr(const char *sym, unsigned long long addr)
+static int check_symbol_range(const char *sym, unsigned long long addr,
+			      struct addr_range *ranges, int entries)
 {
 	size_t i;
-	struct text_range *tr;
+	struct addr_range *ar;
 
-	for (i = 0; i < ARRAY_SIZE(text_ranges); ++i) {
-		tr = &text_ranges[i];
+	for (i = 0; i < entries; ++i) {
+		ar = &ranges[i];
 
-		if (strcmp(sym, tr->stext) == 0) {
-			tr->start = addr;
+		if (strcmp(sym, ar->start_sym) == 0) {
+			ar->start = addr;
 			return 0;
-		} else if (strcmp(sym, tr->etext) == 0) {
-			tr->end = addr;
+		} else if (strcmp(sym, ar->end_sym) == 0) {
+			ar->end = addr;
 			return 0;
 		}
 	}
@@ -130,7 +131,8 @@ static int read_symbol(FILE *in, struct sym_entry *s)
 	/* Ignore most absolute/undefined (?) symbols. */
 	if (strcmp(sym, "_text") == 0)
 		_text = s->addr;
-	else if (read_symbol_tr(sym, s->addr) == 0)
+	else if (check_symbol_range(sym, s->addr, text_ranges,
+				    ARRAY_SIZE(text_ranges)) == 0)
 		/* nothing to do */;
 	else if (toupper(stype) == 'A')
 	{
@@ -167,19 +169,21 @@ static int read_symbol(FILE *in, struct sym_entry *s)
 	return 0;
 }
 
-static int symbol_valid_tr(struct sym_entry *s)
+/* Return which addr_range index an entry matches, or -1 if not found. */
+static int symbol_in_range(struct sym_entry *s, struct addr_range *ranges,
+			   int entries)
 {
 	size_t i;
-	struct text_range *tr;
+	struct addr_range *ar;
 
-	for (i = 0; i < ARRAY_SIZE(text_ranges); ++i) {
-		tr = &text_ranges[i];
+	for (i = 0; i < entries; ++i) {
+		ar = &ranges[i];
 
-		if (s->addr >= tr->start && s->addr <= tr->end)
-			return 1;
+		if (s->addr >= ar->start && s->addr <= ar->end)
+			return i;
 	}
 
-	return 0;
+	return -1;
 }
 
 static int symbol_valid(struct sym_entry *s)
@@ -214,7 +218,8 @@ static int symbol_valid(struct sym_entry *s)
 	/* if --all-symbols is not specified, then symbols outside the text
 	 * and inittext sections are discarded */
 	if (!all_symbols) {
-		if (symbol_valid_tr(s) == 0)
+		if (symbol_in_range(s, text_ranges,
+				    ARRAY_SIZE(text_ranges)) < 0)
 			return 0;
 		/* Corner case.  Discard any symbols with the same value as
 		 * _etext _einittext; they can move between pass 1 and 2 when
@@ -223,9 +228,11 @@ static int symbol_valid(struct sym_entry *s)
 		 * rules.
 		 */
 		if ((s->addr == text_range_text->end &&
-				strcmp((char *)s->sym + offset, text_range_text->etext)) ||
+				strcmp((char *)s->sym + offset,
+				       text_range_text->end_sym)) ||
 		    (s->addr == text_range_inittext->end &&
-				strcmp((char *)s->sym + offset, text_range_inittext->etext)))
+				strcmp((char *)s->sym + offset,
+				       text_range_inittext->end_sym)))
 			return 0;
 	}
 
@@ -298,6 +305,11 @@ static int expand_symbol(unsigned char *data, int len, char *result)
 	return total;
 }
 
+static int symbol_absolute(struct sym_entry *s)
+{
+	return toupper(s->sym[0]) == 'A';
+}
+
 static void write_src(void)
 {
 	unsigned int i, k, off;
@@ -325,7 +337,7 @@ static void write_src(void)
 	 */
 	output_label("kallsyms_addresses");
 	for (i = 0; i < table_cnt; i++) {
-		if (toupper(table[i].sym[0]) != 'A') {
+		if (!symbol_absolute(&table[i])) {
 			if (_text <= table[i].addr)
 				printf("\tPTR\t_text + %#llx\n",
 					table[i].addr - _text);
-- 
1.7.9.5

[PATCH v2 2/2] kallsyms: handle special absolute symbols

[Kees Cook]

This forces the entire per_cpu range to be reported as absolute
without losing their linker symbol types, when the per_cpu area is
0-based. Without this, the variables are incorrectly shown as relocated
under kASLR on x86_64.

Several kallsyms output in different boot states for comparison of
various symbols:

$ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.nokaslr
0000000000000000 D __per_cpu_start
0000000000004000 D gdt_page
0000000000014280 D __per_cpu_end
ffffffff810001c8 T _stext
ffffffff81ee53c0 D __per_cpu_offset
$ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.kaslr1
000000001f200000 D __per_cpu_start
000000001f204000 D gdt_page
000000001f214280 D __per_cpu_end
ffffffffa02001c8 T _stext
ffffffffa10e53c0 D __per_cpu_offset
$ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.kaslr2
000000000d400000 D __per_cpu_start
000000000d404000 D gdt_page
000000000d414280 D __per_cpu_end
ffffffff8e4001c8 T _stext
ffffffff8f2e53c0 D __per_cpu_offset
$ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.kaslr-fixed
0000000000000000 D __per_cpu_start
0000000000004000 D gdt_page
0000000000014280 D __per_cpu_end
ffffffffadc001c8 T _stext
ffffffffaeae53c0 D __per_cpu_offset

Signed-off-by: Kees Cook <keescook@chromium.org>
---
v2:
 - only force absolute when per_cpu starts at 0.
---
 scripts/kallsyms.c |   20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 08f30ac5b07d..d3f93b8eb277 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;
+	int force_absolute;
 };
 
 struct addr_range {
@@ -51,6 +52,14 @@ static struct addr_range text_ranges[] = {
 #define text_range_text     (&text_ranges[0])
 #define text_range_inittext (&text_ranges[1])
 
+/*
+ * Variables in these ranges, when the start is 0 based, will be forced to
+ * be handled as absolute addresses.
+ */
+static struct addr_range abs_ranges[] = {
+	{ "__per_cpu_start",	"__per_cpu_end", -1ULL, 0 },
+};
+
 static struct sym_entry *table;
 static unsigned int table_size, table_cnt;
 static int all_symbols = 0;
@@ -165,6 +174,10 @@ static int read_symbol(FILE *in, struct sym_entry *s)
 	}
 	strcpy((char *)s->sym + 1, str);
 	s->sym[0] = stype;
+	s->force_absolute = 0;
+
+	/* Check if we've found special absolute symbol range. */
+	check_symbol_range(sym, s->addr, abs_ranges, ARRAY_SIZE(abs_ranges));
 
 	return 0;
 }
@@ -211,6 +224,11 @@ static int symbol_valid(struct sym_entry *s)
 	if (s->addr < kernel_start_addr)
 		return 0;
 
+	/* Force zero-based range special symbols into being absolute. */
+	i = symbol_in_range(s, abs_ranges, ARRAY_SIZE(abs_ranges));
+	if (i >= 0 && abs_ranges[i].start == 0)
+		s->force_absolute = 1;
+
 	/* skip prefix char */
 	if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
 		offset++;
@@ -307,7 +325,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->force_absolute || toupper(s->sym[0]) == 'A';
 }
 
 static void write_src(void)
-- 
1.7.9.5

Re: [PATCH v2 0/2] kallsyms: handle special absolute symbols

[Andrew Morton]

On Fri,  7 Mar 2014 17:00:23 -0800 Kees Cook <keescook@chromium.org> wrote:

> Handles 0-based per_cpu variables as being absolute so they are
> not relocated under kASLR on x86_64.
> 

Would it be prudent to revert 0f55159d091cb1e5 ("kallsyms: fix absolute
addresses for kASLR") then sort all this out for 3.15?

Re: [PATCH v2 0/2] kallsyms: handle special absolute symbols

[Kees Cook]

On Mon, Mar 10, 2014 at 12:06 PM, Andrew Morton
<akpm@linux-foundation.org> wrote:
> On Fri,  7 Mar 2014 17:00:23 -0800 Kees Cook <keescook@chromium.org> wrote:
>> Handles 0-based per_cpu variables as being absolute so they are
>> not relocated under kASLR on x86_64.
>
> Would it be prudent to revert 0f55159d091cb1e5 ("kallsyms: fix absolute
> addresses for kASLR") then sort all this out for 3.15?

My opinion is that if it breaks a real-life case (avr32), it should be
reverted. The only people affected by the kallsyms per_cpu relocation
reporting bug are those using kASLR on x86, and even then the bug is a
corner case on live kernel debugging.

I am fine either way.

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH v2 0/2] kallsyms: handle special absolute symbols

[Andrew Morton]

On Mon, 10 Mar 2014 12:58:06 -0700 Kees Cook <keescook@chromium.org> wrote:

> On Mon, Mar 10, 2014 at 12:06 PM, Andrew Morton
> <akpm@linux-foundation.org> wrote:
> > On Fri,  7 Mar 2014 17:00:23 -0800 Kees Cook <keescook@chromium.org> wrote:
> >> Handles 0-based per_cpu variables as being absolute so they are
> >> not relocated under kASLR on x86_64.
> >
> > Would it be prudent to revert 0f55159d091cb1e5 ("kallsyms: fix absolute
> > addresses for kASLR") then sort all this out for 3.15?
> 
> My opinion is that if it breaks a real-life case (avr32), it should be
> reverted.

We aren't going to be able to test this on 40 architectures so yes,
let's take the cautious approach.

> The only people affected by the kallsyms per_cpu relocation
> reporting bug are those using kASLR on x86, and even then the bug is a
> corner case on live kernel debugging.
> 
> I am fine either way.

Re: [PATCH v2 2/2] kallsyms: handle special absolute symbols

[Kees Cook]

On Fri, Mar 7, 2014 at 5:00 PM, Kees Cook <keescook@chromium.org> wrote:
> This forces the entire per_cpu range to be reported as absolute
> without losing their linker symbol types, when the per_cpu area is
> 0-based. Without this, the variables are incorrectly shown as relocated
> under kASLR on x86_64.
>
> Several kallsyms output in different boot states for comparison of
> various symbols:
>
> $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.nokaslr
> 0000000000000000 D __per_cpu_start
> 0000000000004000 D gdt_page
> 0000000000014280 D __per_cpu_end
> ffffffff810001c8 T _stext
> ffffffff81ee53c0 D __per_cpu_offset
> $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.kaslr1
> 000000001f200000 D __per_cpu_start
> 000000001f204000 D gdt_page
> 000000001f214280 D __per_cpu_end
> ffffffffa02001c8 T _stext
> ffffffffa10e53c0 D __per_cpu_offset
> $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.kaslr2
> 000000000d400000 D __per_cpu_start
> 000000000d404000 D gdt_page
> 000000000d414280 D __per_cpu_end
> ffffffff8e4001c8 T _stext
> ffffffff8f2e53c0 D __per_cpu_offset
> $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.kaslr-fixed
> 0000000000000000 D __per_cpu_start
> 0000000000004000 D gdt_page
> 0000000000014280 D __per_cpu_end
> ffffffffadc001c8 T _stext
> ffffffffaeae53c0 D __per_cpu_offset
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
> ---
> v2:
>  - only force absolute when per_cpu starts at 0.
> ---
>  scripts/kallsyms.c |   20 +++++++++++++++++++-
>  1 file changed, 19 insertions(+), 1 deletion(-)
>
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index 08f30ac5b07d..d3f93b8eb277 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;
> +       int force_absolute;
>  };
>
>  struct addr_range {
> @@ -51,6 +52,14 @@ static struct addr_range text_ranges[] = {
>  #define text_range_text     (&text_ranges[0])
>  #define text_range_inittext (&text_ranges[1])
>
> +/*
> + * Variables in these ranges, when the start is 0 based, will be forced to
> + * be handled as absolute addresses.
> + */
> +static struct addr_range abs_ranges[] = {
> +       { "__per_cpu_start",    "__per_cpu_end", -1ULL, 0 },
> +};
> +
>  static struct sym_entry *table;
>  static unsigned int table_size, table_cnt;
>  static int all_symbols = 0;
> @@ -165,6 +174,10 @@ static int read_symbol(FILE *in, struct sym_entry *s)
>         }
>         strcpy((char *)s->sym + 1, str);
>         s->sym[0] = stype;
> +       s->force_absolute = 0;
> +
> +       /* Check if we've found special absolute symbol range. */
> +       check_symbol_range(sym, s->addr, abs_ranges, ARRAY_SIZE(abs_ranges));
>
>         return 0;
>  }
> @@ -211,6 +224,11 @@ static int symbol_valid(struct sym_entry *s)
>         if (s->addr < kernel_start_addr)
>                 return 0;
>
> +       /* Force zero-based range special symbols into being absolute. */
> +       i = symbol_in_range(s, abs_ranges, ARRAY_SIZE(abs_ranges));
> +       if (i >= 0 && abs_ranges[i].start == 0)
> +               s->force_absolute = 1;

Rusty, is this 0-detection workable for you? If so, should you or akpm
carry this series for 3.15?

Thanks!

-Kees

> +
>         /* skip prefix char */
>         if (symbol_prefix_char && *(s->sym + 1) == symbol_prefix_char)
>                 offset++;
> @@ -307,7 +325,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->force_absolute || toupper(s->sym[0]) == 'A';
>  }
>
>  static void write_src(void)
> --
> 1.7.9.5
>



-- 
Kees Cook
Chrome OS Security

Re: [PATCH v2 2/2] kallsyms: handle special absolute symbols

[Rusty Russell]

Kees Cook <keescook@chromium.org> writes:
> On Fri, Mar 7, 2014 at 5:00 PM, Kees Cook <keescook@chromium.org> wrote:
>> This forces the entire per_cpu range to be reported as absolute
>> without losing their linker symbol types, when the per_cpu area is
>> 0-based. Without this, the variables are incorrectly shown as relocated
>> under kASLR on x86_64.
>>
>> Several kallsyms output in different boot states for comparison of
>> various symbols:
>>
>> $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.nokaslr
>> 0000000000000000 D __per_cpu_start
>> 0000000000004000 D gdt_page
>> 0000000000014280 D __per_cpu_end
>> ffffffff810001c8 T _stext
>> ffffffff81ee53c0 D __per_cpu_offset
>> $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.kaslr1
>> 000000001f200000 D __per_cpu_start
>> 000000001f204000 D gdt_page
>> 000000001f214280 D __per_cpu_end
>> ffffffffa02001c8 T _stext
>> ffffffffa10e53c0 D __per_cpu_offset
>> $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.kaslr2
>> 000000000d400000 D __per_cpu_start
>> 000000000d404000 D gdt_page
>> 000000000d414280 D __per_cpu_end
>> ffffffff8e4001c8 T _stext
>> ffffffff8f2e53c0 D __per_cpu_offset
>> $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.kaslr-fixed
>> 0000000000000000 D __per_cpu_start
>> 0000000000004000 D gdt_page
>> 0000000000014280 D __per_cpu_end
>> ffffffffadc001c8 T _stext
>> ffffffffaeae53c0 D __per_cpu_offset
>>
>> Signed-off-by: Kees Cook <keescook@chromium.org>
>> ---
>> v2:
>>  - only force absolute when per_cpu starts at 0.
>> ---
>>  scripts/kallsyms.c |   20 +++++++++++++++++++-
>>  1 file changed, 19 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
>> index 08f30ac5b07d..d3f93b8eb277 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;
>> +       int force_absolute;
>>  };
>>
>>  struct addr_range {
>> @@ -51,6 +52,14 @@ static struct addr_range text_ranges[] = {
>>  #define text_range_text     (&text_ranges[0])
>>  #define text_range_inittext (&text_ranges[1])
>>
>> +/*
>> + * Variables in these ranges, when the start is 0 based, will be forced to
>> + * be handled as absolute addresses.
>> + */
>> +static struct addr_range abs_ranges[] = {
>> +       { "__per_cpu_start",    "__per_cpu_end", -1ULL, 0 },
>> +};
>> +
>>  static struct sym_entry *table;
>>  static unsigned int table_size, table_cnt;
>>  static int all_symbols = 0;
>> @@ -165,6 +174,10 @@ static int read_symbol(FILE *in, struct sym_entry *s)
>>         }
>>         strcpy((char *)s->sym + 1, str);
>>         s->sym[0] = stype;
>> +       s->force_absolute = 0;
>> +
>> +       /* Check if we've found special absolute symbol range. */
>> +       check_symbol_range(sym, s->addr, abs_ranges, ARRAY_SIZE(abs_ranges));
>>
>>         return 0;
>>  }
>> @@ -211,6 +224,11 @@ static int symbol_valid(struct sym_entry *s)
>>         if (s->addr < kernel_start_addr)
>>                 return 0;
>>
>> +       /* Force zero-based range special symbols into being absolute. */
>> +       i = symbol_in_range(s, abs_ranges, ARRAY_SIZE(abs_ranges));
>> +       if (i >= 0 && abs_ranges[i].start == 0)
>> +               s->force_absolute = 1;
>
> Rusty, is this 0-detection workable for you? If so, should you or akpm
> carry this series for 3.15?

Damn, sorry, I wrote this patch and seems like I didn't actually send it
out.  No wonder you didn't respond :)

This applies on top of your first cleanup patch:

kallsyms: fix percpu vars on x86-64 with relocation.

x86-64 has a problem: per-cpu variables are actually represented by
their absolute offsets within the per-cpu area, but the symbols are
not emitted as absolute.  Thus kallsyms naively creates them as offsets
from _text, meaning their values change if the kernel is relocated
(especially noticeable with CONFIG_RANDOMIZE_BASE):

 $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.nokaslr
 0000000000000000 D __per_cpu_start
 0000000000004000 D gdt_page
 0000000000014280 D __per_cpu_end
 ffffffff810001c8 T _stext
 ffffffff81ee53c0 D __per_cpu_offset
 $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.kaslr1
 000000001f200000 D __per_cpu_start
 000000001f204000 D gdt_page
 000000001f214280 D __per_cpu_end
 ffffffffa02001c8 T _stext
 ffffffffa10e53c0 D __per_cpu_offset

Making them absolute symbols is the Right Thing, but requires fixes to
the relocs tool.  So for the moment, we add a --absolute-percpu option
which makes them absolute from a kallsyms perspective:

 $ egrep ' (gdt_|_(stext|_per_cpu_))' /proc/kallsyms # no KASLR
 0000000000000000 A __per_cpu_start
 000000000000a000 A gdt_page
 0000000000013040 A __per_cpu_end
 ffffffff802001c8 T _stext
 ffffffff8099b180 D __per_cpu_offset
 ffffffff809a3000 D __per_cpu_load
 $ egrep ' (gdt_|_(stext|_per_cpu_))' /proc/kallsyms # With KASLR
 0000000000000000 A __per_cpu_start
 000000000000a000 A gdt_page
 0000000000013040 A __per_cpu_end
 ffffffff89c001c8 T _stext
 ffffffff8a39d180 D __per_cpu_offset
 ffffffff8a3a5000 D __per_cpu_load

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
index 3c6224728960..21013e739773 100644
--- a/scripts/kallsyms.c
+++ b/scripts/kallsyms.c
@@ -51,9 +51,14 @@ static struct addr_range text_ranges[] = {
 #define text_range_text     (&text_ranges[0])
 #define text_range_inittext (&text_ranges[1])
 
+static struct addr_range percpu_range = {
+	"__per_cpu_start", "__per_cpu_end", -1ULL, 0
+};
+
 static struct sym_entry *table;
 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;
 
@@ -166,6 +171,9 @@ static int read_symbol(FILE *in, struct sym_entry *s)
 	strcpy((char *)s->sym + 1, str);
 	s->sym[0] = stype;
 
+	/* Record if we've found __per_cpu_start/end. */
+	check_symbol_range(sym, s->addr, &percpu_range, 1);
+
 	return 0;
 }
 
@@ -656,6 +664,15 @@ static void sort_symbols(void)
 	qsort(table, table_cnt, sizeof(struct sym_entry), compare_symbols);
 }
 
+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))
+			table[i].sym[0] = 'A';
+}
+
 int main(int argc, char **argv)
 {
 	if (argc >= 2) {
@@ -663,6 +683,8 @@ int main(int argc, char **argv)
 		for (i = 1; i < argc; i++) {
 			if(strcmp(argv[i], "--all-symbols") == 0)
 				all_symbols = 1;
+			else if (strcmp(argv[i], "--absolute-percpu") == 0)
+				absolute_percpu = 1;
 			else if (strncmp(argv[i], "--symbol-prefix=", 16) == 0) {
 				char *p = &argv[i][16];
 				/* skip quote */
@@ -679,6 +701,8 @@ int main(int argc, char **argv)
 		usage();
 
 	read_map(stdin);
+	if (absolute_percpu)
+		make_percpus_absolute();
 	sort_symbols();
 	optimize_token_table();
 	write_src();
diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
index 2dcb37736d84..86a4fe75f453 100644
--- a/scripts/link-vmlinux.sh
+++ b/scripts/link-vmlinux.sh
@@ -86,6 +86,10 @@ kallsyms()
 		kallsymopt="${kallsymopt} --page-offset=$CONFIG_PAGE_OFFSET"
 	fi
 
+	if [ -n "${CONFIG_X86_64}" ]; then
+		kallsymopt="${kallsymopt} --absolute-percpu"
+	fi
+
 	local aflags="${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL}               \
 		      ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS}"
 

Re: [PATCH v2 2/2] kallsyms: handle special absolute symbols

[Kees Cook]

On Wed, Mar 12, 2014 at 5:30 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Kees Cook <keescook@chromium.org> writes:
>> On Fri, Mar 7, 2014 at 5:00 PM, Kees Cook <keescook@chromium.org> wrote:
>>> This forces the entire per_cpu range to be reported as absolute
>>> without losing their linker symbol types, when the per_cpu area is
>>> 0-based. Without this, the variables are incorrectly shown as relocated
>>> under kASLR on x86_64.
>>>
>>> Several kallsyms output in different boot states for comparison of
>>> various symbols:
>>>
>>> $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.nokaslr
>>> 0000000000000000 D __per_cpu_start
>>> 0000000000004000 D gdt_page
>>> 0000000000014280 D __per_cpu_end
>>> ffffffff810001c8 T _stext
>>> ffffffff81ee53c0 D __per_cpu_offset
>>> $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.kaslr1
>>> 000000001f200000 D __per_cpu_start
>>> 000000001f204000 D gdt_page
>>> 000000001f214280 D __per_cpu_end
>>> ffffffffa02001c8 T _stext
>>> ffffffffa10e53c0 D __per_cpu_offset
>>> $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.kaslr2
>>> 000000000d400000 D __per_cpu_start
>>> 000000000d404000 D gdt_page
>>> 000000000d414280 D __per_cpu_end
>>> ffffffff8e4001c8 T _stext
>>> ffffffff8f2e53c0 D __per_cpu_offset
>>> $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.kaslr-fixed
>>> 0000000000000000 D __per_cpu_start
>>> 0000000000004000 D gdt_page
>>> 0000000000014280 D __per_cpu_end
>>> ffffffffadc001c8 T _stext
>>> ffffffffaeae53c0 D __per_cpu_offset
>>>
>>> Signed-off-by: Kees Cook <keescook@chromium.org>
>>> ---
>>> v2:
>>>  - only force absolute when per_cpu starts at 0.
>>> ---
>>>  scripts/kallsyms.c |   20 +++++++++++++++++++-
>>>  1 file changed, 19 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
>>> index 08f30ac5b07d..d3f93b8eb277 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;
>>> +       int force_absolute;
>>>  };
>>>
>>>  struct addr_range {
>>> @@ -51,6 +52,14 @@ static struct addr_range text_ranges[] = {
>>>  #define text_range_text     (&text_ranges[0])
>>>  #define text_range_inittext (&text_ranges[1])
>>>
>>> +/*
>>> + * Variables in these ranges, when the start is 0 based, will be forced to
>>> + * be handled as absolute addresses.
>>> + */
>>> +static struct addr_range abs_ranges[] = {
>>> +       { "__per_cpu_start",    "__per_cpu_end", -1ULL, 0 },
>>> +};
>>> +
>>>  static struct sym_entry *table;
>>>  static unsigned int table_size, table_cnt;
>>>  static int all_symbols = 0;
>>> @@ -165,6 +174,10 @@ static int read_symbol(FILE *in, struct sym_entry *s)
>>>         }
>>>         strcpy((char *)s->sym + 1, str);
>>>         s->sym[0] = stype;
>>> +       s->force_absolute = 0;
>>> +
>>> +       /* Check if we've found special absolute symbol range. */
>>> +       check_symbol_range(sym, s->addr, abs_ranges, ARRAY_SIZE(abs_ranges));
>>>
>>>         return 0;
>>>  }
>>> @@ -211,6 +224,11 @@ static int symbol_valid(struct sym_entry *s)
>>>         if (s->addr < kernel_start_addr)
>>>                 return 0;
>>>
>>> +       /* Force zero-based range special symbols into being absolute. */
>>> +       i = symbol_in_range(s, abs_ranges, ARRAY_SIZE(abs_ranges));
>>> +       if (i >= 0 && abs_ranges[i].start == 0)
>>> +               s->force_absolute = 1;
>>
>> Rusty, is this 0-detection workable for you? If so, should you or akpm
>> carry this series for 3.15?
>
> Damn, sorry, I wrote this patch and seems like I didn't actually send it
> out.  No wonder you didn't respond :)

Ah-ha, I was wondering. :)

> This applies on top of your first cleanup patch:
>
> kallsyms: fix percpu vars on x86-64 with relocation.
>
> x86-64 has a problem: per-cpu variables are actually represented by
> their absolute offsets within the per-cpu area, but the symbols are
> not emitted as absolute.  Thus kallsyms naively creates them as offsets
> from _text, meaning their values change if the kernel is relocated
> (especially noticeable with CONFIG_RANDOMIZE_BASE):
>
>  $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.nokaslr
>  0000000000000000 D __per_cpu_start
>  0000000000004000 D gdt_page
>  0000000000014280 D __per_cpu_end
>  ffffffff810001c8 T _stext
>  ffffffff81ee53c0 D __per_cpu_offset
>  $ egrep ' (gdt_|_(stext|_per_cpu_))' /root/kallsyms.kaslr1
>  000000001f200000 D __per_cpu_start
>  000000001f204000 D gdt_page
>  000000001f214280 D __per_cpu_end
>  ffffffffa02001c8 T _stext
>  ffffffffa10e53c0 D __per_cpu_offset
>
> Making them absolute symbols is the Right Thing, but requires fixes to
> the relocs tool.  So for the moment, we add a --absolute-percpu option
> which makes them absolute from a kallsyms perspective:

Why not just do this with 0-base-address detection like my v2? That
would mean we don't need to remember to add this flag in the future to
imagined new architectures that might want this 0-based per_cpu
feature.

-Kees

>
>  $ egrep ' (gdt_|_(stext|_per_cpu_))' /proc/kallsyms # no KASLR
>  0000000000000000 A __per_cpu_start
>  000000000000a000 A gdt_page
>  0000000000013040 A __per_cpu_end
>  ffffffff802001c8 T _stext
>  ffffffff8099b180 D __per_cpu_offset
>  ffffffff809a3000 D __per_cpu_load
>  $ egrep ' (gdt_|_(stext|_per_cpu_))' /proc/kallsyms # With KASLR
>  0000000000000000 A __per_cpu_start
>  000000000000a000 A gdt_page
>  0000000000013040 A __per_cpu_end
>  ffffffff89c001c8 T _stext
>  ffffffff8a39d180 D __per_cpu_offset
>  ffffffff8a3a5000 D __per_cpu_load
>
> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
>
> diff --git a/scripts/kallsyms.c b/scripts/kallsyms.c
> index 3c6224728960..21013e739773 100644
> --- a/scripts/kallsyms.c
> +++ b/scripts/kallsyms.c
> @@ -51,9 +51,14 @@ static struct addr_range text_ranges[] = {
>  #define text_range_text     (&text_ranges[0])
>  #define text_range_inittext (&text_ranges[1])
>
> +static struct addr_range percpu_range = {
> +       "__per_cpu_start", "__per_cpu_end", -1ULL, 0
> +};
> +
>  static struct sym_entry *table;
>  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;
>
> @@ -166,6 +171,9 @@ static int read_symbol(FILE *in, struct sym_entry *s)
>         strcpy((char *)s->sym + 1, str);
>         s->sym[0] = stype;
>
> +       /* Record if we've found __per_cpu_start/end. */
> +       check_symbol_range(sym, s->addr, &percpu_range, 1);
> +
>         return 0;
>  }
>
> @@ -656,6 +664,15 @@ static void sort_symbols(void)
>         qsort(table, table_cnt, sizeof(struct sym_entry), compare_symbols);
>  }
>
> +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))
> +                       table[i].sym[0] = 'A';
> +}
> +
>  int main(int argc, char **argv)
>  {
>         if (argc >= 2) {
> @@ -663,6 +683,8 @@ int main(int argc, char **argv)
>                 for (i = 1; i < argc; i++) {
>                         if(strcmp(argv[i], "--all-symbols") == 0)
>                                 all_symbols = 1;
> +                       else if (strcmp(argv[i], "--absolute-percpu") == 0)
> +                               absolute_percpu = 1;
>                         else if (strncmp(argv[i], "--symbol-prefix=", 16) == 0) {
>                                 char *p = &argv[i][16];
>                                 /* skip quote */
> @@ -679,6 +701,8 @@ int main(int argc, char **argv)
>                 usage();
>
>         read_map(stdin);
> +       if (absolute_percpu)
> +               make_percpus_absolute();
>         sort_symbols();
>         optimize_token_table();
>         write_src();
> diff --git a/scripts/link-vmlinux.sh b/scripts/link-vmlinux.sh
> index 2dcb37736d84..86a4fe75f453 100644
> --- a/scripts/link-vmlinux.sh
> +++ b/scripts/link-vmlinux.sh
> @@ -86,6 +86,10 @@ kallsyms()
>                 kallsymopt="${kallsymopt} --page-offset=$CONFIG_PAGE_OFFSET"
>         fi
>
> +       if [ -n "${CONFIG_X86_64}" ]; then
> +               kallsymopt="${kallsymopt} --absolute-percpu"
> +       fi
> +
>         local aflags="${KBUILD_AFLAGS} ${KBUILD_AFLAGS_KERNEL}               \
>                       ${NOSTDINC_FLAGS} ${LINUXINCLUDE} ${KBUILD_CPPFLAGS}"
>
>



-- 
Kees Cook
Chrome OS Security

Re: [PATCH v2 2/2] kallsyms: handle special absolute symbols

[Rusty Russell]

Kees Cook <keescook@chromium.org> writes:
> Why not just do this with 0-base-address detection like my v2? That
> would mean we don't need to remember to add this flag in the future to
> imagined new architectures that might want this 0-based per_cpu
> feature.

Because future architectures will get this right and emit absolute
symbols.  I hope!

I'm swamped at the moment, but am hoping to investigate that for
x86-64.  This is a stop-gap.

Cheers,
Rusty.

Re: [PATCH v2 2/2] kallsyms: handle special absolute symbols

[Kees Cook]

On Wed, Mar 12, 2014 at 8:40 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
> Kees Cook <keescook@chromium.org> writes:
>> Why not just do this with 0-base-address detection like my v2? That
>> would mean we don't need to remember to add this flag in the future to
>> imagined new architectures that might want this 0-based per_cpu
>> feature.
>
> Because future architectures will get this right and emit absolute
> symbols.  I hope!
>
> I'm swamped at the moment, but am hoping to investigate that for
> x86-64.  This is a stop-gap.

Okay, I'm convinced. :)

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

Thanks!

-Kees

-- 
Kees Cook
Chrome OS Security

Re: [PATCH v2 2/2] kallsyms: handle special absolute symbols

[Rusty Russell]

Kees Cook <keescook@chromium.org> writes:
> On Wed, Mar 12, 2014 at 8:40 PM, Rusty Russell <rusty@rustcorp.com.au> wrote:
>> Kees Cook <keescook@chromium.org> writes:
>>> Why not just do this with 0-base-address detection like my v2? That
>>> would mean we don't need to remember to add this flag in the future to
>>> imagined new architectures that might want this 0-based per_cpu
>>> feature.
>>
>> Because future architectures will get this right and emit absolute
>> symbols.  I hope!
>>
>> I'm swamped at the moment, but am hoping to investigate that for
>> x86-64.  This is a stop-gap.
>
> Okay, I'm convinced. :)
>
> Acked-by: Kees Cook <keescook@chromium.org>
>
> Thanks!

Applied, thanks.

Cheers,
Rusty.