From 7684bd334d9d4ca4f09873e88d9c0131a2cf6c3b Mon Sep 17 00:00:00 2001 From: Peng Wang Date: Tue, 30 Oct 2018 15:52:34 +0800 Subject: pstore: Avoid duplicate call of persistent_ram_zap() When initialing a prz, if invalid data is found (no PERSISTENT_RAM_SIG), the function call path looks like this: ramoops_init_prz -> persistent_ram_new -> persistent_ram_post_init -> persistent_ram_zap persistent_ram_zap As we can see, persistent_ram_zap() is called twice. We can avoid this by adding an option to persistent_ram_new(), and only call persistent_ram_zap() when it is needed. Signed-off-by: Peng Wang [kees: minor tweak to exit path and commit log] Signed-off-by: Kees Cook --- include/linux/pstore_ram.h | 1 + 1 file changed, 1 insertion(+) (limited to 'include') diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h index 602d64725222..6e94980357d2 100644 --- a/include/linux/pstore_ram.h +++ b/include/linux/pstore_ram.h @@ -30,6 +30,7 @@ * PRZ_FLAG_NO_LOCK is used. For all other cases, locking is required. */ #define PRZ_FLAG_NO_LOCK BIT(0) +#define PRZ_FLAG_ZAP_OLD BIT(1) struct persistent_ram_buffer; struct rs_control; -- cgit v1.2.3 From c208f7d4b037e1c71e5c839bb5dfcc3e0df19890 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 1 Nov 2018 15:11:47 -0700 Subject: pstore/ram: Add kern-doc for struct persistent_ram_zone The struct persistent_ram_zone wasn't well documented. This adds kern-doc for it. Signed-off-by: Kees Cook --- include/linux/pstore_ram.h | 46 +++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 43 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h index 6e94980357d2..5d10ad51c1c4 100644 --- a/include/linux/pstore_ram.h +++ b/include/linux/pstore_ram.h @@ -30,6 +30,10 @@ * 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; @@ -43,17 +47,53 @@ 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 + * @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; - struct persistent_ram_buffer *buffer; - size_t buffer_size; u32 flags; + raw_spinlock_t buffer_lock; + struct persistent_ram_buffer *buffer; + size_t buffer_size; - /* ECC correction */ char *par_buffer; char *par_header; struct rs_control *rs_decoder; -- cgit v1.2.3 From 0eed84ffb094bbddfb4b9378ef0a2eccf4dda99c Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 1 Nov 2018 14:03:07 -0700 Subject: pstore: Improve and update some comments and status output This improves and updates some comments: - dump handler comment out of sync from calling convention - fix kern-doc typo and improves status output: - reminder that only kernel crash dumps are compressed - do not be silent about ECC infrastructure failures Signed-off-by: Kees Cook --- include/linux/pstore.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'include') diff --git a/include/linux/pstore.h b/include/linux/pstore.h index 30fcec375a3a..81669aa80027 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -85,7 +85,7 @@ struct pstore_record { /** * struct pstore_info - backend pstore driver structure * - * @owner: module which is repsonsible for this backend driver + * @owner: module which is responsible for this backend driver * @name: name of the backend driver * * @buf_lock: spinlock to serialize access to @buf -- cgit v1.2.3 From 4af62a6423d0ad98e3eee2bec4305dde8deefefe Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 1 Nov 2018 15:30:05 -0700 Subject: pstore: Replace open-coded << with BIT() Minor clean-up to use BIT() (as already done in pstore_ram.h). Signed-off-by: Kees Cook --- include/linux/pstore.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/include/linux/pstore.h b/include/linux/pstore.h index 81669aa80027..f46e5df76b58 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -192,10 +192,10 @@ struct pstore_info { }; /* Supported frontends */ -#define PSTORE_FLAGS_DMESG (1 << 0) -#define PSTORE_FLAGS_CONSOLE (1 << 1) -#define PSTORE_FLAGS_FTRACE (1 << 2) -#define PSTORE_FLAGS_PMSG (1 << 3) +#define PSTORE_FLAGS_DMESG BIT(0) +#define PSTORE_FLAGS_CONSOLE BIT(1) +#define PSTORE_FLAGS_FTRACE BIT(2) +#define PSTORE_FLAGS_PMSG BIT(3) extern int pstore_register(struct pstore_info *); extern void pstore_unregister(struct pstore_info *); -- cgit v1.2.3 From f0f23e5469dc80b482d985898a930be0e249a162 Mon Sep 17 00:00:00 2001 From: "Joel Fernandes (Google)" Date: Sat, 3 Nov 2018 16:38:16 -0700 Subject: pstore: Map PSTORE_TYPE_* to strings In later patches we will need to map types to names, so create a constant table for that which can also be used in different parts of old and new code. This saves the type in the PRZ which will be useful in later patches. Instead of having an explicit PSTORE_TYPE_UNKNOWN, just use ..._MAX. This includes removing the now redundant filename templates which can use a single format string. Also, there's no reason to limit the "is it still compressed?" test to only PSTORE_TYPE_DMESG when building the pstorefs filename. Records are zero-initialized, so a backend would need to have explicitly set compressed=1. Signed-off-by: Joel Fernandes (Google) Co-developed-by: Kees Cook Signed-off-by: Kees Cook --- include/linux/pstore.h | 17 ++++++++++++++--- include/linux/pstore_ram.h | 3 +++ 2 files changed, 17 insertions(+), 3 deletions(-) (limited to 'include') diff --git a/include/linux/pstore.h b/include/linux/pstore.h index f46e5df76b58..a9ec285d85d1 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -32,21 +32,32 @@ struct module; -/* pstore record types (see fs/pstore/inode.c for filename templates) */ +/* + * pstore record types (see fs/pstore/platform.c for pstore_type_names[]) + * These values may be written to storage (see EFI vars backend), so + * they are kind of an ABI. Be careful changing the mappings. + */ enum pstore_type_id { + /* Frontend storage types */ PSTORE_TYPE_DMESG = 0, PSTORE_TYPE_MCE = 1, PSTORE_TYPE_CONSOLE = 2, PSTORE_TYPE_FTRACE = 3, - /* PPC64 partition types */ + + /* PPC64-specific partition types */ PSTORE_TYPE_PPC_RTAS = 4, PSTORE_TYPE_PPC_OF = 5, PSTORE_TYPE_PPC_COMMON = 6, PSTORE_TYPE_PMSG = 7, PSTORE_TYPE_PPC_OPAL = 8, - PSTORE_TYPE_UNKNOWN = 255 + + /* End of the list */ + PSTORE_TYPE_MAX }; +const char *pstore_type_to_name(enum pstore_type_id type); +enum pstore_type_id pstore_name_to_type(const char *name); + struct pstore_info; /** * struct pstore_record - details of a pstore record entry diff --git a/include/linux/pstore_ram.h b/include/linux/pstore_ram.h index 5d10ad51c1c4..337971c41980 100644 --- a/include/linux/pstore_ram.h +++ b/include/linux/pstore_ram.h @@ -22,6 +22,7 @@ #include #include #include +#include #include /* @@ -54,6 +55,7 @@ struct persistent_ram_ecc_info { * @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: @@ -88,6 +90,7 @@ struct persistent_ram_zone { size_t size; void *vaddr; char *label; + enum pstore_type_id type; u32 flags; raw_spinlock_t buffer_lock; -- cgit v1.2.3 From ea84b580b95521644429cc6748b6c2bf27c8b0f3 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 30 Nov 2018 14:36:58 -0800 Subject: pstore: Convert buf_lock to semaphore Instead of running with interrupts disabled, use a semaphore. This should make it easier for backends that may need to sleep (e.g. EFI) when performing a write: |BUG: sleeping function called from invalid context at kernel/sched/completion.c:99 |in_atomic(): 1, irqs_disabled(): 1, pid: 2236, name: sig-xstate-bum |Preemption disabled at: |[] pstore_dump+0x72/0x330 |CPU: 26 PID: 2236 Comm: sig-xstate-bum Tainted: G D 4.20.0-rc3 #45 |Call Trace: | dump_stack+0x4f/0x6a | ___might_sleep.cold.91+0xd3/0xe4 | __might_sleep+0x50/0x90 | wait_for_completion+0x32/0x130 | virt_efi_query_variable_info+0x14e/0x160 | efi_query_variable_store+0x51/0x1a0 | efivar_entry_set_safe+0xa3/0x1b0 | efi_pstore_write+0x109/0x140 | pstore_dump+0x11c/0x330 | kmsg_dump+0xa4/0xd0 | oops_exit+0x22/0x30 ... Reported-by: Sebastian Andrzej Siewior Fixes: 21b3ddd39fee ("efi: Don't use spinlocks for efi vars") Signed-off-by: Kees Cook --- include/linux/pstore.h | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) (limited to 'include') diff --git a/include/linux/pstore.h b/include/linux/pstore.h index a9ec285d85d1..b146181e8709 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -26,7 +26,7 @@ #include #include #include -#include +#include #include #include @@ -99,7 +99,7 @@ struct pstore_record { * @owner: module which is responsible for this backend driver * @name: name of the backend driver * - * @buf_lock: spinlock to serialize access to @buf + * @buf_lock: semaphore to serialize access to @buf * @buf: preallocated crash dump buffer * @bufsize: size of @buf available for crash dump bytes (must match * smallest number of bytes available for writing to a @@ -184,7 +184,7 @@ struct pstore_info { struct module *owner; char *name; - spinlock_t buf_lock; + struct semaphore buf_lock; char *buf; size_t bufsize; @@ -210,7 +210,6 @@ struct pstore_info { extern int pstore_register(struct pstore_info *); extern void pstore_unregister(struct pstore_info *); -extern bool pstore_cannot_block_path(enum kmsg_dump_reason reason); struct pstore_ftrace_record { unsigned long ip; -- cgit v1.2.3