Commit ff534f03 authored by Josh Boyer's avatar Josh Boyer Committed by Greg Kroah-Hartman

efivars: Disable external interrupt while holding efivars->lock

commit 81fa4e58 upstream.

[Problem]
There is a scenario which efi_pstore fails to log messages in a panic case.

 - CPUA holds an efi_var->lock in either efivarfs parts
   or efi_pstore with interrupt enabled.
 - CPUB panics and sends IPI to CPUA in smp_send_stop().
 - CPUA stops with holding the lock.
 - CPUB kicks efi_pstore_write() via kmsg_dump(KSMG_DUMP_PANIC)
   but it returns without logging messages.

[Patch Description]
This patch disables an external interruption while holding efivars->lock
as follows.

In efi_pstore_write() and get_var_data(), spin_lock/spin_unlock is
replaced by spin_lock_irqsave/spin_unlock_irqrestore because they may
be called in an interrupt context.

In other functions, they are replaced by spin_lock_irq/spin_unlock_irq.
because they are all called from a process context.

By applying this patch, we can avoid the problem above with
a following senario.

 - CPUA holds an efi_var->lock with interrupt disabled.
 - CPUB panics and sends IPI to CPUA in smp_send_stop().
 - CPUA receives the IPI after releasing the lock because it is
   disabling interrupt while holding the lock.
 - CPUB waits for one sec until CPUA releases the lock.
 - CPUB kicks efi_pstore_write() via kmsg_dump(KSMG_DUMP_PANIC)
   And it can hold the lock successfully.
Signed-off-by: default avatarSeiji Aguchi <seiji.aguchi@hds.com>
Acked-by: default avatarMike Waychison <mikew@google.com>
Acked-by: default avatarMatt Fleming <matt.fleming@intel.com>
Signed-off-by: default avatarTony Luck <tony.luck@intel.com>
[bwh: Backported to 3.2:
 - Drop efivarfs changes
 - Adjust context
 - Drop change to efi_pstore_erase(), which is implemented using
   efi_pstore_write() here]
