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 ++++++-------- drivers/firmware/efi/efi-pstore.c | 104 ++++++++++++++++---------------------- 2 files changed, 59 insertions(+), 83 deletions(-) (limited to 'drivers') 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); diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c index f402ba2eed46..bda24129e85b 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; } -- cgit v1.2.1 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 --- drivers/acpi/apei/erst.c | 18 +++++++----------- drivers/firmware/efi/efi-pstore.c | 18 ++++++++---------- 2 files changed, 15 insertions(+), 21 deletions(-) (limited to 'drivers') 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; }; -- cgit v1.2.1 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 +++++++++++--------------- 2 files changed, 14 insertions(+), 20 deletions(-) (limited to 'drivers') 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()) -- cgit v1.2.1