From d80a361d779a9f19498943d1ca84243209cd5647 Mon Sep 17 00:00:00 2001 From: Seiji Aguchi Date: Wed, 14 Nov 2012 20:25:37 +0000 Subject: [PATCH 1/7] efi_pstore: Check remaining space with QueryVariableInfo() before writing data [Issue] As discussed in a thread below, Running out of space in EFI isn't a well-tested scenario. And we wouldn't expect all firmware to handle it gracefully. http://marc.info/?l=linux-kernel&m=134305325801789&w=2 On the other hand, current efi_pstore doesn't check a remaining space of storage at writing time. Therefore, efi_pstore may not work if it tries to write a large amount of data. [Patch Description] To avoid handling the situation above, this patch checks if there is a space enough to log with QueryVariableInfo() before writing data. Signed-off-by: Seiji Aguchi Acked-by: Mike Waychison Signed-off-by: Tony Luck --- drivers/firmware/efivars.c | 18 ++++++++++++++++++ include/linux/efi.h | 1 + 2 files changed, 19 insertions(+) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index d10c9873dd9a..37ac21a08751 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -707,12 +707,29 @@ static int efi_pstore_write(enum pstore_type_id type, struct efivars *efivars = psi->data; struct efivar_entry *entry, *found = NULL; int i, ret = 0; + u64 storage_space, remaining_space, max_variable_size; + efi_status_t status = EFI_NOT_FOUND; sprintf(stub_name, "dump-type%u-%u-", type, part); sprintf(name, "%s%lu", stub_name, get_seconds()); spin_lock(&efivars->lock); + /* + * Check if there is a space enough to log. + * size: a size of logging data + * DUMP_NAME_LEN * 2: a maximum size of variable name + */ + status = efivars->ops->query_variable_info(PSTORE_EFI_ATTRIBUTES, + &storage_space, + &remaining_space, + &max_variable_size); + if (status || remaining_space < size + DUMP_NAME_LEN * 2) { + spin_unlock(&efivars->lock); + *id = part; + return -ENOSPC; + } + for (i = 0; i < DUMP_NAME_LEN; i++) efi_name[i] = stub_name[i]; @@ -1237,6 +1254,7 @@ efivars_init(void) ops.get_variable = efi.get_variable; ops.set_variable = efi.set_variable; ops.get_next_variable = efi.get_next_variable; + ops.query_variable_info = efi.query_variable_info; error = register_efivars(&__efivars, &ops, efi_kobj); if (error) goto err_put; diff --git a/include/linux/efi.h b/include/linux/efi.h index 8670eb1eb8cd..c47ec36f3f39 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -643,6 +643,7 @@ struct efivar_operations { efi_get_variable_t *get_variable; efi_get_next_variable_t *get_next_variable; efi_set_variable_t *set_variable; + efi_query_variable_info_t *query_variable_info; }; struct efivars { From dd230fecab5e5833b11941f7f4a5732be78b1975 Mon Sep 17 00:00:00 2001 From: Seiji Aguchi Date: Wed, 14 Nov 2012 20:26:21 +0000 Subject: [PATCH 2/7] efi_pstore: Add a logic erasing entries to an erase callback [Issue] Currently, efi_pstore driver simply overwrites existing panic messages in NVRAM. So, in the following scenario, we will lose 1st panic messages. 1. kernel panics. 2. efi_pstore is kicked and writes panic messages to NVRAM. 3. system reboots. 4. kernel panics again before a user checks the 1st panic messages in NVRAM. [Solution] A reasonable solution to fix the issue is just holding multiple logs without erasing existing entries. This patch freshly adds a logic erasing existing entries, which shared with a write callback, to an erase callback. To support holding multiple logs, the write callback doesn't need to erase any entries and it will be removed in a subsequent patch. Signed-off-by: Seiji Aguchi Acked-by: Mike Waychison Signed-off-by: Tony Luck --- drivers/firmware/efivars.c | 46 +++++++++++++++++++++++++++++++++++++- 1 file changed, 45 insertions(+), 1 deletion(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index 37ac21a08751..bee14cc47c03 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -784,7 +784,51 @@ static int efi_pstore_write(enum pstore_type_id type, static int efi_pstore_erase(enum pstore_type_id type, u64 id, struct pstore_info *psi) { - efi_pstore_write(type, 0, &id, (unsigned int)id, 0, psi); + char stub_name[DUMP_NAME_LEN]; + efi_char16_t efi_name[DUMP_NAME_LEN]; + efi_guid_t vendor = LINUX_EFI_CRASH_GUID; + struct efivars *efivars = psi->data; + struct efivar_entry *entry, *found = NULL; + int i; + + sprintf(stub_name, "dump-type%u-%u-", type, (unsigned int)id); + + spin_lock(&efivars->lock); + + for (i = 0; i < DUMP_NAME_LEN; i++) + efi_name[i] = stub_name[i]; + + /* + * Clean up any entries with the same name + */ + + list_for_each_entry(entry, &efivars->list, list) { + get_var_data_locked(efivars, &entry->var); + + if (efi_guidcmp(entry->var.VendorGuid, vendor)) + continue; + if (utf16_strncmp(entry->var.VariableName, efi_name, + utf16_strlen(efi_name))) + continue; + /* Needs to be a prefix */ + if (entry->var.VariableName[utf16_strlen(efi_name)] == 0) + continue; + + /* found */ + found = entry; + efivars->ops->set_variable(entry->var.VariableName, + &entry->var.VendorGuid, + PSTORE_EFI_ATTRIBUTES, + 0, NULL); + } + + if (found) + list_del(&found->list); + + spin_unlock(&efivars->lock); + + if (found) + efivar_unregister(found); return 0; } From 96480d9c8fcfd7e325e9be6a6c6846689707f8e0 Mon Sep 17 00:00:00 2001 From: Seiji Aguchi Date: Wed, 14 Nov 2012 20:26:46 +0000 Subject: [PATCH 3/7] efi_pstore: Remove a logic erasing entries from a write callback to hold multiple logs [Issue] Currently, efi_pstore driver simply overwrites existing panic messages in NVRAM. So, in the following scenario, we will lose 1st panic messages. 1. kernel panics. 2. efi_pstore is kicked and writes panic messages to NVRAM. 3. system reboots. 4. kernel panics again before a user checks the 1st panic messages in NVRAM. [Solution] A reasonable solution to fix the issue is just holding multiple logs without erasing existing entries. This patch removes a logic erasing existing entries in a write callback because the logic is not needed in the write callback to support holding multiple logs. Signed-off-by: Seiji Aguchi Acked-by: Mike Waychison Signed-off-by: Tony Luck --- drivers/firmware/efivars.c | 39 ++------------------------------------ 1 file changed, 2 insertions(+), 37 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index bee14cc47c03..fbe9202c2678 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -701,18 +701,13 @@ static int efi_pstore_write(enum pstore_type_id type, unsigned int part, size_t size, struct pstore_info *psi) { char name[DUMP_NAME_LEN]; - char stub_name[DUMP_NAME_LEN]; efi_char16_t efi_name[DUMP_NAME_LEN]; efi_guid_t vendor = LINUX_EFI_CRASH_GUID; struct efivars *efivars = psi->data; - struct efivar_entry *entry, *found = NULL; int i, ret = 0; u64 storage_space, remaining_space, max_variable_size; efi_status_t status = EFI_NOT_FOUND; - sprintf(stub_name, "dump-type%u-%u-", type, part); - sprintf(name, "%s%lu", stub_name, get_seconds()); - spin_lock(&efivars->lock); /* @@ -730,35 +725,8 @@ static int efi_pstore_write(enum pstore_type_id type, return -ENOSPC; } - for (i = 0; i < DUMP_NAME_LEN; i++) - efi_name[i] = stub_name[i]; - - /* - * Clean up any entries with the same name - */ - - list_for_each_entry(entry, &efivars->list, list) { - get_var_data_locked(efivars, &entry->var); - - if (efi_guidcmp(entry->var.VendorGuid, vendor)) - continue; - if (utf16_strncmp(entry->var.VariableName, efi_name, - utf16_strlen(efi_name))) - continue; - /* Needs to be a prefix */ - if (entry->var.VariableName[utf16_strlen(efi_name)] == 0) - continue; - - /* found */ - found = entry; - efivars->ops->set_variable(entry->var.VariableName, - &entry->var.VendorGuid, - PSTORE_EFI_ATTRIBUTES, - 0, NULL); - } - - if (found) - list_del(&found->list); + sprintf(name, "dump-type%u-%u-%lu", type, part, + get_seconds()); for (i = 0; i < DUMP_NAME_LEN; i++) efi_name[i] = name[i]; @@ -768,9 +736,6 @@ static int efi_pstore_write(enum pstore_type_id type, spin_unlock(&efivars->lock); - if (found) - efivar_unregister(found); - if (size) ret = efivar_create_sysfs_entry(efivars, utf16_strsize(efi_name, From a9efd39cd547223597cfe7c53acec44c099b9264 Mon Sep 17 00:00:00 2001 From: Seiji Aguchi Date: Wed, 14 Nov 2012 20:27:28 +0000 Subject: [PATCH 4/7] efi_pstore: Add ctime to argument of erase callback [Issue] Currently, a variable name, which is used to identify each log entry, consists of type, id and ctime. But an erase callback does not use ctime. If efi_pstore supported just one log, type and id were enough. However, in case of supporting multiple logs, it doesn't work because it can't distinguish each entry without ctime at erasing time. As you can see below, efi_pstore can't differentiate first event from second one without ctime. a variable name of first event: dump-type0-1-12345678 a variable name of second event: dump-type0-1-23456789 type:0 id:1 ctime:12345678, 23456789 [Solution] This patch adds ctime to an argument of an erase callback. It works across reboots because ctime of pstore means the date that the record was originally stored. To do this, efi_pstore saves the ctime to variable name at writing time and passes it to pstore at reading time. Signed-off-by: Seiji Aguchi Acked-by: Mike Waychison Signed-off-by: Tony Luck --- drivers/acpi/apei/erst.c | 4 ++-- drivers/firmware/efivars.c | 17 ++++++++--------- fs/pstore/inode.c | 3 ++- fs/pstore/ram.c | 2 +- include/linux/pstore.h | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index e4d9d24eb73d..0bd6ae4a899f 100644 --- a/drivers/acpi/apei/erst.c +++ b/drivers/acpi/apei/erst.c @@ -938,7 +938,7 @@ static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason, u64 *id, unsigned int part, size_t size, struct pstore_info *psi); static int erst_clearer(enum pstore_type_id type, u64 id, - struct pstore_info *psi); + struct timespec time, struct pstore_info *psi); static struct pstore_info erst_info = { .owner = THIS_MODULE, @@ -1102,7 +1102,7 @@ static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason, } static int erst_clearer(enum pstore_type_id type, u64 id, - struct pstore_info *psi) + struct timespec time, struct pstore_info *psi) { return erst_clear(id); } diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index fbe9202c2678..3803621c0d45 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -747,24 +747,25 @@ static int efi_pstore_write(enum pstore_type_id type, }; static int efi_pstore_erase(enum pstore_type_id type, u64 id, - struct pstore_info *psi) + struct timespec time, struct pstore_info *psi) { - char stub_name[DUMP_NAME_LEN]; + char name[DUMP_NAME_LEN]; efi_char16_t efi_name[DUMP_NAME_LEN]; efi_guid_t vendor = LINUX_EFI_CRASH_GUID; struct efivars *efivars = psi->data; struct efivar_entry *entry, *found = NULL; int i; - sprintf(stub_name, "dump-type%u-%u-", type, (unsigned int)id); + sprintf(name, "dump-type%u-%u-%lu", type, (unsigned int)id, + time.tv_sec); spin_lock(&efivars->lock); for (i = 0; i < DUMP_NAME_LEN; i++) - efi_name[i] = stub_name[i]; + efi_name[i] = name[i]; /* - * Clean up any entries with the same name + * Clean up an entry with the same name */ list_for_each_entry(entry, &efivars->list, list) { @@ -775,9 +776,6 @@ static int efi_pstore_erase(enum pstore_type_id type, u64 id, if (utf16_strncmp(entry->var.VariableName, efi_name, utf16_strlen(efi_name))) continue; - /* Needs to be a prefix */ - if (entry->var.VariableName[utf16_strlen(efi_name)] == 0) - continue; /* found */ found = entry; @@ -785,6 +783,7 @@ static int efi_pstore_erase(enum pstore_type_id type, u64 id, &entry->var.VendorGuid, PSTORE_EFI_ATTRIBUTES, 0, NULL); + break; } if (found) @@ -823,7 +822,7 @@ static int efi_pstore_write(enum pstore_type_id type, } static int efi_pstore_erase(enum pstore_type_id type, u64 id, - struct pstore_info *psi) + struct timespec time, struct pstore_info *psi) { return 0; } diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c index 4ab572e6d277..4300af654710 100644 --- a/fs/pstore/inode.c +++ b/fs/pstore/inode.c @@ -175,7 +175,8 @@ static int pstore_unlink(struct inode *dir, struct dentry *dentry) struct pstore_private *p = dentry->d_inode->i_private; if (p->psi->erase) - p->psi->erase(p->type, p->id, p->psi); + p->psi->erase(p->type, p->id, dentry->d_inode->i_ctime, + p->psi); return simple_unlink(dir, dentry); } diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 1a4f6da58eab..749693fcb75a 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -237,7 +237,7 @@ static int notrace ramoops_pstore_write_buf(enum pstore_type_id type, } static int ramoops_pstore_erase(enum pstore_type_id type, u64 id, - struct pstore_info *psi) + struct timespec time, struct pstore_info *psi) { struct ramoops_context *cxt = psi->data; struct persistent_ram_zone *prz; diff --git a/include/linux/pstore.h b/include/linux/pstore.h index ee3034a40884..f6e93362d259 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -60,7 +60,7 @@ struct pstore_info { unsigned int part, const char *buf, size_t size, struct pstore_info *psi); int (*erase)(enum pstore_type_id type, u64 id, - struct pstore_info *psi); + struct timespec time, struct pstore_info *psi); void *data; }; From 755d4fe46529018ae45bc7c86df682de45ace764 Mon Sep 17 00:00:00 2001 From: Seiji Aguchi Date: Mon, 26 Nov 2012 16:07:44 -0800 Subject: [PATCH 5/7] efi_pstore: Add a sequence counter to a variable name [Issue] Currently, a variable name, which identifies each entry, consists of type, id and ctime. But if multiple events happens in a short time, a second/third event may fail to log because efi_pstore can't distinguish each event with current variable name. [Solution] A reasonable way to identify all events precisely is introducing a sequence counter to the variable name. The sequence counter has already supported in a pstore layer with "oopscount". So, this patch adds it to a variable name. Also, it is passed to read/erase callbacks of platform drivers in accordance with the modification of the variable name. a variable name of first event: dump-type0-1-12345678 a variable name of second event: dump-type0-1-12345678 type:0 id:1 ctime:12345678 If multiple events happen in a short time, efi_pstore can't distinguish them because variable names are same among them. it can be distinguishable by adding a sequence counter as follows. a variable name of first event: dump-type0-1-1-12345678 a variable name of Second event: dump-type0-1-2-12345678 type:0 id:1 sequence counter: 1(first event), 2(second event) ctime:12345678 In case of a write callback executed in pstore_console_write(), "0" is added to an argument of the write callback because it just logs all kernel messages and doesn't need to care about multiple events. Signed-off-by: Seiji Aguchi Acked-by: Rafael J. Wysocki Acked-by: Mike Waychison Signed-off-by: Tony Luck --- drivers/acpi/apei/erst.c | 12 ++++++------ drivers/firmware/efivars.c | 23 ++++++++++++++--------- fs/pstore/inode.c | 8 +++++--- fs/pstore/internal.h | 2 +- fs/pstore/platform.c | 13 +++++++------ fs/pstore/ram.c | 7 +++---- include/linux/pstore.h | 8 +++++--- 7 files changed, 41 insertions(+), 32 deletions(-) diff --git a/drivers/acpi/apei/erst.c b/drivers/acpi/apei/erst.c index 0bd6ae4a899f..6d894bfd8b8f 100644 --- a/drivers/acpi/apei/erst.c +++ b/drivers/acpi/apei/erst.c @@ -931,13 +931,13 @@ 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, +static ssize_t erst_reader(u64 *id, enum pstore_type_id *type, int *count, struct timespec *time, char **buf, struct pstore_info *psi); static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason, - u64 *id, unsigned int part, + u64 *id, unsigned int part, int count, size_t size, struct pstore_info *psi); -static int erst_clearer(enum pstore_type_id type, u64 id, +static int erst_clearer(enum pstore_type_id type, u64 id, int count, struct timespec time, struct pstore_info *psi); static struct pstore_info erst_info = { @@ -987,7 +987,7 @@ static int erst_close_pstore(struct pstore_info *psi) return 0; } -static ssize_t erst_reader(u64 *id, enum pstore_type_id *type, +static ssize_t erst_reader(u64 *id, enum pstore_type_id *type, int *count, struct timespec *time, char **buf, struct pstore_info *psi) { @@ -1055,7 +1055,7 @@ static ssize_t erst_reader(u64 *id, enum pstore_type_id *type, } static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason, - u64 *id, unsigned int part, + u64 *id, unsigned int part, int count, size_t size, struct pstore_info *psi) { struct cper_pstore_record *rcd = (struct cper_pstore_record *) @@ -1101,7 +1101,7 @@ static int erst_writer(enum pstore_type_id type, enum kmsg_dump_reason reason, return ret; } -static int erst_clearer(enum pstore_type_id type, u64 id, +static int erst_clearer(enum pstore_type_id type, u64 id, int count, struct timespec time, struct pstore_info *psi) { return erst_clear(id); diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index 3803621c0d45..7ad3aae6e085 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -658,13 +658,14 @@ static int efi_pstore_close(struct pstore_info *psi) } static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type, - struct timespec *timespec, + int *count, struct timespec *timespec, char **buf, struct pstore_info *psi) { efi_guid_t vendor = LINUX_EFI_CRASH_GUID; struct efivars *efivars = psi->data; char name[DUMP_NAME_LEN]; int i; + int cnt; unsigned int part, size; unsigned long time; @@ -674,8 +675,10 @@ static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type, for (i = 0; i < DUMP_NAME_LEN; i++) { name[i] = efivars->walk_entry->var.VariableName[i]; } - if (sscanf(name, "dump-type%u-%u-%lu", type, &part, &time) == 3) { + if (sscanf(name, "dump-type%u-%u-%d-%lu", + type, &part, &cnt, &time) == 4) { *id = part; + *count = cnt; timespec->tv_sec = time; timespec->tv_nsec = 0; get_var_data_locked(efivars, &efivars->walk_entry->var); @@ -698,7 +701,8 @@ static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type, static int efi_pstore_write(enum pstore_type_id type, enum kmsg_dump_reason reason, u64 *id, - unsigned int part, size_t size, struct pstore_info *psi) + unsigned int part, int count, size_t size, + struct pstore_info *psi) { char name[DUMP_NAME_LEN]; efi_char16_t efi_name[DUMP_NAME_LEN]; @@ -725,7 +729,7 @@ static int efi_pstore_write(enum pstore_type_id type, return -ENOSPC; } - sprintf(name, "dump-type%u-%u-%lu", type, part, + sprintf(name, "dump-type%u-%u-%d-%lu", type, part, count, get_seconds()); for (i = 0; i < DUMP_NAME_LEN; i++) @@ -746,7 +750,7 @@ static int efi_pstore_write(enum pstore_type_id type, return ret; }; -static int efi_pstore_erase(enum pstore_type_id type, u64 id, +static int efi_pstore_erase(enum pstore_type_id type, u64 id, int count, struct timespec time, struct pstore_info *psi) { char name[DUMP_NAME_LEN]; @@ -756,7 +760,7 @@ static int efi_pstore_erase(enum pstore_type_id type, u64 id, struct efivar_entry *entry, *found = NULL; int i; - sprintf(name, "dump-type%u-%u-%lu", type, (unsigned int)id, + sprintf(name, "dump-type%u-%u-%d-%lu", type, (unsigned int)id, count, time.tv_sec); spin_lock(&efivars->lock); @@ -807,7 +811,7 @@ static int efi_pstore_close(struct pstore_info *psi) return 0; } -static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type, +static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type, int *count, struct timespec *timespec, char **buf, struct pstore_info *psi) { @@ -816,12 +820,13 @@ static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type, static int efi_pstore_write(enum pstore_type_id type, enum kmsg_dump_reason reason, u64 *id, - unsigned int part, size_t size, struct pstore_info *psi) + unsigned int part, int count, size_t size, + struct pstore_info *psi) { return 0; } -static int efi_pstore_erase(enum pstore_type_id type, u64 id, +static int efi_pstore_erase(enum pstore_type_id type, u64 id, int count, struct timespec time, struct pstore_info *psi) { return 0; diff --git a/fs/pstore/inode.c b/fs/pstore/inode.c index 4300af654710..ed1d8c7212da 100644 --- a/fs/pstore/inode.c +++ b/fs/pstore/inode.c @@ -49,6 +49,7 @@ struct pstore_private { struct pstore_info *psi; enum pstore_type_id type; u64 id; + int count; ssize_t size; char data[]; }; @@ -175,8 +176,8 @@ static int pstore_unlink(struct inode *dir, struct dentry *dentry) struct pstore_private *p = dentry->d_inode->i_private; if (p->psi->erase) - p->psi->erase(p->type, p->id, dentry->d_inode->i_ctime, - p->psi); + p->psi->erase(p->type, p->id, p->count, + dentry->d_inode->i_ctime, p->psi); return simple_unlink(dir, dentry); } @@ -271,7 +272,7 @@ int 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 pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, int count, char *data, size_t size, struct timespec time, struct pstore_info *psi) { @@ -307,6 +308,7 @@ int pstore_mkfile(enum pstore_type_id type, char *psname, u64 id, goto fail_alloc; private->type = type; private->id = id; + private->count = count; private->psi = psi; switch (type) { diff --git a/fs/pstore/internal.h b/fs/pstore/internal.h index 4847f588b7d5..937d820f273c 100644 --- a/fs/pstore/internal.h +++ b/fs/pstore/internal.h @@ -50,7 +50,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, - char *data, size_t size, + int count, char *data, size_t size, struct timespec time, struct pstore_info *psi); extern int pstore_is_mounted(void); diff --git a/fs/pstore/platform.c b/fs/pstore/platform.c index 947fbe06c3b1..5ea2e77ff023 100644 --- a/fs/pstore/platform.c +++ b/fs/pstore/platform.c @@ -136,7 +136,7 @@ static void pstore_dump(struct kmsg_dumper *dumper, break; ret = psinfo->write(PSTORE_TYPE_DMESG, reason, &id, part, - hsize + len, psinfo); + oopscount, hsize + len, psinfo); if (ret == 0 && reason == KMSG_DUMP_OOPS && pstore_is_mounted()) pstore_new_entry = 1; @@ -173,7 +173,7 @@ static void pstore_console_write(struct console *con, const char *s, unsigned c) spin_lock_irqsave(&psinfo->buf_lock, flags); } memcpy(psinfo->buf, s, c); - psinfo->write(PSTORE_TYPE_CONSOLE, 0, &id, 0, c, psinfo); + psinfo->write(PSTORE_TYPE_CONSOLE, 0, &id, 0, 0, c, psinfo); spin_unlock_irqrestore(&psinfo->buf_lock, flags); s += c; c = e - s; @@ -197,7 +197,7 @@ static void pstore_register_console(void) {} static int pstore_write_compat(enum pstore_type_id type, enum kmsg_dump_reason reason, - u64 *id, unsigned int part, + u64 *id, unsigned int part, int count, size_t size, struct pstore_info *psi) { return psi->write_buf(type, reason, id, part, psinfo->buf, size, psi); @@ -267,6 +267,7 @@ void pstore_get_records(int quiet) char *buf = NULL; ssize_t size; u64 id; + int count; enum pstore_type_id type; struct timespec time; int failed = 0, rc; @@ -278,9 +279,9 @@ void pstore_get_records(int quiet) if (psi->open && psi->open(psi)) goto out; - while ((size = psi->read(&id, &type, &time, &buf, psi)) > 0) { - rc = pstore_mkfile(type, psi->name, id, buf, (size_t)size, - time, psi); + while ((size = psi->read(&id, &type, &count, &time, &buf, psi)) > 0) { + rc = pstore_mkfile(type, psi->name, id, count, buf, + (size_t)size, time, psi); kfree(buf); buf = NULL; if (rc && (rc != -EEXIST || !quiet)) diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c index 749693fcb75a..2bfa36e0ffe8 100644 --- a/fs/pstore/ram.c +++ b/fs/pstore/ram.c @@ -132,9 +132,8 @@ ramoops_get_next_prz(struct persistent_ram_zone *przs[], uint *c, uint max, } static ssize_t ramoops_pstore_read(u64 *id, enum pstore_type_id *type, - struct timespec *time, - char **buf, - struct pstore_info *psi) + int *count, struct timespec *time, + char **buf, struct pstore_info *psi) { ssize_t size; struct ramoops_context *cxt = psi->data; @@ -236,7 +235,7 @@ static int notrace ramoops_pstore_write_buf(enum pstore_type_id type, return 0; } -static int ramoops_pstore_erase(enum pstore_type_id type, u64 id, +static int ramoops_pstore_erase(enum pstore_type_id type, u64 id, int count, struct timespec time, struct pstore_info *psi) { struct ramoops_context *cxt = psi->data; diff --git a/include/linux/pstore.h b/include/linux/pstore.h index f6e93362d259..1788909d9a99 100644 --- a/include/linux/pstore.h +++ b/include/linux/pstore.h @@ -50,17 +50,19 @@ 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, - struct timespec *time, char **buf, + int *count, struct timespec *time, char **buf, struct pstore_info *psi); int (*write)(enum pstore_type_id type, enum kmsg_dump_reason reason, u64 *id, - unsigned int part, size_t size, struct pstore_info *psi); + unsigned int part, int count, size_t size, + struct pstore_info *psi); int (*write_buf)(enum pstore_type_id type, enum kmsg_dump_reason reason, u64 *id, unsigned int part, const char *buf, size_t size, struct pstore_info *psi); int (*erase)(enum pstore_type_id type, u64 id, - struct timespec time, struct pstore_info *psi); + int count, struct timespec time, + struct pstore_info *psi); void *data; }; From 0f7de85a94de553c6cb222b70ac4032d265b362d Mon Sep 17 00:00:00 2001 From: Seiji Aguchi Date: Wed, 14 Nov 2012 20:28:50 +0000 Subject: [PATCH 6/7] efi_pstore: Add a format check for an existing variable name at reading time [Issue] a format of variable name has been updated to type, id, count and ctime to support holding multiple logs. Format of current variable name dump-type0-1-2-12345678 type:0 id:1 count:2 ctime:12345678 On the other hand, if an old variable name before being updated remains, users can't read it via /dev/pstore. Format of old variable name dump-type0-1-12345678 type:0 id:1 ctime:12345678 [Solution] This patch add a format check for the old variable name in a read callback to make it readable. Signed-off-by: Seiji Aguchi Acked-by: Mike Waychison Signed-off-by: Tony Luck --- drivers/firmware/efivars.c | 38 ++++++++++++++++++++++++++++---------- 1 file changed, 28 insertions(+), 10 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index 7ad3aae6e085..bf7a6f994a3c 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -681,17 +681,35 @@ static ssize_t efi_pstore_read(u64 *id, enum pstore_type_id *type, *count = cnt; timespec->tv_sec = time; timespec->tv_nsec = 0; - get_var_data_locked(efivars, &efivars->walk_entry->var); - size = efivars->walk_entry->var.DataSize; - *buf = kmalloc(size, GFP_KERNEL); - if (*buf == NULL) - return -ENOMEM; - memcpy(*buf, efivars->walk_entry->var.Data, - size); - efivars->walk_entry = list_entry(efivars->walk_entry->list.next, - struct efivar_entry, list); - return size; + } else if (sscanf(name, "dump-type%u-%u-%lu", + type, &part, &time) == 3) { + /* + * Check if an old format, + * which doesn't support holding + * multiple logs, remains. + */ + *id = part; + *count = 0; + timespec->tv_sec = time; + timespec->tv_nsec = 0; + } else { + efivars->walk_entry = list_entry( + efivars->walk_entry->list.next, + struct efivar_entry, list); + continue; } + + get_var_data_locked(efivars, &efivars->walk_entry->var); + size = efivars->walk_entry->var.DataSize; + *buf = kmalloc(size, GFP_KERNEL); + if (*buf == NULL) + return -ENOMEM; + memcpy(*buf, efivars->walk_entry->var.Data, + size); + efivars->walk_entry = list_entry( + efivars->walk_entry->list.next, + struct efivar_entry, list); + return size; } efivars->walk_entry = list_entry(efivars->walk_entry->list.next, struct efivar_entry, list); From f94ec0c0594ef73ab3a2f1f32735aca8ddaf65e2 Mon Sep 17 00:00:00 2001 From: Seiji Aguchi Date: Wed, 14 Nov 2012 20:29:21 +0000 Subject: [PATCH 7/7] efi_pstore: Add a format check for an existing variable name at erasing time [Issue] a format of variable name has been updated to type, id, count and ctime to support holding multiple logs. Format of current variable name dump-type0-1-2-12345678 type:0 id:1 count:2 ctime:12345678 On the other hand, if an old variable name before being updated remains, users can't erase it via /dev/pstore. Format of old variable name dump-type0-1-12345678 type:0 id:1 ctime:12345678 [Solution] This patch add a format check for the old variable name in a erase callback to make it erasable. Signed-off-by: Seiji Aguchi Acked-by: Mike Waychison Signed-off-by: Tony Luck --- drivers/firmware/efivars.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index bf7a6f994a3c..6e51c1e81f14 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -773,6 +773,8 @@ static int efi_pstore_erase(enum pstore_type_id type, u64 id, int count, { char name[DUMP_NAME_LEN]; efi_char16_t efi_name[DUMP_NAME_LEN]; + char name_old[DUMP_NAME_LEN]; + efi_char16_t efi_name_old[DUMP_NAME_LEN]; efi_guid_t vendor = LINUX_EFI_CRASH_GUID; struct efivars *efivars = psi->data; struct efivar_entry *entry, *found = NULL; @@ -796,8 +798,22 @@ static int efi_pstore_erase(enum pstore_type_id type, u64 id, int count, if (efi_guidcmp(entry->var.VendorGuid, vendor)) continue; if (utf16_strncmp(entry->var.VariableName, efi_name, - utf16_strlen(efi_name))) - continue; + utf16_strlen(efi_name))) { + /* + * Check if an old format, + * which doesn't support holding + * multiple logs, remains. + */ + sprintf(name_old, "dump-type%u-%u-%lu", type, + (unsigned int)id, time.tv_sec); + + for (i = 0; i < DUMP_NAME_LEN; i++) + efi_name_old[i] = name_old[i]; + + if (utf16_strncmp(entry->var.VariableName, efi_name_old, + utf16_strlen(efi_name_old))) + continue; + } /* found */ found = entry;