Signed-off-by: default avatarBen Hutchings <ben@decadent.org.uk>
Cc: Rui Xiang <rui.xiang@huawei.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 8e74ecf7
...@@ -396,10 +396,11 @@ static efi_status_t ...@@ -396,10 +396,11 @@ static efi_status_t
get_var_data(struct efivars *efivars, struct efi_variable *var) get_var_data(struct efivars *efivars, struct efi_variable *var)
{ {
efi_status_t status; efi_status_t status;
unsigned long flags;
spin_lock(&efivars->lock); spin_lock_irqsave(&efivars->lock, flags);
status = get_var_data_locked(efivars, var); status = get_var_data_locked(efivars, var);
spin_unlock(&efivars->lock); spin_unlock_irqrestore(&efivars->lock, flags);
if (status != EFI_SUCCESS) { if (status != EFI_SUCCESS) {
printk(KERN_WARNING "efivars: get_variable() failed 0x%lx!\n", printk(KERN_WARNING "efivars: get_variable() failed 0x%lx!\n",
...@@ -528,14 +529,14 @@ efivar_store_raw(struct efivar_entry *entry, const char *buf, size_t count) ...@@ -528,14 +529,14 @@ efivar_store_raw(struct efivar_entry *entry, const char *buf, size_t count)
return -EINVAL; return -EINVAL;
} }
spin_lock(&efivars->lock); spin_lock_irq(&efivars->lock);
status = efivars->ops->set_variable(new_var->VariableName, status = efivars->ops->set_variable(new_var->VariableName,
&new_var->VendorGuid, &new_var->VendorGuid,
new_var->Attributes, new_var->Attributes,
new_var->DataSize, new_var->DataSize,
new_var->Data); new_var->Data);
spin_unlock(&efivars->lock); spin_unlock_irq(&efivars->lock);
if (status != EFI_SUCCESS) { if (status != EFI_SUCCESS) {
printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n", printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n",
...@@ -646,7 +647,7 @@ static int efi_pstore_open(struct pstore_info *psi) ...@@ -646,7 +647,7 @@ static int efi_pstore_open(struct pstore_info *psi)
{ {
struct efivars *efivars = psi->data; struct efivars *efivars = psi->data;
spin_lock(&efivars->lock); spin_lock_irq(&efivars->lock);
efivars->walk_entry = list_first_entry(&efivars->list, efivars->walk_entry = list_first_entry(&efivars->list,
struct efivar_entry, list); struct efivar_entry, list);
return 0; return 0;
...@@ -656,7 +657,7 @@ static int efi_pstore_close(struct pstore_info *psi) ...@@ -656,7 +657,7 @@ static int efi_pstore_close(struct pstore_info *psi)
{ {
struct efivars *efivars = psi->data; struct efivars *efivars = psi->data;
spin_unlock(&efivars->lock); spin_unlock_irq(&efivars->lock);
return 0; return 0;
} }
...@@ -712,11 +713,12 @@ static int efi_pstore_write(enum pstore_type_id type, ...@@ -712,11 +713,12 @@ static int efi_pstore_write(enum pstore_type_id type,
int i, ret = 0; int i, ret = 0;
u64 storage_space, remaining_space, max_variable_size; u64 storage_space, remaining_space, max_variable_size;
efi_status_t status = EFI_NOT_FOUND; efi_status_t status = EFI_NOT_FOUND;
unsigned long flags;
sprintf(stub_name, "dump-type%u-%u-", type, part); sprintf(stub_name, "dump-type%u-%u-", type, part);
sprintf(name, "%s%lu", stub_name, get_seconds()); sprintf(name, "%s%lu", stub_name, get_seconds());
spin_lock(&efivars->lock); spin_lock_irqsave(&efivars->lock, flags);
/* /*
* Check if there is a space enough to log. * Check if there is a space enough to log.
...@@ -728,7 +730,7 @@ static int efi_pstore_write(enum pstore_type_id type, ...@@ -728,7 +730,7 @@ static int efi_pstore_write(enum pstore_type_id type,
&remaining_space, &remaining_space,
&max_variable_size); &max_variable_size);
if (status || remaining_space < size + DUMP_NAME_LEN * 2) { if (status || remaining_space < size + DUMP_NAME_LEN * 2) {
spin_unlock(&efivars->lock); spin_unlock_irqrestore(&efivars->lock, flags);
*id = part; *id = part;
return -ENOSPC; return -ENOSPC;
} }
...@@ -769,7 +771,7 @@ static int efi_pstore_write(enum pstore_type_id type, ...@@ -769,7 +771,7 @@ static int efi_pstore_write(enum pstore_type_id type,
efivars->ops->set_variable(efi_name, &vendor, PSTORE_EFI_ATTRIBUTES, efivars->ops->set_variable(efi_name, &vendor, PSTORE_EFI_ATTRIBUTES,
size, psi->buf); size, psi->buf);
spin_unlock(&efivars->lock); spin_unlock_irqrestore(&efivars->lock, flags);
if (found) if (found)
efivar_unregister(found); efivar_unregister(found);
...@@ -853,7 +855,7 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj, ...@@ -853,7 +855,7 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
return -EINVAL; return -EINVAL;
} }
spin_lock(&efivars->lock); spin_lock_irq(&efivars->lock);
/* /*
* Does this variable already exist? * Does this variable already exist?
...@@ -871,7 +873,7 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj, ...@@ -871,7 +873,7 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
} }
} }
if (found) { if (found) {
spin_unlock(&efivars->lock); spin_unlock_irq(&efivars->lock);
return -EINVAL; return -EINVAL;
} }
...@@ -885,10 +887,10 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj, ...@@ -885,10 +887,10 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
if (status != EFI_SUCCESS) { if (status != EFI_SUCCESS) {
printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n", printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n",
status); status);
spin_unlock(&efivars->lock); spin_unlock_irq(&efivars->lock);
return -EIO; return -EIO;
} }
spin_unlock(&efivars->lock); spin_unlock_irq(&efivars->lock);
/* Create the entry in sysfs. Locking is not required here */ /* Create the entry in sysfs. Locking is not required here */
status = efivar_create_sysfs_entry(efivars, status = efivar_create_sysfs_entry(efivars,
...@@ -916,7 +918,7 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj, ...@@ -916,7 +918,7 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
if (!capable(CAP_SYS_ADMIN)) if (!capable(CAP_SYS_ADMIN))
return -EACCES; return -EACCES;
spin_lock(&efivars->lock); spin_lock_irq(&efivars->lock);
/* /*
* Does this variable already exist? * Does this variable already exist?
...@@ -934,7 +936,7 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj, ...@@ -934,7 +936,7 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
} }
} }
if (!found) { if (!found) {
spin_unlock(&efivars->lock); spin_unlock_irq(&efivars->lock);
return -EINVAL; return -EINVAL;
} }
/* force the Attributes/DataSize to 0 to ensure deletion */ /* force the Attributes/DataSize to 0 to ensure deletion */
...@@ -950,12 +952,12 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj, ...@@ -950,12 +952,12 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
if (status != EFI_SUCCESS) { if (status != EFI_SUCCESS) {
printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n", printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n",
status); status);
spin_unlock(&efivars->lock); spin_unlock_irq(&efivars->lock);
return -EIO; return -EIO;
} }
list_del(&search_efivar->list); list_del(&search_efivar->list);
/* We need to release this lock before unregistering. */ /* We need to release this lock before unregistering. */
spin_unlock(&efivars->lock); spin_unlock_irq(&efivars->lock);
efivar_unregister(search_efivar); efivar_unregister(search_efivar);
/* It's dead Jim.... */ /* It's dead Jim.... */
...@@ -1110,9 +1112,9 @@ efivar_create_sysfs_entry(struct efivars *efivars, ...@@ -1110,9 +1112,9 @@ efivar_create_sysfs_entry(struct efivars *efivars,
kfree(short_name); kfree(short_name);
short_name = NULL; short_name = NULL;
spin_lock(&efivars->lock); spin_lock_irq(&efivars->lock);
list_add(&new_efivar->list, &efivars->list); list_add(&new_efivar->list, &efivars->list);
spin_unlock(&efivars->lock); spin_unlock_irq(&efivars->lock);
return 0; return 0;
} }
...@@ -1181,9 +1183,9 @@ void unregister_efivars(struct efivars *efivars) ...@@ -1181,9 +1183,9 @@ void unregister_efivars(struct efivars *efivars)
struct efivar_entry *entry, *n; struct efivar_entry *entry, *n;
list_for_each_entry_safe(entry, n, &efivars->list, list) { list_for_each_entry_safe(entry, n, &efivars->list, list) {
spin_lock(&efivars->lock); spin_lock_irq(&efivars->lock);
list_del(&entry->list); list_del(&entry->list);
spin_unlock(&efivars->lock); spin_unlock_irq(&efivars->lock);
efivar_unregister(entry); efivar_unregister(entry);
} }
if (efivars->new_var) if (efivars->new_var)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment