Skip to content

Commit 7fb54ba

Browse files
Matt Fleminggregkh
authored andcommitted
efivars: Handle duplicate names from get_next_variable()
commit e971318 upstream. Some firmware exhibits a bug where the same VariableName and VendorGuid values are returned on multiple invocations of GetNextVariableName(). See, https://bugzilla.kernel.org/show_bug.cgi?id=47631 As a consequence of such a bug, Andre reports hitting the following WARN_ON() in the sysfs code after updating the BIOS on his, "Gigabyte Technology Co., Ltd. To be filled by O.E.M./Z77X-UD3H, BIOS F19e 11/21/2012)" machine, [ 0.581554] EFI Variables Facility v0.08 2004-May-17 [ 0.584914] ------------[ cut here ]------------ [ 0.585639] WARNING: at /home/andre/linux/fs/sysfs/dir.c:536 sysfs_add_one+0xd4/0x100() [ 0.586381] Hardware name: To be filled by O.E.M. [ 0.587123] sysfs: cannot create duplicate filename '/firmware/efi/vars/SbAslBufferPtrVar-01f33c25-764d-43ea-aeea-6b5a41f3f3e8' [ 0.588694] Modules linked in: [ 0.589484] Pid: 1, comm: swapper/0 Not tainted 3.8.0+ coolya#7 [ 0.590280] Call Trace: [ 0.591066] [<ffffffff81208954>] ? sysfs_add_one+0xd4/0x100 [ 0.591861] [<ffffffff810587bf>] warn_slowpath_common+0x7f/0xc0 [ 0.592650] [<ffffffff810588bc>] warn_slowpath_fmt+0x4c/0x50 [ 0.593429] [<ffffffff8134dd85>] ? strlcat+0x65/0x80 [ 0.594203] [<ffffffff81208954>] sysfs_add_one+0xd4/0x100 [ 0.594979] [<ffffffff81208b78>] create_dir+0x78/0xd0 [ 0.595753] [<ffffffff81208ec6>] sysfs_create_dir+0x86/0xe0 [ 0.596532] [<ffffffff81347e4c>] kobject_add_internal+0x9c/0x220 [ 0.597310] [<ffffffff81348307>] kobject_init_and_add+0x67/0x90 [ 0.598083] [<ffffffff81584a71>] ? efivar_create_sysfs_entry+0x61/0x1c0 [ 0.598859] [<ffffffff81584b2b>] efivar_create_sysfs_entry+0x11b/0x1c0 [ 0.599631] [<ffffffff8158517e>] register_efivars+0xde/0x420 [ 0.600395] [<ffffffff81d430a7>] ? edd_init+0x2f5/0x2f5 [ 0.601150] [<ffffffff81d4315f>] efivars_init+0xb8/0x104 [ 0.601903] [<ffffffff8100215a>] do_one_initcall+0x12a/0x180 [ 0.602659] [<ffffffff81d05d80>] kernel_init_freeable+0x13e/0x1c6 [ 0.603418] [<ffffffff81d05586>] ? loglevel+0x31/0x31 [ 0.604183] [<ffffffff816a6530>] ? rest_init+0x80/0x80 [ 0.604936] [<ffffffff816a653e>] kernel_init+0xe/0xf0 [ 0.605681] [<ffffffff816ce7ec>] ret_from_fork+0x7c/0xb0 [ 0.606414] [<ffffffff816a6530>] ? rest_init+0x80/0x80 [ 0.607143] ---[ end trace 1609741ab737eb29 ]--- There's not much we can do to work around and keep traversing the variable list once we hit this firmware bug. Our only solution is to terminate the loop because, as Lingzhu reports, some machines get stuck when they encounter duplicate names, > I had an IBM System x3100 M4 and x3850 X5 on which kernel would > get stuck in infinite loop creating duplicate sysfs files because, > for some reason, there are several duplicate boot entries in nvram > getting GetNextVariableName into a circle of iteration (with > period > 2). Also disable the workqueue, as efivar_update_sysfs_entries() uses GetNextVariableName() to figure out which variables have been created since the last iteration. That algorithm isn't going to work if GetNextVariableName() returns duplicates. Note that we don't disable EFI variable creation completely on the affected machines, it's just that any pstore dump-* files won't appear in sysfs until the next boot. [Backported for 3.0-stable. Removed code related to pstore workqueue but pulled in helper function variable_is_present from a93bc0c; Moved the definition of __efivars to the top for being referenced in variable_is_present.] Reported-by: Andre Heider <a.heider@gmail.com> Reported-by: Lingzhu Xiang <lxiang@redhat.com> Tested-by: Lingzhu Xiang <lxiang@redhat.com> Cc: Seiji Aguchi <seiji.aguchi@hds.com> Signed-off-by: Matt Fleming <matt.fleming@intel.com> Signed-off-by: Lingzhu Xiang <lxiang@redhat.com> Reviewed-by: CAI Qian <caiqian@redhat.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent c2ff015 commit 7fb54ba

