The pstore ftrace frontend works by purely collecting the instruction address, saving it on the persistent area through the backend and when the log is read, on next boot for example, the address is then resolved by using the regular printk symbol lookup (%pS for example).
Problem: if we are running a relocatable kernel with KASLR enabled, this is a recipe for failure in the symbol resolution on next boots, since the addresses are offset'ed by the KASLR address. So, naturally the way to go is factor the KASLR address out of instruction address collection, and adding the fresh offset when resolving the symbol on future boots. Problem #2: modules also have varying addresses that float based on module base address and potentially the module ordering in memory, meaning factoring KASLR offset for them is useless. So, let's hereby only take KASLR offset into account for core kernel addresses, leaving module ones as is. And we have yet a 3rd complexity: not necessarily the check range for core kernel addresses holds true on future boots, since the module base address will vary. With that, the choice was to mark the addresses as being core vs module based on its MSB. And with that... ...we have the 4th challenge here: for some "simple" architectures, the CPU number is saved bit-encoded on the instruction pointer, to allow bigger timestamps - this is set through the PSTORE_CPU_IN_IP define for such architectures. Hence, the approach here is to skip such architectures (at least in a first moment). TL;DR: let's factor KASLR offsets on pstore/ftrace for core kernel addresses, leaving module addresses out and also leaving the architectures that define PSTORE_CPU_IN_IP. Signed-off-by: Guilherme G. Piccoli <[email protected]> --- Hi folks, first of all thanks in advance for reviews and comments! I was testing a pstore/ftrace patch the other day and noticed the lack of the KASLR support. But to my surprise, it was not as easy to fix up as I expected heh Main reason is the obvious thing with modules: the way to go, I think, is to somehow save the module name (or some other id?) and the instruction offset inside such module, to then resolve that in next boot, when printing. But that would require more intrusive changes in the way pstore/ftrace saves the IP (which is quite simple now), leading to some potentially meaningful perf overhead. Hence, I've decided to just mess with core kernel addresses so far, lemme know WDYT - should I somehow pursue fixing modules addr resolution as well? Or doesn't worth the changes? Any ideas on how to approach that? I noticed that currently, modules' symbols are sometimes resolved fine, sometimes they're bogus but point to the module at least (not some other random code), but eventually they are just nonsense addresses. Regarding the choice of using the MSB to store if an addr is core kernel or module, well this was also a choice taking into account simplicity and performance, lemme know please if it's no good and any suggestions on how to better do it, I can easily re-implement! Thanks again, Guilherme fs/pstore/ftrace.c | 33 +++++++++++++++++++++++++++++++-- fs/pstore/inode.c | 6 ++++-- fs/pstore/internal.h | 2 ++ 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/fs/pstore/ftrace.c b/fs/pstore/ftrace.c index 13db123beac1..58f8204b23af 100644 --- a/fs/pstore/ftrace.c +++ b/fs/pstore/ftrace.c @@ -18,10 +18,35 @@ #include <linux/cache.h> #include <linux/slab.h> #include <asm/barrier.h> +#include <asm/setup.h> #include "internal.h" /* This doesn't need to be atomic: speed is chosen over correctness here. */ static u64 pstore_ftrace_stamp; +unsigned long kaslr_off; + +static inline unsigned long adjust_ip(unsigned long ip) +{ +#ifndef PSTORE_CPU_IN_IP + if (core_kernel_text(ip)) + return ip - kaslr_off; + + __clear_bit(BITS_PER_LONG - 1, &ip); +#endif + return ip; +} + +inline unsigned long decode_ip(unsigned long ip) +{ +#ifndef PSTORE_CPU_IN_IP + if (test_bit(BITS_PER_LONG - 1, &ip)) + return ip + kaslr_off; + + __set_bit(BITS_PER_LONG - 1, &ip); + +#endif + return ip; +} static void notrace pstore_ftrace_call(unsigned long ip, unsigned long parent_ip, @@ -47,8 +72,8 @@ static void notrace pstore_ftrace_call(unsigned long ip, local_irq_save(flags); - rec.ip = ip; - rec.parent_ip = parent_ip; + rec.ip = adjust_ip(ip); + rec.parent_ip = adjust_ip(parent_ip); pstore_ftrace_write_timestamp(&rec, pstore_ftrace_stamp++); pstore_ftrace_encode_cpu(&rec, raw_smp_processor_id()); psinfo->write(&record); @@ -132,6 +157,10 @@ void pstore_register_ftrace(void) if (!psinfo->write) return; +#ifdef CONFIG_RANDOMIZE_BASE + kaslr_off = kaslr_offset(); +#endif + pstore_ftrace_dir = debugfs_create_dir("pstore", NULL); pstore_set_ftrace_enabled(record_ftrace); diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c index 83fa0bb3435a..62e678f3527d 100644 --- a/fs/pstore/inode.c +++ b/fs/pstore/inode.c @@ -105,17 +105,19 @@ static int pstore_ftrace_seq_show(struct seq_file *s, void *v) struct pstore_private *ps = s->private; struct pstore_ftrace_seq_data *data = v; struct pstore_ftrace_record *rec; + unsigned long ip, parent_ip; if (!data) return 0; rec = (struct pstore_ftrace_record *)(ps->record->buf + data->off); + ip = decode_ip(rec->ip); + parent_ip = decode_ip(rec->parent_ip); seq_printf(s, "CPU:%d ts:%llu %08lx %08lx %ps <- %pS\n", pstore_ftrace_decode_cpu(rec), pstore_ftrace_read_timestamp(rec), - rec->ip, rec->parent_ip, (void *)rec->ip, - (void *)rec->parent_ip); + ip, parent_ip, (void *)ip, (void *)parent_ip); return 0; } diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h index a0fc51196910..079284120db9 100644 --- a/fs/pstore/internal.h +++ b/fs/pstore/internal.h @@ -9,6 +9,7 @@ extern unsigned int kmsg_bytes; #ifdef CONFIG_PSTORE_FTRACE +extern unsigned long decode_ip(unsigned long ip); extern void pstore_register_ftrace(void); extern void pstore_unregister_ftrace(void); ssize_t pstore_ftrace_combine_log(char **dest_log, size_t *dest_log_size, @@ -16,6 +17,7 @@ ssize_t pstore_ftrace_combine_log(char **dest_log, size_t *dest_log_size, #else static inline void pstore_register_ftrace(void) {} static inline void pstore_unregister_ftrace(void) {} +static inline unsigned long decode_ip(unsigned long ip) { return ip; } static inline ssize_t pstore_ftrace_combine_log(char **dest_log, size_t *dest_log_size, const char *src_log, size_t src_log_size) -- 2.50.1
