From 4ef6255cc56343bc90d82420b49dab1b11dee414 Mon Sep 17 00:00:00 2001 From: Andrey Skvortsov Date: Sun, 15 Feb 2026 21:51:55 +0300 Subject: [PATCH 1/6] pstore: fix ftrace dump, when ECC is enabled total_size is sum of record->size and record->ecc_notice_size (ECC: No errors detected). When ECC is not used, then there is no problem. When ECC is enabled, then ftrace dump is decoded incorrectly after restart. First this affects starting offset calculation, that breaks reading of all ftrace records. CPU:66 ts:51646260179894273 3818ffff80008002 fe00ffff800080f0 0x3818ffff80008002 <- 0xfe00ffff800080f0 CPU:66 ts:56589664458375169 3818ffff80008002 ff02ffff800080f0 0x3818ffff80008002 <- 0xff02ffff800080f0 CPU:67 ts:13194139533313 afe4ffff80008002 1ffff800080f0 0xafe4ffff80008002 <- 0x1ffff800080f0 CPU:67 ts:13194139533313 b7d0ffff80008001 100ffff80008002 0xb7d0ffff80008001 <- 0x100ffff80008002 CPU:67 ts:51646260179894273 8de0ffff80008001 202ffff80008002 0x8de0ffff80008001 <- 0x202ffff80008002 Second ECC notice message is printed like ftrace record and as a result couple of last records are completely wrong. For example, when the starting offset is fixed: CPU:0 ts:113 ffffffc00879bd04 ffffffc0080dc08c cpuidle_enter <- do_idle+0x20c/0x290 CPU:0 ts:114 ffffffc00879bd04 ffffffc0080dc08c cpuidle_enter <- do_idle+0x20c/0x290 CPU:100 ts:28259048229270629 6f4e203a4343450a 2073726f72726520 0x6f4e203a4343450a <- 0x2073726f72726520 Signed-off-by: Andrey Skvortsov Tested-by: Guilherme G. Piccoli Link: https://patch.msgid.link/20260215185156.317394-1-andrej.skvortzov@gmail.com Signed-off-by: Kees Cook --- fs/pstore/inode.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c index 83fa0bb3435a..d0ca91040591 100644 --- a/fs/pstore/inode.c +++ b/fs/pstore/inode.c @@ -74,9 +74,9 @@ static void *pstore_ftrace_seq_start(struct seq_file *s, loff_t *pos) if (!data) return NULL; - data->off = ps->total_size % REC_SIZE; + data->off = ps->record->size % REC_SIZE; data->off += *pos * REC_SIZE; - if (data->off + REC_SIZE > ps->total_size) + if (data->off + REC_SIZE > ps->record->size) return NULL; return_ptr(data); @@ -94,7 +94,7 @@ static void *pstore_ftrace_seq_next(struct seq_file *s, void *v, loff_t *pos) (*pos)++; data->off += REC_SIZE; - if (data->off + REC_SIZE > ps->total_size) + if (data->off + REC_SIZE > ps->record->size) return NULL; return data; From 80632e333b0bd3cf188cff4e7ff52114506f5612 Mon Sep 17 00:00:00 2001 From: "Guilherme G. Piccoli" Date: Wed, 18 Feb 2026 16:37:31 -0300 Subject: [PATCH 2/6] pstore/ramoops: Remove useless memblock header Seems the linux/memblock.h inclusion was added early on due to usage of some memblock allocation routine. But that was removed, header was forgotten, hence let's remove that. Signed-off-by: Guilherme G. Piccoli Link: https://patch.msgid.link/20260218193940.912143-2-gpiccoli@igalia.com Signed-off-by: Kees Cook --- fs/pstore/ram_core.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c index ed97494abf60..d47ba14e47bd 100644 --- a/fs/pstore/ram_core.c +++ b/fs/pstore/ram_core.c @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include From b22462c79179f228327b98313b47369129114d6a Mon Sep 17 00:00:00 2001 From: "Guilherme G. Piccoli" Date: Wed, 18 Feb 2026 16:37:32 -0300 Subject: [PATCH 3/6] pstore/ramoops: Fix ECC parameter help text In order to set ECC on ramoops, the parameter "ecc" should be used. The variable that carries this information is "ramoops_ecc". Due to some confusion in the parameter setting functions, modinfo ends-up showing both "ecc" and "ramoops_ecc" as valid parameters, but only "ecc" is the valid one, hence this fix to the parameter help text. Signed-off-by: Guilherme G. Piccoli Link: https://patch.msgid.link/20260218193940.912143-3-gpiccoli@igalia.com Signed-off-by: Kees Cook --- fs/pstore/ram.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index d1f895e57a95..2eb0d4fa2186 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -71,7 +71,7 @@ MODULE_PARM_DESC(max_reason, static int ramoops_ecc; module_param_named(ecc, ramoops_ecc, int, 0400); -MODULE_PARM_DESC(ramoops_ecc, +MODULE_PARM_DESC(ecc, "if non-zero, the option enables ECC support and specifies " "ECC buffer size in bytes (1 is a special value, means 16 " "bytes ECC)"); From 2ddb69f686ef7a621645e97fc7329c50edf5d0e5 Mon Sep 17 00:00:00 2001 From: Cole Leavitt Date: Wed, 25 Feb 2026 16:54:06 -0700 Subject: [PATCH 4/6] pstore/ram: fix resource leak when ioremap() fails In persistent_ram_iomap(), ioremap() or ioremap_wc() may return NULL on failure. Currently, if this happens, the function returns NULL without releasing the memory region acquired by request_mem_region(). This leads to a resource leak where the memory region remains reserved but unusable. Additionally, the caller persistent_ram_buffer_map() handles NULL correctly by returning -ENOMEM, but without this check, a NULL return combined with request_mem_region() succeeding leaves resources in an inconsistent state. This is the ioremap() counterpart to commit 05363abc7625 ("pstore: ram_core: fix incorrect success return when vmap() fails") which fixed a similar issue in the vmap() path. Fixes: 404a6043385d ("staging: android: persistent_ram: handle reserving and mapping memory") Signed-off-by: Cole Leavitt Link: https://patch.msgid.link/20260225235406.11790-1-cole@unwrap.rs Signed-off-by: Kees Cook --- fs/pstore/ram_core.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c index d47ba14e47bd..05048c6f787a 100644 --- a/fs/pstore/ram_core.c +++ b/fs/pstore/ram_core.c @@ -487,6 +487,10 @@ static void *persistent_ram_iomap(phys_addr_t start, size_t size, else va = ioremap_wc(start, size); + /* We must release the mem region if ioremap fails. */ + if (!va) + release_mem_region(start, size); + /* * Since request_mem_region() and ioremap() are byte-granularity * there is no need handle anything special like we do when the From 421a41c485dde449cbf90ba610b805bd99e3ae78 Mon Sep 17 00:00:00 2001 From: "Guilherme G. Piccoli" Date: Sun, 1 Mar 2026 16:26:36 -0300 Subject: [PATCH 5/6] pstore/ftrace: Keep ftrace module parameter and debugfs switch in sync MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Commit a5d05b07961a ("pstore/ftrace: Allow immediate recording") introduced a kernel parameter to enable early-boot collection for ftrace frontend. But then, if we enable the debugfs later, the parameter remains set as N. This is not a biggie, things work fine; but at the same time, why not have both in sync if possible, right? Cc: Uwe Kleine-König Signed-off-by: Guilherme G. Piccoli Link: https://patch.msgid.link/20260301192704.1263589-1-gpiccoli@igalia.com Signed-off-by: Kees Cook --- fs/pstore/ftrace.c | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/fs/pstore/ftrace.c b/fs/pstore/ftrace.c index 776cae20af4e..13db123beac1 100644 --- a/fs/pstore/ftrace.c +++ b/fs/pstore/ftrace.c @@ -62,13 +62,16 @@ static struct ftrace_ops pstore_ftrace_ops __read_mostly = { }; static DEFINE_MUTEX(pstore_ftrace_lock); -static bool pstore_ftrace_enabled; +static bool record_ftrace; +module_param(record_ftrace, bool, 0400); +MODULE_PARM_DESC(record_ftrace, + "enable ftrace recording immediately (default: off)"); static int pstore_set_ftrace_enabled(bool on) { ssize_t ret; - if (on == pstore_ftrace_enabled) + if (on == record_ftrace) return 0; if (on) { @@ -82,7 +85,7 @@ static int pstore_set_ftrace_enabled(bool on) pr_err("%s: unable to %sregister ftrace ops: %zd\n", __func__, on ? "" : "un", ret); } else { - pstore_ftrace_enabled = on; + record_ftrace = on; } return ret; @@ -111,7 +114,7 @@ static ssize_t pstore_ftrace_knob_write(struct file *f, const char __user *buf, static ssize_t pstore_ftrace_knob_read(struct file *f, char __user *buf, size_t count, loff_t *ppos) { - char val[] = { '0' + pstore_ftrace_enabled, '\n' }; + char val[] = { '0' + record_ftrace, '\n' }; return simple_read_from_buffer(buf, count, ppos, val, sizeof(val)); } @@ -124,11 +127,6 @@ static const struct file_operations pstore_knob_fops = { static struct dentry *pstore_ftrace_dir; -static bool record_ftrace; -module_param(record_ftrace, bool, 0400); -MODULE_PARM_DESC(record_ftrace, - "enable ftrace recording immediately (default: off)"); - void pstore_register_ftrace(void) { if (!psinfo->write) @@ -145,9 +143,9 @@ void pstore_register_ftrace(void) void pstore_unregister_ftrace(void) { mutex_lock(&pstore_ftrace_lock); - if (pstore_ftrace_enabled) { + if (record_ftrace) { unregister_ftrace_function(&pstore_ftrace_ops); - pstore_ftrace_enabled = false; + record_ftrace = false; } mutex_unlock(&pstore_ftrace_lock); From 24b8f8dcb9a139a36cf48bfbe935e8dc1f33ed79 Mon Sep 17 00:00:00 2001 From: "Guilherme G. Piccoli" Date: Fri, 10 Apr 2026 17:49:26 -0300 Subject: [PATCH 6/6] pstore/ftrace: Factor KASLR offset in the core kernel instruction addresses 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). Finished? No. On top of all previous complexities, we have one extra pain point: kaslr_offset() is inlined and fully "resolved" at boot-time, after kernel decompression, through ELF relocation mechanism. Once the offset is known, it's patched to the kernel text area, wherever it is used. The mechanism, and its users, are only built-in - incompatible with module usage. Though there are possibly some hacks (as computing the offset using some kallsym lookup), the choice here is to restrict this optimization to the (hopefully common) case of CONFIG_PSTORE=y. TL;DR: let's factor KASLR offsets on pstore/ftrace for core kernel addresses, only when PSTORE is built-in and leaving module addresses out, as well as architectures that define PSTORE_CPU_IN_IP. Signed-off-by: Guilherme G. Piccoli Link: https://patch.msgid.link/20260410205848.2607169-1-gpiccoli@igalia.com Signed-off-by: Kees Cook --- fs/pstore/ftrace.c | 28 ++++++++++++++++++++++++++-- fs/pstore/inode.c | 6 ++++-- fs/pstore/internal.h | 2 ++ 3 files changed, 32 insertions(+), 4 deletions(-) diff --git a/fs/pstore/ftrace.c b/fs/pstore/ftrace.c index 13db123beac1..f937c6a33bea 100644 --- a/fs/pstore/ftrace.c +++ b/fs/pstore/ftrace.c @@ -18,11 +18,35 @@ #include #include #include +#include #include "internal.h" /* This doesn't need to be atomic: speed is chosen over correctness here. */ static u64 pstore_ftrace_stamp; +static inline unsigned long adjust_ip(unsigned long ip) +{ +#if defined(CONFIG_RANDOMIZE_BASE) && !defined(PSTORE_CPU_IN_IP) && IS_BUILTIN(CONFIG_PSTORE) + if (core_kernel_text(ip)) + return ip - kaslr_offset(); + + __clear_bit(BITS_PER_LONG - 1, &ip); +#endif + return ip; +} + +inline unsigned long decode_ip(unsigned long ip) +{ +#if defined(CONFIG_RANDOMIZE_BASE) && !defined(PSTORE_CPU_IN_IP) && IS_BUILTIN(CONFIG_PSTORE) + if (test_bit(BITS_PER_LONG - 1, &ip)) + return ip + kaslr_offset(); + + __set_bit(BITS_PER_LONG - 1, &ip); + +#endif + return ip; +} + static void notrace pstore_ftrace_call(unsigned long ip, unsigned long parent_ip, struct ftrace_ops *op, @@ -47,8 +71,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); diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c index d0ca91040591..cf499ba72702 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)