From 99b3b837855b987563bcfb397cf9ddd88262814b Mon Sep 17 00:00:00 2001 From: Qiujun Huang Date: Sun, 4 Sep 2022 23:17:13 +0800 Subject: [PATCH 01/13] pstore/zone: Use GFP_ATOMIC to allocate zone buffer There is a case found when triggering a panic_on_oom, pstore fails to dump kmsg. Because psz_kmsg_write_record can't get the new buffer. Handle this by using GFP_ATOMIC to allocate a buffer at lower watermark. Signed-off-by: Qiujun Huang Fixes: 335426c6dcdd ("pstore/zone: Provide way to skip "broken" zone for MTD devices") Cc: WeiXiong Liao Cc: stable@vger.kernel.org Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/CAJRQjofRCF7wjrYmw3D7zd5QZnwHQq+F8U-mJDJ6NZ4bddYdLA@mail.gmail.com --- fs/pstore/zone.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/pstore/zone.c b/fs/pstore/zone.c index 017d0d4ad329..2770746bb7aa 100644 --- a/fs/pstore/zone.c +++ b/fs/pstore/zone.c @@ -761,7 +761,7 @@ static inline int notrace psz_kmsg_write_record(struct psz_context *cxt, /* avoid destroying old data, allocate a new one */ len = zone->buffer_size + sizeof(*zone->buffer); zone->oldbuf = zone->buffer; - zone->buffer = kzalloc(len, GFP_KERNEL); + zone->buffer = kzalloc(len, GFP_ATOMIC); if (!zone->buffer) { zone->buffer = zone->oldbuf; return -ENOMEM; From d85644dc5cf4023bf1a325f476d58a16bcc0798b Mon Sep 17 00:00:00 2001 From: "Guilherme G. Piccoli" Date: Thu, 6 Oct 2022 19:42:05 -0300 Subject: [PATCH 02/13] pstore: Improve error reporting in case of backend overlap The pstore infrastructure supports one single backend at a time; trying to load a another backend causes an error and displays a message, introduced on commit 0d7cd09a3dbb ("pstore: Improve register_pstore() error reporting"). Happens that this message is not really clear about the situation, also the current error returned (-EPERM) isn't accurate, whereas -EBUSY makes more sense. We have another place in the code that relies in the -EBUSY return for a similar check. So, make it consistent here by returning -EBUSY and using a similar message in both scenarios. Signed-off-by: Guilherme G. Piccoli Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20221006224212.569555-2-gpiccoli@igalia.com --- fs/pstore/platform.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 0c034ea39954..c32957e4b256 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -562,8 +562,9 @@ out: int pstore_register(struct pstore_info *psi) { if (backend && strcmp(backend, psi->name)) { - pr_warn("ignoring unexpected backend '%s'\n", psi->name); - return -EPERM; + pr_warn("backend '%s' already in use: ignoring '%s'\n", + backend, psi->name); + return -EBUSY; } /* Sanity check flags. */ From 8f5de3fd38b7e64112017bb1630f9df6e1b3331d Mon Sep 17 00:00:00 2001 From: "Guilherme G. Piccoli" Date: Thu, 6 Oct 2022 19:42:06 -0300 Subject: [PATCH 03/13] pstore: Expose kmsg_bytes as a module parameter Currently this tuning is only exposed as a filesystem option, but most Linux distros automatically mount pstore, hence changing this setting requires remounting it. Also, if that mount option wasn't explicitly set it doesn't show up in mount information, so users cannot check what is the current value of kmsg_bytes. Let's then expose it as a module parameter, allowing both user visibility at all times (even if not manually set) and also the possibility of setting that as a boot/module parameter. Signed-off-by: Guilherme G. Piccoli Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20221006224212.569555-3-gpiccoli@igalia.com --- fs/pstore/platform.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index c32957e4b256..be05090076ce 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -89,6 +89,11 @@ static char *compress = module_param(compress, charp, 0444); MODULE_PARM_DESC(compress, "compression to use"); +/* How much of the kernel log to snapshot */ +unsigned long kmsg_bytes = CONFIG_PSTORE_DEFAULT_KMSG_BYTES; +module_param(kmsg_bytes, ulong, 0444); +MODULE_PARM_DESC(kmsg_bytes, "amount of kernel log to snapshot (in bytes)"); + /* Compression parameters */ static struct crypto_comp *tfm; @@ -100,9 +105,6 @@ struct pstore_zbackend { static char *big_oops_buf; static size_t big_oops_buf_sz; -/* How much of the console log to snapshot */ -unsigned long kmsg_bytes = CONFIG_PSTORE_DEFAULT_KMSG_BYTES; - void pstore_set_kmsg_bytes(int bytes) { kmsg_bytes = bytes; From 6a14f1982e6b476c84938c9ca011d258a6bba24d Mon Sep 17 00:00:00 2001 From: "Guilherme G. Piccoli" Date: Thu, 6 Oct 2022 19:42:07 -0300 Subject: [PATCH 04/13] pstore: Inform unregistered backend names as well Currently we only show the registered ones in the kernel log; users that change backend for some reason require first to unregister the current one, so let's confirm this operation with a message in the log. Signed-off-by: Guilherme G. Piccoli Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20221006224212.569555-4-gpiccoli@igalia.com --- fs/pstore/platform.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index be05090076ce..06c2c66af332 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -665,6 +665,8 @@ void pstore_unregister(struct pstore_info *psi) psinfo = NULL; kfree(backend); backend = NULL; + + pr_info("Unregistered %s as persistent store backend\n", psi->name); mutex_unlock(&psinfo_lock); } EXPORT_SYMBOL_GPL(pstore_unregister); From 893c5f1de620fb0134ddef37dfd850f6c639162b Mon Sep 17 00:00:00 2001 From: "Guilherme G. Piccoli" Date: Thu, 6 Oct 2022 19:42:11 -0300 Subject: [PATCH 05/13] efi: pstore: Follow convention for the efi-pstore backend name For some reason, the efi-pstore backend name (exposed through the pstore infrastructure) is hardcoded as "efi", whereas all the other backends follow a kind of convention in using the module name. Let's do it here as well, to make user's life easier (they might use this info for unloading the module backend, for example). Cc: Ard Biesheuvel Signed-off-by: Guilherme G. Piccoli Acked-by: Ard Biesheuvel Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20221006224212.569555-8-gpiccoli@igalia.com --- drivers/firmware/efi/efi-pstore.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c index 3bddc152fcd4..97a9e84840a0 100644 --- a/drivers/firmware/efi/efi-pstore.c +++ b/drivers/firmware/efi/efi-pstore.c @@ -207,7 +207,7 @@ static int efi_pstore_erase(struct pstore_record *record) static struct pstore_info efi_pstore_info = { .owner = THIS_MODULE, - .name = "efi", + .name = KBUILD_MODNAME, .flags = PSTORE_FLAGS_DMESG, .open = efi_pstore_open, .close = efi_pstore_close, From 3219122b8cdd580f0793e803d4e80ecd7384cf17 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 11 Oct 2022 13:01:08 -0700 Subject: [PATCH 06/13] pstore/ram: Consolidate kfree() paths There's no reason to keep separate kfree() paths: either all allocations succeeded, or not. Everything is torn down in the case of failure, so adjust the callers to reflect this. Cc: Anton Vorontsov Cc: Colin Cross Cc: Tony Luck Signed-off-by: Kees Cook Reviewed-and-tested-by: Guilherme G. Piccoli Link: https://lore.kernel.org/r/20221011200112.731334-2-keescook@chromium.org --- fs/pstore/ram.c | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index fefe3d391d3a..348820a3ce88 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -451,6 +451,12 @@ static void ramoops_free_przs(struct ramoops_context *cxt) { int i; + /* Free pmsg PRZ */ + persistent_ram_free(cxt->mprz); + + /* Free console PRZ */ + persistent_ram_free(cxt->cprz); + /* Free dump PRZs */ if (cxt->dprzs) { for (i = 0; i < cxt->max_dump_cnt; i++) @@ -772,12 +778,12 @@ static int ramoops_probe(struct platform_device *pdev) dump_mem_sz, cxt->record_size, &cxt->max_dump_cnt, 0, 0); if (err) - goto fail_out; + goto fail_init; err = ramoops_init_prz("console", dev, cxt, &cxt->cprz, &paddr, cxt->console_size, 0); if (err) - goto fail_init_cprz; + goto fail_init; cxt->max_ftrace_cnt = (cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU) ? nr_cpu_ids @@ -788,12 +794,12 @@ static int ramoops_probe(struct platform_device *pdev) (cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU) ? PRZ_FLAG_NO_LOCK : 0); if (err) - goto fail_init_fprz; + goto fail_init; err = ramoops_init_prz("pmsg", dev, cxt, &cxt->mprz, &paddr, cxt->pmsg_size, 0); if (err) - goto fail_init_mprz; + goto fail_init; cxt->pstore.data = cxt; /* @@ -857,11 +863,7 @@ fail_buf: kfree(cxt->pstore.buf); fail_clear: cxt->pstore.bufsize = 0; - persistent_ram_free(cxt->mprz); -fail_init_mprz: -fail_init_fprz: - persistent_ram_free(cxt->cprz); -fail_init_cprz: +fail_init: ramoops_free_przs(cxt); fail_out: return err; @@ -876,8 +878,6 @@ static int ramoops_remove(struct platform_device *pdev) kfree(cxt->pstore.buf); cxt->pstore.bufsize = 0; - persistent_ram_free(cxt->mprz); - persistent_ram_free(cxt->cprz); ramoops_free_przs(cxt); return 0; From 6daf4e82bd54ef004ace4dd6deed60a32c282a95 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 11 Oct 2022 13:01:09 -0700 Subject: [PATCH 07/13] pstore/ram: Move pmsg init earlier Since the ftrace area can vary in size based on CPU count, move pmsg initialization earlier so it will have a stable location. Suggested-by: Paramjit Oberoi Cc: Anton Vorontsov Cc: Colin Cross Cc: Tony Luck Signed-off-by: Kees Cook Reviewed: Guilherme G. Piccoli Link: https://lore.kernel.org/r/20221011200112.731334-3-keescook@chromium.org --- fs/pstore/ram.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 348820a3ce88..2f18563c8141 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -785,6 +785,11 @@ static int ramoops_probe(struct platform_device *pdev) if (err) goto fail_init; + err = ramoops_init_prz("pmsg", dev, cxt, &cxt->mprz, &paddr, + cxt->pmsg_size, 0); + if (err) + goto fail_init; + cxt->max_ftrace_cnt = (cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU) ? nr_cpu_ids : 1; @@ -796,11 +801,6 @@ static int ramoops_probe(struct platform_device *pdev) if (err) goto fail_init; - err = ramoops_init_prz("pmsg", dev, cxt, &cxt->mprz, &paddr, - cxt->pmsg_size, 0); - if (err) - goto fail_init; - cxt->pstore.data = cxt; /* * Prepare frontend flags based on which areas are initialized. From 8bd4da0f0626ae9a82099d3d99cd6efd4355879b Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 11 Oct 2022 13:01:10 -0700 Subject: [PATCH 08/13] pstore/ram: Move internal definitions out of kernel-wide include Most of the details of the ram backend are entirely internal to the backend itself. Leave only what is needed to instantiate a ram backend in the kernel-wide header. Cc: Anton Vorontsov Cc: Colin Cross Cc: Tony Luck Signed-off-by: Kees Cook Reviewed-and-tested-by: Guilherme G. Piccoli Link: https://lore.kernel.org/r/20221011200112.731334-4-keescook@chromium.org --- fs/pstore/ram.c | 3 +- fs/pstore/ram_core.c | 3 +- fs/pstore/ram_internal.h | 98 +++++++++++++++++++++++++++++++++++++ include/linux/pstore_ram.h | 99 -------------------------------------- 4 files changed, 102 insertions(+), 101 deletions(-) create mode 100644 fs/pstore/ram_internal.h diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 2f18563c8141..f5bf360cf905 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -18,10 +18,11 @@ #include #include #include -#include #include #include + #include "internal.h" +#include "ram_internal.h" #define RAMOOPS_KERNMSG_HDR "====" #define MIN_MEM_SIZE 4096UL diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c index a89e33719fcf..9e1047f4316d 100644 --- a/fs/pstore/ram_core.c +++ b/fs/pstore/ram_core.c @@ -13,13 +13,14 @@ #include #include #include -#include #include #include #include #include #include +#include "ram_internal.h" + /** * struct persistent_ram_buffer - persistent circular RAM buffer * diff --git a/fs/pstore/ram_internal.h b/fs/pstore/ram_internal.h new file mode 100644 index 000000000000..440ee7a35e10 --- /dev/null +++ b/fs/pstore/ram_internal.h @@ -0,0 +1,98 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/* + * Copyright (C) 2010 Marco Stornelli + * Copyright (C) 2011 Kees Cook + * Copyright (C) 2011 Google, Inc. + */ + +#include + +/* + * Choose whether access to the RAM zone requires locking or not. If a zone + * can be written to from different CPUs like with ftrace for example, then + * PRZ_FLAG_NO_LOCK is used. For all other cases, locking is required. + */ +#define PRZ_FLAG_NO_LOCK BIT(0) +/* + * If a PRZ should only have a single-boot lifetime, this marks it as + * getting wiped after its contents get copied out after boot. + */ +#define PRZ_FLAG_ZAP_OLD BIT(1) + +/** + * struct persistent_ram_zone - Details of a persistent RAM zone (PRZ) + * used as a pstore backend + * + * @paddr: physical address of the mapped RAM area + * @size: size of mapping + * @label: unique name of this PRZ + * @type: frontend type for this PRZ + * @flags: holds PRZ_FLAGS_* bits + * + * @buffer_lock: + * locks access to @buffer "size" bytes and "start" offset + * @buffer: + * pointer to actual RAM area managed by this PRZ + * @buffer_size: + * bytes in @buffer->data (not including any trailing ECC bytes) + * + * @par_buffer: + * pointer into @buffer->data containing ECC bytes for @buffer->data + * @par_header: + * pointer into @buffer->data containing ECC bytes for @buffer header + * (i.e. all fields up to @data) + * @rs_decoder: + * RSLIB instance for doing ECC calculations + * @corrected_bytes: + * ECC corrected bytes accounting since boot + * @bad_blocks: + * ECC uncorrectable bytes accounting since boot + * @ecc_info: + * ECC configuration details + * + * @old_log: + * saved copy of @buffer->data prior to most recent wipe + * @old_log_size: + * bytes contained in @old_log + * + */ +struct persistent_ram_zone { + phys_addr_t paddr; + size_t size; + void *vaddr; + char *label; + enum pstore_type_id type; + u32 flags; + + raw_spinlock_t buffer_lock; + struct persistent_ram_buffer *buffer; + size_t buffer_size; + + char *par_buffer; + char *par_header; + struct rs_control *rs_decoder; + int corrected_bytes; + int bad_blocks; + struct persistent_ram_ecc_info ecc_info; + + char *old_log; + size_t old_log_size; +}; + +struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size, + u32 sig, struct persistent_ram_ecc_info *ecc_info, + unsigned int memtype, u32 flags, char *label); +void persistent_ram_free(struct persistent_ram_zone *prz); +void persistent_ram_zap(struct persistent_ram_zone *prz); + +int persistent_ram_write(struct persistent_ram_zone *prz, const void *s, + unsigned int count); +int persistent_ram_write_user(struct persistent_ram_zone *prz, + const void __user *s, unsigned int count); + +void persistent_ram_save_old(struct persistent_ram_zone *prz); +size_t persistent_ram_old_size(struct persistent_ram_zone *prz); +void *persistent_ram_old(struct persistent_ram_zone *prz); +void persistent_ram_free_old(struct persistent_ram_zone *prz); +ssize_t persistent_ram_ecc_string(struct persistent_ram_zone *prz, + char *str, size_t len); diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h index 9f16afec7290..9d65ff94e216 100644 --- a/include/linux/pstore_ram.h +++ b/include/linux/pstore_ram.h @@ -8,28 +8,7 @@ #ifndef __LINUX_PSTORE_RAM_H__ #define __LINUX_PSTORE_RAM_H__ -#include -#include -#include -#include -#include #include -#include - -/* - * Choose whether access to the RAM zone requires locking or not. If a zone - * can be written to from different CPUs like with ftrace for example, then - * PRZ_FLAG_NO_LOCK is used. For all other cases, locking is required. - */ -#define PRZ_FLAG_NO_LOCK BIT(0) -/* - * If a PRZ should only have a single-boot lifetime, this marks it as - * getting wiped after its contents get copied out after boot. - */ -#define PRZ_FLAG_ZAP_OLD BIT(1) - -struct persistent_ram_buffer; -struct rs_control; struct persistent_ram_ecc_info { int block_size; @@ -39,84 +18,6 @@ struct persistent_ram_ecc_info { uint16_t *par; }; -/** - * struct persistent_ram_zone - Details of a persistent RAM zone (PRZ) - * used as a pstore backend - * - * @paddr: physical address of the mapped RAM area - * @size: size of mapping - * @label: unique name of this PRZ - * @type: frontend type for this PRZ - * @flags: holds PRZ_FLAGS_* bits - * - * @buffer_lock: - * locks access to @buffer "size" bytes and "start" offset - * @buffer: - * pointer to actual RAM area managed by this PRZ - * @buffer_size: - * bytes in @buffer->data (not including any trailing ECC bytes) - * - * @par_buffer: - * pointer into @buffer->data containing ECC bytes for @buffer->data - * @par_header: - * pointer into @buffer->data containing ECC bytes for @buffer header - * (i.e. all fields up to @data) - * @rs_decoder: - * RSLIB instance for doing ECC calculations - * @corrected_bytes: - * ECC corrected bytes accounting since boot - * @bad_blocks: - * ECC uncorrectable bytes accounting since boot - * @ecc_info: - * ECC configuration details - * - * @old_log: - * saved copy of @buffer->data prior to most recent wipe - * @old_log_size: - * bytes contained in @old_log - * - */ -struct persistent_ram_zone { - phys_addr_t paddr; - size_t size; - void *vaddr; - char *label; - enum pstore_type_id type; - u32 flags; - - raw_spinlock_t buffer_lock; - struct persistent_ram_buffer *buffer; - size_t buffer_size; - - char *par_buffer; - char *par_header; - struct rs_control *rs_decoder; - int corrected_bytes; - int bad_blocks; - struct persistent_ram_ecc_info ecc_info; - - char *old_log; - size_t old_log_size; -}; - -struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size, - u32 sig, struct persistent_ram_ecc_info *ecc_info, - unsigned int memtype, u32 flags, char *label); -void persistent_ram_free(struct persistent_ram_zone *prz); -void persistent_ram_zap(struct persistent_ram_zone *prz); - -int persistent_ram_write(struct persistent_ram_zone *prz, const void *s, - unsigned int count); -int persistent_ram_write_user(struct persistent_ram_zone *prz, - const void __user *s, unsigned int count); - -void persistent_ram_save_old(struct persistent_ram_zone *prz); -size_t persistent_ram_old_size(struct persistent_ram_zone *prz); -void *persistent_ram_old(struct persistent_ram_zone *prz); -void persistent_ram_free_old(struct persistent_ram_zone *prz); -ssize_t persistent_ram_ecc_string(struct persistent_ram_zone *prz, - char *str, size_t len); - /* * Ramoops platform data * @mem_size memory size for ramoops From 06b4e09aab6c1ae6b3991a729d2fcb31e358e7f3 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 11 Oct 2022 13:01:11 -0700 Subject: [PATCH 09/13] pstore/ram: Set freed addresses to NULL For good measure, set all the freed addresses to NULL when managing przs. Cc: Anton Vorontsov Cc: Colin Cross Cc: Tony Luck Signed-off-by: Kees Cook Reviewed-and-tested-by: Guilherme G. Piccoli Link: https://lore.kernel.org/r/20221011200112.731334-5-keescook@chromium.org --- fs/pstore/ram.c | 13 ++++++++----- fs/pstore/ram_core.c | 11 +++++++++-- fs/pstore/ram_internal.h | 2 +- 3 files changed, 18 insertions(+), 8 deletions(-) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index f5bf360cf905..5a39fa8522b0 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -453,25 +453,27 @@ static void ramoops_free_przs(struct ramoops_context *cxt) int i; /* Free pmsg PRZ */ - persistent_ram_free(cxt->mprz); + persistent_ram_free(&cxt->mprz); /* Free console PRZ */ - persistent_ram_free(cxt->cprz); + persistent_ram_free(&cxt->cprz); /* Free dump PRZs */ if (cxt->dprzs) { for (i = 0; i < cxt->max_dump_cnt; i++) - persistent_ram_free(cxt->dprzs[i]); + persistent_ram_free(&cxt->dprzs[i]); kfree(cxt->dprzs); + cxt->dprzs = NULL; cxt->max_dump_cnt = 0; } /* Free ftrace PRZs */ if (cxt->fprzs) { for (i = 0; i < cxt->max_ftrace_cnt; i++) - persistent_ram_free(cxt->fprzs[i]); + persistent_ram_free(&cxt->fprzs[i]); kfree(cxt->fprzs); + cxt->fprzs = NULL; cxt->max_ftrace_cnt = 0; } } @@ -555,9 +557,10 @@ static int ramoops_init_przs(const char *name, while (i > 0) { i--; - persistent_ram_free(prz_ar[i]); + persistent_ram_free(&prz_ar[i]); } kfree(prz_ar); + prz_ar = NULL; goto fail; } *paddr += zone_sz; diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c index 9e1047f4316d..97dde525150a 100644 --- a/fs/pstore/ram_core.c +++ b/fs/pstore/ram_core.c @@ -544,8 +544,14 @@ static int persistent_ram_post_init(struct persistent_ram_zone *prz, u32 sig, return 0; } -void persistent_ram_free(struct persistent_ram_zone *prz) +void persistent_ram_free(struct persistent_ram_zone **_prz) { + struct persistent_ram_zone *prz; + + if (!_prz) + return; + + prz = *_prz; if (!prz) return; @@ -569,6 +575,7 @@ void persistent_ram_free(struct persistent_ram_zone *prz) persistent_ram_free_old(prz); kfree(prz->label); kfree(prz); + *_prz = NULL; } struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size, @@ -605,6 +612,6 @@ struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size, return prz; err: - persistent_ram_free(prz); + persistent_ram_free(&prz); return ERR_PTR(ret); } diff --git a/fs/pstore/ram_internal.h b/fs/pstore/ram_internal.h index 440ee7a35e10..5f694698351f 100644 --- a/fs/pstore/ram_internal.h +++ b/fs/pstore/ram_internal.h @@ -82,7 +82,7 @@ struct persistent_ram_zone { struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size, u32 sig, struct persistent_ram_ecc_info *ecc_info, unsigned int memtype, u32 flags, char *label); -void persistent_ram_free(struct persistent_ram_zone *prz); +void persistent_ram_free(struct persistent_ram_zone **_prz); void persistent_ram_zap(struct persistent_ram_zone *prz); int persistent_ram_write(struct persistent_ram_zone *prz, const void *s, From 7d7eca411fa0c662693613805107494a9dc7477f Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 11 Oct 2022 13:01:12 -0700 Subject: [PATCH 10/13] MAINTAINERS: Update pstore maintainers Update pstore to better reflect reality of active contributors: - Remove Anton and Colin (thank you for your help through the years!) - Move Tony to Reviewer - Add Guilherme as Reviewer - Add mailing list - Upgrade to Supported Signed-off-by: Kees Cook Acked-by: Guilherme G. Piccoli Acked-by: Colin Cross Link: https://lore.kernel.org/r/20221011200112.731334-6-keescook@chromium.org --- MAINTAINERS | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/MAINTAINERS b/MAINTAINERS index cf0f18502372..41ae6877dcdf 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -16641,10 +16641,10 @@ F: net/psample PSTORE FILESYSTEM M: Kees Cook -M: Anton Vorontsov -M: Colin Cross -M: Tony Luck -S: Maintained +R: Tony Luck +R: Guilherme G. Piccoli +L: linux-hardening@vger.kernel.org +S: Supported T: git git://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/pstore F: Documentation/admin-guide/ramoops.rst F: Documentation/admin-guide/pstore-blk.rst From 38b91847c314f49c80e30062549d4709a3754ea6 Mon Sep 17 00:00:00 2001 From: "Guilherme G. Piccoli" Date: Thu, 13 Oct 2022 18:06:46 -0300 Subject: [PATCH 11/13] pstore: Alert on backend write error The pstore dump function doesn't alert at all on errors - despite pstore is usually a last resource and if it fails users won't be able to read the kernel log, this is not the case for server users with serial access, for example. So, let's at least attempt to inform such advanced users on the first backend writing error detected during the kmsg dump - this is also very useful for pstore debugging purposes. Signed-off-by: Guilherme G. Piccoli Acked-by: Ard Biesheuvel Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20221013210648.137452-2-gpiccoli@igalia.com --- fs/pstore/platform.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 06c2c66af332..cbc0b468c1ab 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -393,6 +393,7 @@ static void pstore_dump(struct kmsg_dumper *dumper, const char *why; unsigned int part = 1; unsigned long flags = 0; + int saved_ret = 0; int ret; why = kmsg_dump_reason_str(reason); @@ -463,12 +464,21 @@ static void pstore_dump(struct kmsg_dumper *dumper, if (ret == 0 && reason == KMSG_DUMP_OOPS) { pstore_new_entry = 1; pstore_timer_kick(); + } else { + /* Preserve only the first non-zero returned value. */ + if (!saved_ret) + saved_ret = ret; } total += record.size; part++; } spin_unlock_irqrestore(&psinfo->buf_lock, flags); + + if (saved_ret) { + pr_err_once("backend (%s) writing error (%d)\n", psinfo->name, + saved_ret); + } } static struct kmsg_dumper pstore_dumper = { From e1fce564900f8734edf15b87f028c57e14f6e28d Mon Sep 17 00:00:00 2001 From: Wang Yufen Date: Fri, 2 Dec 2022 16:22:54 +0800 Subject: [PATCH 12/13] pstore/ram: Fix error return code in ramoops_probe() In the if (dev_of_node(dev) && !pdata) path, the "err" may be assigned a value of 0, so the error return code -EINVAL may be incorrectly set to 0. To fix set valid return code before calling to goto. Fixes: 35da60941e44 ("pstore/ram: add Device Tree bindings") Signed-off-by: Wang Yufen Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/1669969374-46582-1-git-send-email-wangyufen@huawei.com --- fs/pstore/ram.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 5a39fa8522b0..9a5052431fd3 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -745,6 +745,7 @@ static int ramoops_probe(struct platform_device *pdev) /* Make sure we didn't get bogus platform data pointer. */ if (!pdata) { pr_err("NULL platform data\n"); + err = -EINVAL; goto fail_out; } @@ -752,6 +753,7 @@ static int ramoops_probe(struct platform_device *pdev) !pdata->ftrace_size && !pdata->pmsg_size)) { pr_err("The memory size and the record/console size must be " "non-zero\n"); + err = -EINVAL; goto fail_out; } From e6b842741b4f39007215fd7e545cb55aa3d358a2 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Mon, 5 Dec 2022 15:31:36 -0800 Subject: [PATCH 13/13] pstore: Avoid kcore oops by vmap()ing with VM_IOREMAP An oops can be induced by running 'cat /proc/kcore > /dev/null' on devices using pstore with the ram backend because kmap_atomic() assumes lowmem pages are accessible with __va(). Unable to handle kernel paging request at virtual address ffffff807ff2b000 Mem abort info: ESR = 0x96000006 EC = 0x25: DABT (current EL), IL = 32 bits SET = 0, FnV = 0 EA = 0, S1PTW = 0 FSC = 0x06: level 2 translation fault Data abort info: ISV = 0, ISS = 0x00000006 CM = 0, WnR = 0 swapper pgtable: 4k pages, 39-bit VAs, pgdp=0000000081d87000 [ffffff807ff2b000] pgd=180000017fe18003, p4d=180000017fe18003, pud=180000017fe18003, pmd=0000000000000000 Internal error: Oops: 96000006 [#1] PREEMPT SMP Modules linked in: dm_integrity CPU: 7 PID: 21179 Comm: perf Not tainted 5.15.67-10882-ge4eb2eb988cd #1 baa443fb8e8477896a370b31a821eb2009f9bfba Hardware name: Google Lazor (rev3 - 8) (DT) pstate: a0400009 (NzCv daif +PAN -UAO -TCO -DIT -SSBS BTYPE=--) pc : __memcpy+0x110/0x260 lr : vread+0x194/0x294 sp : ffffffc013ee39d0 x29: ffffffc013ee39f0 x28: 0000000000001000 x27: ffffff807ff2b000 x26: 0000000000001000 x25: ffffffc0085a2000 x24: ffffff802d4b3000 x23: ffffff80f8a60000 x22: ffffff802d4b3000 x21: ffffffc0085a2000 x20: ffffff8080b7bc68 x19: 0000000000001000 x18: 0000000000000000 x17: 0000000000000000 x16: 0000000000000000 x15: ffffffd3073f2e60 x14: ffffffffad588000 x13: 0000000000000000 x12: 0000000000000001 x11: 00000000000001a2 x10: 00680000fff2bf0b x9 : 03fffffff807ff2b x8 : 0000000000000001 x7 : 0000000000000000 x6 : 0000000000000000 x5 : ffffff802d4b4000 x4 : ffffff807ff2c000 x3 : ffffffc013ee3a78 x2 : 0000000000001000 x1 : ffffff807ff2b000 x0 : ffffff802d4b3000 Call trace: __memcpy+0x110/0x260 read_kcore+0x584/0x778 proc_reg_read+0xb4/0xe4 During early boot, memblock reserves the pages for the ramoops reserved memory node in DT that would otherwise be part of the direct lowmem mapping. Pstore's ram backend reuses those reserved pages to change the memory type (writeback or non-cached) by passing the pages to vmap() (see pfn_to_page() usage in persistent_ram_vmap() for more details) with specific flags. When read_kcore() starts iterating over the vmalloc region, it runs over the virtual address that vmap() returned for ramoops. In aligned_vread() the virtual address is passed to vmalloc_to_page() which returns the page struct for the reserved lowmem area. That lowmem page is passed to kmap_atomic(), which effectively calls page_to_virt() that assumes a lowmem page struct must be directly accessible with __va() and friends. These pages are mapped via vmap() though, and the lowmem mapping was never made, so accessing them via the lowmem virtual address oopses like above. Let's side-step this problem by passing VM_IOREMAP to vmap(). This will tell vread() to not include the ramoops region in the kcore. Instead the area will look like a bunch of zeros. The alternative is to teach kmap() about vmalloc areas that intersect with lowmem. Presumably such a change isn't a one-liner, and there isn't much interest in inspecting the ramoops region in kcore files anyway, so the most expedient route is taken for now. Cc: Brian Geffon Cc: Mike Rapoport Cc: Andrew Morton Fixes: 404a6043385d ("staging: android: persistent_ram: handle reserving and mapping memory") Signed-off-by: Stephen Boyd Signed-off-by: Kees Cook Link: https://lore.kernel.org/r/20221205233136.3420802-1-swboyd@chromium.org --- fs/pstore/ram_core.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c index 97dde525150a..966191d3a5ba 100644 --- a/fs/pstore/ram_core.c +++ b/fs/pstore/ram_core.c @@ -440,7 +440,11 @@ static void *persistent_ram_vmap(phys_addr_t start, size_t size, phys_addr_t addr = page_start + i * PAGE_SIZE; pages[i] = pfn_to_page(addr >> PAGE_SHIFT); } - vaddr = vmap(pages, page_count, VM_MAP, prot); + /* + * VM_IOREMAP used here to bypass this region during vread() + * and kmap_atomic() (i.e. kcore) to avoid __va() failures. + */ + vaddr = vmap(pages, page_count, VM_MAP | VM_IOREMAP, prot); kfree(pages); /*