sysfs: cannot create duplicate filename

sysfs: cannot create duplicate filename

[Andre Heider]

Hi,

After a BIOS update I get this in dmesg:

[    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+0x
d4/0x100()
[    0.586381] Hardware name: To be filled by O.E.M.
[    0.587123] sysfs: cannot create duplicate filename '/firmware/efi/vars/SbAsl
BufferPtrVar-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 ]---
[    0.607869] ------------[ cut here ]------------
[    0.608589] WARNING: at /home/andre/linux/lib/kobject.c:196
kobject_add_internal+0x204/0x220()
[    0.609342] Hardware name: To be filled by O.E.M.
[    0.610093] kobject_add_internal failed for
SbAslBufferPtrVar-01f33c25-764d-43ea-aeea-6b5a41f3f3e8 with -EEXIST,
don't try to register things with the same name in the same directory.
[    0.611703] Modules linked in:
[    0.612497] Pid: 1, comm: swapper/0 Tainted: G        W    3.8.0+ #7
[    0.613312] Call Trace:
[    0.614112]  [<ffffffff81347fb4>] ? kobject_add_internal+0x204/0x220
[    0.614931]  [<ffffffff810587bf>] warn_slowpath_common+0x7f/0xc0
[    0.615753]  [<ffffffff810588bc>] warn_slowpath_fmt+0x4c/0x50
[    0.616559]  [<ffffffff81347fb4>] kobject_add_internal+0x204/0x220
[    0.617362]  [<ffffffff81348307>] kobject_init_and_add+0x67/0x90
[    0.618152]  [<ffffffff81584a71>] ? efivar_create_sysfs_entry+0x61/0x1c0
[    0.618936]  [<ffffffff81584b2b>] efivar_create_sysfs_entry+0x11b/0x1c0
[    0.619732]  [<ffffffff8158517e>] register_efivars+0xde/0x420
[    0.620518]  [<ffffffff81d430a7>] ? edd_init+0x2f5/0x2f5
[    0.621297]  [<ffffffff81d4315f>] efivars_init+0xb8/0x104
[    0.622072]  [<ffffffff8100215a>] do_one_initcall+0x12a/0x180
[    0.622836]  [<ffffffff81d05d80>] kernel_init_freeable+0x13e/0x1c6
[    0.623590]  [<ffffffff81d05586>] ? loglevel+0x31/0x31
[    0.624314]  [<ffffffff816a6530>] ? rest_init+0x80/0x80
[    0.625010]  [<ffffffff816a653e>] kernel_init+0xe/0xf0
[    0.625703]  [<ffffffff816ce7ec>] ret_from_fork+0x7c/0xb0
[    0.626393]  [<ffffffff816a6530>] ? rest_init+0x80/0x80
[    0.627078] ---[ end trace 1609741ab737eb2a ]---

Sounds like yet another BIOS issue, and I guess the answer is
"complain to the manufacturer"... But we know how well that works, so
can this be a warning instead of a scary backtrace? :)

(this is: Gigabyte Technology Co., Ltd. To be filled by
O.E.M./Z77X-UD3H, BIOS F19e 11/21/2012)"

Thanks,
Andre

Re: sysfs: cannot create duplicate filename

[Lingzhu Xiang]

On 03/03/2013 02:03 AM, Andre Heider wrote:
> After a BIOS update I get this in dmesg:
> 
> [    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+0x
> d4/0x100()
> [    0.586381] Hardware name: To be filled by O.E.M.
> [    0.587123] sysfs: cannot create duplicate filename '/firmware/efi/vars/SbAsl
> BufferPtrVar-01f33c25-764d-43ea-aeea-6b5a41f3f3e8'
> (snip)
> 
> Sounds like yet another BIOS issue, and I guess the answer is
> "complain to the manufacturer"... But we know how well that works, so
> can this be a warning instead of a scary backtrace? :)
> 
> (this is: Gigabyte Technology Co., Ltd. To be filled by
> O.E.M./Z77X-UD3H, BIOS F19e 11/21/2012)"

Just saw https://bugzilla.kernel.org/show_bug.cgi?id=47631

I think this problem is getting more and more common.

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).

Granted, it's not kernel's fault for duplicate variables, but it'd be much
harder for users to fix their broken firmware than kernel dealing with it
by not dying on the never-ending GetNextVariableName.


Lingzhu Xiang

Re: sysfs: cannot create duplicate filename

[Matt Fleming]

On Tue, 2013-03-05 at 17:39 +0800, Lingzhu Xiang wrote:
> On 03/03/2013 02:03 AM, Andre Heider wrote:
> > After a BIOS update I get this in dmesg:
> > 
> > [    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+0x
> > d4/0x100()
> > [    0.586381] Hardware name: To be filled by O.E.M.
> > [    0.587123] sysfs: cannot create duplicate filename '/firmware/efi/vars/SbAsl
> > BufferPtrVar-01f33c25-764d-43ea-aeea-6b5a41f3f3e8'
> > (snip)
> > 
> > Sounds like yet another BIOS issue, and I guess the answer is
> > "complain to the manufacturer"... But we know how well that works, so
> > can this be a warning instead of a scary backtrace? :)
> > 
> > (this is: Gigabyte Technology Co., Ltd. To be filled by
> > O.E.M./Z77X-UD3H, BIOS F19e 11/21/2012)"
> 
> Just saw https://bugzilla.kernel.org/show_bug.cgi?id=47631
> 
> I think this problem is getting more and more common.
> 
> 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).
> 
> Granted, it's not kernel's fault for duplicate variables, but it'd be much
> harder for users to fix their broken firmware than kernel dealing with it
> by not dying on the never-ending GetNextVariableName.

