From 3faf93543ccb3410bfaf992a0de4a925631a3526 Mon Sep 17 00:00:00 2001 From: Bhumika Goyal Date: Sun, 19 Feb 2017 15:07:53 +0530 Subject: [PATCH 01/23] pstore: constify pstore_zbackend structures The references of pstore_zbackend structures are stored into the pointer zbackend of type struct pstore_zbackend. The pointer zbackend can be made const as it is only dereferenced. After making this change the pstore_zbackend structures whose references are stored into the pointer zbackend can be made const too. File size before: text data bss dec hex filename 4817 541 172 5530 159a fs/pstore/platform.o File size after: text data bss dec hex filename 4865 477 172 5514 158a fs/pstore/platform.o Signed-off-by: Bhumika Goyal Signed-off-by: Kees Cook --- fs/pstore/platform.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index efab7b64925ba7..cfc1abd264d90e 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -267,7 +267,7 @@ static void free_zlib(void) big_oops_buf_sz = 0; } -static struct pstore_zbackend backend_zlib = { +static const struct pstore_zbackend backend_zlib = { .compress = compress_zlib, .decompress = decompress_zlib, .allocate = allocate_zlib, @@ -328,7 +328,7 @@ static void free_lzo(void) big_oops_buf_sz = 0; } -static struct pstore_zbackend backend_lzo = { +static const struct pstore_zbackend backend_lzo = { .compress = compress_lzo, .decompress = decompress_lzo, .allocate = allocate_lzo, @@ -393,7 +393,7 @@ static void free_lz4(void) big_oops_buf_sz = 0; } -static struct pstore_zbackend backend_lz4 = { +static const struct pstore_zbackend backend_lz4 = { .compress = compress_lz4, .decompress = decompress_lz4, .allocate = allocate_lz4, @@ -402,7 +402,7 @@ static struct pstore_zbackend backend_lz4 = { }; #endif -static struct pstore_zbackend *zbackend = +static const struct pstore_zbackend *zbackend = #if defined(CONFIG_PSTORE_ZLIB_COMPRESS) &backend_zlib; #elif defined(CONFIG_PSTORE_LZO_COMPRESS) From e9a330c4289f2ba1ca4bf98c2b430ab165a8931b Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sun, 5 Mar 2017 22:08:58 -0800 Subject: [PATCH 02/23] pstore: Use dynamic spinlock initializer The per-prz spinlock should be using the dynamic initializer so that lockdep can correctly track it. Without this, under lockdep, we get a warning at boot that the lock is in non-static memory. Fixes: 109704492ef6 ("pstore: Make spinlock per zone instead of global") Fixes: 76d5692a5803 ("pstore: Correctly initialize spinlock and flags") Signed-off-by: Kees Cook Cc: stable@vger.kernel.org --- fs/pstore/ram_core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c index bc927e30bdccd4..e11672aa457514 100644 --- a/fs/pstore/ram_core.c +++ b/fs/pstore/ram_core.c @@ -532,7 +532,7 @@ struct persistent_ram_zone *persistent_ram_new(phys_addr_t start, size_t size, } /* Initialize general buffer state. */ - prz->buffer_lock = __RAW_SPIN_LOCK_UNLOCKED(buffer_lock); + raw_spin_lock_init(&prz->buffer_lock); prz->flags = flags; ret = persistent_ram_buffer_map(start, size, prz, memtype); From 6330d5534786d5315d56d558aa6d20740f97d80a Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Mon, 6 Mar 2017 12:42:12 -0800 Subject: [PATCH 03/23] pstore: Shut down worker when unregistering When built as a module and running with update_ms >= 0, pstore will Oops during module unload since the work timer is still running. This makes sure the worker is stopped before unloading. Signed-off-by: Kees Cook Cc: stable@vger.kernel.org --- fs/pstore/platform.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index cfc1abd264d90e..074fe85a207806 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -709,6 +709,7 @@ int pstore_register(struct pstore_info *psi) if (psi->flags & PSTORE_FLAGS_PMSG) pstore_register_pmsg(); + /* Start watching for new records, if desired. */ if (pstore_update_ms >= 0) { pstore_timer.expires = jiffies + msecs_to_jiffies(pstore_update_ms); @@ -731,6 +732,11 @@ EXPORT_SYMBOL_GPL(pstore_register); void pstore_unregister(struct pstore_info *psi) { + /* Stop timer and make sure all work has finished. */ + pstore_update_ms = -1; + del_timer_sync(&pstore_timer); + flush_work(&pstore_work); + if (psi->flags & PSTORE_FLAGS_PMSG) pstore_unregister_pmsg(); if (psi->flags & PSTORE_FLAGS_FTRACE) @@ -830,7 +836,9 @@ static void pstore_timefunc(unsigned long dummy) schedule_work(&pstore_work); } - mod_timer(&pstore_timer, jiffies + msecs_to_jiffies(pstore_update_ms)); + if (pstore_update_ms >= 0) + mod_timer(&pstore_timer, + jiffies + msecs_to_jiffies(pstore_update_ms)); } module_param(backend, charp, 0444); From 1344dd86f35c7669c94aceb2273676e356cff848 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 3 Mar 2017 17:45:38 -0800 Subject: [PATCH 04/23] pstore: Avoid race in module unloading Technically, it might be possible for struct pstore_info to go out of scope after the module_put(), so report the backend name first. Signed-off-by: Kees Cook --- fs/pstore/platform.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 074fe85a207806..d69ef8a840b961 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -722,10 +722,10 @@ int pstore_register(struct pstore_info *psi) */ backend = psi->name; - module_put(owner); - pr_info("Registered %s as persistent store backend\n", psi->name); + module_put(owner); + return 0; } EXPORT_SYMBOL_GPL(pstore_register); From 0d7cd09a3dbbdb3d4932cddbef613b2a6de28b75 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 3 Mar 2017 12:11:40 -0800 Subject: [PATCH 05/23] pstore: Improve register_pstore() error reporting Uncommon errors are better to get reported to dmesg so developers can more easily figure out why pstore is unhappy with a backend attempting to register. Signed-off-by: Kees Cook --- fs/pstore/platform.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index d69ef8a840b961..320a673ecb5be4 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -673,11 +673,15 @@ int pstore_register(struct pstore_info *psi) { struct module *owner = psi->owner; - if (backend && strcmp(backend, psi->name)) + if (backend && strcmp(backend, psi->name)) { + pr_warn("ignoring unexpected backend '%s'\n", psi->name); return -EPERM; + } spin_lock(&pstore_lock); if (psinfo) { + pr_warn("backend '%s' already loaded: ignoring '%s'\n", + psinfo->name, psi->name); spin_unlock(&pstore_lock); return -EBUSY; } From 0edae0b3ffa6fc968d63932347a4d74b0ad0340b Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 3 Mar 2017 12:16:16 -0800 Subject: [PATCH 06/23] pstore: Add kernel-doc for struct pstore_info This adds documentation for struct pstore_info, which also includes the basic API the backends need to implement. Signed-off-by: Kees Cook --- include/linux/pstore.h | 133 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 128 insertions(+), 5 deletions(-) diff --git a/include/linux/pstore.h b/include/linux/pstore.h index 0da29cae009b18..56477ce6806a60 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -30,7 +30,7 @@ #include #include -/* types */ +/* pstore record types (see fs/pstore/inode.c for filename templates) */ enum pstore_type_id { PSTORE_TYPE_DMESG = 0, PSTORE_TYPE_MCE = 1, @@ -47,14 +47,138 @@ enum pstore_type_id { struct module; +/** + * struct pstore_info - backend pstore driver structure + * + * @owner: module which is repsonsible for this backend driver + * @name: name of the backend driver + * + * @buf_lock: spinlock to serialize access to @buf + * @buf: preallocated crash dump buffer + * @bufsize: size of @buf available for crash dump writes + * + * @read_mutex: serializes @open, @read, @close, and @erase callbacks + * @flags: bitfield of frontends the backend can accept writes for + * @data: backend-private pointer passed back during callbacks + * + * Callbacks: + * + * @open: + * Notify backend that pstore is starting a full read of backend + * records. Followed by one or more @read calls, and a final @close. + * + * @psi: in: pointer to the struct pstore_info for the backend + * + * Returns 0 on success, and non-zero on error. + * + * @close: + * Notify backend that pstore has finished a full read of backend + * records. Always preceded by an @open call and one or more @read + * calls. + * + * @psi: in: pointer to the struct pstore_info for the backend + * + * Returns 0 on success, and non-zero on error. (Though pstore will + * ignore the error.) + * + * @read: + * Read next available backend record. Called after a successful + * @open. + * + * @id: out: unique identifier for the record + * @type: out: pstore record type + * @count: out: for PSTORE_TYPE_DMESG, the Oops count. + * @time: out: timestamp for the record + * @buf: out: kmalloc copy of record contents, to be freed by pstore + * @compressed: + * out: if the record contents are compressed + * @ecc_notice_size: + * out: ECC information + * @psi: in: pointer to the struct pstore_info for the backend + * + * Returns record size on success, zero when no more records are + * available, or negative on error. + * + * @write: + * Perform a frontend notification of a write to a backend record. The + * data to be stored has already been written to the registered @buf + * of the @psi structure. + * + * @type: in: pstore record type to write + * @reason: + * in: pstore write reason + * @id: out: unique identifier for the record + * @part: in: position in a multipart write + * @count: in: increasing from 0 since boot, the number of this Oops + * @compressed: + * in: if the record is compressed + * @size: in: size of the write + * @psi: in: pointer to the struct pstore_info for the backend + * + * Returns 0 on success, and non-zero on error. + * + * @write_buf: + * Perform a frontend write to a backend record, using a specified + * buffer. Unlike @write, this does not use the @psi @buf. + * + * @type: in: pstore record type to write + * @reason: + * in: pstore write reason + * @id: out: unique identifier for the record + * @part: in: position in a multipart write + * @buf: in: pointer to contents to write to backend record + * @compressed: + * in: if the record is compressed + * @size: in: size of the write + * @psi: in: pointer to the struct pstore_info for the backend + * + * Returns 0 on success, and non-zero on error. + * + * @write_buf_user: + * Perform a frontend write to a backend record, using a specified + * buffer that is coming directly from userspace. + * + * @type: in: pstore record type to write + * @reason: + * in: pstore write reason + * @id: out: unique identifier for the record + * @part: in: position in a multipart write + * @buf: in: pointer to userspace contents to write to backend record + * @compressed: + * in: if the record is compressed + * @size: in: size of the write + * @psi: in: pointer to the struct pstore_info for the backend + * + * Returns 0 on success, and non-zero on error. + * + * @erase: + * Delete a record from backend storage. Different backends + * identify records differently, so all possible methods of + * identification are included to help the backend locate the + * record to remove. + * + * @type: in: pstore record type to write + * @id: in: per-type unique identifier for the record + * @count: in: Oops count + * @time: in: timestamp for the record + * @psi: in: pointer to the struct pstore_info for the backend + * + * Returns 0 on success, and non-zero on error. + * + */ struct pstore_info { struct module *owner; char *name; - spinlock_t buf_lock; /* serialize access to 'buf' */ + + spinlock_t buf_lock; char *buf; size_t bufsize; - struct mutex read_mutex; /* serialize open/read/close */ + + struct mutex read_mutex; + int flags; + void *data; + int (*open)(struct pstore_info *psi); int (*close)(struct pstore_info *psi); ssize_t (*read)(u64 *id, enum pstore_type_id *type, @@ -76,11 +200,10 @@ struct pstore_info { int (*erase)(enum pstore_type_id type, u64 id, int count, struct timespec time, struct pstore_info *psi); - void *data; }; +/* Supported frontends */ #define PSTORE_FLAGS_DMESG (1 << 0) -#define PSTORE_FLAGS_FRAGILE PSTORE_FLAGS_DMESG #define PSTORE_FLAGS_CONSOLE (1 << 1) #define PSTORE_FLAGS_FTRACE (1 << 2) #define PSTORE_FLAGS_PMSG (1 << 3) From 9abdcccc3d5f3c72f25cd48160f60d911353bee9 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 3 Mar 2017 16:59:29 -0800 Subject: [PATCH 07/23] pstore: Extract common arguments into structure The read/mkfile pair pass the same arguments and should be cleared between calls. Move to a structure and wipe it after every loop. Signed-off-by: Kees Cook --- fs/pstore/platform.c | 55 +++++++++++++++++++++++------------------- include/linux/pstore.h | 28 ++++++++++++++++++++- 2 files changed, 57 insertions(+), 26 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 320a673ecb5be4..f45228eac3e684 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -766,16 +766,9 @@ EXPORT_SYMBOL_GPL(pstore_unregister); void pstore_get_records(int quiet) { struct pstore_info *psi = psinfo; - char *buf = NULL; - ssize_t size; - u64 id; - int count; - enum pstore_type_id type; - struct timespec time; + struct pstore_record record = { .psi = psi, }; int failed = 0, rc; - bool compressed; int unzipped_len = -1; - ssize_t ecc_notice_size = 0; if (!psi) return; @@ -784,39 +777,51 @@ void pstore_get_records(int quiet) if (psi->open && psi->open(psi)) goto out; - while ((size = psi->read(&id, &type, &count, &time, &buf, &compressed, - &ecc_notice_size, psi)) > 0) { - if (compressed && (type == PSTORE_TYPE_DMESG)) { + while ((record.size = psi->read(&record.id, &record.type, + &record.count, &record.time, + &record.buf, &record.compressed, + &record.ecc_notice_size, + record.psi)) > 0) { + if (record.compressed && + record.type == PSTORE_TYPE_DMESG) { if (big_oops_buf) - unzipped_len = pstore_decompress(buf, - big_oops_buf, size, + unzipped_len = pstore_decompress( + record.buf, + big_oops_buf, + record.size, big_oops_buf_sz); if (unzipped_len > 0) { - if (ecc_notice_size) + if (record.ecc_notice_size) memcpy(big_oops_buf + unzipped_len, - buf + size, ecc_notice_size); - kfree(buf); - buf = big_oops_buf; - size = unzipped_len; - compressed = false; + record.buf + record.size, + record.ecc_notice_size); + kfree(record.buf); + record.buf = big_oops_buf; + record.size = unzipped_len; + record.compressed = false; } else { pr_err("decompression failed;returned %d\n", unzipped_len); - compressed = true; + record.compressed = true; } } - rc = pstore_mkfile(type, psi->name, id, count, buf, - compressed, size + ecc_notice_size, - time, psi); + rc = pstore_mkfile(record.type, psi->name, record.id, + record.count, record.buf, + record.compressed, + record.size + record.ecc_notice_size, + record.time, record.psi); if (unzipped_len < 0) { /* Free buffer other than big oops */ - kfree(buf); - buf = NULL; + kfree(record.buf); + record.buf = NULL; } else unzipped_len = -1; if (rc && (rc != -EEXIST || !quiet)) failed++; + + memset(&record, 0, sizeof(record)); + record.psi = psi; } if (psi->close) psi->close(psi); diff --git a/include/linux/pstore.h b/include/linux/pstore.h index 56477ce6806a60..745468072d6e0a 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -30,6 +30,8 @@ #include #include +struct module; + /* pstore record types (see fs/pstore/inode.c for filename templates) */ enum pstore_type_id { PSTORE_TYPE_DMESG = 0, @@ -45,7 +47,31 @@ enum pstore_type_id { PSTORE_TYPE_UNKNOWN = 255 }; -struct module; +struct pstore_info; +/** + * struct pstore_record - details of a pstore record entry + * @psi: pstore backend driver information + * @type: pstore record type + * @id: per-type unique identifier for record + * @time: timestamp of the record + * @count: for PSTORE_TYPE_DMESG, the Oops count. + * @compressed: for PSTORE_TYPE_DMESG, whether the buffer is compressed + * @buf: pointer to record contents + * @size: size of @buf + * @ecc_notice_size: + * ECC information for @buf + */ +struct pstore_record { + struct pstore_info *psi; + enum pstore_type_id type; + u64 id; + struct timespec time; + int count; + bool compressed; + char *buf; + ssize_t size; + ssize_t ecc_notice_size; +}; /** * struct pstore_info - backend pstore driver structure From 634f8f5167c88052ce6d28e519ff6325172191dc Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 3 Mar 2017 17:35:25 -0800 Subject: [PATCH 08/23] pstore: Move record decompression to function This moves the record decompression logic out to a separate function to avoid the deep indentation. Signed-off-by: Kees Cook --- fs/pstore/platform.c | 67 ++++++++++++++++++++++++-------------------- 1 file changed, 37 insertions(+), 30 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index f45228eac3e684..0503380704debf 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -757,6 +757,37 @@ void pstore_unregister(struct pstore_info *psi) } EXPORT_SYMBOL_GPL(pstore_unregister); +static void decompress_record(struct pstore_record *record) +{ + int unzipped_len; + + /* Only PSTORE_TYPE_DMESG support compression. */ + if (!record->compressed || record->type != PSTORE_TYPE_DMESG) { + pr_warn("ignored compressed record type %d\n", record->type); + return; + } + + /* No compression method has created the common buffer. */ + if (!big_oops_buf) { + pr_warn("no decompression buffer allocated\n"); + return; + } + + unzipped_len = pstore_decompress(record->buf, big_oops_buf, + record->size, big_oops_buf_sz); + if (unzipped_len > 0) { + if (record->ecc_notice_size) + memcpy(big_oops_buf + unzipped_len, + record->buf + record->size, + record->ecc_notice_size); + kfree(record->buf); + record->buf = big_oops_buf; + record->size = unzipped_len; + record->compressed = false; + } else + pr_err("decompression failed: %d\n", unzipped_len); +} + /* * Read all the records from the persistent store. Create * files in our filesystem. Don't warn about -EEXIST errors @@ -768,7 +799,6 @@ void pstore_get_records(int quiet) struct pstore_info *psi = psinfo; struct pstore_record record = { .psi = psi, }; int failed = 0, rc; - int unzipped_len = -1; if (!psi) return; @@ -782,41 +812,18 @@ void pstore_get_records(int quiet) &record.buf, &record.compressed, &record.ecc_notice_size, record.psi)) > 0) { - if (record.compressed && - record.type == PSTORE_TYPE_DMESG) { - if (big_oops_buf) - unzipped_len = pstore_decompress( - record.buf, - big_oops_buf, - record.size, - big_oops_buf_sz); - - if (unzipped_len > 0) { - if (record.ecc_notice_size) - memcpy(big_oops_buf + unzipped_len, - record.buf + record.size, - record.ecc_notice_size); - kfree(record.buf); - record.buf = big_oops_buf; - record.size = unzipped_len; - record.compressed = false; - } else { - pr_err("decompression failed;returned %d\n", - unzipped_len); - record.compressed = true; - } - } + + decompress_record(&record); rc = pstore_mkfile(record.type, psi->name, record.id, record.count, record.buf, record.compressed, record.size + record.ecc_notice_size, record.time, record.psi); - if (unzipped_len < 0) { - /* Free buffer other than big oops */ + + /* Free buffer other than big oops */ + if (record.buf != big_oops_buf) kfree(record.buf); - record.buf = NULL; - } else - unzipped_len = -1; + if (rc && (rc != -EEXIST || !quiet)) failed++; From 1edd1aa397ad3ca5f1fca1961c13910ef53f16e8 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 3 Mar 2017 18:16:32 -0800 Subject: [PATCH 09/23] pstore: Switch pstore_mkfile to pass record Instead of the long list of arguments, just pass the new record struct. Signed-off-by: Kees Cook --- fs/pstore/inode.c | 57 +++++++++++++++++++++++++------------------- fs/pstore/internal.h | 5 +--- fs/pstore/platform.c | 6 +---- 3 files changed, 34 insertions(+), 34 deletions(-) diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c index 57c0646479f51c..a98787bab3e64d 100644 --- a/fs/pstore/inode.c +++ b/fs/pstore/inode.c @@ -302,9 +302,7 @@ bool pstore_is_mounted(void) * Load it up with "size" bytes of data from "buf". * Set the mtime & ctime to the date that this record was originally stored. */ -int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, - char *data, bool compressed, size_t size, - struct timespec time, struct pstore_info *psi) +int pstore_mkfile(struct pstore_record *record) { struct dentry *root = pstore_sb->s_root; struct dentry *dentry; @@ -313,12 +311,13 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, char name[PSTORE_NAMELEN]; struct pstore_private *private, *pos; unsigned long flags; + size_t size = record->size + record->ecc_notice_size; spin_lock_irqsave(&allpstore_lock, flags); list_for_each_entry(pos, &allpstore, list) { - if (pos->type == type && - pos->id == id && - pos->psi == psi) { + if (pos->type == record->type && + pos->id == record->id && + pos->psi == record->psi) { rc = -EEXIST; break; } @@ -336,48 +335,56 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, private = kmalloc(sizeof *private + size, GFP_KERNEL); if (!private) goto fail_alloc; - private->type = type; - private->id = id; - private->count = count; - private->psi = psi; + private->type = record->type; + private->id = record->id; + private->count = record->count; + private->psi = record->psi; - switch (type) { + switch (record->type) { case PSTORE_TYPE_DMESG: scnprintf(name, sizeof(name), "dmesg-%s-%lld%s", - psname, id, compressed ? ".enc.z" : ""); + record->psi->name, record->id, + record->compressed ? ".enc.z" : ""); break; case PSTORE_TYPE_CONSOLE: - scnprintf(name, sizeof(name), "console-%s-%lld", psname, id); + scnprintf(name, sizeof(name), "console-%s-%lld", + record->psi->name, record->id); break; case PSTORE_TYPE_FTRACE: - scnprintf(name, sizeof(name), "ftrace-%s-%lld", psname, id); + scnprintf(name, sizeof(name), "ftrace-%s-%lld", + record->psi->name, record->id); break; case PSTORE_TYPE_MCE: - scnprintf(name, sizeof(name), "mce-%s-%lld", psname, id); + scnprintf(name, sizeof(name), "mce-%s-%lld", + record->psi->name, record->id); break; case PSTORE_TYPE_PPC_RTAS: - scnprintf(name, sizeof(name), "rtas-%s-%lld", psname, id); + scnprintf(name, sizeof(name), "rtas-%s-%lld", + record->psi->name, record->id); break; case PSTORE_TYPE_PPC_OF: scnprintf(name, sizeof(name), "powerpc-ofw-%s-%lld", - psname, id); + record->psi->name, record->id); break; case PSTORE_TYPE_PPC_COMMON: scnprintf(name, sizeof(name), "powerpc-common-%s-%lld", - psname, id); + record->psi->name, record->id); break; case PSTORE_TYPE_PMSG: - scnprintf(name, sizeof(name), "pmsg-%s-%lld", psname, id); + scnprintf(name, sizeof(name), "pmsg-%s-%lld", + record->psi->name, record->id); break; case PSTORE_TYPE_PPC_OPAL: - sprintf(name, "powerpc-opal-%s-%lld", psname, id); + scnprintf(name, sizeof(name), "powerpc-opal-%s-%lld", + record->psi->name, record->id); break; case PSTORE_TYPE_UNKNOWN: - scnprintf(name, sizeof(name), "unknown-%s-%lld", psname, id); + scnprintf(name, sizeof(name), "unknown-%s-%lld", + record->psi->name, record->id); break; default: scnprintf(name, sizeof(name), "type%d-%s-%lld", - type, psname, id); + record->type, record->psi->name, record->id); break; } @@ -387,13 +394,13 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, if (!dentry) goto fail_lockedalloc; - memcpy(private->data, data, size); + memcpy(private->data, record->buf, size); inode->i_size = private->size = size; inode->i_private = private; - if (time.tv_sec) - inode->i_mtime = inode->i_ctime = time; + if (record->time.tv_sec) + inode->i_mtime = inode->i_ctime = record->time; d_add(dentry, inode); diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h index da416e6591c9d9..af1df5a36d860b 100644 --- a/fs/pstore/internal.h +++ b/fs/pstore/internal.h @@ -25,10 +25,7 @@ extern struct pstore_info *psinfo; extern void pstore_set_kmsg_bytes(int); extern void pstore_get_records(int); -extern int pstore_mkfile(enum pstore_type_id, char *psname, u64 id, - int count, char *data, bool compressed, - size_t size, struct timespec time, - struct pstore_info *psi); +extern int pstore_mkfile(struct pstore_record *record); extern bool pstore_is_mounted(void); #endif diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 0503380704debf..168e03fd5e5843 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -814,11 +814,7 @@ void pstore_get_records(int quiet) record.psi)) > 0) { decompress_record(&record); - rc = pstore_mkfile(record.type, psi->name, record.id, - record.count, record.buf, - record.compressed, - record.size + record.ecc_notice_size, - record.time, record.psi); + rc = pstore_mkfile(&record); /* Free buffer other than big oops */ if (record.buf != big_oops_buf) From 125cc42baf8ab2149c207f8a360ea25668b8422d Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 3 Mar 2017 22:09:18 -0800 Subject: [PATCH 10/23] pstore: Replace arguments for read() API The argument list for the pstore_read() interface is unwieldy. This changes passes the new struct pstore_record instead. The erst backend was already doing something similar internally. Signed-off-by: Kees Cook --- arch/powerpc/kernel/nvram_64.c | 61 +++++++++--------- drivers/acpi/apei/erst.c | 38 +++++------ drivers/firmware/efi/efi-pstore.c | 104 ++++++++++++------------------ fs/pstore/platform.c | 7 +- fs/pstore/ram.c | 53 ++++++++------- include/linux/pstore.h | 20 ++---- 6 files changed, 124 insertions(+), 159 deletions(-) diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c index d5e2b83099398d..7f192001d09a21 100644 --- a/arch/powerpc/kernel/nvram_64.c +++ b/arch/powerpc/kernel/nvram_64.c @@ -442,10 +442,7 @@ static int nvram_pstore_write(enum pstore_type_id type, * Returns the length of the data we read from each partition. * Returns 0 if we've been called before. */ -static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type, - int *count, struct timespec *time, char **buf, - bool *compressed, ssize_t *ecc_notice_size, - struct pstore_info *psi) +static ssize_t nvram_pstore_read(struct pstore_record *record) { struct oops_log_info *oops_hdr; unsigned int err_type, id_no, size = 0; @@ -459,40 +456,40 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type, switch (nvram_type_ids[read_type]) { case PSTORE_TYPE_DMESG: part = &oops_log_partition; - *type = PSTORE_TYPE_DMESG; + record->type = PSTORE_TYPE_DMESG; break; case PSTORE_TYPE_PPC_COMMON: sig = NVRAM_SIG_SYS; part = &common_partition; - *type = PSTORE_TYPE_PPC_COMMON; - *id = PSTORE_TYPE_PPC_COMMON; - time->tv_sec = 0; - time->tv_nsec = 0; + record->type = PSTORE_TYPE_PPC_COMMON; + record->id = PSTORE_TYPE_PPC_COMMON; + record->time.tv_sec = 0; + record->time.tv_nsec = 0; break; #ifdef CONFIG_PPC_PSERIES case PSTORE_TYPE_PPC_RTAS: part = &rtas_log_partition; - *type = PSTORE_TYPE_PPC_RTAS; - time->tv_sec = last_rtas_event; - time->tv_nsec = 0; + record->type = PSTORE_TYPE_PPC_RTAS; + record->time.tv_sec = last_rtas_event; + record->time.tv_nsec = 0; break; case PSTORE_TYPE_PPC_OF: sig = NVRAM_SIG_OF; part = &of_config_partition; - *type = PSTORE_TYPE_PPC_OF; - *id = PSTORE_TYPE_PPC_OF; - time->tv_sec = 0; - time->tv_nsec = 0; + record->type = PSTORE_TYPE_PPC_OF; + record->id = PSTORE_TYPE_PPC_OF; + record->time.tv_sec = 0; + record->time.tv_nsec = 0; break; #endif #ifdef CONFIG_PPC_POWERNV case PSTORE_TYPE_PPC_OPAL: sig = NVRAM_SIG_FW; part = &skiboot_partition; - *type = PSTORE_TYPE_PPC_OPAL; - *id = PSTORE_TYPE_PPC_OPAL; - time->tv_sec = 0; - time->tv_nsec = 0; + record->type = PSTORE_TYPE_PPC_OPAL; + record->id = PSTORE_TYPE_PPC_OPAL; + record->time.tv_sec = 0; + record->time.tv_nsec = 0; break; #endif default: @@ -520,10 +517,10 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type, return 0; } - *count = 0; + record->count = 0; if (part->os_partition) - *id = id_no; + record->id = id_no; if (nvram_type_ids[read_type] == PSTORE_TYPE_DMESG) { size_t length, hdr_size; @@ -533,28 +530,28 @@ static ssize_t nvram_pstore_read(u64 *id, enum pstore_type_id *type, /* Old format oops header had 2-byte record size */ hdr_size = sizeof(u16); length = be16_to_cpu(oops_hdr->version); - time->tv_sec = 0; - time->tv_nsec = 0; + record->time.tv_sec = 0; + record->time.tv_nsec = 0; } else { hdr_size = sizeof(*oops_hdr); length = be16_to_cpu(oops_hdr->report_length); - time->tv_sec = be64_to_cpu(oops_hdr->timestamp); - time->tv_nsec = 0; + record->time.tv_sec = be64_to_cpu(oops_hdr->timestamp); + record->time.tv_nsec = 0; } - *buf = kmemdup(buff + hdr_size, length, GFP_KERNEL); + record->buf = kmemdup(buff + hdr_size, length, GFP_KERNEL); kfree(buff); - if (*buf == NULL) + if (record->buf == NULL) return -ENOMEM; - *ecc_notice_size = 0; + record->ecc_notice_size = 0; if (err_type == ERR_TYPE_KERNEL_PANIC_GZ) - *compressed = true; + record->compressed = true; else - *compressed = false; + record->compressed = false; return length; } - *buf = buff; + record->buf = buff; return part->size; } diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index ec4f507b524fa8..bbefb7522f80c4 100644 --- a/drivers/acpi/apei/erst.c +++ b/drivers/acpi/apei/erst.c @@ -925,10 +925,7 @@ static int erst_check_table(struct acpi_table_erst *erst_tab) static int erst_open_pstore(struct pstore_info *psi); static int erst_close_pstore(struct pstore_info *psi); -static ssize_t erst_reader(u64 *id, enum pstore_type_id *type, int *count, - struct timespec *time, char **buf, - bool *compressed, ssize_t *ecc_notice_size, - struct pstore_info *psi); +static ssize_t erst_reader(struct pstore_record *record); static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason, u64 *id, unsigned int part, int count, bool compressed, size_t size, struct pstore_info *psi); @@ -986,10 +983,7 @@ static int erst_close_pstore(struct pstore_info *psi) return 0; } -static ssize_t erst_reader(u64 *id, enum pstore_type_id *type, int *count, - struct timespec *time, char **buf, - bool *compressed, ssize_t *ecc_notice_size, - struct pstore_info *psi) +static ssize_t erst_reader(struct pstore_record *record) { int rc; ssize_t len = 0; @@ -1027,33 +1021,33 @@ static ssize_t erst_reader(u64 *id, enum pstore_type_id *type, int *count, if (uuid_le_cmp(rcd->hdr.creator_id, CPER_CREATOR_PSTORE) != 0) goto skip; - *buf = kmalloc(len, GFP_KERNEL); - if (*buf == NULL) { + record->buf = kmalloc(len, GFP_KERNEL); + if (record->buf == NULL) { rc = -ENOMEM; goto out; } - memcpy(*buf, rcd->data, len - sizeof(*rcd)); - *id = record_id; - *compressed = false; - *ecc_notice_size = 0; + memcpy(record->buf, rcd->data, len - sizeof(*rcd)); + record->id = record_id; + record->compressed = false; + record->ecc_notice_size = 0; if (uuid_le_cmp(rcd->sec_hdr.section_type, CPER_SECTION_TYPE_DMESG_Z) == 0) { - *type = PSTORE_TYPE_DMESG; - *compressed = true; + record->type = PSTORE_TYPE_DMESG; + record->compressed = true; } else if (uuid_le_cmp(rcd->sec_hdr.section_type, CPER_SECTION_TYPE_DMESG) == 0) - *type = PSTORE_TYPE_DMESG; + record->type = PSTORE_TYPE_DMESG; else if (uuid_le_cmp(rcd->sec_hdr.section_type, CPER_SECTION_TYPE_MCE) == 0) - *type = PSTORE_TYPE_MCE; + record->type = PSTORE_TYPE_MCE; else - *type = PSTORE_TYPE_UNKNOWN; + record->type = PSTORE_TYPE_UNKNOWN; if (rcd->hdr.validation_bits & CPER_VALID_TIMESTAMP) - time->tv_sec = rcd->hdr.timestamp; + record->time.tv_sec = rcd->hdr.timestamp; else - time->tv_sec = 0; - time->tv_nsec = 0; + record->time.tv_sec = 0; + record->time.tv_nsec = 0; out: kfree(rcd); diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c index f402ba2eed4646..bda24129e85ba7 100644 --- a/drivers/firmware/efi/efi-pstore.c +++ b/drivers/firmware/efi/efi-pstore.c @@ -28,26 +28,16 @@ static int efi_pstore_close(struct pstore_info *psi) return 0; } -struct pstore_read_data { - u64 *id; - enum pstore_type_id *type; - int *count; - struct timespec *timespec; - bool *compressed; - ssize_t *ecc_notice_size; - char **buf; -}; - static inline u64 generic_id(unsigned long timestamp, unsigned int part, int count) { return ((u64) timestamp * 100 + part) * 1000 + count; } -static int efi_pstore_read_func(struct efivar_entry *entry, void *data) +static int efi_pstore_read_func(struct efivar_entry *entry, + struct pstore_record *record) { efi_guid_t vendor = LINUX_EFI_CRASH_GUID; - struct pstore_read_data *cb_data = data; char name[DUMP_NAME_LEN], data_type; int i; int cnt; @@ -61,37 +51,37 @@ static int efi_pstore_read_func(struct efivar_entry *entry, void *data) name[i] = entry->var.VariableName[i]; if (sscanf(name, "dump-type%u-%u-%d-%lu-%c", - cb_data->type, &part, &cnt, &time, &data_type) == 5) { - *cb_data->id = generic_id(time, part, cnt); - *cb_data->count = cnt; - cb_data->timespec->tv_sec = time; - cb_data->timespec->tv_nsec = 0; + &record->type, &part, &cnt, &time, &data_type) == 5) { + record->id = generic_id(time, part, cnt); + record->count = cnt; + record->time.tv_sec = time; + record->time.tv_nsec = 0; if (data_type == 'C') - *cb_data->compressed = true; + record->compressed = true; else - *cb_data->compressed = false; - *cb_data->ecc_notice_size = 0; + record->compressed = false; + record->ecc_notice_size = 0; } else if (sscanf(name, "dump-type%u-%u-%d-%lu", - cb_data->type, &part, &cnt, &time) == 4) { - *cb_data->id = generic_id(time, part, cnt); - *cb_data->count = cnt; - cb_data->timespec->tv_sec = time; - cb_data->timespec->tv_nsec = 0; - *cb_data->compressed = false; - *cb_data->ecc_notice_size = 0; + &record->type, &part, &cnt, &time) == 4) { + record->id = generic_id(time, part, cnt); + record->count = cnt; + record->time.tv_sec = time; + record->time.tv_nsec = 0; + record->compressed = false; + record->ecc_notice_size = 0; } else if (sscanf(name, "dump-type%u-%u-%lu", - cb_data->type, &part, &time) == 3) { + &record->type, &part, &time) == 3) { /* * Check if an old format, * which doesn't support holding * multiple logs, remains. */ - *cb_data->id = generic_id(time, part, 0); - *cb_data->count = 0; - cb_data->timespec->tv_sec = time; - cb_data->timespec->tv_nsec = 0; - *cb_data->compressed = false; - *cb_data->ecc_notice_size = 0; + record->id = generic_id(time, part, 0); + record->count = 0; + record->time.tv_sec = time; + record->time.tv_nsec = 0; + record->compressed = false; + record->ecc_notice_size = 0; } else return 0; @@ -99,7 +89,7 @@ static int efi_pstore_read_func(struct efivar_entry *entry, void *data) __efivar_entry_get(entry, &entry->var.Attributes, &entry->var.DataSize, entry->var.Data); size = entry->var.DataSize; - memcpy(*cb_data->buf, entry->var.Data, + memcpy(record->buf, entry->var.Data, (size_t)min_t(unsigned long, EFIVARS_DATA_SIZE_MAX, size)); return size; @@ -164,7 +154,7 @@ static int efi_pstore_scan_sysfs_exit(struct efivar_entry *pos, /** * efi_pstore_sysfs_entry_iter * - * @data: function-specific data to pass to callback + * @record: pstore record to pass to callback * @pos: entry to begin iterating from * * You MUST call efivar_enter_iter_begin() before this function, and @@ -175,7 +165,8 @@ static int efi_pstore_scan_sysfs_exit(struct efivar_entry *pos, * the next entry of the last one passed to efi_pstore_read_func(). * To begin iterating from the beginning of the list @pos must be %NULL. */ -static int efi_pstore_sysfs_entry_iter(void *data, struct efivar_entry **pos) +static int efi_pstore_sysfs_entry_iter(struct pstore_record *record, + struct efivar_entry **pos) { struct efivar_entry *entry, *n; struct list_head *head = &efivar_sysfs_list; @@ -186,7 +177,7 @@ static int efi_pstore_sysfs_entry_iter(void *data, struct efivar_entry **pos) list_for_each_entry_safe(entry, n, head, list) { efi_pstore_scan_sysfs_enter(entry, n, head); - size = efi_pstore_read_func(entry, data); + size = efi_pstore_read_func(entry, record); ret = efi_pstore_scan_sysfs_exit(entry, n, head, size < 0); if (ret) @@ -201,7 +192,7 @@ static int efi_pstore_sysfs_entry_iter(void *data, struct efivar_entry **pos) list_for_each_entry_safe_from((*pos), n, head, list) { efi_pstore_scan_sysfs_enter((*pos), n, head); - size = efi_pstore_read_func((*pos), data); + size = efi_pstore_read_func((*pos), record); ret = efi_pstore_scan_sysfs_exit((*pos), n, head, size < 0); if (ret) return ret; @@ -225,36 +216,27 @@ static int efi_pstore_sysfs_entry_iter(void *data, struct efivar_entry **pos) * size < 0: Failed to get data of entry logging via efi_pstore_write(), * and pstore will stop reading entry. */ -static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type, - int *count, struct timespec *timespec, - char **buf, bool *compressed, - ssize_t *ecc_notice_size, - struct pstore_info *psi) +static ssize_t efi_pstore_read(struct pstore_record *record) { - struct pstore_read_data data; + struct efivar_entry *entry = (struct efivar_entry *)record->psi->data; ssize_t size; - data.id = id; - data.type = type; - data.count = count; - data.timespec = timespec; - data.compressed = compressed; - data.ecc_notice_size = ecc_notice_size; - data.buf = buf; - - *data.buf = kzalloc(EFIVARS_DATA_SIZE_MAX, GFP_KERNEL); - if (!*data.buf) + record->buf = kzalloc(EFIVARS_DATA_SIZE_MAX, GFP_KERNEL); + if (!record->buf) return -ENOMEM; if (efivar_entry_iter_begin()) { - kfree(*data.buf); - return -EINTR; + size = -EINTR; + goto out; } - size = efi_pstore_sysfs_entry_iter(&data, - (struct efivar_entry **)&psi->data); + size = efi_pstore_sysfs_entry_iter(record, &entry); efivar_entry_iter_end(); - if (size <= 0) - kfree(*data.buf); + +out: + if (size <= 0) { + kfree(record->buf); + record->buf = NULL; + } return size; } diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 168e03fd5e5843..47968c2f2d0dff 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -807,12 +807,7 @@ void pstore_get_records(int quiet) if (psi->open && psi->open(psi)) goto out; - while ((record.size = psi->read(&record.id, &record.type, - &record.count, &record.time, - &record.buf, &record.compressed, - &record.ecc_notice_size, - record.psi)) > 0) { - + while ((record.size = psi->read(&record)) > 0) { decompress_record(&record); rc = pstore_mkfile(&record); diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 11f918d34b1e50..ca6e2a814e37b2 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -235,35 +235,34 @@ static ssize_t ftrace_log_combine(struct persistent_ram_zone *dest, return 0; } -static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type, - int *count, struct timespec *time, - char **buf, bool *compressed, - ssize_t *ecc_notice_size, - struct pstore_info *psi) +static ssize_t ramoops_pstore_read(struct pstore_record *record) { ssize_t size = 0; - struct ramoops_context *cxt = psi->data; + struct ramoops_context *cxt = record->psi->data; struct persistent_ram_zone *prz = NULL; int header_length = 0; bool free_prz = false; - /* Ramoops headers provide time stamps for PSTORE_TYPE_DMESG, but + /* + * Ramoops headers provide time stamps for PSTORE_TYPE_DMESG, but * PSTORE_TYPE_CONSOLE and PSTORE_TYPE_FTRACE don't currently have * valid time stamps, so it is initialized to zero. */ - time->tv_sec = 0; - time->tv_nsec = 0; - *compressed = false; + record->time.tv_sec = 0; + record->time.tv_nsec = 0; + record->compressed = false; /* Find the next valid persistent_ram_zone for DMESG */ while (cxt->dump_read_cnt < cxt->max_dump_cnt && !prz) { prz = ramoops_get_next_prz(cxt->dprzs, &cxt->dump_read_cnt, - cxt->max_dump_cnt, id, type, + cxt->max_dump_cnt, &record->id, + &record->type, PSTORE_TYPE_DMESG, 1); if (!prz_ok(prz)) continue; header_length = ramoops_read_kmsg_hdr(persistent_ram_old(prz), - time, compressed); + &record->time, + &record->compressed); /* Clear and skip this DMESG record if it has no valid header */ if (!header_length) { persistent_ram_free_old(prz); @@ -274,18 +273,20 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type, if (!prz_ok(prz)) prz = ramoops_get_next_prz(&cxt->cprz, &cxt->console_read_cnt, - 1, id, type, PSTORE_TYPE_CONSOLE, 0); + 1, &record->id, &record->type, + PSTORE_TYPE_CONSOLE, 0); if (!prz_ok(prz)) prz = ramoops_get_next_prz(&cxt->mprz, &cxt->pmsg_read_cnt, - 1, id, type, PSTORE_TYPE_PMSG, 0); + 1, &record->id, &record->type, + PSTORE_TYPE_PMSG, 0); /* ftrace is last since it may want to dynamically allocate memory. */ if (!prz_ok(prz)) { if (!(cxt->flags & RAMOOPS_FLAG_FTRACE_PER_CPU)) { prz = ramoops_get_next_prz(cxt->fprzs, - &cxt->ftrace_read_cnt, 1, id, type, - PSTORE_TYPE_FTRACE, 0); + &cxt->ftrace_read_cnt, 1, &record->id, + &record->type, PSTORE_TYPE_FTRACE, 0); } else { /* * Build a new dummy record which combines all the @@ -302,8 +303,10 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type, while (cxt->ftrace_read_cnt < cxt->max_ftrace_cnt) { prz_next = ramoops_get_next_prz(cxt->fprzs, &cxt->ftrace_read_cnt, - cxt->max_ftrace_cnt, id, - type, PSTORE_TYPE_FTRACE, 0); + cxt->max_ftrace_cnt, + &record->id, + &record->type, + PSTORE_TYPE_FTRACE, 0); if (!prz_ok(prz_next)) continue; @@ -316,7 +319,7 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type, if (size) goto out; } - *id = 0; + record->id = 0; prz = tmp_prz; } } @@ -329,17 +332,19 @@ static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type, size = persistent_ram_old_size(prz) - header_length; /* ECC correction notice */ - *ecc_notice_size = persistent_ram_ecc_string(prz, NULL, 0); + record->ecc_notice_size = persistent_ram_ecc_string(prz, NULL, 0); - *buf = kmalloc(size + *ecc_notice_size + 1, GFP_KERNEL); - if (*buf == NULL) { + record->buf = kmalloc(size + record->ecc_notice_size + 1, GFP_KERNEL); + if (record->buf == NULL) { size = -ENOMEM; goto out; } - memcpy(*buf, (char *)persistent_ram_old(prz) + header_length, size); + memcpy(record->buf, (char *)persistent_ram_old(prz) + header_length, + size); - persistent_ram_ecc_string(prz, *buf + size, *ecc_notice_size + 1); + persistent_ram_ecc_string(prz, record->buf + size, + record->ecc_notice_size + 1); out: if (free_prz) { diff --git a/include/linux/pstore.h b/include/linux/pstore.h index 745468072d6e0a..22a46ebbe04171 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -111,16 +111,11 @@ struct pstore_record { * Read next available backend record. Called after a successful * @open. * - * @id: out: unique identifier for the record - * @type: out: pstore record type - * @count: out: for PSTORE_TYPE_DMESG, the Oops count. - * @time: out: timestamp for the record - * @buf: out: kmalloc copy of record contents, to be freed by pstore - * @compressed: - * out: if the record contents are compressed - * @ecc_notice_size: - * out: ECC information - * @psi: in: pointer to the struct pstore_info for the backend + * @record: + * pointer to record to populate. @buf should be allocated + * by the backend and filled. At least @type and @id should + * be populated, since these are used when creating pstorefs + * file names. * * Returns record size on success, zero when no more records are * available, or negative on error. @@ -207,10 +202,7 @@ struct pstore_info { int (*open)(struct pstore_info *psi); int (*close)(struct pstore_info *psi); - ssize_t (*read)(u64 *id, enum pstore_type_id *type, - int *count, struct timespec *time, char **buf, - bool *compressed, ssize_t *ecc_notice_size, - struct pstore_info *psi); + ssize_t (*read)(struct pstore_record *record); int (*write)(enum pstore_type_id type, enum kmsg_dump_reason reason, u64 *id, unsigned int part, int count, bool compressed, From 76cc9580e3fbd323651d06e8184a5a54e0e1066e Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 3 Mar 2017 23:28:53 -0800 Subject: [PATCH 11/23] pstore: Replace arguments for write() API Similar to the pstore_info read() callback, there were too many arguments. This switches to the new struct pstore_record pointer instead. This adds "reason" and "part" to the record structure as well. Signed-off-by: Kees Cook --- arch/powerpc/kernel/nvram_64.c | 27 ++++---------- drivers/acpi/apei/erst.c | 18 ++++----- drivers/firmware/efi/efi-pstore.c | 18 ++++----- fs/pstore/platform.c | 62 +++++++++++++++++-------------- include/linux/pstore.h | 36 +++++++++--------- 5 files changed, 76 insertions(+), 85 deletions(-) diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c index 7f192001d09a21..caf2e1f36d6b1b 100644 --- a/arch/powerpc/kernel/nvram_64.c +++ b/arch/powerpc/kernel/nvram_64.c @@ -389,51 +389,40 @@ static int nvram_pstore_open(struct pstore_info *psi) /** * nvram_pstore_write - pstore write callback for nvram - * @type: Type of message logged - * @reason: reason behind dump (oops/panic) - * @id: identifier to indicate the write performed - * @part: pstore writes data to registered buffer in parts, - * part number will indicate the same. - * @count: Indicates oops count - * @compressed: Flag to indicate the log is compressed - * @size: number of bytes written to the registered buffer - * @psi: registered pstore_info structure + * @record: pstore record to write, with @id to be set * * Called by pstore_dump() when an oops or panic report is logged in the * printk buffer. * Returns 0 on successful write. */ -static int nvram_pstore_write(enum pstore_type_id type, - enum kmsg_dump_reason reason, - u64 *id, unsigned int part, int count, - bool compressed, size_t size, - struct pstore_info *psi) +static int nvram_pstore_write(struct pstore_record *record) { int rc; unsigned int err_type = ERR_TYPE_KERNEL_PANIC; struct oops_log_info *oops_hdr = (struct oops_log_info *) oops_buf; /* part 1 has the recent messages from printk buffer */ - if (part > 1 || (type != PSTORE_TYPE_DMESG)) + if (record->part > 1 || (record->type != PSTORE_TYPE_DMESG)) return -1; if (clobbering_unread_rtas_event()) return -1; oops_hdr->version = cpu_to_be16(OOPS_HDR_VERSION); - oops_hdr->report_length = cpu_to_be16(size); + oops_hdr->report_length = cpu_to_be16(record->size); oops_hdr->timestamp = cpu_to_be64(ktime_get_real_seconds()); - if (compressed) + if (record->compressed) err_type = ERR_TYPE_KERNEL_PANIC_GZ; rc = nvram_write_os_partition(&oops_log_partition, oops_buf, - (int) (sizeof(*oops_hdr) + size), err_type, count); + (int) (sizeof(*oops_hdr) + record->size), err_type, + record->count); if (rc != 0) return rc; - *id = part; + record->id = record->part; return 0; } diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index bbefb7522f80c4..440588d189e78f 100644 --- a/drivers/acpi/apei/erst.c +++ b/drivers/acpi/apei/erst.c @@ -926,9 +926,7 @@ static int erst_check_table(struct acpi_table_erst *erst_tab) static int erst_open_pstore(struct pstore_info *psi); static int erst_close_pstore(struct pstore_info *psi); static ssize_t erst_reader(struct pstore_record *record); -static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason, - u64 *id, unsigned int part, int count, bool compressed, - size_t size, struct pstore_info *psi); +static int erst_writer(struct pstore_record *record); static int erst_clearer(enum pstore_type_id type, u64 id, int count, struct timespec time, struct pstore_info *psi); @@ -1054,9 +1052,7 @@ static ssize_t erst_reader(struct pstore_record *record) return (rc < 0) ? rc : (len - sizeof(*rcd)); } -static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason, - u64 *id, unsigned int part, int count, bool compressed, - size_t size, struct pstore_info *psi) +static int erst_writer(struct pstore_record *record) { struct cper_pstore_record *rcd = (struct cper_pstore_record *) (erst_info.buf - sizeof(*rcd)); @@ -1071,21 +1067,21 @@ static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason, /* timestamp valid. platform_id, partition_id are invalid */ rcd->hdr.validation_bits = CPER_VALID_TIMESTAMP; rcd->hdr.timestamp = get_seconds(); - rcd->hdr.record_length = sizeof(*rcd) + size; + rcd->hdr.record_length = sizeof(*rcd) + record->size; rcd->hdr.creator_id = CPER_CREATOR_PSTORE; rcd->hdr.notification_type = CPER_NOTIFY_MCE; rcd->hdr.record_id = cper_next_record_id(); rcd->hdr.flags = CPER_HW_ERROR_FLAGS_PREVERR; rcd->sec_hdr.section_offset = sizeof(*rcd); - rcd->sec_hdr.section_length = size; + rcd->sec_hdr.section_length = record->size; rcd->sec_hdr.revision = CPER_SEC_REV; /* fru_id and fru_text is invalid */ rcd->sec_hdr.validation_bits = 0; rcd->sec_hdr.flags = CPER_SEC_PRIMARY; - switch (type) { + switch (record->type) { case PSTORE_TYPE_DMESG: - if (compressed) + if (record->compressed) rcd->sec_hdr.section_type = CPER_SECTION_TYPE_DMESG_Z; else rcd->sec_hdr.section_type = CPER_SECTION_TYPE_DMESG; @@ -1099,7 +1095,7 @@ static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason, rcd->sec_hdr.section_severity = CPER_SEV_FATAL; ret = erst_write(&rcd->hdr); - *id = rcd->hdr.record_id; + record->id = rcd->hdr.record_id; return ret; } diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c index bda24129e85ba7..f81e3ec6f1c01d 100644 --- a/drivers/firmware/efi/efi-pstore.c +++ b/drivers/firmware/efi/efi-pstore.c @@ -240,30 +240,28 @@ static ssize_t efi_pstore_read(struct pstore_record *record) return size; } -static int efi_pstore_write(enum pstore_type_id type, - enum kmsg_dump_reason reason, u64 *id, - unsigned int part, int count, bool compressed, size_t size, - struct pstore_info *psi) +static int efi_pstore_write(struct pstore_record *record) { char name[DUMP_NAME_LEN]; efi_char16_t efi_name[DUMP_NAME_LEN]; efi_guid_t vendor = LINUX_EFI_CRASH_GUID; int i, ret = 0; - sprintf(name, "dump-type%u-%u-%d-%lu-%c", type, part, count, - get_seconds(), compressed ? 'C' : 'D'); + snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lu-%c", + record->type, record->part, record->count, + get_seconds(), record->compressed ? 'C' : 'D'); for (i = 0; i < DUMP_NAME_LEN; i++) efi_name[i] = name[i]; efivar_entry_set_safe(efi_name, vendor, PSTORE_EFI_ATTRIBUTES, - !pstore_cannot_block_path(reason), - size, psi->buf); + !pstore_cannot_block_path(record->reason), + record->size, record->psi->buf); - if (reason == KMSG_DUMP_OOPS) + if (record->reason == KMSG_DUMP_OOPS) efivar_run_worker(); - *id = part; + record->id = record->part; return ret; }; diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 47968c2f2d0dff..879658b4c6795f 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -484,7 +484,6 @@ static void pstore_dump(struct kmsg_dumper *dumper, { unsigned long total = 0; const char *why; - u64 id; unsigned int part = 1; unsigned long flags = 0; int is_locked; @@ -506,48 +505,59 @@ static void pstore_dump(struct kmsg_dumper *dumper, oopscount++; while (total < kmsg_bytes) { char *dst; - unsigned long size; - int hsize; + size_t dst_size; + int header_size; int zipped_len = -1; - size_t len; - bool compressed = false; - size_t total_len; + size_t dump_size; + struct pstore_record record = { + .type = PSTORE_TYPE_DMESG, + .count = oopscount, + .reason = reason, + .part = part, + .compressed = false, + .buf = psinfo->buf, + .psi = psinfo, + }; if (big_oops_buf && is_locked) { dst = big_oops_buf; - size = big_oops_buf_sz; + dst_size = big_oops_buf_sz; } else { dst = psinfo->buf; - size = psinfo->bufsize; + dst_size = psinfo->bufsize; } - hsize = sprintf(dst, "%s#%d Part%u\n", why, oopscount, part); - size -= hsize; + /* Write dump header. */ + header_size = snprintf(dst, dst_size, "%s#%d Part%u\n", why, + oopscount, part); + dst_size -= header_size; - if (!kmsg_dump_get_buffer(dumper, true, dst + hsize, - size, &len)) + /* Write dump contents. */ + if (!kmsg_dump_get_buffer(dumper, true, dst + header_size, + dst_size, &dump_size)) break; if (big_oops_buf && is_locked) { zipped_len = pstore_compress(dst, psinfo->buf, - hsize + len, psinfo->bufsize); + header_size + dump_size, + psinfo->bufsize); if (zipped_len > 0) { - compressed = true; - total_len = zipped_len; + record.compressed = true; + record.size = zipped_len; } else { - total_len = copy_kmsg_to_buffer(hsize, len); + record.size = copy_kmsg_to_buffer(header_size, + dump_size); } } else { - total_len = hsize + len; + record.size = header_size + dump_size; } - ret = psinfo->write(PSTORE_TYPE_DMESG, reason, &id, part, - oopscount, compressed, total_len, psinfo); + ret = psinfo->write(&record); if (ret == 0 && reason == KMSG_DUMP_OOPS && pstore_is_mounted()) pstore_new_entry = 1; - total += total_len; + total += record.size; part++; } if (is_locked) @@ -618,14 +628,12 @@ static void pstore_register_console(void) {} static void pstore_unregister_console(void) {} #endif -static int pstore_write_compat(enum pstore_type_id type, - enum kmsg_dump_reason reason, - u64 *id, unsigned int part, int count, - bool compressed, size_t size, - struct pstore_info *psi) +static int pstore_write_compat(struct pstore_record *record) { - return psi->write_buf(type, reason, id, part, psinfo->buf, compressed, - size, psi); + return record->psi->write_buf(record->type, record->reason, + &record->id, record->part, + psinfo->buf, record->compressed, + record->size, record->psi); } static int pstore_write_buf_user_compat(enum pstore_type_id type, diff --git a/include/linux/pstore.h b/include/linux/pstore.h index 22a46ebbe04171..9335f75c3ddbe5 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -54,23 +54,32 @@ struct pstore_info; * @type: pstore record type * @id: per-type unique identifier for record * @time: timestamp of the record - * @count: for PSTORE_TYPE_DMESG, the Oops count. - * @compressed: for PSTORE_TYPE_DMESG, whether the buffer is compressed * @buf: pointer to record contents * @size: size of @buf * @ecc_notice_size: * ECC information for @buf + * + * Valid for PSTORE_TYPE_DMESG @type: + * + * @count: Oops count since boot + * @reason: kdump reason for notification + * @part: position in a multipart record + * @compressed: whether the buffer is compressed + * */ struct pstore_record { struct pstore_info *psi; enum pstore_type_id type; u64 id; struct timespec time; - int count; - bool compressed; char *buf; ssize_t size; ssize_t ecc_notice_size; + + int count; + enum kmsg_dump_reason reason; + unsigned int part; + bool compressed; }; /** @@ -125,16 +134,10 @@ struct pstore_record { * data to be stored has already been written to the registered @buf * of the @psi structure. * - * @type: in: pstore record type to write - * @reason: - * in: pstore write reason - * @id: out: unique identifier for the record - * @part: in: position in a multipart write - * @count: in: increasing from 0 since boot, the number of this Oops - * @compressed: - * in: if the record is compressed - * @size: in: size of the write - * @psi: in: pointer to the struct pstore_info for the backend + * @record: + * pointer to record metadata. Note that @buf is NULL, since + * the @buf registered with @psi is what has been written. The + * backend is expected to update @id. * * Returns 0 on success, and non-zero on error. * @@ -203,10 +206,7 @@ struct pstore_info { int (*open)(struct pstore_info *psi); int (*close)(struct pstore_info *psi); ssize_t (*read)(struct pstore_record *record); - int (*write)(enum pstore_type_id type, - enum kmsg_dump_reason reason, u64 *id, - unsigned int part, int count, bool compressed, - size_t size, struct pstore_info *psi); + int (*write)(struct pstore_record *record); int (*write_buf)(enum pstore_type_id type, enum kmsg_dump_reason reason, u64 *id, unsigned int part, const char *buf, bool compressed, From 7e8cc8dce17574e432945fa75882cd401c3ef673 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sat, 4 Mar 2017 22:28:46 -0800 Subject: [PATCH 12/23] pstore: Always allocate buffer for decompression Currently, pstore_mkfile() performs a memcpy() of the record contents, so it can live anywhere. However, this is needlessly wasteful. In preparation of pstore_mkfile() keeping the record contents, always allocate a buffer for the contents. Signed-off-by: Kees Cook --- fs/pstore/platform.c | 38 ++++++++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 879658b4c6795f..c0d401e732e6e3 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -768,6 +768,7 @@ EXPORT_SYMBOL_GPL(pstore_unregister); static void decompress_record(struct pstore_record *record) { int unzipped_len; + char *decompressed; /* Only PSTORE_TYPE_DMESG support compression. */ if (!record->compressed || record->type != PSTORE_TYPE_DMESG) { @@ -783,17 +784,29 @@ static void decompress_record(struct pstore_record *record) unzipped_len = pstore_decompress(record->buf, big_oops_buf, record->size, big_oops_buf_sz); - if (unzipped_len > 0) { - if (record->ecc_notice_size) - memcpy(big_oops_buf + unzipped_len, - record->buf + record->size, - record->ecc_notice_size); - kfree(record->buf); - record->buf = big_oops_buf; - record->size = unzipped_len; - record->compressed = false; - } else + if (unzipped_len <= 0) { pr_err("decompression failed: %d\n", unzipped_len); + return; + } + + /* Build new buffer for decompressed contents. */ + decompressed = kmalloc(unzipped_len + record->ecc_notice_size, + GFP_KERNEL); + if (!decompressed) { + pr_err("decompression ran out of memory\n"); + return; + } + memcpy(decompressed, big_oops_buf, unzipped_len); + + /* Append ECC notice to decompressed buffer. */ + memcpy(decompressed + unzipped_len, record->buf + record->size, + record->ecc_notice_size); + + /* Swap out compresed contents with decompressed contents. */ + kfree(record->buf); + record->buf = decompressed; + record->size = unzipped_len; + record->compressed = false; } /* @@ -819,13 +832,10 @@ void pstore_get_records(int quiet) decompress_record(&record); rc = pstore_mkfile(&record); - /* Free buffer other than big oops */ - if (record.buf != big_oops_buf) - kfree(record.buf); - if (rc && (rc != -EEXIST || !quiet)) failed++; + kfree(record.buf); memset(&record, 0, sizeof(record)); record.psi = psi; } From 1dfff7dd67d1a3be4d0ab4a5313f0363966bc70d Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sat, 4 Mar 2017 22:46:41 -0800 Subject: [PATCH 13/23] pstore: Pass record contents instead of copying pstore_mkfile() shouldn't have to memcpy the record contents. It can use the existing copy instead. This adjusts the allocation lifetime management and renames the contents variable from "data" to "buf" to assist moving to struct pstore_record in the future. Signed-off-by: Kees Cook --- fs/pstore/inode.c | 22 +++++++++++++++------- fs/pstore/platform.c | 16 ++++++++++++---- 2 files changed, 27 insertions(+), 11 deletions(-) diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c index a98787bab3e64d..3d1f047e4f4132 100644 --- a/fs/pstore/inode.c +++ b/fs/pstore/inode.c @@ -52,7 +52,7 @@ struct pstore_private { u64 id; int count; ssize_t size; - char data[]; + char *buf; }; struct pstore_ftrace_seq_data { @@ -63,6 +63,14 @@ struct pstore_ftrace_seq_data { #define REC_SIZE sizeof(struct pstore_ftrace_record) +static void free_pstore_private(struct pstore_private *private) +{ + if (!private) + return; + kfree(private->buf); + kfree(private); +} + static void *pstore_ftrace_seq_start(struct seq_file *s, loff_t *pos) { struct pstore_private *ps = s->private; @@ -105,7 +113,7 @@ 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 = (void *)(ps->data + data->off); + struct pstore_ftrace_record *rec = (void *)(ps->buf + data->off); seq_printf(s, "CPU:%d ts:%llu %08lx %08lx %pf <- %pF\n", pstore_ftrace_decode_cpu(rec), @@ -143,7 +151,7 @@ static ssize_t pstore_file_read(struct file *file, char __user *userbuf, if (ps->type == PSTORE_TYPE_FTRACE) return seq_read(file, userbuf, count, ppos); - return simple_read_from_buffer(userbuf, count, ppos, ps->data, ps->size); + return simple_read_from_buffer(userbuf, count, ppos, ps->buf, ps->size); } static int pstore_file_open(struct inode *inode, struct file *file) @@ -221,7 +229,7 @@ static void pstore_evict_inode(struct inode *inode) spin_lock_irqsave(&allpstore_lock, flags); list_del(&p->list); spin_unlock_irqrestore(&allpstore_lock, flags); - kfree(p); + free_pstore_private(p); } } @@ -332,7 +340,7 @@ int pstore_mkfile(struct pstore_record *record) goto fail; inode->i_mode = S_IFREG | 0444; inode->i_fop = &pstore_file_operations; - private = kmalloc(sizeof *private + size, GFP_KERNEL); + private = kzalloc(sizeof(*private), GFP_KERNEL); if (!private) goto fail_alloc; private->type = record->type; @@ -394,7 +402,7 @@ int pstore_mkfile(struct pstore_record *record) if (!dentry) goto fail_lockedalloc; - memcpy(private->data, record->buf, size); + private->buf = record->buf; inode->i_size = private->size = size; inode->i_private = private; @@ -414,7 +422,7 @@ int pstore_mkfile(struct pstore_record *record) fail_lockedalloc: inode_unlock(d_inode(root)); - kfree(private); + free_pstore_private(private); fail_alloc: iput(inode); diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index c0d401e732e6e3..d897e2f11b6a57 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -828,14 +828,22 @@ void pstore_get_records(int quiet) if (psi->open && psi->open(psi)) goto out; + /* + * Backend callback read() allocates record.buf. decompress_record() + * may reallocate record.buf. On success, pstore_mkfile() will keep + * the record.buf, so free it only on failure. + */ while ((record.size = psi->read(&record)) > 0) { decompress_record(&record); rc = pstore_mkfile(&record); + if (rc) { + /* pstore_mkfile() did not take buf, so free it. */ + kfree(record.buf); + if (rc != -EEXIST || !quiet) + failed++; + } - if (rc && (rc != -EEXIST || !quiet)) - failed++; - - kfree(record.buf); + /* Reset for next record. */ memset(&record, 0, sizeof(record)); record.psi = psi; } From 2a2b0acf768cfb2a9cf6c3b42b0e0eb25cbb5814 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sat, 4 Mar 2017 22:57:26 -0800 Subject: [PATCH 14/23] pstore: Allocate records on heap instead of stack In preparation for handling records off to pstore_mkfile(), allocate the record instead of reusing stack. This still always frees the record, though, since pstore_mkfile() isn't yet keeping it. Signed-off-by: Kees Cook --- fs/pstore/platform.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index d897e2f11b6a57..07232662562907 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -818,8 +818,7 @@ static void decompress_record(struct pstore_record *record) void pstore_get_records(int quiet) { struct pstore_info *psi = psinfo; - struct pstore_record record = { .psi = psi, }; - int failed = 0, rc; + int failed = 0; if (!psi) return; @@ -833,19 +832,34 @@ void pstore_get_records(int quiet) * may reallocate record.buf. On success, pstore_mkfile() will keep * the record.buf, so free it only on failure. */ - while ((record.size = psi->read(&record)) > 0) { - decompress_record(&record); - rc = pstore_mkfile(&record); + for (;;) { + struct pstore_record *record; + int rc; + + record = kzalloc(sizeof(*record), GFP_KERNEL); + if (!record) { + pr_err("out of memory creating record\n"); + break; + } + record->psi = psi; + + record->size = psi->read(record); + + /* No more records left in backend? */ + if (record->size <= 0) + break; + + decompress_record(record); + rc = pstore_mkfile(record); if (rc) { /* pstore_mkfile() did not take buf, so free it. */ - kfree(record.buf); + kfree(record->buf); if (rc != -EEXIST || !quiet) failed++; } /* Reset for next record. */ - memset(&record, 0, sizeof(record)); - record.psi = psi; + kfree(record); } if (psi->close) psi->close(psi); From 83f70f0769ddd8a368cb6346a918102818232962 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sat, 4 Mar 2017 23:12:24 -0800 Subject: [PATCH 15/23] pstore: Do not duplicate record metadata This switches the inode-private data from carrying duplicate metadata to keeping the record passed in during pstore_mkfile(). Signed-off-by: Kees Cook --- fs/pstore/inode.c | 57 ++++++++++++++++++++++---------------------- fs/pstore/platform.c | 6 ++--- 2 files changed, 30 insertions(+), 33 deletions(-) diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c index 3d1f047e4f4132..0ea281b457face 100644 --- a/fs/pstore/inode.c +++ b/fs/pstore/inode.c @@ -47,12 +47,8 @@ static LIST_HEAD(allpstore); struct pstore_private { struct list_head list; - struct pstore_info *psi; - enum pstore_type_id type; - u64 id; - int count; - ssize_t size; - char *buf; + struct pstore_record *record; + size_t total_size; }; struct pstore_ftrace_seq_data { @@ -67,7 +63,10 @@ static void free_pstore_private(struct pstore_private *private) { if (!private) return; - kfree(private->buf); + if (private->record) { + kfree(private->record->buf); + kfree(private->record); + } kfree(private); } @@ -80,9 +79,9 @@ static void *pstore_ftrace_seq_start(struct seq_file *s, loff_t *pos) if (!data) return NULL; - data->off = ps->size % REC_SIZE; + data->off = ps->total_size % REC_SIZE; data->off += *pos * REC_SIZE; - if (data->off + REC_SIZE > ps->size) { + if (data->off + REC_SIZE > ps->total_size) { kfree(data); return NULL; } @@ -102,7 +101,7 @@ static void *pstore_ftrace_seq_next(struct seq_file *s, void *v, loff_t *pos) struct pstore_ftrace_seq_data *data = v; data->off += REC_SIZE; - if (data->off + REC_SIZE > ps->size) + if (data->off + REC_SIZE > ps->total_size) return NULL; (*pos)++; @@ -113,7 +112,9 @@ 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 = (void *)(ps->buf + data->off); + struct pstore_ftrace_record *rec; + + rec = (struct pstore_ftrace_record *)(ps->record->buf + data->off); seq_printf(s, "CPU:%d ts:%llu %08lx %08lx %pf <- %pF\n", pstore_ftrace_decode_cpu(rec), @@ -133,7 +134,7 @@ static const struct seq_operations pstore_ftrace_seq_ops = { static int pstore_check_syslog_permissions(struct pstore_private *ps) { - switch (ps->type) { + switch (ps->record->type) { case PSTORE_TYPE_DMESG: case PSTORE_TYPE_CONSOLE: return check_syslog_permissions(SYSLOG_ACTION_READ_ALL, @@ -149,9 +150,10 @@ static ssize_t pstore_file_read(struct file *file, char __user *userbuf, struct seq_file *sf = file->private_data; struct pstore_private *ps = sf->private; - if (ps->type == PSTORE_TYPE_FTRACE) + if (ps->record->type == PSTORE_TYPE_FTRACE) return seq_read(file, userbuf, count, ppos); - return simple_read_from_buffer(userbuf, count, ppos, ps->buf, ps->size); + return simple_read_from_buffer(userbuf, count, ppos, + ps->record->buf, ps->total_size); } static int pstore_file_open(struct inode *inode, struct file *file) @@ -165,7 +167,7 @@ static int pstore_file_open(struct inode *inode, struct file *file) if (err) return err; - if (ps->type == PSTORE_TYPE_FTRACE) + if (ps->record->type == PSTORE_TYPE_FTRACE) sops = &pstore_ftrace_seq_ops; err = seq_open(file, sops); @@ -201,17 +203,18 @@ static const struct file_operations pstore_file_operations = { static int pstore_unlink(struct inode *dir, struct dentry *dentry) { struct pstore_private *p = d_inode(dentry)->i_private; + struct pstore_record *record = p->record; int err; err = pstore_check_syslog_permissions(p); if (err) return err; - if (p->psi->erase) { - mutex_lock(&p->psi->read_mutex); - p->psi->erase(p->type, p->id, p->count, - d_inode(dentry)->i_ctime, p->psi); - mutex_unlock(&p->psi->read_mutex); + if (record->psi->erase) { + mutex_lock(&record->psi->read_mutex); + record->psi->erase(record->type, record->id, record->count, + d_inode(dentry)->i_ctime, record->psi); + mutex_unlock(&record->psi->read_mutex); } else { return -EPERM; } @@ -323,9 +326,9 @@ int pstore_mkfile(struct pstore_record *record) spin_lock_irqsave(&allpstore_lock, flags); list_for_each_entry(pos, &allpstore, list) { - if (pos->type == record->type && - pos->id == record->id && - pos->psi == record->psi) { + if (pos->record->type == record->type && + pos->record->id == record->id && + pos->record->psi == record->psi) { rc = -EEXIST; break; } @@ -343,10 +346,7 @@ int pstore_mkfile(struct pstore_record *record) private = kzalloc(sizeof(*private), GFP_KERNEL); if (!private) goto fail_alloc; - private->type = record->type; - private->id = record->id; - private->count = record->count; - private->psi = record->psi; + private->record = record; switch (record->type) { case PSTORE_TYPE_DMESG: @@ -402,8 +402,7 @@ int pstore_mkfile(struct pstore_record *record) if (!dentry) goto fail_lockedalloc; - private->buf = record->buf; - inode->i_size = private->size = size; + inode->i_size = private->total_size = size; inode->i_private = private; diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 07232662562907..aa3d6e572ede6a 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -852,14 +852,12 @@ void pstore_get_records(int quiet) decompress_record(record); rc = pstore_mkfile(record); if (rc) { - /* pstore_mkfile() did not take buf, so free it. */ + /* pstore_mkfile() did not take record, so free it. */ kfree(record->buf); + kfree(record); if (rc != -EEXIST || !quiet) failed++; } - - /* Reset for next record. */ - kfree(record); } if (psi->close) psi->close(psi); From a61072aae693ba08390f92eed1dd0573fa5c3cd9 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sat, 4 Mar 2017 23:31:19 -0800 Subject: [PATCH 16/23] pstore: Replace arguments for erase() API This removes the argument list for the erase() callback and replaces it with a pointer to the backend record details to be removed. Signed-off-by: Kees Cook --- drivers/acpi/apei/erst.c | 8 +++----- drivers/firmware/efi/efi-pstore.c | 26 +++++++++++--------------- fs/pstore/inode.c | 12 +++++------- fs/pstore/ram.c | 15 +++++++-------- include/linux/pstore.h | 16 +++++----------- 5 files changed, 31 insertions(+), 46 deletions(-) diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index 440588d189e78f..7207e5fc9d3d3e 100644 --- a/drivers/acpi/apei/erst.c +++ b/drivers/acpi/apei/erst.c @@ -927,8 +927,7 @@ static int erst_open_pstore(struct pstore_info *psi); static int erst_close_pstore(struct pstore_info *psi); static ssize_t erst_reader(struct pstore_record *record); static int erst_writer(struct pstore_record *record); -static int erst_clearer(enum pstore_type_id type, u64 id, int count, - struct timespec time, struct pstore_info *psi); +static int erst_clearer(struct pstore_record *record); static struct pstore_info erst_info = { .owner = THIS_MODULE, @@ -1100,10 +1099,9 @@ static int erst_writer(struct pstore_record *record) return ret; } -static int erst_clearer(enum pstore_type_id type, u64 id, int count, - struct timespec time, struct pstore_info *psi) +static int erst_clearer(struct pstore_record *record) { - return erst_clear(id); + return erst_clear(record->id); } static int __init erst_init(void) diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c index f81e3ec6f1c01d..93d8cdbe7ef493 100644 --- a/drivers/firmware/efi/efi-pstore.c +++ b/drivers/firmware/efi/efi-pstore.c @@ -266,10 +266,7 @@ static int efi_pstore_write(struct pstore_record *record) }; struct pstore_erase_data { - u64 id; - enum pstore_type_id type; - int count; - struct timespec time; + struct pstore_record *record; efi_char16_t *name; }; @@ -295,8 +292,9 @@ static int efi_pstore_erase_func(struct efivar_entry *entry, void *data) * Check if an old format, which doesn't support * holding multiple logs, remains. */ - sprintf(name_old, "dump-type%u-%u-%lu", ed->type, - (unsigned int)ed->id, ed->time.tv_sec); + snprintf(name_old, sizeof(name_old), "dump-type%u-%u-%lu", + ed->record->type, (unsigned int)ed->record->id, + ed->record->time.tv_sec); for (i = 0; i < DUMP_NAME_LEN; i++) efi_name_old[i] = name_old[i]; @@ -321,8 +319,7 @@ static int efi_pstore_erase_func(struct efivar_entry *entry, void *data) return 1; } -static int efi_pstore_erase(enum pstore_type_id type, u64 id, int count, - struct timespec time, struct pstore_info *psi) +static int efi_pstore_erase(struct pstore_record *record) { struct pstore_erase_data edata; struct efivar_entry *entry = NULL; @@ -331,17 +328,16 @@ static int efi_pstore_erase(enum pstore_type_id type, u64 id, int count, int found, i; unsigned int part; - do_div(id, 1000); - part = do_div(id, 100); - sprintf(name, "dump-type%u-%u-%d-%lu", type, part, count, time.tv_sec); + do_div(record->id, 1000); + part = do_div(record->id, 100); + snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lu", + record->type, record->part, record->count, + record->time.tv_sec); for (i = 0; i < DUMP_NAME_LEN; i++) efi_name[i] = name[i]; - edata.id = part; - edata.type = type; - edata.count = count; - edata.time = time; + edata.record = record; edata.name = efi_name; if (efivar_entry_iter_begin()) diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c index 0ea281b457face..06504b69575bfb 100644 --- a/fs/pstore/inode.c +++ b/fs/pstore/inode.c @@ -210,14 +210,12 @@ static int pstore_unlink(struct inode *dir, struct dentry *dentry) if (err) return err; - if (record->psi->erase) { - mutex_lock(&record->psi->read_mutex); - record->psi->erase(record->type, record->id, record->count, - d_inode(dentry)->i_ctime, record->psi); - mutex_unlock(&record->psi->read_mutex); - } else { + if (!record->psi->erase) return -EPERM; - } + + mutex_lock(&record->psi->read_mutex); + record->psi->erase(record); + mutex_unlock(&record->psi->read_mutex); return simple_unlink(dir, dentry); } diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index ca6e2a814e37b2..a18575fe32e99c 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -469,25 +469,24 @@ static int notrace ramoops_pstore_write_buf_user(enum pstore_type_id type, return -EINVAL; } -static int ramoops_pstore_erase(enum pstore_type_id type, u64 id, int count, - struct timespec time, struct pstore_info *psi) +static int ramoops_pstore_erase(struct pstore_record *record) { - struct ramoops_context *cxt = psi->data; + struct ramoops_context *cxt = record->psi->data; struct persistent_ram_zone *prz; - switch (type) { + switch (record->type) { case PSTORE_TYPE_DMESG: - if (id >= cxt->max_dump_cnt) + if (record->id >= cxt->max_dump_cnt) return -EINVAL; - prz = cxt->dprzs[id]; + prz = cxt->dprzs[record->id]; break; case PSTORE_TYPE_CONSOLE: prz = cxt->cprz; break; case PSTORE_TYPE_FTRACE: - if (id >= cxt->max_ftrace_cnt) + if (record->id >= cxt->max_ftrace_cnt) return -EINVAL; - prz = cxt->fprzs[id]; + prz = cxt->fprzs[record->id]; break; case PSTORE_TYPE_PMSG: prz = cxt->mprz; diff --git a/include/linux/pstore.h b/include/linux/pstore.h index 9335f75c3ddbe5..2cd1979d1f9af1 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -177,15 +177,11 @@ struct pstore_record { * * @erase: * Delete a record from backend storage. Different backends - * identify records differently, so all possible methods of - * identification are included to help the backend locate the - * record to remove. + * identify records differently, so entire original record is + * passed back to assist in identification of what the backend + * should remove from storage. * - * @type: in: pstore record type to write - * @id: in: per-type unique identifier for the record - * @count: in: Oops count - * @time: in: timestamp for the record - * @psi: in: pointer to the struct pstore_info for the backend + * @record: pointer to record metadata. * * Returns 0 on success, and non-zero on error. * @@ -215,9 +211,7 @@ struct pstore_info { enum kmsg_dump_reason reason, u64 *id, unsigned int part, const char __user *buf, bool compressed, size_t size, struct pstore_info *psi); - int (*erase)(enum pstore_type_id type, u64 id, - int count, struct timespec time, - struct pstore_info *psi); + int (*erase)(struct pstore_record *record); }; /* Supported frontends */ From b10b471145f28c219d9ddcc309a67e053776865a Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sun, 5 Mar 2017 00:27:54 -0800 Subject: [PATCH 17/23] pstore: Replace arguments for write_buf() API As with the other API updates, this removes the long argument list in favor of passing a single pstore recaord. Signed-off-by: Kees Cook --- fs/pstore/ftrace.c | 9 +++++++-- fs/pstore/platform.c | 30 +++++++++++++++++++--------- fs/pstore/ram.c | 44 +++++++++++++++++++++--------------------- include/linux/pstore.h | 21 +++++--------------- 4 files changed, 55 insertions(+), 49 deletions(-) diff --git a/fs/pstore/ftrace.c b/fs/pstore/ftrace.c index 899d0ba0bd6cca..a5506ec6995e0b 100644 --- a/fs/pstore/ftrace.c +++ b/fs/pstore/ftrace.c @@ -37,6 +37,12 @@ static void notrace pstore_ftrace_call(unsigned long ip, { unsigned long flags; struct pstore_ftrace_record rec = {}; + struct pstore_record record = { + .type = PSTORE_TYPE_FTRACE, + .buf = (char *)&rec, + .size = sizeof(rec), + .psi = psinfo, + }; if (unlikely(oops_in_progress)) return; @@ -47,8 +53,7 @@ static void notrace pstore_ftrace_call(unsigned long ip, rec.parent_ip = parent_ip; pstore_ftrace_write_timestamp(&rec, pstore_ftrace_stamp++); pstore_ftrace_encode_cpu(&rec, raw_smp_processor_id()); - psinfo->write_buf(PSTORE_TYPE_FTRACE, 0, NULL, 0, (void *)&rec, - 0, sizeof(rec), psinfo); + psinfo->write_buf(&record); local_irq_restore(flags); } diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index aa3d6e572ede6a..5eecf9012459dd 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -587,8 +587,11 @@ static void pstore_console_write(struct console *con, const char *s, unsigned c) const char *e = s + c; while (s < e) { + struct pstore_record record = { + .type = PSTORE_TYPE_CONSOLE, + .psi = psinfo, + }; unsigned long flags; - u64 id; if (c > psinfo->bufsize) c = psinfo->bufsize; @@ -599,8 +602,9 @@ static void pstore_console_write(struct console *con, const char *s, unsigned c) } else { spin_lock_irqsave(&psinfo->buf_lock, flags); } - psinfo->write_buf(PSTORE_TYPE_CONSOLE, 0, &id, 0, - s, 0, c, psinfo); + record.buf = (char *)s; + record.size = c; + psinfo->write_buf(&record); spin_unlock_irqrestore(&psinfo->buf_lock, flags); s += c; c = e - s; @@ -630,10 +634,9 @@ static void pstore_unregister_console(void) {} static int pstore_write_compat(struct pstore_record *record) { - return record->psi->write_buf(record->type, record->reason, - &record->id, record->part, - psinfo->buf, record->compressed, - record->size, record->psi); + record->buf = psinfo->buf; + + return record->psi->write_buf(record); } static int pstore_write_buf_user_compat(enum pstore_type_id type, @@ -653,6 +656,15 @@ static int pstore_write_buf_user_compat(enum pstore_type_id type, bufsize = psinfo->bufsize; spin_lock_irqsave(&psinfo->buf_lock, flags); for (i = 0; i < size; ) { + struct pstore_record record = { + .type = type, + .reason = reason, + .id = id, + .part = part, + .buf = psinfo->buf, + .compressed = compressed, + .psi = psi, + }; size_t c = min(size - i, bufsize); ret = __copy_from_user(psinfo->buf, buf + i, c); @@ -660,8 +672,8 @@ static int pstore_write_buf_user_compat(enum pstore_type_id type, ret = -EFAULT; break; } - ret = psi->write_buf(type, reason, id, part, psinfo->buf, - compressed, c, psi); + record.size = c; + ret = psi->write_buf(&record); if (unlikely(ret < 0)) break; i += c; diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index a18575fe32e99c..a7cdde60b1f93d 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -378,23 +378,18 @@ static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz, return len; } -static int notrace ramoops_pstore_write_buf(enum pstore_type_id type, - enum kmsg_dump_reason reason, - u64 *id, unsigned int part, - const char *buf, - bool compressed, size_t size, - struct pstore_info *psi) +static int notrace ramoops_pstore_write_buf(struct pstore_record *record) { - struct ramoops_context *cxt = psi->data; + struct ramoops_context *cxt = record->psi->data; struct persistent_ram_zone *prz; - size_t hlen; + size_t size, hlen; - if (type == PSTORE_TYPE_CONSOLE) { + if (record->type == PSTORE_TYPE_CONSOLE) { if (!cxt->cprz) return -ENOMEM; - persistent_ram_write(cxt->cprz, buf, size); + persistent_ram_write(cxt->cprz, record->buf, record->size); return 0; - } else if (type == PSTORE_TYPE_FTRACE) { + } else if (record->type == PSTORE_TYPE_FTRACE) { int zonenum; if (!cxt->fprzs) @@ -407,33 +402,36 @@ static int notrace ramoops_pstore_write_buf(enum pstore_type_id type, else zonenum = 0; - persistent_ram_write(cxt->fprzs[zonenum], buf, size); + persistent_ram_write(cxt->fprzs[zonenum], record->buf, + record->size); return 0; - } else if (type == PSTORE_TYPE_PMSG) { + } else if (record->type == PSTORE_TYPE_PMSG) { pr_warn_ratelimited("PMSG shouldn't call %s\n", __func__); return -EINVAL; } - if (type != PSTORE_TYPE_DMESG) + if (record->type != PSTORE_TYPE_DMESG) return -EINVAL; - /* Out of the various dmesg dump types, ramoops is currently designed + /* + * Out of the various dmesg dump types, ramoops is currently designed * to only store crash logs, rather than storing general kernel logs. */ - if (reason != KMSG_DUMP_OOPS && - reason != KMSG_DUMP_PANIC) + if (record->reason != KMSG_DUMP_OOPS && + record->reason != KMSG_DUMP_PANIC) return -EINVAL; /* Skip Oopes when configured to do so. */ - if (reason == KMSG_DUMP_OOPS && !cxt->dump_oops) + if (record->reason == KMSG_DUMP_OOPS && !cxt->dump_oops) return -EINVAL; - /* Explicitly only take the first part of any new crash. + /* + * Explicitly only take the first part of any new crash. * If our buffer is larger than kmsg_bytes, this can never happen, * and if our buffer is smaller than kmsg_bytes, we don't want the * report split across multiple records. */ - if (part != 1) + if (record->part != 1) return -ENOSPC; if (!cxt->dprzs) @@ -441,10 +439,12 @@ static int notrace ramoops_pstore_write_buf(enum pstore_type_id type, prz = cxt->dprzs[cxt->dump_write_cnt]; - hlen = ramoops_write_kmsg_hdr(prz, compressed); + /* Build header and append record contents. */ + hlen = ramoops_write_kmsg_hdr(prz, record->compressed); + size = record->size; if (size + hlen > prz->buffer_size) size = prz->buffer_size - hlen; - persistent_ram_write(prz, buf, size); + persistent_ram_write(prz, record->buf, size); cxt->dump_write_cnt = (cxt->dump_write_cnt + 1) % cxt->max_dump_cnt; diff --git a/include/linux/pstore.h b/include/linux/pstore.h index 2cd1979d1f9af1..cbf5e561778d87 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -142,19 +142,11 @@ struct pstore_record { * Returns 0 on success, and non-zero on error. * * @write_buf: - * Perform a frontend write to a backend record, using a specified - * buffer. Unlike @write, this does not use the @psi @buf. + * Perform a frontend write to a backend record. The record contains + * all metadata and the buffer to write to backend storage. (Unlike + * @write, this does not use the @psi @buf.) * - * @type: in: pstore record type to write - * @reason: - * in: pstore write reason - * @id: out: unique identifier for the record - * @part: in: position in a multipart write - * @buf: in: pointer to contents to write to backend record - * @compressed: - * in: if the record is compressed - * @size: in: size of the write - * @psi: in: pointer to the struct pstore_info for the backend + * @record: pointer to record metadata. * * Returns 0 on success, and non-zero on error. * @@ -203,10 +195,7 @@ struct pstore_info { int (*close)(struct pstore_info *psi); ssize_t (*read)(struct pstore_record *record); int (*write)(struct pstore_record *record); - int (*write_buf)(enum pstore_type_id type, - enum kmsg_dump_reason reason, u64 *id, - unsigned int part, const char *buf, bool compressed, - size_t size, struct pstore_info *psi); + int (*write_buf)(struct pstore_record *record); int (*write_buf_user)(enum pstore_type_id type, enum kmsg_dump_reason reason, u64 *id, unsigned int part, const char __user *buf, From fdd0311863b32b42bb2c54e60c987bbbabc0c430 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sun, 5 Mar 2017 00:56:38 -0800 Subject: [PATCH 18/23] pstore: Replace arguments for write_buf_user() API Removes argument list in favor of pstore record, though the user buffer remains passed separately since it must carry the __user annotation. Signed-off-by: Kees Cook --- fs/pstore/platform.c | 35 ++++++++++++----------------------- fs/pstore/pmsg.c | 9 ++++++--- fs/pstore/ram.c | 14 +++++--------- include/linux/pstore.h | 23 +++++++---------------- 4 files changed, 30 insertions(+), 51 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 5eecf9012459dd..1e6642a2063ee7 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -639,47 +639,36 @@ static int pstore_write_compat(struct pstore_record *record) return record->psi->write_buf(record); } -static int pstore_write_buf_user_compat(enum pstore_type_id type, - enum kmsg_dump_reason reason, - u64 *id, unsigned int part, - const char __user *buf, - bool compressed, size_t size, - struct pstore_info *psi) +static int pstore_write_buf_user_compat(struct pstore_record *record, + const char __user *buf) { unsigned long flags = 0; - size_t i, bufsize = size; + size_t i, bufsize, total_size = record->size; long ret = 0; - if (unlikely(!access_ok(VERIFY_READ, buf, size))) + if (unlikely(!access_ok(VERIFY_READ, buf, total_size))) return -EFAULT; + bufsize = total_size; if (bufsize > psinfo->bufsize) bufsize = psinfo->bufsize; + record->buf = psinfo->buf; spin_lock_irqsave(&psinfo->buf_lock, flags); - for (i = 0; i < size; ) { - struct pstore_record record = { - .type = type, - .reason = reason, - .id = id, - .part = part, - .buf = psinfo->buf, - .compressed = compressed, - .psi = psi, - }; - size_t c = min(size - i, bufsize); + for (i = 0; i < total_size; ) { + size_t c = min(total_size - i, bufsize); - ret = __copy_from_user(psinfo->buf, buf + i, c); + ret = __copy_from_user(record->buf, buf + i, c); if (unlikely(ret != 0)) { ret = -EFAULT; break; } - record.size = c; - ret = psi->write_buf(&record); + record->size = c; + ret = record->psi->write_buf(record); if (unlikely(ret < 0)) break; i += c; } spin_unlock_irqrestore(&psinfo->buf_lock, flags); - return unlikely(ret < 0) ? ret : size; + return unlikely(ret < 0) ? ret : total_size; } /* diff --git a/fs/pstore/pmsg.c b/fs/pstore/pmsg.c index 78f6176c020f82..ce35907602de64 100644 --- a/fs/pstore/pmsg.c +++ b/fs/pstore/pmsg.c @@ -23,7 +23,11 @@ static DEFINE_MUTEX(pmsg_lock); static ssize_t write_pmsg(struct file *file, const char __user *buf, size_t count, loff_t *ppos) { - u64 id; + struct pstore_record record = { + .type = PSTORE_TYPE_PMSG, + .size = count, + .psi = psinfo, + }; int ret; if (!count) @@ -34,8 +38,7 @@ static ssize_t write_pmsg(struct file *file, const char __user *buf, return -EFAULT; mutex_lock(&pmsg_lock); - ret = psinfo->write_buf_user(PSTORE_TYPE_PMSG, 0, &id, 0, buf, 0, count, - psinfo); + ret = psinfo->write_buf_user(&record, buf); mutex_unlock(&pmsg_lock); return ret ? ret : count; } diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index a7cdde60b1f93d..d85e1adae1b693 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -451,19 +451,15 @@ static int notrace ramoops_pstore_write_buf(struct pstore_record *record) return 0; } -static int notrace ramoops_pstore_write_buf_user(enum pstore_type_id type, - enum kmsg_dump_reason reason, - u64 *id, unsigned int part, - const char __user *buf, - bool compressed, size_t size, - struct pstore_info *psi) +static int notrace ramoops_pstore_write_buf_user(struct pstore_record *record, + const char __user *buf) { - if (type == PSTORE_TYPE_PMSG) { - struct ramoops_context *cxt = psi->data; + if (record->type == PSTORE_TYPE_PMSG) { + struct ramoops_context *cxt = record->psi->data; if (!cxt->mprz) return -ENOMEM; - return persistent_ram_write_user(cxt->mprz, buf, size); + return persistent_ram_write_user(cxt->mprz, buf, record->size); } return -EINVAL; diff --git a/include/linux/pstore.h b/include/linux/pstore.h index cbf5e561778d87..9b85d3eeca83ec 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -152,18 +152,11 @@ struct pstore_record { * * @write_buf_user: * Perform a frontend write to a backend record, using a specified - * buffer that is coming directly from userspace. - * - * @type: in: pstore record type to write - * @reason: - * in: pstore write reason - * @id: out: unique identifier for the record - * @part: in: position in a multipart write - * @buf: in: pointer to userspace contents to write to backend record - * @compressed: - * in: if the record is compressed - * @size: in: size of the write - * @psi: in: pointer to the struct pstore_info for the backend + * buffer that is coming directly from userspace, instead of the + * @record @buf. + * + * @record: pointer to record metadata. + * @buf: pointer to userspace contents to write to backend * * Returns 0 on success, and non-zero on error. * @@ -196,10 +189,8 @@ struct pstore_info { ssize_t (*read)(struct pstore_record *record); int (*write)(struct pstore_record *record); int (*write_buf)(struct pstore_record *record); - int (*write_buf_user)(enum pstore_type_id type, - enum kmsg_dump_reason reason, u64 *id, - unsigned int part, const char __user *buf, - bool compressed, size_t size, struct pstore_info *psi); + int (*write_buf_user)(struct pstore_record *record, + const char __user *buf); int (*erase)(struct pstore_record *record); }; From 4c9ec219766a217468fb94a281c416455a884dda Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sun, 5 Mar 2017 22:41:10 -0800 Subject: [PATCH 19/23] pstore: Remove write_buf() callback Now that write() and write_buf() are functionally identical, this removes write_buf(), and renames write_buf_user() to write_user(). Additionally adds sanity-checks for pstore_info's declared functions and flags at registration time. Signed-off-by: Kees Cook --- fs/pstore/ftrace.c | 4 ++-- fs/pstore/platform.c | 35 ++++++++++++++++++++--------------- fs/pstore/pmsg.c | 4 ++-- fs/pstore/ram.c | 10 +++++----- include/linux/pstore.h | 29 ++++++++++------------------- 5 files changed, 39 insertions(+), 43 deletions(-) diff --git a/fs/pstore/ftrace.c b/fs/pstore/ftrace.c index a5506ec6995e0b..06aab07b6bb71b 100644 --- a/fs/pstore/ftrace.c +++ b/fs/pstore/ftrace.c @@ -53,7 +53,7 @@ static void notrace pstore_ftrace_call(unsigned long ip, rec.parent_ip = parent_ip; pstore_ftrace_write_timestamp(&rec, pstore_ftrace_stamp++); pstore_ftrace_encode_cpu(&rec, raw_smp_processor_id()); - psinfo->write_buf(&record); + psinfo->write(&record); local_irq_restore(flags); } @@ -122,7 +122,7 @@ void pstore_register_ftrace(void) { struct dentry *file; - if (!psinfo->write_buf) + if (!psinfo->write) return; pstore_ftrace_dir = debugfs_create_dir("pstore", NULL); diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 1e6642a2063ee7..e79f170fa79b88 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -604,7 +604,7 @@ static void pstore_console_write(struct console *con, const char *s, unsigned c) } record.buf = (char *)s; record.size = c; - psinfo->write_buf(&record); + psinfo->write(&record); spin_unlock_irqrestore(&psinfo->buf_lock, flags); s += c; c = e - s; @@ -632,15 +632,8 @@ static void pstore_register_console(void) {} static void pstore_unregister_console(void) {} #endif -static int pstore_write_compat(struct pstore_record *record) -{ - record->buf = psinfo->buf; - - return record->psi->write_buf(record); -} - -static int pstore_write_buf_user_compat(struct pstore_record *record, - const char __user *buf) +static int pstore_write_user_compat(struct pstore_record *record, + const char __user *buf) { unsigned long flags = 0; size_t i, bufsize, total_size = record->size; @@ -662,7 +655,7 @@ static int pstore_write_buf_user_compat(struct pstore_record *record, break; } record->size = c; - ret = record->psi->write_buf(record); + ret = record->psi->write(record); if (unlikely(ret < 0)) break; i += c; @@ -687,6 +680,20 @@ int pstore_register(struct pstore_info *psi) return -EPERM; } + /* Sanity check flags. */ + if (!psi->flags) { + pr_warn("backend '%s' must support at least one frontend\n", + psi->name); + return -EINVAL; + } + + /* Check for required functions. */ + if (!psi->read || !psi->write) { + pr_warn("backend '%s' must implement read() and write()\n", + psi->name); + return -EINVAL; + } + spin_lock(&pstore_lock); if (psinfo) { pr_warn("backend '%s' already loaded: ignoring '%s'\n", @@ -695,10 +702,8 @@ int pstore_register(struct pstore_info *psi) return -EBUSY; } - if (!psi->write) - psi->write = pstore_write_compat; - if (!psi->write_buf_user) - psi->write_buf_user = pstore_write_buf_user_compat; + if (!psi->write_user) + psi->write_user = pstore_write_user_compat; psinfo = psi; mutex_init(&psinfo->read_mutex); spin_unlock(&pstore_lock); diff --git a/fs/pstore/pmsg.c b/fs/pstore/pmsg.c index ce35907602de64..c16a2477e106cd 100644 --- a/fs/pstore/pmsg.c +++ b/fs/pstore/pmsg.c @@ -33,12 +33,12 @@ static ssize_t write_pmsg(struct file *file, const char __user *buf, if (!count) return 0; - /* check outside lock, page in any data. write_buf_user also checks */ + /* check outside lock, page in any data. write_user also checks */ if (!access_ok(VERIFY_READ, buf, count)) return -EFAULT; mutex_lock(&pmsg_lock); - ret = psinfo->write_buf_user(&record, buf); + ret = psinfo->write_user(&record, buf); mutex_unlock(&pmsg_lock); return ret ? ret : count; } diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index d85e1adae1b693..5523df7f17ef05 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -378,7 +378,7 @@ static size_t ramoops_write_kmsg_hdr(struct persistent_ram_zone *prz, return len; } -static int notrace ramoops_pstore_write_buf(struct pstore_record *record) +static int notrace ramoops_pstore_write(struct pstore_record *record) { struct ramoops_context *cxt = record->psi->data; struct persistent_ram_zone *prz; @@ -451,8 +451,8 @@ static int notrace ramoops_pstore_write_buf(struct pstore_record *record) return 0; } -static int notrace ramoops_pstore_write_buf_user(struct pstore_record *record, - const char __user *buf) +static int notrace ramoops_pstore_write_user(struct pstore_record *record, + const char __user *buf) { if (record->type == PSTORE_TYPE_PMSG) { struct ramoops_context *cxt = record->psi->data; @@ -503,8 +503,8 @@ static struct ramoops_context oops_cxt = { .name = "ramoops", .open = ramoops_pstore_open, .read = ramoops_pstore_read, - .write_buf = ramoops_pstore_write_buf, - .write_buf_user = ramoops_pstore_write_buf_user, + .write = ramoops_pstore_write, + .write_user = ramoops_pstore_write_user, .erase = ramoops_pstore_erase, }, }; diff --git a/include/linux/pstore.h b/include/linux/pstore.h index 9b85d3eeca83ec..e2233f50f42848 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -130,27 +130,19 @@ struct pstore_record { * available, or negative on error. * * @write: - * Perform a frontend notification of a write to a backend record. The - * data to be stored has already been written to the registered @buf - * of the @psi structure. + * A newly generated record needs to be written to backend storage. * * @record: - * pointer to record metadata. Note that @buf is NULL, since - * the @buf registered with @psi is what has been written. The - * backend is expected to update @id. + * pointer to record metadata. When @type is PSTORE_TYPE_DMESG, + * @buf will be pointing to the preallocated @psi.buf, since + * memory allocation may be broken during an Oops. Regardless, + * @buf must be proccesed or copied before returning. The + * backend is also expected to write @id with something that + 8 can help identify this record to a future @erase callback. * * Returns 0 on success, and non-zero on error. * - * @write_buf: - * Perform a frontend write to a backend record. The record contains - * all metadata and the buffer to write to backend storage. (Unlike - * @write, this does not use the @psi @buf.) - * - * @record: pointer to record metadata. - * - * Returns 0 on success, and non-zero on error. - * - * @write_buf_user: + * @write_user: * Perform a frontend write to a backend record, using a specified * buffer that is coming directly from userspace, instead of the * @record @buf. @@ -188,9 +180,8 @@ struct pstore_info { int (*close)(struct pstore_info *psi); ssize_t (*read)(struct pstore_record *record); int (*write)(struct pstore_record *record); - int (*write_buf)(struct pstore_record *record); - int (*write_buf_user)(struct pstore_record *record, - const char __user *buf); + int (*write_user)(struct pstore_record *record, + const char __user *buf); int (*erase)(struct pstore_record *record); }; From 30800d9977ec271a7836d777848dba6773d12a3b Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Tue, 7 Mar 2017 13:57:11 -0800 Subject: [PATCH 20/23] pstore: simplify write_user_compat() Nothing actually uses write_user_compat() currently, but there is no reason to reuse the dmesg buffer. Instead, just allocate a new record buffer, copy in from userspace, and pass it to write() as normal. Signed-off-by: Kees Cook --- fs/pstore/platform.c | 46 +++++++++++++++++++------------------------- 1 file changed, 20 insertions(+), 26 deletions(-) diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index e79f170fa79b88..43b3ca5e045ffc 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -635,33 +635,27 @@ static void pstore_unregister_console(void) {} static int pstore_write_user_compat(struct pstore_record *record, const char __user *buf) { - unsigned long flags = 0; - size_t i, bufsize, total_size = record->size; - long ret = 0; - - if (unlikely(!access_ok(VERIFY_READ, buf, total_size))) - return -EFAULT; - bufsize = total_size; - if (bufsize > psinfo->bufsize) - bufsize = psinfo->bufsize; - record->buf = psinfo->buf; - spin_lock_irqsave(&psinfo->buf_lock, flags); - for (i = 0; i < total_size; ) { - size_t c = min(total_size - i, bufsize); - - ret = __copy_from_user(record->buf, buf + i, c); - if (unlikely(ret != 0)) { - ret = -EFAULT; - break; - } - record->size = c; - ret = record->psi->write(record); - if (unlikely(ret < 0)) - break; - i += c; + int ret = 0; + + if (record->buf) + return -EINVAL; + + record->buf = kmalloc(record->size, GFP_KERNEL); + if (!record->buf) + return -ENOMEM; + + if (unlikely(copy_from_user(record->buf, buf, record->size))) { + ret = -EFAULT; + goto out; } - spin_unlock_irqrestore(&psinfo->buf_lock, flags); - return unlikely(ret < 0) ? ret : total_size; + + ret = record->psi->write(record); + +out: + kfree(record->buf); + record->buf = NULL; + + return unlikely(ret < 0) ? ret : record->size; } /* From 3509d048c8a813a8f903ffd5f8dc5ca66e3d8716 Mon Sep 17 00:00:00 2001 From: Geliang Tang Date: Thu, 23 Mar 2017 21:15:13 +0800 Subject: [PATCH 21/23] pstore: Remove unused vmalloc.h in pmsg Since the vmalloc code has been removed from write_pmsg() in the commit "5bf6d1b pstore/pmsg: drop bounce buffer", remove the unused header vmalloc.h. Signed-off-by: Geliang Tang Signed-off-by: Kees Cook --- fs/pstore/pmsg.c | 1 - 1 file changed, 1 deletion(-) diff --git a/fs/pstore/pmsg.c b/fs/pstore/pmsg.c index c16a2477e106cd..209755e0d7c8ee 100644 --- a/fs/pstore/pmsg.c +++ b/fs/pstore/pmsg.c @@ -15,7 +15,6 @@ #include #include #include -#include #include "internal.h" static DEFINE_MUTEX(pmsg_lock); From 041939c1ec54208b42f5cd819209173d52a29d34 Mon Sep 17 00:00:00 2001 From: Ankit Kumar Date: Thu, 27 Apr 2017 17:03:13 +0530 Subject: [PATCH 22/23] pstore: Fix flags to enable dumps on powerpc After commit c950fd6f201a kernel registers pstore write based on flag set. Pstore write for powerpc is broken as flags(PSTORE_FLAGS_DMESG) is not set for powerpc architecture. On panic, kernel doesn't write message to /fs/pstore/dmesg*(Entry doesn't gets created at all). This patch enables pstore write for powerpc architecture by setting PSTORE_FLAGS_DMESG flag. Fixes: c950fd6f201a ("pstore: Split pstore fragile flags") Cc: stable@vger.kernel.org # v4.9+ Signed-off-by: Ankit Kumar Signed-off-by: Kees Cook --- arch/powerpc/kernel/nvram_64.c | 1 + 1 file changed, 1 insertion(+) diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c index caf2e1f36d6b1b..eae61b044e9e35 100644 --- a/arch/powerpc/kernel/nvram_64.c +++ b/arch/powerpc/kernel/nvram_64.c @@ -547,6 +547,7 @@ static ssize_t nvram_pstore_read(struct pstore_record *record) static struct pstore_info nvram_pstore_info = { .owner = THIS_MODULE, .name = "nvram", + .flags = PSTORE_FLAGS_DMESG, .open = nvram_pstore_open, .read = nvram_pstore_read, .write = nvram_pstore_write, From 3a7d2fd16c57a1ef47dc2891171514231c9c7c6e Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Thu, 27 Apr 2017 15:53:21 -0700 Subject: [PATCH 23/23] pstore: Solve lockdep warning by moving inode locks Lockdep complains about a possible deadlock between mount and unlink (which is technically impossible), but fixing this improves possible future multiple-backend support, and keeps locking in the right order. The lockdep warning could be triggered by unlinking a file in the pstore filesystem: -> #1 (&sb->s_type->i_mutex_key#14){++++++}: lock_acquire+0xc9/0x220 down_write+0x3f/0x70 pstore_mkfile+0x1f4/0x460 pstore_get_records+0x17a/0x320 pstore_fill_super+0xa4/0xc0 mount_single+0x89/0xb0 pstore_mount+0x13/0x20 mount_fs+0xf/0x90 vfs_kern_mount+0x66/0x170 do_mount+0x190/0xd50 SyS_mount+0x90/0xd0 entry_SYSCALL_64_fastpath+0x1c/0xb1 -> #0 (&psinfo->read_mutex){+.+.+.}: __lock_acquire+0x1ac0/0x1bb0 lock_acquire+0xc9/0x220 __mutex_lock+0x6e/0x990 mutex_lock_nested+0x16/0x20 pstore_unlink+0x3f/0xa0 vfs_unlink+0xb5/0x190 do_unlinkat+0x24c/0x2a0 SyS_unlinkat+0x16/0x30 entry_SYSCALL_64_fastpath+0x1c/0xb1 Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&sb->s_type->i_mutex_key#14); lock(&psinfo->read_mutex); lock(&sb->s_type->i_mutex_key#14); lock(&psinfo->read_mutex); Reported-by: Marta Lofstedt Reported-by: Chris Wilson Signed-off-by: Kees Cook Acked-by: Namhyung Kim --- fs/pstore/inode.c | 37 +++++++++++++++++++++++++++---------- fs/pstore/internal.h | 5 ++++- fs/pstore/platform.c | 10 +++++----- 3 files changed, 36 insertions(+), 16 deletions(-) diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c index 06504b69575bfb..792a4e5f9226d1 100644 --- a/fs/pstore/inode.c +++ b/fs/pstore/inode.c @@ -311,9 +311,8 @@ bool pstore_is_mounted(void) * Load it up with "size" bytes of data from "buf". * Set the mtime & ctime to the date that this record was originally stored. */ -int pstore_mkfile(struct pstore_record *record) +int pstore_mkfile(struct dentry *root, struct pstore_record *record) { - struct dentry *root = pstore_sb->s_root; struct dentry *dentry; struct inode *inode; int rc = 0; @@ -322,6 +321,8 @@ int pstore_mkfile(struct pstore_record *record) unsigned long flags; size_t size = record->size + record->ecc_notice_size; + WARN_ON(!inode_is_locked(d_inode(root))); + spin_lock_irqsave(&allpstore_lock, flags); list_for_each_entry(pos, &allpstore, list) { if (pos->record->type == record->type && @@ -336,7 +337,7 @@ int pstore_mkfile(struct pstore_record *record) return rc; rc = -ENOMEM; - inode = pstore_get_inode(pstore_sb); + inode = pstore_get_inode(root->d_sb); if (!inode) goto fail; inode->i_mode = S_IFREG | 0444; @@ -394,11 +395,9 @@ int pstore_mkfile(struct pstore_record *record) break; } - inode_lock(d_inode(root)); - dentry = d_alloc_name(root, name); if (!dentry) - goto fail_lockedalloc; + goto fail_private; inode->i_size = private->total_size = size; @@ -413,12 +412,9 @@ int pstore_mkfile(struct pstore_record *record) list_add(&private->list, &allpstore); spin_unlock_irqrestore(&allpstore_lock, flags); - inode_unlock(d_inode(root)); - return 0; -fail_lockedalloc: - inode_unlock(d_inode(root)); +fail_private: free_pstore_private(private); fail_alloc: iput(inode); @@ -427,6 +423,27 @@ int pstore_mkfile(struct pstore_record *record) return rc; } +/* + * Read all the records from the persistent store. Create + * files in our filesystem. Don't warn about -EEXIST errors + * when we are re-scanning the backing store looking to add new + * error records. + */ +void pstore_get_records(int quiet) +{ + struct pstore_info *psi = psinfo; + struct dentry *root; + + if (!psi || !pstore_sb) + return; + + root = pstore_sb->s_root; + + inode_lock(d_inode(root)); + pstore_get_backend_records(psi, root, quiet); + inode_unlock(d_inode(root)); +} + static int pstore_fill_super(struct super_block *sb, void *data, int silent) { struct inode *inode; diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h index af1df5a36d860b..c416e653dc4f59 100644 --- a/fs/pstore/internal.h +++ b/fs/pstore/internal.h @@ -25,7 +25,10 @@ extern struct pstore_info *psinfo; extern void pstore_set_kmsg_bytes(int); extern void pstore_get_records(int); -extern int pstore_mkfile(struct pstore_record *record); +extern void pstore_get_backend_records(struct pstore_info *psi, + struct dentry *root, int quiet); +extern int pstore_mkfile(struct dentry *root, + struct pstore_record *record); extern bool pstore_is_mounted(void); #endif diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 43b3ca5e045ffc..d468eec9b8a6ba 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -810,17 +810,17 @@ static void decompress_record(struct pstore_record *record) } /* - * Read all the records from the persistent store. Create + * Read all the records from one persistent store backend. Create * files in our filesystem. Don't warn about -EEXIST errors * when we are re-scanning the backing store looking to add new * error records. */ -void pstore_get_records(int quiet) +void pstore_get_backend_records(struct pstore_info *psi, + struct dentry *root, int quiet) { - struct pstore_info *psi = psinfo; int failed = 0; - if (!psi) + if (!psi || !root) return; mutex_lock(&psi->read_mutex); @@ -850,7 +850,7 @@ void pstore_get_records(int quiet) break; decompress_record(record); - rc = pstore_mkfile(record); + rc = pstore_mkfile(root, record); if (rc) { /* pstore_mkfile() did not take record, so free it. */ kfree(record->buf);