1 file changed

Lines changed: 62 additions & 3 deletions

File tree

drivers/firmware/efivars.c

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,8 @@ struct efivar_attribute {
119119
ssize_t (*store)(struct efivar_entry *entry, const char *buf, size_t count);
120120
};
121121

122+
static struct efivars __efivars;
123+
static struct efivar_operations ops;
122124

123125
#define EFIVAR_ATTR(_name, _mode, _show, _store) \
124126
struct efivar_attribute efivar_attr_##_name = { \
@@ -730,6 +732,28 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
730732
return count;
731733
}
732734

735+
static bool variable_is_present(efi_char16_t *variable_name, efi_guid_t *vendor)
736+
{
737+
struct efivar_entry *entry, *n;
738+
struct efivars *efivars = &__efivars;
739+
unsigned long strsize1, strsize2;
740+
bool found = false;
741+
742+
strsize1 = utf16_strsize(variable_name, 1024);
743+
list_for_each_entry_safe(entry, n, &efivars->list, list) {
744+
strsize2 = utf16_strsize(entry->var.VariableName, 1024);
745+
if (strsize1 == strsize2 &&
746+
!memcmp(variable_name, &(entry->var.VariableName),
747+
strsize2) &&
748+
!efi_guidcmp(entry->var.VendorGuid,
749+
*vendor)) {
750+
found = true;
751+
break;
752+
}
753+
}
754+
return found;
755+
}
756+
733757
/*
734758
* Returns the size of variable_name, in bytes, including the
735759
* terminating NULL character, or variable_name_size if no NULL
@@ -942,6 +966,28 @@ void unregister_efivars(struct efivars *efivars)
942966
}
943967
EXPORT_SYMBOL_GPL(unregister_efivars);
944968

969+
/*
970+
* Print a warning when duplicate EFI variables are encountered and
971+
* disable the sysfs workqueue since the firmware is buggy.
972+
*/
973+
static void dup_variable_bug(efi_char16_t *s16, efi_guid_t *vendor_guid,
974+
unsigned long len16)
975+
{
976+
size_t i, len8 = len16 / sizeof(efi_char16_t);
977+
char *s8;
978+
979+
s8 = kzalloc(len8, GFP_KERNEL);
980+
if (!s8)
981+
return;
982+
983+
for (i = 0; i < len8; i++)
984+
s8[i] = s16[i];
985+
986+
printk(KERN_WARNING "efivars: duplicate variable: %s-%pUl\n",
987+
s8, vendor_guid);
988+
kfree(s8);
989+
}
990+
945991
int register_efivars(struct efivars *efivars,
946992
const struct efivar_operations *ops,
947993
struct kobject *parent_kobj)
@@ -984,6 +1030,22 @@ int register_efivars(struct efivars *efivars,
9841030
case EFI_SUCCESS:
9851031
variable_name_size = var_name_strnsize(variable_name,
9861032
variable_name_size);
1033+
1034+
/*
1035+
* Some firmware implementations return the
1036+
* same variable name on multiple calls to
1037+
* get_next_variable(). Terminate the loop
1038+
* immediately as there is no guarantee that
1039+
* we'll ever see a different variable name,
1040+
* and may end up looping here forever.
1041+
*/
1042+
if (variable_is_present(variable_name, &vendor_guid)) {
1043+
dup_variable_bug(variable_name, &vendor_guid,
1044+
variable_name_size);
1045+
status = EFI_NOT_FOUND;
1046+
break;
1047+
}
1048+
9871049
efivar_create_sysfs_entry(efivars,
9881050
variable_name_size,
9891051
variable_name,
@@ -1010,9 +1072,6 @@ int register_efivars(struct efivars *efivars,
10101072
}
10111073
EXPORT_SYMBOL_GPL(register_efivars);
10121074

1013-
static struct efivars __efivars;
1014-
static struct efivar_operations ops;
1015-
10161075
/*
10171076
* For now we register the efi subsystem with the firmware subsystem
10181077
* and the vars subsystem with the efi subsystem. In the future, it

0 commit comments

Comments
 (0)