It's not just about the manufacturers fixing their firmware and
distributing a BIOS update. It's also the fact few users are likely to
apply it before installing Linux on a new machine.

Yeah, we need something in the kernel to handle this.

Thanks for the report guys.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: sysfs: cannot create duplicate filename

[shea-yfkUTty7RcRWk0Htik3J/w]

Hi Matt,

On 2013-03-06 08:19, Matt Fleming wrote:
> On Tue, 2013-03-05 at 17:39 +0800, Lingzhu Xiang wrote:
>> On 03/03/2013 02:03 AM, Andre Heider wrote:
>> > After a BIOS update I get this in dmesg:
>> >
>> > [    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+0x
>> > d4/0x100()
>> > [    0.586381] Hardware name: To be filled by O.E.M.
>> > [    0.587123] sysfs: cannot create duplicate filename 
>> '/firmware/efi/vars/SbAsl
>> > BufferPtrVar-01f33c25-764d-43ea-aeea-6b5a41f3f3e8'
>> > (snip)
>> >
>> > Sounds like yet another BIOS issue, and I guess the answer is
>> > "complain to the manufacturer"... But we know how well that works, 
>> so
>> > can this be a warning instead of a scary backtrace? :)
>> >
>> > (this is: Gigabyte Technology Co., Ltd. To be filled by
>> > O.E.M./Z77X-UD3H, BIOS F19e 11/21/2012)"
>>
>> Just saw https://bugzilla.kernel.org/show_bug.cgi?id=47631
>>
>> I think this problem is getting more and more common.
>>
>> 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).
>>
>> Granted, it's not kernel's fault for duplicate variables, but it'd 
>> be much
>> harder for users to fix their broken firmware than kernel dealing 
>> with it
>> by not dying on the never-ending GetNextVariableName.
>
> It's not just about the manufacturers fixing their firmware and
> distributing a BIOS update. It's also the fact few users are likely 
> to
> apply it before installing Linux on a new machine.
>
> Yeah, we need something in the kernel to handle this.
>

Unrelated to the initial issue, but with the growing number of fixes 
for buggy firmware out there, maybe we should consider an approach 
similar to CONFIG_PCI_QUIRKS and the separate drivers/pci/quirks.c, 
where all of the system specific EFI quirks are isolated and can be 
compiled out if you know your firmware is good?

Also might be nice to have a separate config option specifically for 
all the Apple quirks, as their EFI seems to be the worst out there with 
respect to following standards.

>
> Thanks for the report guys.

Re: sysfs: cannot create duplicate filename

[Matt Fleming]

On Wed, 2013-03-06 at 08:35 -0500, shea-yfkUTty7RcRWk0Htik3J/w@public.gmane.org wrote:
> Unrelated to the initial issue, but with the growing number of fixes 
> for buggy firmware out there, maybe we should consider an approach 
> similar to CONFIG_PCI_QUIRKS and the separate drivers/pci/quirks.c, 
> where all of the system specific EFI quirks are isolated and can be 
> compiled out if you know your firmware is good?

Right, the firmware bug spectrum is indeed starting to grow rapidly. I
was hoping we could avoid doing something like PCI quirks because that
whole thing is a mess and I am concerned about an explosion of quirks
that would need to be maintained, some being used in core architecture
code. Upto now, we've been catering for the least common denominator,
which has worked, but obviously isn't ideal.

My plan has always been to try and hold off on introducing something
like quirks until my hand is forced. But it is likely that will happen
sooner rather than later. 

One problem we have though, is clearly identifying machines that suffer
from these bugs because of the cross-pollination of firmware from
Independent BIOS Vendors (IBVs) and OEMs. In theory, we could use the
FirmwareRevision field in the EFI System Table, along with DMI strings
and such.

> Also might be nice to have a separate config option specifically for 
> all the Apple quirks, as their EFI seems to be the worst out there with 
> respect to following standards.

AFAIK, it's that they're based on EFI 1.10, so pre-UEFI days.

-- 
Matt Fleming, Intel Open Source Technology Center

Re: sysfs: cannot create duplicate filename

[Matt Fleming]

On Tue, 2013-03-05 at 17:39 +0800, Lingzhu Xiang wrote:
> On 03/03/2013 02:03 AM, Andre Heider wrote:
> > After a BIOS update I get this in dmesg:
> > 
> > [    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+0x
> > d4/0x100()
> > [    0.586381] Hardware name: To be filled by O.E.M.
> > [    0.587123] sysfs: cannot create duplicate filename '/firmware/efi/vars/SbAsl
> > BufferPtrVar-01f33c25-764d-43ea-aeea-6b5a41f3f3e8'
> > (snip)
> > 
> > Sounds like yet another BIOS issue, and I guess the answer is
> > "complain to the manufacturer"... But we know how well that works, so
> > can this be a warning instead of a scary backtrace? :)
> > 
> > (this is: Gigabyte Technology Co., Ltd. To be filled by
> > O.E.M./Z77X-UD3H, BIOS F19e 11/21/2012)"
> 
> Just saw https://bugzilla.kernel.org/show_bug.cgi?id=47631
> 
> I think this problem is getting more and more common.
> 
> 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).
> 
> Granted, it's not kernel's fault for duplicate variables, but it'd be much
> harder for users to fix their broken firmware than kernel dealing with it
> by not dying on the never-ending GetNextVariableName.

Guys, could you test the following patch?

---

>From bb94d021f900a990def5891a7cb480cc6d3e5247 Mon Sep 17 00:00:00 2001
From: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Date: Thu, 7 Mar 2013 11:59:14 +0000
Subject: [PATCH] efivars: Handle duplicate names from get_next_variable()

