lkml.org 
[lkml]   [2013]   [Mar]   [24]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[ 079/104] efivars: Handle duplicate names from get_next_variable()
    3.2-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Matt Fleming <matt.fleming@intel.com>

    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+ #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.

    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>
    [bwh: Backported to 3.2: reason is not checked in efi_pstore_write()]
    Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
    ---
    --- a/drivers/firmware/efivars.c
    +++ b/drivers/firmware/efivars.c
    @@ -159,6 +159,7 @@ efivar_create_sysfs_entry(struct efivars

    static void efivar_update_sysfs_entries(struct work_struct *);
    static DECLARE_WORK(efivar_work, efivar_update_sysfs_entries);
    +static bool efivar_wq_enabled = true;

    /* Return the number of unicode characters in data */
    static unsigned long
    @@ -842,7 +843,8 @@ static int efi_pstore_write(enum pstore_
    if (found)
    efivar_unregister(found);

    - schedule_work(&efivar_work);
    + if (efivar_wq_enabled)
    + schedule_work(&efivar_work);

    *id = part;
    return ret;
    @@ -1305,6 +1307,35 @@ void unregister_efivars(struct 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;
    +
    + /*
    + * Disable the workqueue since the algorithm it uses for
    + * detecting new variables won't work with this buggy
    + * implementation of GetNextVariableName().
    + */
    + efivar_wq_enabled = false;
    +
    + 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)
    @@ -1347,6 +1378,22 @@ int register_efivars(struct efivars *efi
    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,



    \
     
     \ /
      Last update: 2013-03-25 02:41    [W:3.638 / U:0.204 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site