efi-pstore: Refactor erase routine

Right now, every pass through the EFI variables during erase would build
a copy of the old format variable name. Instead, try each name one time
through the EFI variables list. Additionally bump up the buffer size to
avoid truncation in pathological cases, and wipe the write name buffer.

Signed-off-by: Kees Cook <keescook@chromium.org>

diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
index ef1fafdad4008ae7fa1824396592e0e53cf61002..d30dc935e20ede22808e26e08a58917be5aee490 100644 (file)
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -4,7 +4,7 @@
 #include <linux/slab.h>
 #include <linux/ucs2_string.h>
 
-#define DUMP_NAME_LEN 52
+#define DUMP_NAME_LEN 66
 
 static bool efivars_pstore_disable =
        IS_ENABLED(CONFIG_EFI_VARS_PSTORE_DEFAULT_DISABLE);
@@ -250,6 +250,9 @@ static int efi_pstore_write(struct pstore_record *record)
        record->id = generic_id(record->time.tv_sec, record->part,
                                record->count);
 
+       /* Since we copy the entire length of name, make sure it is wiped. */
+       memset(name, 0, sizeof(name));
+
        snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lu-%c",
                 record->type, record->part, record->count,
                 record->time.tv_sec, record->compressed ? 'C' : 'D');
@@ -267,44 +270,20 @@ static int efi_pstore_write(struct pstore_record *record)
        return ret;
 };
 
-struct pstore_erase_data {
-       struct pstore_record *record;
-       efi_char16_t *name;
-};
-
 /*
  * Clean up an entry with the same name
  */
 static int efi_pstore_erase_func(struct efivar_entry *entry, void *data)
 {
-       struct pstore_erase_data *ed = data;
+       efi_char16_t *efi_name = data;
        efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
-       efi_char16_t efi_name_old[DUMP_NAME_LEN];
-       efi_char16_t *efi_name = ed->name;
-       unsigned long ucs2_len = ucs2_strlen(ed->name);
-       char name_old[DUMP_NAME_LEN];
-       int i;
+       unsigned long ucs2_len = ucs2_strlen(efi_name);
 
        if (efi_guidcmp(entry->var.VendorGuid, vendor))
                return 0;
 
-       if (ucs2_strncmp(entry->var.VariableName,
-                         efi_name, (size_t)ucs2_len)) {
-               /*
-                * Check if an old format, which doesn't support
-                * holding multiple logs, remains.
-                */
-               snprintf(name_old, sizeof(name_old), "dump-type%u-%u-%lu",
-                       ed->record->type, ed->record->part,
-                       ed->record->time.tv_sec);
-
-               for (i = 0; i < DUMP_NAME_LEN; i++)
-                       efi_name_old[i] = name_old[i];
-
-               if (ucs2_strncmp(entry->var.VariableName, efi_name_old,
-                                 ucs2_strlen(efi_name_old)))
-                       return 0;
-       }
+       if (ucs2_strncmp(entry->var.VariableName, efi_name, (size_t)ucs2_len))
+               return 0;
 
        if (entry->scanning) {
                /*
@@ -321,35 +300,48 @@ static int efi_pstore_erase_func(struct efivar_entry *entry, void *data)
        return 1;
 }
 
-static int efi_pstore_erase(struct pstore_record *record)
+static int efi_pstore_erase_name(const char *name)
 {
-       struct pstore_erase_data edata;
        struct efivar_entry *entry = NULL;
-       char name[DUMP_NAME_LEN];
        efi_char16_t efi_name[DUMP_NAME_LEN];
        int found, i;
 
-       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++)
+       for (i = 0; i < DUMP_NAME_LEN; i++) {
                efi_name[i] = name[i];
-
-       edata.record = record;
-       edata.name = efi_name;
+               if (name[i] == '\0')
+                       break;
+       }
 
        if (efivar_entry_iter_begin())
                return -EINTR;
-       found = __efivar_entry_iter(efi_pstore_erase_func, &efivar_sysfs_list, &edata, &entry);
 
-       if (found && !entry->scanning) {
-               efivar_entry_iter_end();
+       found = __efivar_entry_iter(efi_pstore_erase_func, &efivar_sysfs_list,
+                                   efi_name, &entry);
+       efivar_entry_iter_end();
+
+       if (found && !entry->scanning)
                efivar_unregister(entry);
-       } else
-               efivar_entry_iter_end();
 
-       return 0;
+       return found ? 0 : -ENOENT;
+}
+
+static int efi_pstore_erase(struct pstore_record *record)
+{
+       char name[DUMP_NAME_LEN];
+       int ret;
+
+       snprintf(name, sizeof(name), "dump-type%u-%u-%d-%lu",
+                record->type, record->part, record->count,
+                record->time.tv_sec);
+       ret = efi_pstore_erase_name(name);
+       if (ret != -ENOENT)
+               return ret;
+
+       snprintf(name, sizeof(name), "dump-type%u-%u-%lu",
+               record->type, record->part, record->time.tv_sec);
+       ret = efi_pstore_erase_name(name);
+
+       return ret;
 }
 
 static struct pstore_info efi_pstore_info = {