Some firmware exhibits a bug where the same VariableName and
VendorGuid values are returned on consecutive 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).

Reported-by: Andre Heider <a.heider-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reported-by: Lingzhu Xiang <lxiang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Signed-off-by: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/firmware/efivars.c | 29 ++++++++++++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index bea32d1..ea415df 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -1962,8 +1962,9 @@ int register_efivars(struct efivars *efivars,
 		     struct kobject *parent_kobj)
 {
 	efi_status_t status = EFI_NOT_FOUND;
-	efi_guid_t vendor_guid;
+	efi_guid_t vendor_guid, prev_guid;
 	efi_char16_t *variable_name;
+	efi_char16_t *prev_name = NULL;
 	unsigned long variable_name_size = 1024;
 	int error = 0;
 
@@ -1973,6 +1974,13 @@ int register_efivars(struct efivars *efivars,
 		return -ENOMEM;
 	}
 
+	prev_name = kzalloc(variable_name_size, GFP_KERNEL);
+	if (!prev_name) {
+		printk(KERN_ERR "efivars: Memory allocation failed.\n");
+		error = -ENOMEM;
+		goto out;
+	}
+
 	spin_lock_init(&efivars->lock);
 	INIT_LIST_HEAD(&efivars->list);
 	efivars->ops = ops;
@@ -2005,6 +2013,21 @@ int register_efivars(struct efivars *efivars,
 						&vendor_guid);
 		switch (status) {
 		case EFI_SUCCESS:
+			/*
+			 * Some firmware implementations return the
+			 * same variable name on consecutive 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 (!efi_guidcmp(prev_guid, vendor_guid) &&
+			     !memcmp(prev_name, variable_name, variable_name_size)) {
+				printk(KERN_WARNING "efivars: duplicate variable name.\n");
+				status = EFI_NOT_FOUND;
+				break;
+			}
+
 			efivar_create_sysfs_entry(efivars,
 						  variable_name_size,
 						  variable_name,
@@ -2018,6 +2041,9 @@ int register_efivars(struct efivars *efivars,
 			status = EFI_NOT_FOUND;
 			break;
 		}
+
+		memcpy(prev_name, variable_name, variable_name_size);
+		memcpy(&prev_guid, &vendor_guid, sizeof(efi_guid_t));
 	} while (status != EFI_NOT_FOUND);
 
 	error = create_efivars_bin_attributes(efivars);
@@ -2037,6 +2063,7 @@ int register_efivars(struct efivars *efivars,
 	register_filesystem(&efivarfs_type);
 
 out:
+	kfree(prev_name);
 	kfree(variable_name);
 
 	return error;
-- 
1.7.11.7

Re: sysfs: cannot create duplicate filename

[Lingzhu Xiang]

On 03/08/2013 11:11 PM, Matt Fleming wrote:
> +			/*
> +			 * Some firmware implementations return the
> +			 * same variable name on consecutive 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 (!efi_guidcmp(prev_guid, vendor_guid) &&
> +			     !memcmp(prev_name, variable_name, variable_name_size)) {
> +				printk(KERN_WARNING "efivars: duplicate variable name.\n");
> +				status = EFI_NOT_FOUND;
> +				break;
> +			}

Comparing with the immediately previous one won't detect larger loop
like how The IBM x3100 M4 in question was looping as below.

(There is some log loss; probably too much data for serial.)
[    9.783119] sysfs: cannot create duplicate filename '/firmware/efi/vars/MemoryTypeInformation-4c19049f-4137-4dd3-9c10-8b97a83ffdfa'
[    9.852017] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0003-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[    9.856520] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0003-d665f20a-f2b7-485e-a321-542a196f40c7'
[    9.860840] sysfs: cannot create duplicate filename '/firmware/efi/vars/AcpiGlobalVariable-c020489e-6db2-4ef2-9aa5-ca06fc11d36a'
[    9.865735] sysfs: cannot create duplicate filename '/firmware/efi/vars/NodalSetup-ec87d643-eba4-4bb5-a1e5-3f3e36b20da9'
[    9.871157] sysfs: cannot create duplicate filename '/firmware/efi/vars/DIMMStatusVariable-c020489e-6db2-4fe2-a2b1-ca05cca1336a'
[    9.876473] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0007-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   11.541964] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBootValidate-d665f20a-f2b7-485e-a321-542a196f40c7'
[   11.556008] sysfs: cannot create duplicate filename '/firmware/efi/vars/MemoryTypeInformation-4c19049f-4137-4dd3-9c10-8b97a83ffdfa'
[   11.560022] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0003-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   11.564583] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0003-d665f20a-f2b7-485e-a321-542a196f40c7'
[   11.568977] sysfs: cannot create duplicate filename '/firmware/efi/vars/AcpiGlobalVariable-c020489e-6db2-4ef2-9aa5-ca06fc11d36a'
[   11.573929] sysfs: cannot create duplicate filename '/firmware/efi/vars/NodalSetup-ec87d643-eba4-4bb5-a1e5-3f3e36b20da9'
[   13.278655] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBootValidate-d665f20a-f2b7-485e-a321-542a196f40c7'
[   13.292703] sysfs: cannot create duplicate filename '/firmware/efi/vars/MemoryTypeInformation-4c19049f-4137-4dd3-9c10-8b97a83ffdfa'
[   13.296716] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0003-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   13.301283] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0003-d665f20a-f2b7-485e-a321-542a196f40c7'
[   13.305669] sysfs: cannot create duplicate filename '/firmware/efi/vars/AcpiGlobalVariable-c020489e-6db2-4ef2-9aa5-ca06fc11d36a'
[   13.310628] sysfs: cannot create duplicate filename '/firmware/efi/vars/NodalSetup-ec87d643-eba4-4bb5-a1e5-3f3e36b20da9'
[   13.316108] sysfs: cannot create duplicate filename '/firmware/efi/vars/DIMMStatusVariable-c020489e-6db2-4fe2-a2b1-ca05cca1336a'
[   15.004171] sysfs: cannot create duplicate filename '/firmware/efi/vars/BootOrder-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   15.017737] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBootValidate-d665f20a-f2b7-485e-a321-542a196f40c7'
[   15.031713] sysfs: cannot create duplicate filename '/firmware/efi/vars/MemoryTypeInformation-4c19049f-4137-4dd3-9c10-8b97a83ffdfa'
[   15.035730] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0003-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   15.040355] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0003-d665f20a-f2b7-485e-a321-542a196f40c7'
[   15.044743] sysfs: cannot create duplicate filename '/firmware/efi/vars/AcpiGlobalVariable-c020489e-6db2-4ef2-9aa5-ca06fc11d36a'
[   15.049700] sysfs: cannot create duplicate filename '/firmware/efi/vars/NodalSetup-ec87d643-eba4-4bb5-a1e5-3f3e36b20da9'
[   16.729287] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0001-d665f20a-f2b7-485e-a321-542a196f40c7'
[   16.742919] sysfs: cannot create duplicate filename '/firmware/efi/vars/BootOrder-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   16.756550] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBootValidate-d665f20a-f2b7-485e-a321-542a196f40c7'
[   16.770526] sysfs: cannot create duplicate filename '/firmware/efi/vars/MemoryTypeInformation-4c19049f-4137-4dd3-9c10-8b97a83ffdfa'
[   16.774532] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0003-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   16.779093] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0003-d665f20a-f2b7-485e-a321-542a196f40c7'
[   16.783542] sysfs: cannot create duplicate filename '/firmware/efi/vars/AcpiGlobalVariable-c020489e-6db2-4ef2-9aa5-ca06fc11d36a'
[   18.467922] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0001-d665f20a-f2b7-485e-a321-542a196f40c7'
[   18.481615] sysfs: cannot create duplicate filename '/firmware/efi/vars/BootOrder-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   18.495194] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBootValidate-d665f20a-f2b7-485e-a321-542a196f40c7'
[   18.509242] sysfs: cannot create duplicate filename '/firmware/efi/vars/MemoryTypeInformation-4c19049f-4137-4dd3-9c10-8b97a83ffdfa'
[   18.513252] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0003-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   18.517806] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0003-d665f20a-f2b7-485e-a321-542a196f40c7'
[   20.193306] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0001-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   20.206649] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0001-d665f20a-f2b7-485e-a321-542a196f40c7'
[   20.220341] sysfs: cannot create duplicate filename '/firmware/efi/vars/BootOrder-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   20.233916] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBootValidate-d665f20a-f2b7-485e-a321-542a196f40c7'
[   20.247957] sysfs: cannot create duplicate filename '/firmware/efi/vars/MemoryTypeInformation-4c19049f-4137-4dd3-9c10-8b97a83ffdfa'
[   20.251970] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0003-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   21.918960] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0004-d665f20a-f2b7-485e-a321-542a196f40c7'
[   21.931944] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0001-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   21.945286] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0001-d665f20a-f2b7-485e-a321-542a196f40c7'
[   21.958984] sysfs: cannot create duplicate filename '/firmware/efi/vars/BootOrder-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   21.972559] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBootValidate-d665f20a-f2b7-485e-a321-542a196f40c7'
[   21.986608] sysfs: cannot create duplicate filename '/firmware/efi/vars/MemoryTypeInformation-4c19049f-4137-4dd3-9c10-8b97a83ffdfa'
[   23.644961] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0004-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   23.657688] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0004-d665f20a-f2b7-485e-a321-542a196f40c7'
[   23.670724] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0001-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   23.684062] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0001-d665f20a-f2b7-485e-a321-542a196f40c7'
[   23.697692] sysfs: cannot create duplicate filename '/firmware/efi/vars/BootOrder-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   23.711324] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBootValidate-d665f20a-f2b7-485e-a321-542a196f40c7'
[   25.370482] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0006-d665f20a-f2b7-485e-a321-542a196f40c7'
[   25.382749] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0004-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   25.395419] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0004-d665f20a-f2b7-485e-a321-542a196f40c7'
[   25.408343] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0001-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   25.421625] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0001-d665f20a-f2b7-485e-a321-542a196f40c7'
[   25.435192] sysfs: cannot create duplicate filename '/firmware/efi/vars/BootOrder-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   27.087790] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0006-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   27.099845] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0006-d665f20a-f2b7-485e-a321-542a196f40c7'
[   27.112118] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0004-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   27.124788] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0004-d665f20a-f2b7-485e-a321-542a196f40c7'
[   27.137702] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0001-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   27.150990] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0001-d665f20a-f2b7-485e-a321-542a196f40c7'
[   28.805316] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0005-d665f20a-f2b7-485e-a321-542a196f40c7'
[   28.816944] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0006-8be4df61-93ca-11d2-aa0d-00e098032b8c'
[   28.829010] sysfs: cannot create duplicate filename '/firmware/efi/vars/CFGBoot0006-d665f20a-f2b7-485e-a321-542a196f40c7'
[   28.841278] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0004-8be4df61-93ca-11d2-aa0d-00e098032b8c'

Re: sysfs: cannot create duplicate filename

[Matt Fleming]

On Sat, 2013-03-09 at 02:17 +0800, Lingzhu Xiang wrote:
> Comparing with the immediately previous one won't detect larger loop
> like how The IBM x3100 M4 in question was looping as below.
> 
> (There is some log loss; probably too much data for serial.)
> [    9.783119] sysfs: cannot create duplicate filename '/firmware/efi/vars/MemoryTypeInformation-4c19049f-4137-4dd3-9c10-8b97a83ffdfa'
> [    9.852017] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0003-8be4df61-93ca-11d2-aa0d-00e098032b8c'

Ah, thanks. I misunderstood the bug.

How about something like this? We're just going to have to suck it up
and iterate the entire list. This could be made more optimal, but this
isn't a fast path anyway, so I'm not sure it's worth optimising
immediately.

---

>From c8b6c9c175f4459197e2a13f29a2e4125372d771 Mon Sep 17 00:00:00 2001
From: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Date: Thu, 7 Mar 2013 11:59:14 +0000
Subject: [PATCH v2] efivars: Handle duplicate names from get_next_variable()

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).

Reported-by: Andre Heider <a.heider-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reported-by: Lingzhu Xiang <lxiang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Signed-off-by: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---

v2: iterate the entire list when checking for duplicates.

 drivers/firmware/efivars.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index bea32d1..d1656e3 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -2005,6 +2005,20 @@ int register_efivars(struct efivars *efivars,
 						&vendor_guid);
 		switch (status) {
 		case EFI_SUCCESS:
+			/*
+			 * 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)) {
+				printk(KERN_WARNING "efivars: duplicate variable name.\n");
+				status = EFI_NOT_FOUND;
+				break;
+			}
+
 			efivar_create_sysfs_entry(efivars,
 						  variable_name_size,
 						  variable_name,
-- 
1.7.11.7


-- 
Matt Fleming, Intel Open Source Technology Center

Re: sysfs: cannot create duplicate filename

[Lingzhu Xiang]

(Cc'ing Seiji Aguchi for the bug related to sysfs workqueue)

On 03/12/2013 06:08 PM, Matt Fleming wrote:
> On Sat, 2013-03-09 at 02:17 +0800, Lingzhu Xiang wrote:
>> Comparing with the immediately previous one won't detect larger loop
>> like how The IBM x3100 M4 in question was looping as below.
>>
>> (There is some log loss; probably too much data for serial.)
>> [    9.783119] sysfs: cannot create duplicate filename '/firmware/efi/vars/MemoryTypeInformation-4c19049f-4137-4dd3-9c10-8b97a83ffdfa'
>> [    9.852017] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0003-8be4df61-93ca-11d2-aa0d-00e098032b8c'
> 
> Ah, thanks. I misunderstood the bug.
> 
> How about something like this? We're just going to have to suck it up
> and iterate the entire list. This could be made more optimal, but this
> isn't a fast path anyway, so I'm not sure it's worth optimising
> immediately.
> 
> ---
> 
>  From c8b6c9c175f4459197e2a13f29a2e4125372d771 Mon Sep 17 00:00:00 2001
> From: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Date: Thu, 7 Mar 2013 11:59:14 +0000
> Subject: [PATCH v2] efivars: Handle duplicate names from get_next_variable()
> 
> 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).
> 
> Reported-by: Andre Heider <a.heider-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Reported-by: Lingzhu Xiang <lxiang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Signed-off-by: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
> 
> v2: iterate the entire list when checking for duplicates.
> 
>   drivers/firmware/efivars.c | 14 ++++++++++++++
>   1 file changed, 14 insertions(+)
> 
> diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
> index bea32d1..d1656e3 100644
> --- a/drivers/firmware/efivars.c
> +++ b/drivers/firmware/efivars.c
> @@ -2005,6 +2005,20 @@ int register_efivars(struct efivars *efivars,
>   						&vendor_guid);
>   		switch (status) {
>   		case EFI_SUCCESS:
> +			/*
> +			 * 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)) {
> +				printk(KERN_WARNING "efivars: duplicate variable name.\n");
> +				status = EFI_NOT_FOUND;
> +				break;
> +			}
> +
>   			efivar_create_sysfs_entry(efivars,
>   						  variable_name_size,
>   						  variable_name,
> 

I think using what we have is good enough for this. I'll verify
boot time tomorrow.

status = EFI_NOT_FOUND here doesn't seem to do anything.

By the way, efivar_update_sysfs_entries from Seiji's sysfs workqueue
patch may also get stuck in loop in the same way. 


Lingzhu Xiang

Re: sysfs: cannot create duplicate filename

[Andre Heider]

On Tue, Mar 12, 2013 at 11:08 AM, Matt Fleming <matt-HNK1S37rvNbeXh+fF434Mdi2O/JbrIOy@public.gmane.org> wrote:

> From: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Date: Thu, 7 Mar 2013 11:59:14 +0000
> Subject: [PATCH v2] efivars: Handle duplicate names from get_next_variable()
>
> 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,

Thanks for working on this, Matt.

Unfortunately I cannot test this patch anymore... This BIOS seems
beyond buggy, and today I didn't get anything on screen, it didn't
boot anything nor could I even enter it.
The only thing to make this box usable again was to hit the CMOS reset
button, and since then I do not get the WARN_ON() anymore. Seems like
the duplicate entry was an artifact of a BIOS upgrade with old CMOS
settings.

Thanks,
Andre

Re: sysfs: cannot create duplicate filename

[Matt Fleming]

On Tue, 2013-03-12 at 18:45 +0800, Lingzhu Xiang wrote:
> (Cc'ing Seiji Aguchi for the bug related to sysfs workqueue)
> 
> On 03/12/2013 06:08 PM, Matt Fleming wrote:
> > On Sat, 2013-03-09 at 02:17 +0800, Lingzhu Xiang wrote:
> >> Comparing with the immediately previous one won't detect larger loop
> >> like how The IBM x3100 M4 in question was looping as below.
> >>
> >> (There is some log loss; probably too much data for serial.)
> >> [    9.783119] sysfs: cannot create duplicate filename '/firmware/efi/vars/MemoryTypeInformation-4c19049f-4137-4dd3-9c10-8b97a83ffdfa'
> >> [    9.852017] sysfs: cannot create duplicate filename '/firmware/efi/vars/Boot0003-8be4df61-93ca-11d2-aa0d-00e098032b8c'
> > 
> > Ah, thanks. I misunderstood the bug.
> > 
> > How about something like this? We're just going to have to suck it up
> > and iterate the entire list. This could be made more optimal, but this
> > isn't a fast path anyway, so I'm not sure it's worth optimising
> > immediately.
> > 
> > ---
> > 
> >  From c8b6c9c175f4459197e2a13f29a2e4125372d771 Mon Sep 17 00:00:00 2001
> > From: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > Date: Thu, 7 Mar 2013 11:59:14 +0000
> > Subject: [PATCH v2] efivars: Handle duplicate names from get_next_variable()
> > 
> > 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).
> > 
> > Reported-by: Andre Heider <a.heider-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> > Reported-by: Lingzhu Xiang <lxiang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> > Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> > Signed-off-by: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> > ---
> > 
> > v2: iterate the entire list when checking for duplicates.
> > 
> >   drivers/firmware/efivars.c | 14 ++++++++++++++
> >   1 file changed, 14 insertions(+)
> > 
> > diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
> > index bea32d1..d1656e3 100644
> > --- a/drivers/firmware/efivars.c
> > +++ b/drivers/firmware/efivars.c
> > @@ -2005,6 +2005,20 @@ int register_efivars(struct efivars *efivars,
> >   						&vendor_guid);
> >   		switch (status) {
> >   		case EFI_SUCCESS:
> > +			/*
> > +			 * 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)) {
> > +				printk(KERN_WARNING "efivars: duplicate variable name.\n");
> > +				status = EFI_NOT_FOUND;
> > +				break;
> > +			}
> > +
> >   			efivar_create_sysfs_entry(efivars,
> >   						  variable_name_size,
> >   						  variable_name,
> > 
> 
> I think using what we have is good enough for this. I'll verify
> boot time tomorrow.

That would be great, thank you.

> status = EFI_NOT_FOUND here doesn't seem to do anything.

It terminates the enclosing do-while loop.

> By the way, efivar_update_sysfs_entries from Seiji's sysfs workqueue
> patch may also get stuck in loop in the same way. 

You mean if GetNextVariableName() never terminates? Yeah,
efivar_update_sysfs_entries() looks like it needs protecting from that
too, good catch.

-- 
Matt Fleming, Intel Open Source Technology Center

RE: sysfs: cannot create duplicate filename

[Seiji Aguchi]

> > By the way, efivar_update_sysfs_entries from Seiji's sysfs workqueue
> > patch may also get stuck in loop in the same way.

In my understanding, a problem reported initially is that WARN_ON() in create_sysfs_entries() hits because
multiple sysfs entries of a same variable name are created.

On the other hand, in efivar_update_sysfs_entries(), the WARN_ON never hits because
If a variable is already present, create_sysfs_entries() is never called.

> 
> You mean if GetNextVariableName() never terminates? Yeah,
> efivar_update_sysfs_entries() looks like it needs protecting from that too, good catch.

If there is a case where GetNextVariableName() never terminates, I agree that efivar_update_sysfs_entries()
should be protected from that .
But I'm not sure if the case is related to a bug in this thread..

Seiji

Re: sysfs: cannot create duplicate filename

[Lingzhu Xiang]

On 03/13/2013 12:35 AM, Matt Fleming wrote:
>> I think using what we have is good enough for this. I'll verify
>> boot time tomorrow.
>
> That would be great, thank you.

On a Windows 8 logo desktop with 91 variables, the average time (30 runs 
each) of variable iteration goes from 0.009672 second to 0.010180 second 
(+0.000508 second, or +5%). Negligible.

>> status = EFI_NOT_FOUND here doesn't seem to do anything.
>
> It terminates the enclosing do-while loop.

Oops, my brain fart.

>> By the way, efivar_update_sysfs_entries from Seiji's sysfs workqueue
>> patch may also get stuck in loop in the same way.
>
> You mean if GetNextVariableName() never terminates? Yeah,
> efivar_update_sysfs_entries() looks like it needs protecting from that
> too, good catch.

Yes that's what I meant.


Lingzhu Xiang

Re: sysfs: cannot create duplicate filename

[Matt Fleming]

On Wed, 2013-03-13 at 18:47 +0800, Lingzhu Xiang wrote:
> On 03/13/2013 12:35 AM, Matt Fleming wrote:
> >> I think using what we have is good enough for this. I'll verify
> >> boot time tomorrow.
> >
> > That would be great, thank you.
> 
> On a Windows 8 logo desktop with 91 variables, the average time (30 runs 
> each) of variable iteration goes from 0.009672 second to 0.010180 second 
> (+0.000508 second, or +5%). Negligible.

Thanks for testing that.

[...]

> >> By the way, efivar_update_sysfs_entries from Seiji's sysfs workqueue
> >> patch may also get stuck in loop in the same way.
> >
> > You mean if GetNextVariableName() never terminates? Yeah,
> > efivar_update_sysfs_entries() looks like it needs protecting from that
> > too, good catch.
> 
> Yes that's what I meant.

Hmm.. actually, I'm not at all sure how to fix that case, since the
algorithm expects to find duplicates.

Seiji, any thoughts here? For a recap of the discussion see,

	http://article.gmane.org/gmane.linux.kernel.efi/835

-- 
Matt Fleming, Intel Open Source Technology Center

RE: sysfs: cannot create duplicate filename

[Seiji Aguchi]

> Hmm.. actually, I'm not at all sure how to fix that case, since the algorithm expects to find duplicates.
> 
> Seiji, any thoughts here? For a recap of the discussion see,
> 
> 	http://article.gmane.org/gmane.linux.kernel.efi/835

Thank you for providing this.

If GetNextVariableName() never terminates, the firmware is probably broken.
In this case, efivars, including efi_pstore, can't guarantee to work reliably...

>Granted, it's not kernel's fault for duplicate variables, but it'd be much harder for users 
>to fix their broken firmware than kernel dealing with it by not dying on the never-ending 
>GetNextVariableName.

To fix the broken firmware, users need to erase the duplicated variable.
Is it correct?
If yes, how about printing the duplicated variable name as follows?
Or add printk() persuade to upgrading/downgrading firmware?

+ printk(KERN_WARNING "efivars: duplicate variable name. %s\n", variable_name);

As for update_sysfs_entries() , a candidate fix is introducing a global variable like is_firmware_broken.
And if the variable is set, update_sysfs_entries() returns without calling GetNextVariableName().
As I mentioned above, there is no way to make efivars, including efi_pstore, work reliably on the broken firmware..

I admit that introducing a global variable is messy. But I can't find any other way to fix..

If I still misunderstand something, please let me know.

Seiji

Re: sysfs: cannot create duplicate filename

[Matt Fleming]

On Thu, 2013-03-14 at 19:31 +0000, Seiji Aguchi wrote:
> To fix the broken firmware, users need to erase the duplicated variable.
> Is it correct?

I'm unsure whether that will fix the problem for all platforms. Lingzhu,
do you have any idea?

> If yes, how about printing the duplicated variable name as follows?
> Or add printk() persuade to upgrading/downgrading firmware?
> 
> + printk(KERN_WARNING "efivars: duplicate variable name. %s\n", variable_name);

Yes, I think there's merit to printing that. Unfortunately, it's a utf16
string, and printk() doesn't know how to print it.

> As for update_sysfs_entries() , a candidate fix is introducing a
> global variable like is_firmware_broken.
> And if the variable is set, update_sysfs_entries() returns without
> calling GetNextVariableName().
> As I mentioned above, there is no way to make efivars, including
> efi_pstore, work reliably on the broken firmware..
> 
> I admit that introducing a global variable is messy. But I can't find
> any other way to fix..

Right, looks like this is the only way to go.

How does everyone feel about this version? Lingzhu, if I could get a
Tested-by for this version that would be great. This, plus some other
patches, are sitting on the 'urgent' branch.

---

>From 544a6c06479e4ea4e7acc7eea589a82f0ff9aa6d Mon Sep 17 00:00:00 2001
From: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Date: Thu, 7 Mar 2013 11:59:14 +0000
Subject: [PATCH] efivars: Handle duplicate names from get_next_variable()

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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reported-by: Lingzhu Xiang <lxiang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
Cc: Seiji Aguchi <seiji.aguchi-7rDLJAbr9SE@public.gmane.org>
Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Signed-off-by: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
---
 drivers/firmware/efivars.c | 48 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 47 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index ae26d5e..a1a3d01 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -170,6 +170,7 @@ efivar_create_sysfs_entry(struct efivars *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
@@ -1443,7 +1444,7 @@ static int efi_pstore_write(enum pstore_type_id type,
 
 	spin_unlock_irqrestore(&efivars->lock, flags);
 
-	if (reason == KMSG_DUMP_OOPS)
+	if (reason == KMSG_DUMP_OOPS && efivar_wq_enabled)
 		schedule_work(&efivar_work);
 
 	*id = part;
@@ -1974,6 +1975,35 @@ 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;
+
+	/*
+	 * 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)
@@ -2024,6 +2054,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,
-- 
1.7.11.7



-- 
Matt Fleming, Intel Open Source Technology Center

Re: sysfs: cannot create duplicate filename

[Lingzhu Xiang]

On 03/18/2013 10:51 PM, Matt Fleming wrote:
> On Thu, 2013-03-14 at 19:31 +0000, Seiji Aguchi wrote:
>> To fix the broken firmware, users need to erase the duplicated variable.
>> Is it correct?
>
> I'm unsure whether that will fix the problem for all platforms. Lingzhu,
> do you have any idea?

I'm not very sure what is to "erase the duplicated variable". If the 
firmware is broken, the variable interfaces probably are also broken and 
can't be used to erase variable. Users need to reset nvram to a known 
clean state, but often the hardware doesn't provide such facility.

> How does everyone feel about this version? Lingzhu, if I could get a
> Tested-by for this version that would be great. This, plus some other
> patches, are sitting on the 'urgent' branch.

I will test this. The IBM x3100m4 is gone. It takes some time to figure 
out how to corrupt nvram in QEMU...

RE: sysfs: cannot create duplicate filename

[Seiji Aguchi]

Lingzhu,

I have another idea after reading you comment.
 
> If the firmware is broken, the variable interfaces probably are also broken
> and can't be used to erase variable. 

I see, you are concerned that a set_variable service may not work correctly.
If so,  the set_variable service should be disabled in addition to get_next_variable service.

On the other hands, if Linux can boot up, a get_variable service probably works.
Therefore, efivars should be initialized by enabling just get_variable service.

> Users need to reset nvram to a known clean state, but often the hardware doesn't provide such
> facility.

What we can do is to notify that  the firmware/hardware is broken with printk.

Seiji 

Re: sysfs: cannot create duplicate filename

[Matt Fleming]

On 03/19/2013 02:25 PM, Seiji Aguchi wrote:
> I see, you are concerned that a set_variable service may not work correctly.
> If so,  the set_variable service should be disabled in addition to get_next_variable service.

We've no reason to believe that there are broken SetVariable()
implementations in the wild. If any ever crop up, we can deal with them
as and when.

-- 
Matt Fleming, Intel Open Source Technology Center

RE: sysfs: cannot create duplicate filename

[Seiji Aguchi]

> We've no reason to believe that there are broken SetVariable() implementations in the wild. If any ever crop up, we can deal with
> them as and when.

OK. I will make a patch disabling just get_next_variable().
I'm thinking that it is reasonable to set ops->get_next_variable to NULL in register_efivars(),
 instead of introducing a new global variable....

Seiji


> -----Original Message-----
> From: Matt Fleming [mailto:matt@console-pimps.org]
> Sent: Tuesday, March 19, 2013 11:27 AM
> To: Seiji Aguchi
> Cc: Lingzhu Xiang; Andre Heider; linux-efi@vger.kernel.org
> Subject: Re: sysfs: cannot create duplicate filename
> 
> On 03/19/2013 02:25 PM, Seiji Aguchi wrote:
> > I see, you are concerned that a set_variable service may not work correctly.
> > If so,  the set_variable service should be disabled in addition to get_next_variable service.
> 
> We've no reason to believe that there are broken SetVariable() implementations in the wild. If any ever crop up, we can deal with
> them as and when.
> 
> --
> Matt Fleming, Intel Open Source Technology Center

Re: sysfs: cannot create duplicate filename

[Lingzhu Xiang]

On 03/19/2013 10:25 PM, Seiji Aguchi wrote:
>> If the firmware is broken, the variable interfaces probably are also broken
>> and can't be used to erase variable. 
> 
> I see, you are concerned that a set_variable service may not work correctly.
> If so,  the set_variable service should be disabled in addition to get_next_variable service.

Since you're suggesting fixing firmware by erasing, I'm wondering
SetVariable may not actually remove duplicate variables since the way
they exist is already an undefined behavior. And some variables can't
be removed.

GetNextVariable is clearly causing harm in this case. But the rest of efi
runtime services should be left as they are.


Lingzhu Xiang

Re: sysfs: cannot create duplicate filename

[Lingzhu Xiang]

On 03/18/2013 10:51 PM, Matt Fleming wrote:
> How does everyone feel about this version? Lingzhu, if I could get a
> Tested-by for this version that would be great. This, plus some other
> patches, are sitting on the 'urgent' branch.
> 
> ---
> 
>  From 544a6c06479e4ea4e7acc7eea589a82f0ff9aa6d Mon Sep 17 00:00:00 2001
> From: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Date: Thu, 7 Mar 2013 11:59:14 +0000
> Subject: [PATCH] efivars: Handle duplicate names from get_next_variable()
> 
> 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-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Reported-by: Lingzhu Xiang <lxiang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Seiji Aguchi <seiji.aguchi-7rDLJAbr9SE@public.gmane.org>
> Cc: <stable-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
> Signed-off-by: Matt Fleming <matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>

Tested-by: Lingzhu Xiang <lxiang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Reproduced with 3.9-rc3 in QEMU/OVMF. The kernel gets stuck in a loop.

Verified with 3.9-rc3 with this patch. The kernel survives. efivarfs
continues working, though some variables won't show up, as expected.

[    2.033855] efivars: duplicate variable: Boot0001-8be4df61-93ca-11d2-aa0d-00e098032b8c

This patch has no real impact for users with normal firmware as tested
before on a Windows 8 logo desktop.

For the record, here is how I create duplicate variables in QEMU/OVMF:

OVMF EmuVariableFvbRuntimeDxe emulates nvram with a reserved area of
memory (ReserveEmuVariableNvStore). But there seems to be some layers
of cache above this, so directly writing a new duplicate variable here
won't make a difference. I ended up writing an EFI app to scan the
entire memory and replace all L"Boot0004" with L"Boot0001", which
seemed to actually break things (e.g. dmpstore will get stuck).

Re: sysfs: cannot create duplicate filename

[Matt Fleming]

On 03/21/2013 07:22 AM, Lingzhu Xiang wrote:
> Tested-by: Lingzhu Xiang <lxiang-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>

Great, thanks!

> Reproduced with 3.9-rc3 in QEMU/OVMF. The kernel gets stuck in a loop.
> 
> Verified with 3.9-rc3 with this patch. The kernel survives. efivarfs
> continues working, though some variables won't show up, as expected.
> 
> [    2.033855] efivars: duplicate variable: Boot0001-8be4df61-93ca-11d2-aa0d-00e098032b8c
> 
> This patch has no real impact for users with normal firmware as tested
> before on a Windows 8 logo desktop.
> 
> For the record, here is how I create duplicate variables in QEMU/OVMF:
> 
> OVMF EmuVariableFvbRuntimeDxe emulates nvram with a reserved area of
> memory (ReserveEmuVariableNvStore). But there seems to be some layers
> of cache above this, so directly writing a new duplicate variable here
> won't make a difference. I ended up writing an EFI app to scan the
> entire memory and replace all L"Boot0004" with L"Boot0001", which
> seemed to actually break things (e.g. dmpstore will get stuck).

Thanks for going to such lengths to test this.

-- 
Matt Fleming, Intel Open Source Technology Center