Skip to content

Commit

Permalink
efivars: Handle duplicate names from get_next_variable()
Browse files Browse the repository at this point in the history 
commit e971318bbed610e28bb3fde9d548e6aaf0a6b02e 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+ franciscofranco#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.4-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 <[email protected]>
Reported-by: Lingzhu Xiang <[email protected]>
Tested-by: Lingzhu Xiang <[email protected]>
Cc: Seiji Aguchi <[email protected]>
Signed-off-by: Matt Fleming <[email protected]>
Signed-off-by: Lingzhu Xiang <[email protected]>
Reviewed-by: CAI Qian <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
  • Loading branch information
@gregkh
Matt Fleming authored and gregkh committed Apr 5, 2013
1 parent 5c44ddd commit 4025b05
Showing 1 changed file with 63 additions and 3 deletions.
66 changes: 63 additions & 3 deletions drivers/firmware/efivars.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,9 @@ struct efivar_attribute {
ssize_t (*store)(struct efivar_entry *entry, const char *buf, size_t count);
};

static struct efivars __efivars;
static struct efivar_operations ops;

#define PSTORE_EFI_ATTRIBUTES \
(EFI_VARIABLE_NON_VOLATILE | \
EFI_VARIABLE_BOOTSERVICE_ACCESS | \
Expand Down Expand Up @@ -942,6 +945,28 @@ static ssize_t efivar_delete(struct file *filp, struct kobject *kobj,
return count;
}

static bool variable_is_present(efi_char16_t *variable_name, efi_guid_t *vendor)
{
struct efivar_entry *entry, *n;
struct efivars *efivars = &__efivars;
unsigned long strsize1, strsize2;
bool found = false;

strsize1 = utf16_strsize(variable_name, 1024);
list_for_each_entry_safe(entry, n, &efivars->list, list) {
strsize2 = utf16_strsize(entry->var.VariableName, 1024);
if (strsize1 == strsize2 &&
!memcmp(variable_name, &(entry->var.VariableName),
strsize2) &&
!efi_guidcmp(entry->var.VendorGuid,
*vendor)) {
found = true;
break;
}
}
return found;
}

/*
* Returns the size of variable_name, in bytes, including the
* terminating NULL character, or variable_name_size if no NULL
Expand Down Expand Up @@ -1154,6 +1179,28 @@ void unregister_efivars(struct efivars *efivars)
}
EXPORT_SYMBOL_GPL(unregister_efivars);

/*
* Print a warning when duplicate EFI variables are encountered and
* disable the sysfs workqueue since the firmware is buggy.
*/
static void dup_variable_bug(efi_char16_t *s16, efi_guid_t *vendor_guid,
unsigned long len16)
{
size_t i, len8 = len16 / sizeof(efi_char16_t);
char *s8;

s8 = kzalloc(len8, GFP_KERNEL);
if (!s8)
return;

for (i = 0; i < len8; i++)
s8[i] = s16[i];

printk(KERN_WARNING "efivars: duplicate variable: %s-%pUl\n",
s8, vendor_guid);
kfree(s8);
}

int register_efivars(struct efivars *efivars,
const struct efivar_operations *ops,
struct kobject *parent_kobj)
Expand Down Expand Up @@ -1196,6 +1243,22 @@ int register_efivars(struct efivars *efivars,
case EFI_SUCCESS:
variable_name_size = var_name_strnsize(variable_name,
variable_name_size);

/*
* Some firmware implementations return the
* same variable name on multiple calls to
* get_next_variable(). Terminate the loop
* immediately as there is no guarantee that
* we'll ever see a different variable name,
* and may end up looping here forever.
*/
if (variable_is_present(variable_name, &vendor_guid)) {
dup_variable_bug(variable_name, &vendor_guid,
variable_name_size);
status = EFI_NOT_FOUND;
break;
}

efivar_create_sysfs_entry(efivars,
variable_name_size,
variable_name,
Expand Down Expand Up @@ -1232,9 +1295,6 @@ int register_efivars(struct efivars *efivars,
}
EXPORT_SYMBOL_GPL(register_efivars);

static struct efivars __efivars;
static struct efivar_operations ops;

/*
* For now we register the efi subsystem with the firmware subsystem
* and the vars subsystem with the efi subsystem. In the future, it
Expand Down

0 comments on commit 4025b05

Please sign in to comment.