2012-10-26 21:43:29

by Seiji Aguchi

Subject: [PATCH v3 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 adds a format check for the old variable name in a read callback
to make it readable.

Signed-off-by: Seiji Aguchi <[email protected]>
---
drivers/firmware/efivars.c | 38 ++++++++++++++++++++++++++++----------
1 files changed, 28 insertions(+), 10 deletions(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index dc69802..dd228d5 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);
-- 1.7.1

2012-10-29 20:13:24

by Mike Waychison

Subject: Re: [PATCH v3 6/7] efi_pstore: Add a format check for an existing variable name at reading time

On Fri, Oct 26, 2012 at 2:43 PM, Seiji Aguchi <[email protected]> wrote:
> [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 adds a format check for the old variable name in a read callback
> to make it readable.
>
> Signed-off-by: Seiji Aguchi <[email protected]>
> ---
> drivers/firmware/efivars.c | 38 ++++++++++++++++++++++++++++----------
> 1 files changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
> index dc69802..dd228d5 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);

odd wrap.

> + 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);
> -- 1.7.1

2012-10-29 20:39:53

by Seiji Aguchi

Subject: RE: [PATCH v3 6/7] efi_pstore: Add a format check for an existing variable name at reading time

> > + efivars->walk_entry = list_entry(
> > + efivars->walk_entry->list.next,
> > + struct efivar_entry, list);
>
> odd wrap.
>

I will fix it.

Thanks,

Seiji