efi, pstore: Read data from variable store before memcpy()

Seiji reported getting empty dmesg-* files, because the data was never
actually read in efi_pstore_read_func(), and so the memcpy() was copying
garbage data.

This patch necessitated adding __efivar_entry_get() which is callable
between efivar_entry_iter_{begin,end}(). We can also delete
__efivar_entry_size() because efi_pstore_read_func() was the only
caller.

Reported-by: Seiji Aguchi <seiji.aguchi@hds.com>
Tested-by: Seiji Aguchi <seiji.aguchi@hds.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Matthew Garrett <matthew.garrett@nebula.com>
Signed-off-by: Matt Fleming <matt.fleming@intel.com>

diff --git a/drivers/firmware/efi/efi-pstore.c b/drivers/firmware/efi/efi-pstore.c
index af45c42086e1e4c7622bcb9dd5ebb50604ab0bdc..67615d6d038df7a21a1454566cd86698e85c5d19 100644 (file)
--- a/drivers/firmware/efi/efi-pstore.c
+++ b/drivers/firmware/efi/efi-pstore.c
@@ -73,7 +73,11 @@ static int efi_pstore_read_func(struct efivar_entry *entry, void *data)
        } else
                return 0;
 
-       __efivar_entry_size(entry, &size);
+       entry->var.DataSize = 1024;
+       __efivar_entry_get(entry, &entry->var.Attributes,
+                          &entry->var.DataSize, entry->var.Data);
+       size = entry->var.DataSize;
+
        *cb_data->buf = kmalloc(size, GFP_KERNEL);
        if (*cb_data->buf == NULL)
                return -ENOMEM;

diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c
index 1d80c1ca39c5d257a608c1668ba53a550d5cdc56..96d328b21c3e408918bdfb779dc11c6eb7574abb 100644 (file)
--- a/drivers/firmware/efi/vars.c
+++ b/drivers/firmware/efi/vars.c
@@ -689,54 +689,55 @@ struct efivar_entry *efivar_entry_find(efi_char16_t *name, efi_guid_t guid,
 EXPORT_SYMBOL_GPL(efivar_entry_find);
 
 /**
- * __efivar_entry_size - obtain the size of a variable
+ * efivar_entry_size - obtain the size of a variable
  * @entry: entry for this variable
  * @size: location to store the variable's size
- *
- * The caller MUST call efivar_entry_iter_begin() and
- * efivar_entry_iter_end() before and after the invocation of this
- * function, respectively.
  */
-int __efivar_entry_size(struct efivar_entry *entry, unsigned long *size)
+int efivar_entry_size(struct efivar_entry *entry, unsigned long *size)
 {
        const struct efivar_operations *ops = __efivars->ops;
        efi_status_t status;
 
-       WARN_ON(!spin_is_locked(&__efivars->lock));
-
        *size = 0;
+
+       spin_lock_irq(&__efivars->lock);
        status = ops->get_variable(entry->var.VariableName,
                                   &entry->var.VendorGuid, NULL, size, NULL);
+       spin_unlock_irq(&__efivars->lock);
+
        if (status != EFI_BUFFER_TOO_SMALL)
                return efi_status_to_err(status);
 
        return 0;
 }
-EXPORT_SYMBOL_GPL(__efivar_entry_size);
+EXPORT_SYMBOL_GPL(efivar_entry_size);
 
 /**
- * efivar_entry_size - obtain the size of a variable
- * @entry: entry for this variable
- * @size: location to store the variable's size
+ * __efivar_entry_get - call get_variable()
+ * @entry: read data for this variable
+ * @attributes: variable attributes
+ * @size: size of @data buffer
+ * @data: buffer to store variable data
+ *
+ * The caller MUST call efivar_entry_iter_begin() and
+ * efivar_entry_iter_end() before and after the invocation of this
+ * function, respectively.
  */
-int efivar_entry_size(struct efivar_entry *entry, unsigned long *size)
+int __efivar_entry_get(struct efivar_entry *entry, u32 *attributes,
+                      unsigned long *size, void *data)
 {
        const struct efivar_operations *ops = __efivars->ops;
        efi_status_t status;
 
-       *size = 0;
+       WARN_ON(!spin_is_locked(&__efivars->lock));
 
-       spin_lock_irq(&__efivars->lock);
        status = ops->get_variable(entry->var.VariableName,
-                                  &entry->var.VendorGuid, NULL, size, NULL);
-       spin_unlock_irq(&__efivars->lock);
-
-       if (status != EFI_BUFFER_TOO_SMALL)
-               return efi_status_to_err(status);
+                                  &entry->var.VendorGuid,
+                                  attributes, size, data);
 
-       return 0;
+       return efi_status_to_err(status);
 }
-EXPORT_SYMBOL_GPL(efivar_entry_size);
+EXPORT_SYMBOL_GPL(__efivar_entry_get);
 
 /**
  * efivar_entry_get - call get_variable()

diff --git a/include/linux/efi.h b/include/linux/efi.h
index 3f7257f1f5e80b138b2b8fc6ae0f642e69a549cc..2bc0ad78d058d29255b5e7c62d1a91ab0bffcf75 100644 (file)
--- a/include/linux/efi.h
+++ b/include/linux/efi.h
@@ -808,8 +808,9 @@ void efivar_entry_remove(struct efivar_entry *entry);
 int __efivar_entry_delete(struct efivar_entry *entry);
 int efivar_entry_delete(struct efivar_entry *entry);
 
-int __efivar_entry_size(struct efivar_entry *entry, unsigned long *size);
 int efivar_entry_size(struct efivar_entry *entry, unsigned long *size);
+int __efivar_entry_get(struct efivar_entry *entry, u32 *attributes,
+                      unsigned long *size, void *data);
 int efivar_entry_get(struct efivar_entry *entry, u32 *attributes,
                     unsigned long *size, void *data);
 int efivar_entry_set(struct efivar_entry *entry, u32 attributes,