From 125cc42baf8ab2149c207f8a360ea25668b8422d Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 3 Mar 2017 22:09:18 -0800 Subject: 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 --- drivers/acpi/apei/erst.c | 38 ++++++++++++++++---------------------- 1 file changed, 16 insertions(+), 22 deletions(-) (limited to 'drivers/acpi/apei') diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index ec4f507b524f..bbefb7522f80 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 @@ skip: 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); -- cgit v1.2.3-59-g8ed1b From 76cc9580e3fbd323651d06e8184a5a54e0e1066e Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 3 Mar 2017 23:28:53 -0800 Subject: 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(-) (limited to 'drivers/acpi/apei') diff --git a/arch/powerpc/kernel/nvram_64.c b/arch/powerpc/kernel/nvram_64.c index 7f192001d09a..caf2e1f36d6b 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 bbefb7522f80..440588d189e7 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 @@ out: 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 bda24129e85b..f81e3ec6f1c0 100644 --- a/drivers/firmware/efi/efi-pstore.c +++ b/drivers/firmware/efi/efi-pstore.c @@ -240,30 +240,28 @@ out: 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 47968c2f2d0d..879658b4c679 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 22a46ebbe041..9335f75c3ddb 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, -- cgit v1.2.3-59-g8ed1b From a61072aae693ba08390f92eed1dd0573fa5c3cd9 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Sat, 4 Mar 2017 23:31:19 -0800 Subject: 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(-) (limited to 'drivers/acpi/apei') diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index 440588d189e7..7207e5fc9d3d 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 f81e3ec6f1c0..93d8cdbe7ef4 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 0ea281b457fa..06504b69575b 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 ca6e2a814e37..a18575fe32e9 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 9335f75c3ddb..2cd1979d1f9a 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 */ -- cgit v1.2.3-59-g8ed1b