Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

efi_pstore: Introducing workqueue updating sysfs

[Problem]
efi_pstore creates sysfs entries, which enable users to access to NVRAM,
in a write callback. If a kernel panic happens in an interrupt context,
it may fail because it could sleep due to dynamic memory allocations during
creating sysfs entries.

[Patch Description]
This patch removes sysfs operations from a write callback by introducing
a workqueue updating sysfs entries which is scheduled after the write
callback is called.

Also, the workqueue is kicked in a just oops case.
A system will go down in other cases such as panic, clean shutdown and emergency
restart. And we don't need to create sysfs entries because there is no chance for
users to access to them.

efi_pstore will be robust against a kernel panic in an interrupt context with this patch.

Signed-off-by: Seiji Aguchi <seiji.aguchi@hds.com>
Acked-by: Matt Fleming <matt.fleming@intel.com>
Signed-off-by: Tony Luck <tony.luck@intel.com>

authored by

Seiji Aguchi and committed by
Tony Luck
a93bc0c6 81fa4e58

+82 -6
+80 -5
drivers/firmware/efivars.c
··· 158 158 efi_char16_t *variable_name, 159 159 efi_guid_t *vendor_guid); 160 160 161 + /* 162 + * Prototype for workqueue functions updating sysfs entry 163 + */ 164 + 165 + static void efivar_update_sysfs_entries(struct work_struct *); 166 + static DECLARE_WORK(efivar_work, efivar_update_sysfs_entries); 167 + 161 168 /* Return the number of unicode characters in data */ 162 169 static unsigned long 163 170 utf16_strnlen(efi_char16_t *s, size_t maxlength) ··· 1255 1248 1256 1249 spin_unlock_irqrestore(&efivars->lock, flags); 1257 1250 1258 - if (size) 1259 - ret = efivar_create_sysfs_entry(efivars, 1260 - utf16_strsize(efi_name, 1261 - DUMP_NAME_LEN * 2), 1262 - efi_name, &vendor); 1251 + if (reason == KMSG_DUMP_OOPS) 1252 + schedule_work(&efivar_work); 1263 1253 1264 1254 *id = part; 1265 1255 return ret; ··· 1498 1494 1499 1495 /* It's dead Jim.... */ 1500 1496 return count; 1497 + } 1498 + 1499 + static bool variable_is_present(efi_char16_t *variable_name, efi_guid_t *vendor) 1500 + { 1501 + struct efivar_entry *entry, *n; 1502 + struct efivars *efivars = &__efivars; 1503 + unsigned long strsize1, strsize2; 1504 + bool found = false; 1505 + 1506 + strsize1 = utf16_strsize(variable_name, 1024); 1507 + list_for_each_entry_safe(entry, n, &efivars->list, list) { 1508 + strsize2 = utf16_strsize(entry->var.VariableName, 1024); 1509 + if (strsize1 == strsize2 && 1510 + !memcmp(variable_name, &(entry->var.VariableName), 1511 + strsize2) && 1512 + !efi_guidcmp(entry->var.VendorGuid, 1513 + *vendor)) { 1514 + found = true; 1515 + break; 1516 + } 1517 + } 1518 + return found; 1519 + } 1520 + 1521 + static void efivar_update_sysfs_entries(struct work_struct *work) 1522 + { 1523 + struct efivars *efivars = &__efivars; 1524 + efi_guid_t vendor; 1525 + efi_char16_t *variable_name; 1526 + unsigned long variable_name_size = 1024; 1527 + efi_status_t status = EFI_NOT_FOUND; 1528 + bool found; 1529 + 1530 + /* Add new sysfs entries */ 1531 + while (1) { 1532 + variable_name = kzalloc(variable_name_size, GFP_KERNEL); 1533 + if (!variable_name) { 1534 + pr_err("efivars: Memory allocation failed.\n"); 1535 + return; 1536 + } 1537 + 1538 + spin_lock_irq(&efivars->lock); 1539 + found = false; 1540 + while (1) { 1541 + variable_name_size = 1024; 1542 + status = efivars->ops->get_next_variable( 1543 + &variable_name_size, 1544 + variable_name, 1545 + &vendor); 1546 + if (status != EFI_SUCCESS) { 1547 + break; 1548 + } else { 1549 + if (!variable_is_present(variable_name, 1550 + &vendor)) { 1551 + found = true; 1552 + break; 1553 + } 1554 + } 1555 + } 1556 + spin_unlock_irq(&efivars->lock); 1557 + 1558 + if (!found) { 1559 + kfree(variable_name); 1560 + break; 1561 + } else 1562 + efivar_create_sysfs_entry(efivars, 1563 + variable_name_size, 1564 + variable_name, &vendor); 1565 + } 1501 1566 } 1502 1567 1503 1568 /* ··· 1906 1833 static void __exit 1907 1834 efivars_exit(void) 1908 1835 { 1836 + cancel_work_sync(&efivar_work); 1837 + 1909 1838 if (efi_enabled) { 1910 1839 unregister_efivars(&__efivars); 1911 1840 kobject_put(efi_kobj);
+2 -1
include/linux/efi.h
··· 728 728 * 1) ->list - adds, removals, reads, writes 729 729 * 2) ops.[gs]et_variable() calls. 730 730 * It must not be held when creating sysfs entries or calling kmalloc. 731 - * ops.get_next_variable() is only called from register_efivars(), 731 + * ops.get_next_variable() is only called from register_efivars() 732 + * or efivar_update_sysfs_entries(), 732 733 * which is protected by the BKL, so that path is safe. 733 734 */ 734 735 spinlock_t lock;