6:33 p.m.
Changes from v1:
Separated changes in x86_64 to a separate file as they are only related
in that I needed to add support for slighty different kernels in order
to fully test these patches.
Corrected an alignment check from "& 0x1000 == 0" to "&0xFFF ==
0"
Andy Honig (3):
Improve search for __switch_to symbol
Add support to kaslr for offline vmcore files.
Automatically detect kernel aslr offset.
defs.h | 2 ++
main.c | 8 +++--
netdump.c | 11 ++++++
symbols.c | 117 ++++++++++++++++++++++++++++++++++++++++++++++++++------------
x86_64.c | 20 +++++++----
5 files changed, 127 insertions(+), 31 deletions(-)
--
1.9.0.rc1.175.g0b1dcb5
6:33 p.m.
New subject: [PATCH v2 1/3] Improve search for __switch_to symbol
For kernel versions without inlined context_switchm, the disassembly
for the call to switch_to is "call " instead of "callq". This patch
searches for both versions to support a wider range of kernels.
---
x86_64.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/x86_64.c b/x86_64.c
index 8508e4f..b51e285 100644
--- a/x86_64.c
+++ b/x86_64.c
@@ -5414,16 +5414,22 @@ search_for_switch_to(ulong start, ulong end)
{
ulong max_instructions, address;
char buf1[BUFSIZE];
- char buf2[BUFSIZE];
+ char search_string1[BUFSIZE];
+ char search_string2[BUFSIZE];
int found;
max_instructions = end - start;
found = FALSE;
sprintf(buf1, "x/%ldi 0x%lx", max_instructions, start);
- if (symbol_exists("__switch_to"))
- sprintf(buf2, "callq 0x%lx", symbol_value("__switch_to"));
- else
- buf2[0] = NULLCHAR;
+ if (symbol_exists("__switch_to")) {
+ sprintf(search_string1,
+ "callq 0x%lx", symbol_value("__switch_to"));
+ sprintf(search_string2,
+ "call 0x%lx", symbol_value("__switch_to"));
+ } else {
+ search_string1[0] = NULLCHAR;
+ search_string2[0] = NULLCHAR;
+ }
open_tmpfile();
@@ -5436,7 +5442,9 @@ search_for_switch_to(ulong start, ulong end)
break;
if (strstr(buf1, "<__switch_to>"))
found = TRUE;
- if (strlen(buf2) && strstr(buf1, buf2))
+ if (strlen(search_string1) && strstr(buf1, search_string1))
+ found = TRUE;
+ if (strlen(search_string2) && strstr(buf1, search_string2))
found = TRUE;
}
close_tmpfile();
--
1.9.0.rc1.175.g0b1dcb5
2 p.m.
New subject: [PATCH v2 1/3] Improve search for __switch_to symbol
Committed to github:
https://github.com/crash-utility/crash/commit/51f0a176c46ab86eace7a628633...
Queued for crash-7.0.6.
Thanks,
Dave
----- Original Message -----
For kernel versions without inlined context_switchm, the disassembly
for the call to switch_to is "call " instead of "callq". This patch
searches for both versions to support a wider range of kernels.
---
x86_64.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/x86_64.c b/x86_64.c
index 8508e4f..b51e285 100644
--- a/x86_64.c
+++ b/x86_64.c
@@ -5414,16 +5414,22 @@ search_for_switch_to(ulong start, ulong end)
{
ulong max_instructions, address;
char buf1[BUFSIZE];
- char buf2[BUFSIZE];
+ char search_string1[BUFSIZE];
+ char search_string2[BUFSIZE];
int found;
max_instructions = end - start;
found = FALSE;
sprintf(buf1, "x/%ldi 0x%lx", max_instructions, start);
- if (symbol_exists("__switch_to"))
- sprintf(buf2, "callq 0x%lx", symbol_value("__switch_to"));
- else
- buf2[0] = NULLCHAR;
+ if (symbol_exists("__switch_to")) {
+ sprintf(search_string1,
+ "callq 0x%lx", symbol_value("__switch_to"));
+ sprintf(search_string2,
+ "call 0x%lx", symbol_value("__switch_to"));
+ } else {
+ search_string1[0] = NULLCHAR;
+ search_string2[0] = NULLCHAR;
+ }
open_tmpfile();
@@ -5436,7 +5442,9 @@ search_for_switch_to(ulong start, ulong end)
break;
if (strstr(buf1, "<__switch_to>"))
found = TRUE;
- if (strlen(buf2) && strstr(buf1, buf2))
+ if (strlen(search_string1) && strstr(buf1, search_string1))
+ found = TRUE;
+ if (strlen(search_string2) && strstr(buf1, search_string2))
found = TRUE;
}
close_tmpfile();
--
1.9.0.rc1.175.g0b1dcb5
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
6:33 p.m.
New subject: [PATCH v2 2/3] Add support to kaslr for offline vmcore files.
This patch adds a --kaslr command line parameter for loading x86_64
crash dumps with kaslr enabled. This reuses the code from 32-bit
x86 relocations with some small changes. The ASLR offset is postive
instead of negative. Also had to move the code to traverse the
kernel section before the symbol storing code to figure out which
symbols were outside any sections and therefore were not relocated.
Also made a very small change in search_for_switch_to it was
searching through gdb command output for a slightly incorrect syntax.
Tested: Tested by loading kdump files from kernels with aslr enabled
and not enabled. Ran bt, files, and struct file 0xXXXXXX.
Signed-off-by: Andy Honig <ahonig(a)google.com>
---
defs.h | 2 ++
main.c | 8 ++++++--
symbols.c | 66 +++++++++++++++++++++++++++++++++++++++++++++------------------
3 files changed, 55 insertions(+), 21 deletions(-)
diff --git a/defs.h b/defs.h
index 4cae5e5..c9a4b73 100644
--- a/defs.h
+++ b/defs.h
@@ -2417,6 +2417,8 @@ struct symbol_table_data {
ulong __per_cpu_end;
off_t dwarf_debug_frame_file_offset;
ulong dwarf_debug_frame_size;
+ ulong first_section_start;
+ ulong last_section_end;
};
/* flags for st */
diff --git a/main.c b/main.c
index e5829bc..39763da 100644
--- a/main.c
+++ b/main.c
@@ -57,6 +57,7 @@ static struct option long_options[] = {
{"CRASHPAGER", 0, 0, 0},
{"no_scroll", 0, 0, 0},
{"reloc", required_argument, 0, 0},
+ {"kaslr", required_argument, 0, 0},
{"active", 0, 0, 0},
{"minimal", 0, 0, 0},
{"mod", required_argument, 0, 0},
@@ -216,12 +217,15 @@ main(int argc, char **argv)
else if (STREQ(long_options[option_index].name, "mod"))
kt->module_tree = optarg;
- else if (STREQ(long_options[option_index].name, "reloc")) {
+ else if (STREQ(long_options[option_index].name, "reloc") ||
+ STREQ(long_options[option_index].name, "kaslr")) {
if (!calculate(optarg, &kt->relocate, NULL, 0)) {
error(INFO, "invalid --reloc argument: %s\n",
optarg);
program_usage(SHORT_FORM);
- }
+ } else if (STREQ(long_options[option_index].name, "kaslr")) {
+ kt->relocate *= -1;
+ }
kt->flags |= RELOC_SET;
}
diff --git a/symbols.c b/symbols.c
index d063a0a..28378ab 100644
--- a/symbols.c
+++ b/symbols.c
@@ -198,22 +198,6 @@ symtab_init(void)
no_debugging_data(FATAL);
}
- symcount = bfd_read_minisymbols(st->bfd, FALSE, &minisyms, &size);
-
- if (symcount <= 0)
- no_debugging_data(FATAL);
-
- sort_x = bfd_make_empty_symbol(st->bfd);
- sort_y = bfd_make_empty_symbol(st->bfd);
- if (sort_x == NULL || sort_y == NULL)
- error(FATAL, "bfd_make_empty_symbol() failed\n");
-
- gnu_qsort(st->bfd, minisyms, symcount, size, sort_x, sort_y);
-
- store_symbols(st->bfd, FALSE, minisyms, symcount, size);
-
- free(minisyms);
-
/*
* Gather references to the kernel sections.
*/
@@ -222,6 +206,7 @@ symtab_init(void)
error(FATAL, "symbol table section array malloc: %s\n",
strerror(errno));
BZERO(st->sections, st->bfd->section_count * sizeof(struct sec *));
+ st->first_section_start = st->last_section_end = 0;
bfd_map_over_sections(st->bfd, section_header_info, KERNEL_SECTIONS);
if ((st->flags & (NO_SEC_LOAD|NO_SEC_CONTENTS)) ==
@@ -233,6 +218,22 @@ symtab_init(void)
}
}
+ symcount = bfd_read_minisymbols(st->bfd, FALSE, &minisyms, &size);
+
+ if (symcount <= 0)
+ no_debugging_data(FATAL);
+
+ sort_x = bfd_make_empty_symbol(st->bfd);
+ sort_y = bfd_make_empty_symbol(st->bfd);
+ if (sort_x == NULL || sort_y == NULL)
+ error(FATAL, "bfd_make_empty_symbol() failed\n");
+
+ gnu_qsort(st->bfd, minisyms, symcount, size, sort_x, sort_y);
+
+ store_symbols(st->bfd, FALSE, minisyms, symcount, size);
+
+ free(minisyms);
+
symname_hash_init();
symval_hash_init();
}
@@ -590,7 +591,7 @@ store_symbols(bfd *abfd, int dynamic, void *minisyms, long symcount,
st->symcnt = 0;
sp = st->symtable;
- if (machine_type("X86")) {
+ if (machine_type("X86") || machine_type("X86_64")) {
if (!(kt->flags & RELOC_SET))
kt->flags |= RELOC_FORCE;
} else
@@ -663,7 +664,7 @@ store_sysmap_symbols(void)
error(FATAL, "symbol table namespace malloc: %s\n",
strerror(errno));
- if (!machine_type("X86"))
+ if (!machine_type("X86") && !machine_type("X86_64"))
kt->flags &= ~RELOC_SET;
first = 0;
@@ -735,7 +736,20 @@ relocate(ulong symval, char *symname, int first_symbol)
break;
}
- return (symval - kt->relocate);
+ if (machine_type("X86_64")) {
+ /*
+ * There are some symbols which are outside of any section
+ * either because they are offsets or because they are absolute
+ * addresses. These should not be relocated.
+ */
+ if (symval >= st->first_section_start &&
+ symval <= st->last_section_end) {
+ return symval - kt->relocate;
+ } else {
+ return symval;
+ }
+ } else
+ return symval - kt->relocate;
}
/*
@@ -9679,6 +9693,7 @@ section_header_info(bfd *bfd, asection *section, void *reqptr)
struct load_module *lm;
ulong request;
asection **sec;
+ ulong section_end_address;
request = ((ulong)reqptr);
@@ -9697,6 +9712,11 @@ section_header_info(bfd *bfd, asection *section, void *reqptr)
kt->etext_init = kt->stext_init +
(ulong)bfd_section_size(bfd, section);
}
+
+ if (STREQ(bfd_get_section_name(bfd, section), ".text")) {
+ st->first_section_start = (ulong)
+ bfd_get_section_vma(bfd, section);
+ }
if (STREQ(bfd_get_section_name(bfd, section), ".text") ||
STREQ(bfd_get_section_name(bfd, section), ".data")) {
if (!(bfd_get_section_flags(bfd, section) & SEC_LOAD))
@@ -9713,6 +9733,14 @@ section_header_info(bfd *bfd, asection *section, void *reqptr)
st->dwarf_debug_frame_file_offset = (off_t)section->filepos;
st->dwarf_debug_frame_size = (ulong)bfd_section_size(bfd, section);
}
+
+ if (st->first_section_start != 0) {
+ section_end_address =
+ (ulong) bfd_get_section_vma(bfd, section) +
+ (ulong) bfd_section_size(bfd, section);
+ if (section_end_address > st->last_section_end)
+ st->last_section_end = section_end_address;
+ }
break;
case (ulong)MODULE_SECTIONS:
--
1.9.0.rc1.175.g0b1dcb5
9:45 a.m.
New subject: [PATCH v2 2/3] Add support to kaslr for offline vmcore files.
Hi Andy,
I made a few small changes to this patch:
(1) The error message generated if calculate() returns false would be nonsensical
if --kaslr was used, and for clarity's sake I moved the -kaslr option into
its own if statement.
(2) The new st->first_section_start and st->last_section_end symbol_table_data
variables should be dumped by "help -s".
(3) Added a --kaslr entry in the crash.8 man page.
(4) Added a --kaslr display in the "crash --help" output.
Queued for crash-7.0.6 -- GitHub commit:
https://github.com/crash-utility/crash/commit/648d0f62c5b9dbd335984cbdf9a...
Thanks,
Dave
----- Original Message -----
This patch adds a --kaslr command line parameter for loading x86_64
crash dumps with kaslr enabled. This reuses the code from 32-bit
x86 relocations with some small changes. The ASLR offset is postive
instead of negative. Also had to move the code to traverse the
kernel section before the symbol storing code to figure out which
symbols were outside any sections and therefore were not relocated.
Also made a very small change in search_for_switch_to it was
searching through gdb command output for a slightly incorrect syntax.
Tested: Tested by loading kdump files from kernels with aslr enabled
and not enabled. Ran bt, files, and struct file 0xXXXXXX.
Signed-off-by: Andy Honig <ahonig(a)google.com>
---
defs.h | 2 ++
main.c | 8 ++++++--
symbols.c | 66
+++++++++++++++++++++++++++++++++++++++++++++------------------
3 files changed, 55 insertions(+), 21 deletions(-)
diff --git a/defs.h b/defs.h
index 4cae5e5..c9a4b73 100644
--- a/defs.h
+++ b/defs.h
@@ -2417,6 +2417,8 @@ struct symbol_table_data {
ulong __per_cpu_end;
off_t dwarf_debug_frame_file_offset;
ulong dwarf_debug_frame_size;
+ ulong first_section_start;
+ ulong last_section_end;
};
/* flags for st */
diff --git a/main.c b/main.c
index e5829bc..39763da 100644
--- a/main.c
+++ b/main.c
@@ -57,6 +57,7 @@ static struct option long_options[] = {
{"CRASHPAGER", 0, 0, 0},
{"no_scroll", 0, 0, 0},
{"reloc", required_argument, 0, 0},
+ {"kaslr", required_argument, 0, 0},
{"active", 0, 0, 0},
{"minimal", 0, 0, 0},
{"mod", required_argument, 0, 0},
@@ -216,12 +217,15 @@ main(int argc, char **argv)
else if (STREQ(long_options[option_index].name, "mod"))
kt->module_tree = optarg;
- else if (STREQ(long_options[option_index].name, "reloc")) {
+ else if (STREQ(long_options[option_index].name, "reloc") ||
+ STREQ(long_options[option_index].name, "kaslr")) {
if (!calculate(optarg, &kt->relocate, NULL, 0)) {
error(INFO, "invalid --reloc argument: %s\n",
optarg);
program_usage(SHORT_FORM);
- }
+ } else if (STREQ(long_options[option_index].name, "kaslr")) {
+ kt->relocate *= -1;
+ }
kt->flags |= RELOC_SET;
}
diff --git a/symbols.c b/symbols.c
index d063a0a..28378ab 100644
--- a/symbols.c
+++ b/symbols.c
@@ -198,22 +198,6 @@ symtab_init(void)
no_debugging_data(FATAL);
}
- symcount = bfd_read_minisymbols(st->bfd, FALSE, &minisyms, &size);
-
- if (symcount <= 0)
- no_debugging_data(FATAL);
-
- sort_x = bfd_make_empty_symbol(st->bfd);
- sort_y = bfd_make_empty_symbol(st->bfd);
- if (sort_x == NULL || sort_y == NULL)
- error(FATAL, "bfd_make_empty_symbol() failed\n");
-
- gnu_qsort(st->bfd, minisyms, symcount, size, sort_x, sort_y);
-
- store_symbols(st->bfd, FALSE, minisyms, symcount, size);
-
- free(minisyms);
-
/*
* Gather references to the kernel sections.
*/
@@ -222,6 +206,7 @@ symtab_init(void)
error(FATAL, "symbol table section array malloc: %s\n",
strerror(errno));
BZERO(st->sections, st->bfd->section_count * sizeof(struct sec *));
+ st->first_section_start = st->last_section_end = 0;
bfd_map_over_sections(st->bfd, section_header_info, KERNEL_SECTIONS);
if ((st->flags & (NO_SEC_LOAD|NO_SEC_CONTENTS)) ==
@@ -233,6 +218,22 @@ symtab_init(void)
}
}
+ symcount = bfd_read_minisymbols(st->bfd, FALSE, &minisyms, &size);
+
+ if (symcount <= 0)
+ no_debugging_data(FATAL);
+
+ sort_x = bfd_make_empty_symbol(st->bfd);
+ sort_y = bfd_make_empty_symbol(st->bfd);
+ if (sort_x == NULL || sort_y == NULL)
+ error(FATAL, "bfd_make_empty_symbol() failed\n");
+
+ gnu_qsort(st->bfd, minisyms, symcount, size, sort_x, sort_y);
+
+ store_symbols(st->bfd, FALSE, minisyms, symcount, size);
+
+ free(minisyms);
+
symname_hash_init();
symval_hash_init();
}
@@ -590,7 +591,7 @@ store_symbols(bfd *abfd, int dynamic, void *minisyms,
long symcount,
st->symcnt = 0;
sp = st->symtable;
- if (machine_type("X86")) {
+ if (machine_type("X86") || machine_type("X86_64")) {
if (!(kt->flags & RELOC_SET))
kt->flags |= RELOC_FORCE;
} else
@@ -663,7 +664,7 @@ store_sysmap_symbols(void)
error(FATAL, "symbol table namespace malloc: %s\n",
strerror(errno));
- if (!machine_type("X86"))
+ if (!machine_type("X86") && !machine_type("X86_64"))
kt->flags &= ~RELOC_SET;
first = 0;
@@ -735,7 +736,20 @@ relocate(ulong symval, char *symname, int first_symbol)
break;
}
- return (symval - kt->relocate);
+ if (machine_type("X86_64")) {
+ /*
+ * There are some symbols which are outside of any section
+ * either because they are offsets or because they are absolute
+ * addresses. These should not be relocated.
+ */
+ if (symval >= st->first_section_start &&
+ symval <= st->last_section_end) {
+ return symval - kt->relocate;
+ } else {
+ return symval;
+ }
+ } else
+ return symval - kt->relocate;
}
/*
@@ -9679,6 +9693,7 @@ section_header_info(bfd *bfd, asection *section, void
*reqptr)
struct load_module *lm;
ulong request;
asection **sec;
+ ulong section_end_address;
request = ((ulong)reqptr);
@@ -9697,6 +9712,11 @@ section_header_info(bfd *bfd, asection *section, void
*reqptr)
kt->etext_init = kt->stext_init +
(ulong)bfd_section_size(bfd, section);
}
+
+ if (STREQ(bfd_get_section_name(bfd, section), ".text")) {
+ st->first_section_start = (ulong)
+ bfd_get_section_vma(bfd, section);
+ }
if (STREQ(bfd_get_section_name(bfd, section), ".text") ||
STREQ(bfd_get_section_name(bfd, section), ".data")) {
if (!(bfd_get_section_flags(bfd, section) &
SEC_LOAD))
@@ -9713,6 +9733,14 @@ section_header_info(bfd *bfd, asection *section, void
*reqptr)
st->dwarf_debug_frame_file_offset = (off_t)section->filepos;
st->dwarf_debug_frame_size = (ulong)bfd_section_size(bfd, section);
}
+
+ if (st->first_section_start != 0) {
+ section_end_address =
+ (ulong) bfd_get_section_vma(bfd, section) +
+ (ulong) bfd_section_size(bfd, section);
+ if (section_end_address > st->last_section_end)
+ st->last_section_end = section_end_address;
+ }
break;
case (ulong)MODULE_SECTIONS:
--
1.9.0.rc1.175.g0b1dcb5
--
Crash-utility mailing list
Crash-utility(a)redhat.com
https://www.redhat.com/mailman/listinfo/crash-utility
6:33 p.m.
New subject: [PATCH v2 3/3] Automatically detect kernel aslr offset.
This patch improves support for kernel aslr by automatically
finding the aslr offset based on the _stext symbol in the vmcore
info.
Signed-off-by: Andrew Honig <ahonig(a)google.com>
---
netdump.c | 11 +++++++++++
symbols.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++----
2 files changed, 58 insertions(+), 4 deletions(-)
diff --git a/netdump.c b/netdump.c
index 7dc2fca..884dd73 100644
--- a/netdump.c
+++ b/netdump.c
@@ -411,6 +411,17 @@ is_netdump(char *file, ulong source_query)
get_log_from_vmcoreinfo(file, vmcoreinfo_read_string);
}
+ /*
+ * We may need the _stext_SYMBOL from the vmcore_info to adjust for
+ * kaslr and we may not have gotten it elsewhere.
+ */
+ if (source_query == KDUMP_LOCAL) {
+ char *tmpstring = vmcoreinfo_read_string("SYMBOL(_stext)");
+ kt->vmcoreinfo._stext_SYMBOL =
+ htol(tmpstring, RETURN_ON_ERROR, NULL);
+ free(tmpstring);
+ }
+
return nd->header_size;
bailout:
diff --git a/symbols.c b/symbols.c
index 28378ab..a2fc150 100644
--- a/symbols.c
+++ b/symbols.c
@@ -557,6 +557,43 @@ strip_symbol_end(const char *name, char *buf)
}
/*
+ * Derives the kernel aslr offset by comparing the _stext symbol from the
+ * the vmcore_info in the dump file to the _stext symbol in the vmlinux file.
+ */
+static void
+derive_kaslr_offset(bfd *abfd, int dynamic, bfd_byte *start, bfd_byte *end,
+ unsigned int size, asymbol *store)
+{
+ symbol_info syminfo;
+ asymbol *sym;
+ char *name;
+ unsigned long relocate;
+ char buf[BUFSIZE];
+
+ for (; start < end; start += size) {
+ sym = bfd_minisymbol_to_symbol(abfd, dynamic, start, store);
+ if (sym == NULL)
+ error(FATAL, "bfd_minisymbol_to_symbol() failed\n");
+
+ bfd_get_symbol_info(abfd, sym, &syminfo);
+ name = strip_symbol_end(syminfo.name, buf);
+ if (strcmp("_stext", name) == 0) {
+ relocate = syminfo.value - kt->vmcoreinfo._stext_SYMBOL;
+ /*
+ *To avoid mistaking an mismatched kernel version with
+ * a kaslr offset, we make sure that the offset is
+ * aligned by 0x1000, as it always will be for
+ * kaslr.
+ */
+ if ((relocate & 0xFFF) == 0) {
+ kt->relocate = relocate;
+ kt->flags |= RELOC_SET;
+ }
+ }
+ }
+}
+
+/*
* Store the symbols gathered by symtab_init(). The symbols are stored
* in increasing numerical order.
*/
@@ -591,15 +628,21 @@ store_symbols(bfd *abfd, int dynamic, void *minisyms, long
symcount,
st->symcnt = 0;
sp = st->symtable;
+ first = 0;
+ from = (bfd_byte *) minisyms;
+ fromend = from + symcount * size;
+
if (machine_type("X86") || machine_type("X86_64")) {
- if (!(kt->flags & RELOC_SET))
+ if (!(kt->flags & RELOC_SET)) {
+ if (machine_type("X86_64")) {
+ derive_kaslr_offset(abfd, dynamic, from,
+ fromend, size, store);
+ }
kt->flags |= RELOC_FORCE;
+ }
} else
kt->flags &= ~RELOC_SET;
- first = 0;
- from = (bfd_byte *) minisyms;
- fromend = from + symcount * size;
for (; from < fromend; from += size)
{
if ((sym = bfd_minisymbol_to_symbol(abfd, dynamic, from, store))
--
1.9.0.rc1.175.g0b1dcb5
11:06 a.m.
New subject: [PATCH v2 3/3] Automatically detect kernel aslr offset.
Hi Andy,
This patch fails immediately on non-kaslr kernels if _stext is aligned on
a page boundary. Here's why:
----- Original Message -----
... [ cut ] ...
+static void
+derive_kaslr_offset(bfd *abfd, int dynamic, bfd_byte *start, bfd_byte *end,
+ unsigned int size, asymbol *store)
+{
+ symbol_info syminfo;
+ asymbol *sym;
+ char *name;
+ unsigned long relocate;
+ char buf[BUFSIZE];
+
+ for (; start < end; start += size) {
+ sym = bfd_minisymbol_to_symbol(abfd, dynamic, start, store);
+ if (sym == NULL)
+ error(FATAL, "bfd_minisymbol_to_symbol() failed\n");
+
+ bfd_get_symbol_info(abfd, sym, &syminfo);
+ name = strip_symbol_end(syminfo.name, buf);
+ if (strcmp("_stext", name) == 0) {
+ relocate = syminfo.value - kt->vmcoreinfo._stext_SYMBOL;
+ /*
+ *To avoid mistaking an mismatched kernel version with
+ * a kaslr offset, we make sure that the offset is
+ * aligned by 0x1000, as it always will be for
+ * kaslr.
+ */
+ if ((relocate & 0xFFF) == 0) {
+ kt->relocate = relocate;
+ kt->flags |= RELOC_SET;
+ }
+ }
+ }
+}
This function is a waste of time if kt->vmcoreinfo._stext_SYMBOL was never set, say
when running on a live system, or on a dumpfile that has no vmcoreinfo. And what's
worse in those cases, if _stext is aligned on a page boundary, then kt->relocate is
set to the _stext symbol value, and all hell breaks loose.
And also, if kt->relocate/RELOC_SET does get set legitimately, the function should
return immediately rather than cycling through the remaining symbols.
BTW, even though the kernel code seems to indicate that this feature would be
applicable to 32-bit x86, should I restrict the man page and help data to indicate
it only applies to x86_64?
Thanks,
Dave
12:58 p.m.
New subject: [PATCH v2 3/3] Automatically detect kernel aslr offset.
On Fri, Feb 21, 2014 at 9:06 AM, Dave Anderson <anderson(a)redhat.com> wrote:
Hi Andy,
This patch fails immediately on non-kaslr kernels if _stext is aligned on
a page boundary. Here's why:
----- Original Message -----
... [ cut ] ...
> +static void
> +derive_kaslr_offset(bfd *abfd, int dynamic, bfd_byte *start, bfd_byte *end,
> + unsigned int size, asymbol *store)
> +{
> + symbol_info syminfo;
> + asymbol *sym;
> + char *name;
> + unsigned long relocate;
> + char buf[BUFSIZE];
> +
> + for (; start < end; start += size) {
> + sym = bfd_minisymbol_to_symbol(abfd, dynamic, start, store);
> + if (sym == NULL)
> + error(FATAL, "bfd_minisymbol_to_symbol() failed\n");
> +
> + bfd_get_symbol_info(abfd, sym, &syminfo);
> + name = strip_symbol_end(syminfo.name, buf);
> + if (strcmp("_stext", name) == 0) {
> + relocate = syminfo.value - kt->vmcoreinfo._stext_SYMBOL;
> + /*
> + *To avoid mistaking an mismatched kernel version with
> + * a kaslr offset, we make sure that the offset is
> + * aligned by 0x1000, as it always will be for
> + * kaslr.
> + */
> + if ((relocate & 0xFFF) == 0) {
> + kt->relocate = relocate;
> + kt->flags |= RELOC_SET;
> + }
> + }
> + }
> +}
This function is a waste of time if kt->vmcoreinfo._stext_SYMBOL was never set, say
when running on a live system, or on a dumpfile that has no vmcoreinfo. And what's
worse in those cases, if _stext is aligned on a page boundary, then kt->relocate is
set to the _stext symbol value, and all hell breaks loose.
Yes, I see your point. I can easily fix this issue by making sure
_stext_SYMBOL is set before trying to derive the kaslr offset. If
you're happy with that change I'll send a new patch. I am a bit
worried that crash handles so many different cases/situations that I'm
still missing something. I see 3 options
1) Fix this patch by checking _stext_SYMBOL is set before trying to
derive kaslr offset.
2) Make this auto-derive functionality opt-in so it doesn't break
other things (something like --kaslr=auto)
3) Wait until the kaslr offset is properly output in the vmcore and
avoid this somewhat hacky calculation entirely.
I'm fine with any of these, but I figured I'd enumerate them and see
which you'd prefer. If you're comfortable with 1 then I'm happy with
it.
And also, if kt->relocate/RELOC_SET does get set legitimately, the
function should
return immediately rather than cycling through the remaining symbols.
Will do.
BTW, even though the kernel code seems to indicate that this feature
would be
applicable to 32-bit x86, should I restrict the man page and help data to indicate
it only applies to x86_64?
I tried to make it only apply to x86_64, which is why the
drive_kaslr_offset function call is within a machine_type("X86_64") if
clause.
thanks,
Andy
Thanks,
Dave
1:52 p.m.
New subject: [PATCH v2 3/3] Automatically detect kernel aslr offset.
----- Original Message -----
On Fri, Feb 21, 2014 at 9:06 AM, Dave Anderson
<anderson(a)redhat.com> wrote:
>
> Hi Andy,
>
> This patch fails immediately on non-kaslr kernels if _stext is aligned on
> a page boundary. Here's why:
>
> ----- Original Message -----
> ... [ cut ] ...
>> +static void
>> +derive_kaslr_offset(bfd *abfd, int dynamic, bfd_byte *start, bfd_byte
>> *end,
>> + unsigned int size, asymbol *store)
>> +{
>> + symbol_info syminfo;
>> + asymbol *sym;
>> + char *name;
>> + unsigned long relocate;
>> + char buf[BUFSIZE];
>> +
>> + for (; start < end; start += size) {
>> + sym = bfd_minisymbol_to_symbol(abfd, dynamic, start, store);
>> + if (sym == NULL)
>> + error(FATAL, "bfd_minisymbol_to_symbol()
failed\n");
>> +
>> + bfd_get_symbol_info(abfd, sym, &syminfo);
>> + name = strip_symbol_end(syminfo.name, buf);
>> + if (strcmp("_stext", name) == 0) {
>> + relocate = syminfo.value -
kt->vmcoreinfo._stext_SYMBOL;
>> + /*
>> + *To avoid mistaking an mismatched kernel version with
>> + * a kaslr offset, we make sure that the offset is
>> + * aligned by 0x1000, as it always will be for
>> + * kaslr.
>> + */
>> + if ((relocate & 0xFFF) == 0) {
>> + kt->relocate = relocate;
>> + kt->flags |= RELOC_SET;
>> + }
>> + }
>> + }
>> +}
>
> This function is a waste of time if kt->vmcoreinfo._stext_SYMBOL was never set,
say
> when running on a live system, or on a dumpfile that has no vmcoreinfo. And
what's
> worse in those cases, if _stext is aligned on a page boundary, then kt->relocate
is
> set to the _stext symbol value, and all hell breaks loose.
>
Yes, I see your point. I can easily fix this issue by making sure
_stext_SYMBOL is set before trying to derive the kaslr offset. If
you're happy with that change I'll send a new patch. I am a bit
worried that crash handles so many different cases/situations that I'm
still missing something. I see 3 options
1) Fix this patch by checking _stext_SYMBOL is set before trying to
derive kaslr offset.
2) Make this auto-derive functionality opt-in so it doesn't break
other things (something like --kaslr=auto)
3) Wait until the kaslr offset is properly output in the vmcore and
avoid this somewhat hacky calculation entirely.
I'm fine with any of these, but I figured I'd enumerate them and see
which you'd prefer. If you're comfortable with 1 then I'm happy with
it.
Right, it would be nice if the KERNELOFFSET vmcoreinfo item had gone into
the kernel at the same time as kALSR. I see that it's not in Linus' tree
yet -- has it been accepted into any other tree waiting to be pulled?
Anyway, for that reason, I like the idea of the --kaslr=auto option, and
maybe setting a flag somewhere, say in st->flags. And when the KERNELOFFSET
item does eventually show up, the same flag could be set during the initial
scan of the dumpfile header, obviating the need for --kaslr.
Which reminds me -- the "SYMBOL(_stext)" check that you make in is_netdump()
also has to be done in is_diskdump() as well. Currently makedumpfile does
not work with kaslr dumpfiles, but they will eventually get it working.
> And also, if kt->relocate/RELOC_SET does get set legitimately, the function
should
> return immediately rather than cycling through the remaining symbols.
>
Will do.
> BTW, even though the kernel code seems to indicate that this feature would be
> applicable to 32-bit x86, should I restrict the man page and help data to indicate
> it only applies to x86_64?
I tried to make it only apply to x86_64, which is why the
drive_kaslr_offset function call is within a machine_type("X86_64") if
clause.
thanks,
Andy
But the manual setting of --kalsr=<offset> would still be passed through
for 32-bit x86, correct? Maybe just --kaslr=auto could be restricted?
Dave
1:58 p.m.
New subject: [PATCH v2 3/3] Automatically detect kernel aslr offset.
On Fri, Feb 21, 2014 at 11:52 AM, Dave Anderson <anderson(a)redhat.com> wrote:
----- Original Message -----
> On Fri, Feb 21, 2014 at 9:06 AM, Dave Anderson <anderson(a)redhat.com> wrote:
> >
> > Hi Andy,
> >
> > This patch fails immediately on non-kaslr kernels if _stext is aligned on
> > a page boundary. Here's why:
> >
> > ----- Original Message -----
> > ... [ cut ] ...
> >> +static void
> >> +derive_kaslr_offset(bfd *abfd, int dynamic, bfd_byte *start, bfd_byte
> >> *end,
> >> + unsigned int size, asymbol *store)
> >> +{
> >> + symbol_info syminfo;
> >> + asymbol *sym;
> >> + char *name;
> >> + unsigned long relocate;
> >> + char buf[BUFSIZE];
> >> +
> >> + for (; start < end; start += size) {
> >> + sym = bfd_minisymbol_to_symbol(abfd, dynamic, start, store);
> >> + if (sym == NULL)
> >> + error(FATAL, "bfd_minisymbol_to_symbol()
failed\n");
> >> +
> >> + bfd_get_symbol_info(abfd, sym, &syminfo);
> >> + name = strip_symbol_end(syminfo.name, buf);
> >> + if (strcmp("_stext", name) == 0) {
> >> + relocate = syminfo.value -
kt->vmcoreinfo._stext_SYMBOL;
> >> + /*
> >> + *To avoid mistaking an mismatched kernel version
with
> >> + * a kaslr offset, we make sure that the offset is
> >> + * aligned by 0x1000, as it always will be for
> >> + * kaslr.
> >> + */
> >> + if ((relocate & 0xFFF) == 0) {
> >> + kt->relocate = relocate;
> >> + kt->flags |= RELOC_SET;
> >> + }
> >> + }
> >> + }
> >> +}
> >
> > This function is a waste of time if kt->vmcoreinfo._stext_SYMBOL was never
set, say
> > when running on a live system, or on a dumpfile that has no vmcoreinfo. And
what's
> > worse in those cases, if _stext is aligned on a page boundary, then
kt->relocate is
> > set to the _stext symbol value, and all hell breaks loose.
> >
>
> Yes, I see your point. I can easily fix this issue by making sure
> _stext_SYMBOL is set before trying to derive the kaslr offset. If
> you're happy with that change I'll send a new patch. I am a bit
> worried that crash handles so many different cases/situations that I'm
> still missing something. I see 3 options
> 1) Fix this patch by checking _stext_SYMBOL is set before trying to
> derive kaslr offset.
> 2) Make this auto-derive functionality opt-in so it doesn't break
> other things (something like --kaslr=auto)
> 3) Wait until the kaslr offset is properly output in the vmcore and
> avoid this somewhat hacky calculation entirely.
>
> I'm fine with any of these, but I figured I'd enumerate them and see
> which you'd prefer. If you're comfortable with 1 then I'm happy with
> it.
Right, it would be nice if the KERNELOFFSET vmcoreinfo item had gone into
the kernel at the same time as kALSR. I see that it's not in Linus' tree
yet -- has it been accepted into any other tree waiting to be pulled?
Anyway, for that reason, I like the idea of the --kaslr=auto option, and
maybe setting a flag somewhere, say in st->flags. And when the KERNELOFFSET
item does eventually show up, the same flag could be set during the initial
scan of the dumpfile header, obviating the need for --kaslr.
Which reminds me -- the "SYMBOL(_stext)" check that you make in is_netdump()
also has to be done in is_diskdump() as well. Currently makedumpfile does
not work with kaslr dumpfiles, but they will eventually get it working.
Sounds good to me, I'll do that. I'll check with kees on the
KERNELOFFSET patch status.
>
> > And also, if kt->relocate/RELOC_SET does get set legitimately, the function
should
> > return immediately rather than cycling through the remaining symbols.
> >
> Will do.
>
> > BTW, even though the kernel code seems to indicate that this feature would be
> > applicable to 32-bit x86, should I restrict the man page and help data to
indicate
> > it only applies to x86_64?
>
> I tried to make it only apply to x86_64, which is why the
> drive_kaslr_offset function call is within a machine_type("X86_64") if
> clause.
> thanks,
> Andy
But the manual setting of --kalsr=<offset> would still be passed through
for 32-bit x86, correct? Maybe just --kaslr=auto could be restricted?
Yes, the manual setting of --kaslr would work with 32-bit x86, but it
probably shouldn't be used that way. I like the idea of restricting
the man page. I don't think it's worth checking to make sure it's not
set on a 32-bit kernel, but I can add that if you prefer.
Dave
2:16 p.m.
New subject: [PATCH v2 3/3] Automatically detect kernel aslr offset.
----- Original Message -----
> Right, it would be nice if the KERNELOFFSET vmcoreinfo item had
gone into
> the kernel at the same time as kALSR. I see that it's not in Linus' tree
> yet -- has it been accepted into any other tree waiting to be pulled?
>
> Anyway, for that reason, I like the idea of the --kaslr=auto option, and
> maybe setting a flag somewhere, say in st->flags. And when the KERNELOFFSET
> item does eventually show up, the same flag could be set during the initial
> scan of the dumpfile header, obviating the need for --kaslr.
>
> Which reminds me -- the "SYMBOL(_stext)" check that you make in
is_netdump()
> also has to be done in is_diskdump() as well. Currently makedumpfile does
> not work with kaslr dumpfiles, but they will eventually get it working.
>
Sounds good to me, I'll do that. I'll check with kees on the
KERNELOFFSET patch status.
OK.
>>
>> > BTW, even though the kernel code seems to indicate that this feature would
be
>> > applicable to 32-bit x86, should I restrict the man page and help data to
indicate
>> > it only applies to x86_64?
>>
>> I tried to make it only apply to x86_64, which is why the drive_kaslr_offset
>> function call is within a machine_type("X86_64") if clause.
>> thanks,
>> Andy
>
> But the manual setting of --kalsr=<offset> would still be passed through
> for 32-bit x86, correct? Maybe just --kaslr=auto could be restricted?
>
Yes, the manual setting of --kaslr would work with 32-bit x86, but it
probably shouldn't be used that way. I like the idea of restricting
the man page. I don't think it's worth checking to make sure it's not
set on a 32-bit kernel, but I can add that if you prefer.
I personally don't really care that much, given that Red Hat no longer
supports x86 kernels. But it seems like the 32-bit x86 code should either
accept both --kalsr options or neither of them. And eventually when
somebody gets around to testing this stuff on 32-bit, we can open the
door to supporting x86.
One other thing -- this is pretty ugly:
WARNING: kernel relocated [17592186044032MB]: patching 55431 gdb minimal_symbol values
It makes sense with a positive --reloc MB value, but in the case of --kaslr the
message should change the displayed value back to the positive input argument.
Thanks,
Dave
2:37 p.m.
New subject: [PATCH v2 3/3] Automatically detect kernel aslr offset.
On Fri, Feb 21, 2014 at 11:58 AM, Andrew Honig <ahonig(a)google.com> wrote:
On Fri, Feb 21, 2014 at 11:52 AM, Dave Anderson
<anderson(a)redhat.com> wrote:
> Right, it would be nice if the KERNELOFFSET vmcoreinfo item had gone into
> the kernel at the same time as kALSR. I see that it's not in Linus' tree
> yet -- has it been accepted into any other tree waiting to be pulled?
>
> Anyway, for that reason, I like the idea of the --kaslr=auto option, and
> maybe setting a flag somewhere, say in st->flags. And when the KERNELOFFSET
> item does eventually show up, the same flag could be set during the initial
> scan of the dumpfile header, obviating the need for --kaslr.
>
> Which reminds me -- the "SYMBOL(_stext)" check that you make in
is_netdump()
> also has to be done in is_diskdump() as well. Currently makedumpfile does
> not work with kaslr dumpfiles, but they will eventually get it working.
>
Sounds good to me, I'll do that. I'll check with kees on the
KERNELOFFSET patch status.
I CC'd you guys on a ping for that patch. I think Peter Anvin has been
busy so he didn't pick it up yet. If it doesn't get traction soon, I
can see if other x86 folks will take it.
-Kees
--
Kees Cook
Chrome OS Security
4075
days inactive
4076
days old
devel@lists.crash-utility.osci.io
11 comments
4 participants
participants (4)
-
Andrew Honig -
Andy Honig
-
Dave Anderson -
Kees Cook