This is a note to let you know that I've just added the patch titled efivars: Disable external interrupt while holding efivars->lock to the 3.4-stable tree which can be found at: http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary The filename of the patch is: efivars-disable-external-interrupt-while-holding-efivars-lock.patch and it can be found in the queue-3.4 subdirectory. If you, or anyone else, feels it should not be added to the stable tree, please let <stable@xxxxxxxxxxxxxxx> know about it. >From 3b048fc196139b840d79b316405bede9b5d3c4c2 Mon Sep 17 00:00:00 2001 From: Josh Boyer <jwboyer@xxxxxxxxxx> Date: Mon, 11 Mar 2013 17:47:42 -0400 Subject: efivars: Disable external interrupt while holding efivars->lock From: Josh Boyer <jwboyer@xxxxxxxxxx> commit 81fa4e581d9283f7992a0d8c534bb141eb840a14 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: Seiji Aguchi <seiji.aguchi@xxxxxxx> Acked-by: Mike Waychison <mikew@xxxxxxxxxx> Acked-by: Matt Fleming <matt.fleming@xxxxxxxxx> Signed-off-by: Tony Luck <tony.luck@xxxxxxxxx> [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: Ben Hutchings <ben@xxxxxxxxxxxxxxx> Cc: Rui Xiang <rui.xiang@xxxxxxxxxx> Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx> --- drivers/firmware/efivars.c | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -396,10 +396,11 @@ static efi_status_t get_var_data(struct efivars *efivars, struct efi_variable *var) { efi_status_t status; + unsigned long flags; - spin_lock(&efivars->lock); + spin_lock_irqsave(&efivars->lock, flags); status = get_var_data_locked(efivars, var); - spin_unlock(&efivars->lock); + spin_unlock_irqrestore(&efivars->lock, flags); if (status != EFI_SUCCESS) { printk(KERN_WARNING "efivars: get_variable() failed 0x%lx!\n", @@ -528,14 +529,14 @@ efivar_store_raw(struct efivar_entry *en return -EINVAL; } - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); status = efivars->ops->set_variable(new_var->VariableName, &new_var->VendorGuid, new_var->Attributes, new_var->DataSize, new_var->Data); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); if (status != EFI_SUCCESS) { printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n", @@ -646,7 +647,7 @@ static int efi_pstore_open(struct pstore { struct efivars *efivars = psi->data; - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); efivars->walk_entry = list_first_entry(&efivars->list, struct efivar_entry, list); return 0; @@ -656,7 +657,7 @@ static int efi_pstore_close(struct pstor { struct efivars *efivars = psi->data; - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); return 0; } @@ -712,11 +713,12 @@ static int efi_pstore_write(enum pstore_ int i, ret = 0; u64 storage_space, remaining_space, max_variable_size; efi_status_t status = EFI_NOT_FOUND; + unsigned long flags; sprintf(stub_name, "dump-type%u-%u-", type, part); 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. @@ -728,7 +730,7 @@ static int efi_pstore_write(enum pstore_ &remaining_space, &max_variable_size); if (status || remaining_space < size + DUMP_NAME_LEN * 2) { - spin_unlock(&efivars->lock); + spin_unlock_irqrestore(&efivars->lock, flags); *id = part; return -ENOSPC; } @@ -769,7 +771,7 @@ static int efi_pstore_write(enum pstore_ efivars->ops->set_variable(efi_name, &vendor, PSTORE_EFI_ATTRIBUTES, size, psi->buf); - spin_unlock(&efivars->lock); + spin_unlock_irqrestore(&efivars->lock, flags); if (found) efivar_unregister(found); @@ -853,7 +855,7 @@ static ssize_t efivar_create(struct file return -EINVAL; } - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); /* * Does this variable already exist? @@ -871,7 +873,7 @@ static ssize_t efivar_create(struct file } } if (found) { - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); return -EINVAL; } @@ -885,10 +887,10 @@ static ssize_t efivar_create(struct file if (status != EFI_SUCCESS) { printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n", status); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); return -EIO; } - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); /* Create the entry in sysfs. Locking is not required here */ status = efivar_create_sysfs_entry(efivars, @@ -916,7 +918,7 @@ static ssize_t efivar_delete(struct file if (!capable(CAP_SYS_ADMIN)) return -EACCES; - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); /* * Does this variable already exist? @@ -934,7 +936,7 @@ static ssize_t efivar_delete(struct file } } if (!found) { - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); return -EINVAL; } /* force the Attributes/DataSize to 0 to ensure deletion */ @@ -950,12 +952,12 @@ static ssize_t efivar_delete(struct file if (status != EFI_SUCCESS) { printk(KERN_WARNING "efivars: set_variable() failed: status=%lx\n", status); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); return -EIO; } list_del(&search_efivar->list); /* We need to release this lock before unregistering. */ - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); efivar_unregister(search_efivar); /* It's dead Jim.... */ @@ -1110,9 +1112,9 @@ efivar_create_sysfs_entry(struct efivars kfree(short_name); short_name = NULL; - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); list_add(&new_efivar->list, &efivars->list); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); return 0; } @@ -1181,9 +1183,9 @@ void unregister_efivars(struct efivars * struct efivar_entry *entry, *n; list_for_each_entry_safe(entry, n, &efivars->list, list) { - spin_lock(&efivars->lock); + spin_lock_irq(&efivars->lock); list_del(&entry->list); - spin_unlock(&efivars->lock); + spin_unlock_irq(&efivars->lock); efivar_unregister(entry); } if (efivars->new_var) Patches currently in stable-queue which might be from jwboyer@xxxxxxxxxx are queue-3.4/efi-be-more-paranoid-about-available-space-when-creating-variables.patch queue-3.4/efivars-disable-external-interrupt-while-holding-efivars-lock.patch queue-3.4/efivars-allow-disabling-use-as-a-pstore-backend.patch -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html