From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org, Andre Heider <a.heider@gmail.com>,
Lingzhu Xiang <lxiang@redhat.com>,
Seiji Aguchi <seiji.aguchi@hds.com>,
Matt Fleming <matt.fleming@intel.com>,
CAI Qian <caiqian@redhat.com>
Subject: [ 37/56] efivars: Handle duplicate names from get_next_variable()
Date: Tue, 2 Apr 2013 15:50:04 -0700 [thread overview]
Message-ID: <20130402224716.181018832@linuxfoundation.org> (raw)
In-Reply-To: <20130402224711.840825715@linuxfoundation.org>
3.0-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.
[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>
---
drivers/firmware/efivars.c | 65 ++++++++++++++++++++++++++++++++++++++++++---
1 file changed, 62 insertions(+), 3 deletions(-)
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -119,6 +119,8 @@ 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 EFIVAR_ATTR(_name, _mode, _show, _store) \
struct efivar_attribute efivar_attr_##_name = { \
@@ -730,6 +732,28 @@ static ssize_t efivar_delete(struct file
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
@@ -942,6 +966,28 @@ 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;
+
+ 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)
@@ -984,6 +1030,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,
@@ -1010,9 +1072,6 @@ out:
}
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
next prev parent reply other threads:[~2013-04-02 22:50 UTC|newest]
Thread overview: 60+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-04-02 22:49 [ 00/56] 3.0.72-stable review Greg Kroah-Hartman
2013-04-02 22:49 ` [ 01/56] signal: Define __ARCH_HAS_SA_RESTORER so we know whether to clear sa_restorer Greg Kroah-Hartman
2013-04-02 22:49 ` [ 02/56] kernel/signal.c: use __ARCH_HAS_SA_RESTORER instead of SA_RESTORER Greg Kroah-Hartman
2013-04-02 22:49 ` [ 03/56] SUNRPC: Add barriers to ensure read ordering in rpc_wake_up_task_queue_locked Greg Kroah-Hartman
2013-04-02 22:49 ` [ 04/56] Bluetooth: Fix not closing SCO sockets in the BT_CONNECT2 state Greg Kroah-Hartman
2013-04-02 22:49 ` [ 05/56] Bluetooth: Add support for Dell[QCA 0cf3:0036] Greg Kroah-Hartman
2013-04-02 22:49 ` [ 06/56] Bluetooth: Add support for Dell[QCA 0cf3:817a] Greg Kroah-Hartman
2013-04-02 22:49 ` [ 07/56] staging: comedi: s626: fix continuous acquisition Greg Kroah-Hartman
2013-04-02 22:49 ` [ 08/56] sysfs: fix race between readdir and lseek Greg Kroah-Hartman
2013-04-02 22:49 ` [ 09/56] sysfs: handle failure path correctly for readdir() Greg Kroah-Hartman
2013-04-02 22:49 ` [ 10/56] b43: A fix for DMA transmission sequence errors Greg Kroah-Hartman
2013-04-02 22:49 ` [ 11/56] xen-blkback: fix dispatch_rw_block_io() error path Greg Kroah-Hartman
2013-04-02 22:49 ` [ 12/56] usb: ftdi_sio: Add support for Mitsubishi FX-USB-AW/-BD Greg Kroah-Hartman
2013-04-02 22:49 ` [ 13/56] vt: synchronize_rcu() under spinlock is not nice Greg Kroah-Hartman
2013-04-02 22:49 ` [ 14/56] mwifiex: cancel cmd timer and free curr_cmd in shutdown process Greg Kroah-Hartman
2013-04-06 19:55 ` Ben Hutchings
2013-04-08 17:58 ` Bing Zhao
2013-04-02 22:49 ` [ 15/56] net/irda: add missing error path release_sock call Greg Kroah-Hartman
2013-04-02 22:49 ` [ 16/56] usb: xhci: Fix TRB transfer length macro used for Event TRB Greg Kroah-Hartman
2013-04-02 22:49 ` [ 17/56] Btrfs: limit the global reserve to 512mb Greg Kroah-Hartman
2013-04-02 22:49 ` [ 18/56] KVM: Clean up error handling during VCPU creation Greg Kroah-Hartman
2013-04-02 22:49 ` [ 19/56] x25: Validate incoming call user data lengths Greg Kroah-Hartman
2013-04-02 22:49 ` [ 20/56] x25: Handle undersized/fragmented skbs Greg Kroah-Hartman
2013-04-02 22:49 ` [ 21/56] batman-adv: bat_socket_read missing checks Greg Kroah-Hartman
2013-04-02 22:49 ` [ 22/56] batman-adv: Only write requested number of byte to user buffer Greg Kroah-Hartman
2013-04-02 22:49 ` [ 23/56] KVM: x86: Prevent starting PIT timers in the absence of irqchip support Greg Kroah-Hartman
2013-04-02 22:49 ` [ 24/56] NFSv4: include bitmap in nfsv4 get acl data Greg Kroah-Hartman
2013-04-02 22:49 ` [ 25/56] NFSv4: Fix an Oops in the NFSv4 getacl code Greg Kroah-Hartman
2013-04-02 22:49 ` [ 26/56] NFS: nfs_getaclargs.acl_len is a size_t Greg Kroah-Hartman
2013-04-02 22:49 ` [ 27/56] KVM: Ensure all vcpus are consistent with in-kernel irqchip settings Greg Kroah-Hartman
2013-04-02 22:49 ` [ 28/56] macvtap: zerocopy: validate vectors before building skb Greg Kroah-Hartman
2013-04-02 22:49 ` [ 29/56] KVM: Fix buffer overflow in kvm_set_irq() Greg Kroah-Hartman
2013-04-02 22:49 ` [ 30/56] mm/hotplug: correctly add new zone to all other nodes zone lists Greg Kroah-Hartman
2013-04-02 22:49 ` [ 31/56] KVM: x86: invalid opcode oops on SET_SREGS with OSXSAVE bit set (CVE-2012-4461) Greg Kroah-Hartman
2013-04-02 22:49 ` [ 32/56] loop: prevent bdev freeing while device in use Greg Kroah-Hartman
2013-04-02 22:50 ` [ 33/56] nfsd4: reject "negative" acl lengths Greg Kroah-Hartman
2013-04-02 22:50 ` [ 34/56] drm/i915: dont set unpin_work if vblank_get fails Greg Kroah-Hartman
2013-04-02 22:50 ` [ 35/56] drm/i915: Dont clobber crtc->fb when queue_flip fails Greg Kroah-Hartman
2013-04-02 22:50 ` [ 36/56] efivars: explicitly calculate length of VariableName Greg Kroah-Hartman
2013-04-02 22:50 ` Greg Kroah-Hartman [this message]
2013-04-02 22:50 ` [ 38/56] ext4: use atomic64_t for the per-flexbg free_clusters count Greg Kroah-Hartman
2013-04-02 22:50 ` [ 39/56] tracing: Protect tracer flags with trace_types_lock Greg Kroah-Hartman
2013-04-02 22:50 ` [ 40/56] tracing: Prevent buffer overwrite disabled for latency tracers Greg Kroah-Hartman
2013-04-02 22:50 ` [ 41/56] sky2: Receive Overflows not counted Greg Kroah-Hartman
2013-04-02 22:50 ` [ 42/56] sky2: Threshold for Pause Packet is set wrong Greg Kroah-Hartman
2013-04-02 22:50 ` [ 43/56] tcp: preserve ACK clocking in TSO Greg Kroah-Hartman
2013-04-02 22:50 ` [ 44/56] tcp: undo spurious timeout after SACK reneging Greg Kroah-Hartman
2013-04-02 22:50 ` [ 45/56] 8021q: fix a potential use-after-free Greg Kroah-Hartman
2013-04-02 22:50 ` [ 46/56] thermal: shorten too long mcast group name Greg Kroah-Hartman
2013-04-02 22:50 ` [ 47/56] unix: fix a race condition in unix_release() Greg Kroah-Hartman
2013-04-02 22:50 ` [ 48/56] aoe: reserve enough headroom on skbs Greg Kroah-Hartman
2013-04-02 22:50 ` [ 49/56] drivers: net: ethernet: davinci_emac: use netif_wake_queue() while restarting tx queue Greg Kroah-Hartman
2013-04-02 22:50 ` [ 50/56] atl1e: drop pci-msi support because of packet corruption Greg Kroah-Hartman
2013-04-02 22:50 ` [ 51/56] ipv6: fix bad free of addrconf_init_net Greg Kroah-Hartman
2013-04-02 22:50 ` [ 52/56] ks8851: Fix interpretation of rxlen field Greg Kroah-Hartman
2013-04-02 22:50 ` [ 53/56] net: add a synchronize_net() in netdev_rx_handler_unregister() Greg Kroah-Hartman
2013-04-02 22:50 ` [ 54/56] pch_gbe: fix ip_summed checksum reporting on rx Greg Kroah-Hartman
2013-04-02 22:50 ` [ 55/56] smsc75xx: fix jumbo frame support Greg Kroah-Hartman
2013-04-02 22:50 ` [ 56/56] iommu/amd: Make sure dma_ops are set for hotplug devices Greg Kroah-Hartman
2013-04-03 15:19 ` [ 00/56] 3.0.72-stable review Shuah Khan
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20130402224716.181018832@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=a.heider@gmail.com \
--cc=caiqian@redhat.com \
--cc=linux-kernel@vger.kernel.org \
--cc=lxiang@redhat.com \
--cc=matt.fleming@intel.com \
--cc=seiji.aguchi@hds.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).