[kernel] Add 3 fixes for efi issues (rhbz 917984)
Josh Boyer
jwboyer at fedoraproject.org
Tue Mar 5 14:47:51 UTC 2013
commit 4d1cff551b12c409b5f0b59d6646a42b1a6b078b
Author: Josh Boyer <jwboyer at redhat.com>
Date: Tue Mar 5 09:33:50 2013 -0500
Add 3 fixes for efi issues (rhbz 917984)
efi-fixes.patch | 431 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
kernel.spec | 11 ++-
2 files changed, 441 insertions(+), 1 deletions(-)
---
diff --git a/efi-fixes.patch b/efi-fixes.patch
new file mode 100644
index 0000000..6e1e8c1
--- /dev/null
+++ b/efi-fixes.patch
@@ -0,0 +1,431 @@
+From 74d5b500b0184d6ddf4e59328b50a9521c1cd1be Mon Sep 17 00:00:00 2001
+From: Matthew Garrett <matthew.garrett at nebula.com>
+Date: Sat, 2 Mar 2013 19:40:17 -0500
+Subject: [PATCH 1/3] efi: be more paranoid about available space when creating
+ variables
+
+UEFI variables are typically stored in flash. For various reasons, avaiable
+space is typically not reclaimed immediately upon the deletion of a
+variable - instead, the system will garbage collect during initialisation
+after a reboot.
+
+Some systems appear to handle this garbage collection extremely poorly,
+failing if more than 50% of the system flash is in use. This can result in
+the machine refusing to boot. The safest thing to do for the moment is to
+forbid writes if they'd end up using more than half of the storage space.
+We can make this more finegrained later if we come up with a method for
+identifying the broken machines.
+
+Signed-off-by: Matthew Garrett <matthew.garrett at nebula.com>
+Cc: <stable at vger.kernel.org>
+Signed-off-by: Matt Fleming <matt.fleming at intel.com>
+---
+ drivers/firmware/efivars.c | 106 +++++++++++++++++++++++++++++++++------------
+ 1 file changed, 79 insertions(+), 27 deletions(-)
+
+diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
+index 7320bf8..0d50497 100644
+--- a/drivers/firmware/efivars.c
++++ b/drivers/firmware/efivars.c
+@@ -426,6 +426,44 @@ get_var_data(struct efivars *efivars, struct efi_variable *var)
+ return status;
+ }
+
++static efi_status_t
++check_var_size_locked(struct efivars *efivars, u32 attributes,
++ unsigned long size)
++{
++ u64 storage_size, remaining_size, max_size;
++ efi_status_t status;
++ const struct efivar_operations *fops = efivars->ops;
++
++ if (!efivars->ops->query_variable_info)
++ return EFI_UNSUPPORTED;
++
++ status = fops->query_variable_info(attributes, &storage_size,
++ &remaining_size, &max_size);
++
++ if (status != EFI_SUCCESS)
++ return status;
++
++ if (!storage_size || size > remaining_size || size > max_size ||
++ (remaining_size - size) < (storage_size / 2))
++ return EFI_OUT_OF_RESOURCES;
++
++ return status;
++}
++
++
++static efi_status_t
++check_var_size(struct efivars *efivars, u32 attributes, unsigned long size)
++{
++ efi_status_t status;
++ unsigned long flags;
++
++ spin_lock_irqsave(&efivars->lock, flags);
++ status = check_var_size_locked(efivars, attributes, size);
++ spin_unlock_irqrestore(&efivars->lock, flags);
++
++ return status;
++}
++
+ static ssize_t
+ efivar_guid_read(struct efivar_entry *entry, char *buf)
+ {
+@@ -547,11 +585,16 @@ efivar_store_raw(struct efivar_entry *entry, const char *buf, size_t count)
+ }
+
+ spin_lock_irq(&efivars->lock);
+- status = efivars->ops->set_variable(new_var->VariableName,
+- &new_var->VendorGuid,
+- new_var->Attributes,
+- new_var->DataSize,
+- new_var->Data);
++
++ status = check_var_size_locked(efivars, new_var->Attributes,
++ new_var->DataSize + utf16_strsize(new_var->VariableName, 1024));
++
++ if (status == EFI_SUCCESS || status == EFI_UNSUPPORTED)
++ status = efivars->ops->set_variable(new_var->VariableName,
++ &new_var->VendorGuid,
++ new_var->Attributes,
++ new_var->DataSize,
++ new_var->Data);
+
+ spin_unlock_irq(&efivars->lock);
+
+@@ -702,8 +745,7 @@ static ssize_t efivarfs_file_write(struct file *file,
+ u32 attributes;
+ struct inode *inode = file->f_mapping->host;
+ unsigned long datasize = count - sizeof(attributes);
+- unsigned long newdatasize;
+- u64 storage_size, remaining_size, max_size;
++ unsigned long newdatasize, varsize;
+ ssize_t bytes = 0;
+
+ if (count < sizeof(attributes))
+@@ -722,28 +764,18 @@ static ssize_t efivarfs_file_write(struct file *file,
+ * amounts of memory. Pick a default size of 64K if
+ * QueryVariableInfo() isn't supported by the firmware.
+ */
+- spin_lock_irq(&efivars->lock);
+
+- if (!efivars->ops->query_variable_info)
+- status = EFI_UNSUPPORTED;
+- else {
+- const struct efivar_operations *fops = efivars->ops;
+- status = fops->query_variable_info(attributes, &storage_size,
+- &remaining_size, &max_size);
+- }
+-
+- spin_unlock_irq(&efivars->lock);
++ varsize = datasize + utf16_strsize(var->var.VariableName, 1024);
++ status = check_var_size(efivars, attributes, varsize);
+
+ if (status != EFI_SUCCESS) {
+ if (status != EFI_UNSUPPORTED)
+ return efi_status_to_err(status);
+
+- remaining_size = 65536;
++ if (datasize > 65536)
++ return -ENOSPC;
+ }
+
+- if (datasize > remaining_size)
+- return -ENOSPC;
+-
+ data = kmalloc(datasize, GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+@@ -765,6 +797,19 @@ static ssize_t efivarfs_file_write(struct file *file,
+ */
+ spin_lock_irq(&efivars->lock);
+
++ /*
++ * Ensure that the available space hasn't shrunk below the safe level
++ */
++
++ status = check_var_size_locked(efivars, attributes, varsize);
++
++ if (status != EFI_SUCCESS && status != EFI_UNSUPPORTED) {
++ spin_unlock_irq(&efivars->lock);
++ kfree(data);
++
++ return efi_status_to_err(status);
++ }
++
+ status = efivars->ops->set_variable(var->var.VariableName,
+ &var->var.VendorGuid,
+ attributes, datasize,
+@@ -1345,7 +1390,6 @@ static int efi_pstore_write(enum pstore_type_id type,
+ efi_guid_t vendor = LINUX_EFI_CRASH_GUID;
+ struct efivars *efivars = psi->data;
+ int i, ret = 0;
+- u64 storage_space, remaining_space, max_variable_size;
+ efi_status_t status = EFI_NOT_FOUND;
+ unsigned long flags;
+
+@@ -1365,11 +1409,11 @@ static int efi_pstore_write(enum pstore_type_id type,
+ * size: a size of logging data
+ * DUMP_NAME_LEN * 2: a maximum size of variable name
+ */
+- status = efivars->ops->query_variable_info(PSTORE_EFI_ATTRIBUTES,
+- &storage_space,
+- &remaining_space,
+- &max_variable_size);
+- if (status || remaining_space < size + DUMP_NAME_LEN * 2) {
++
++ status = check_var_size_locked(efivars, PSTORE_EFI_ATTRIBUTES,
++ size + DUMP_NAME_LEN * 2);
++
++ if (status) {
+ spin_unlock_irqrestore(&efivars->lock, flags);
+ *id = part;
+ return -ENOSPC;
+@@ -1544,6 +1588,14 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj,
+ return -EINVAL;
+ }
+
++ status = check_var_size_locked(efivars, new_var->Attributes,
++ new_var->DataSize + utf16_strsize(new_var->VariableName, 1024));
++
++ if (status && status != EFI_UNSUPPORTED) {
++ spin_unlock_irq(&efivars->lock);
++ return efi_status_to_err(status);
++ }
++
+ /* now *really* create the variable via EFI */
+ status = efivars->ops->set_variable(new_var->VariableName,
+ &new_var->VendorGuid,
+--
+1.8.1.2
+
+
+From 8200cc0633605f417a1f0c229772f9033d57ea0a Mon Sep 17 00:00:00 2001
+From: Matt Fleming <matt.fleming at intel.com>
+Date: Tue, 5 Mar 2013 07:40:16 +0000
+Subject: [PATCH 2/3] efivars: efivarfs_valid_name() should handle pstore
+ syntax
+
+Stricter validation was introduced with commit da27a24383b2b
+("efivarfs: guid part of filenames are case-insensitive") and commit
+47f531e8ba3b ("efivarfs: Validate filenames much more aggressively"),
+which is necessary for the guid portion of efivarfs filenames, but we
+don't need to be so strict with the first part, the variable name. The
+UEFI specification doesn't impose any constraints on variable names
+other than they be a NULL-terminated string.
+
+The above commits caused a regression that resulted in users seeing
+the following message,
+
+ $ sudo mount -v /sys/firmware/efi/efivars mount: Cannot allocate memory
+
+whenever pstore EFI variables were present in the variable store,
+since their variable names failed to pass the following check,
+
+ /* GUID should be right after the first '-' */
+ if (s - 1 != strchr(str, '-'))
+
+as a typical pstore filename is of the form, dump-type0-10-1-<guid>.
+The fix is trivial since the guid portion of the filename is GUID_LEN
+bytes, we can use (len - GUID_LEN) to ensure the '-' character is
+where we expect it to be.
+
+(The bogus ENOMEM error value will be fixed in a separate patch.)
+
+Reported-by: Joseph Yasi <joe.yasi at gmail.com>
+Reported-by: Lingzhu Xiang <lxiang at redhat.com>
+Cc: Josh Boyer <jwboyer at redhat.com>
+Cc: Jeremy Kerr <jk at ozlabs.org>
+Cc: Matthew Garrett <mjg59 at srcf.ucam.org>
+Cc: <stable at vger.kernel.org>
+Signed-off-by: Matt Fleming <matt.fleming at intel.com>
+---
+ drivers/firmware/efivars.c | 4 +-
+ tools/testing/selftests/efivarfs/efivarfs.sh | 59 ++++++++++++++++++++++++++++
+ 2 files changed, 61 insertions(+), 2 deletions(-)
+
+diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
+index 0d50497..1b9a6e1 100644
+--- a/drivers/firmware/efivars.c
++++ b/drivers/firmware/efivars.c
+@@ -974,8 +974,8 @@ static bool efivarfs_valid_name(const char *str, int len)
+ if (len < GUID_LEN + 2)
+ return false;
+
+- /* GUID should be right after the first '-' */
+- if (s - 1 != strchr(str, '-'))
++ /* GUID must be preceded by a '-' */
++ if (*(s - 1) != '-')
+ return false;
+
+ /*
+diff --git a/tools/testing/selftests/efivarfs/efivarfs.sh b/tools/testing/selftests/efivarfs/efivarfs.sh
+index 880cdd5..77edcdc 100644
+--- a/tools/testing/selftests/efivarfs/efivarfs.sh
++++ b/tools/testing/selftests/efivarfs/efivarfs.sh
+@@ -125,6 +125,63 @@ test_open_unlink()
+ ./open-unlink $file
+ }
+
++# test that we can create a range of filenames
++test_valid_filenames()
++{
++ local attrs='\x07\x00\x00\x00'
++ local ret=0
++
++ local file_list="abc dump-type0-11-1-1362436005 1234 -"
++ for f in $file_list; do
++ local file=$efivarfs_mount/$f-$test_guid
++
++ printf "$attrs\x00" > $file
++
++ if [ ! -e $file ]; then
++ echo "$file could not be created" >&2
++ ret=1
++ else
++ rm $file
++ fi
++ done
++
++ exit $ret
++}
++
++test_invalid_filenames()
++{
++ local attrs='\x07\x00\x00\x00'
++ local ret=0
++
++ local file_list="
++ -1234-1234-1234-123456789abc
++ foo
++ foo-bar
++ -foo-
++ foo-barbazba-foob-foob-foob-foobarbazfoo
++ foo-------------------------------------
++ -12345678-1234-1234-1234-123456789abc
++ a-12345678=1234-1234-1234-123456789abc
++ a-12345678-1234=1234-1234-123456789abc
++ a-12345678-1234-1234=1234-123456789abc
++ a-12345678-1234-1234-1234=123456789abc
++ 1112345678-1234-1234-1234-123456789abc"
++
++ for f in $file_list; do
++ local file=$efivarfs_mount/$f
++
++ printf "$attrs\x00" 2>/dev/null > $file
++
++ if [ -e $file ]; then
++ echo "Creating $file should have failed" >&2
++ rm $file
++ ret=1
++ fi
++ done
++
++ exit $ret
++}
++
+ check_prereqs
+
+ rc=0
+@@ -135,5 +192,7 @@ run_test test_create_read
+ run_test test_delete
+ run_test test_zero_size_delete
+ run_test test_open_unlink
++run_test test_valid_filenames
++run_test test_invalid_filenames
+
+ exit $rc
+--
+1.8.1.2
+
+
+From 396c0285825255c6e2549c9a6eec6c23a35c9f7f Mon Sep 17 00:00:00 2001
+From: Matt Fleming <matt.fleming at intel.com>
+Date: Tue, 5 Mar 2013 12:46:30 +0000
+Subject: [PATCH 3/3] efivarfs: return accurate error code in
+ efivarfs_fill_super()
+
+Joseph was hitting a failure case when mounting efivarfs which
+resulted in an incorrect error message,
+
+ $ sudo mount -v /sys/firmware/efi/efivars mount: Cannot allocate memory
+
+triggered when efivarfs_valid_name() returned -EINVAL.
+
+Make sure we pass accurate return values up the stack if
+efivarfs_fill_super() fails to build inodes for EFI variables.
+
+Reported-by: Joseph Yasi <joe.yasi at gmail.com>
+Reported-by: Lingzhu Xiang <lxiang at redhat.com>
+Cc: Josh Boyer <jwboyer at redhat.com>
+Cc: Jeremy Kerr <jk at ozlabs.org>
+Cc: Matthew Garrett <mjg59 at srcf.ucam.org>
+Cc: <stable at vger.kernel.org>
+Signed-off-by: Matt Fleming <matt.fleming at intel.com>
+---
+ drivers/firmware/efivars.c | 20 +++++++++++++++-----
+ 1 file changed, 15 insertions(+), 5 deletions(-)
+
+diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
+index 1b9a6e1..bea32d1 100644
+--- a/drivers/firmware/efivars.c
++++ b/drivers/firmware/efivars.c
+@@ -1163,15 +1163,22 @@ static struct dentry_operations efivarfs_d_ops = {
+
+ static struct dentry *efivarfs_alloc_dentry(struct dentry *parent, char *name)
+ {
++ struct dentry *d;
+ struct qstr q;
++ int err;
+
+ q.name = name;
+ q.len = strlen(name);
+
+- if (efivarfs_d_hash(NULL, NULL, &q))
+- return NULL;
++ err = efivarfs_d_hash(NULL, NULL, &q);
++ if (err)
++ return ERR_PTR(err);
++
++ d = d_alloc(parent, &q);
++ if (d)
++ return d;
+
+- return d_alloc(parent, &q);
++ return ERR_PTR(-ENOMEM);
+ }
+
+ static int efivarfs_fill_super(struct super_block *sb, void *data, int silent)
+@@ -1181,6 +1188,7 @@ static int efivarfs_fill_super(struct super_block *sb, void *data, int silent)
+ struct efivar_entry *entry, *n;
+ struct efivars *efivars = &__efivars;
+ char *name;
++ int err = -ENOMEM;
+
+ efivarfs_sb = sb;
+
+@@ -1231,8 +1239,10 @@ static int efivarfs_fill_super(struct super_block *sb, void *data, int silent)
+ goto fail_name;
+
+ dentry = efivarfs_alloc_dentry(root, name);
+- if (!dentry)
++ if (IS_ERR(dentry)) {
++ err = PTR_ERR(dentry);
+ goto fail_inode;
++ }
+
+ /* copied by the above to local storage in the dentry. */
+ kfree(name);
+@@ -1259,7 +1269,7 @@ fail_inode:
+ fail_name:
+ kfree(name);
+ fail:
+- return -ENOMEM;
++ return err;
+ }
+
+ static struct dentry *efivarfs_mount(struct file_system_type *fs_type,
+--
+1.8.1.2
+
diff --git a/kernel.spec b/kernel.spec
index ae9c9da..bf9780d 100644
--- a/kernel.spec
+++ b/kernel.spec
@@ -62,7 +62,7 @@ Summary: The Linux kernel
# For non-released -rc kernels, this will be appended after the rcX and
# gitX tags, so a 3 here would become part of release "0.rcX.gitX.3"
#
-%global baserelease 1
+%global baserelease 2
%global fedora_build %{baserelease}
# base_sublevel is the kernel version we're starting with and patching
@@ -738,6 +738,9 @@ Patch21262: x86-mm-Fix-vmalloc_fault-oops-during-lazy-MMU-updates.patch
#rhbz 916833
Patch21263: intel-pstate-do-not-load-on-VM-that-do-not-report-max-P-state.patch
+#rhbz 917984
+Patch21264: efi-fixes.patch
+
Patch22000: weird-root-dentry-name-debug.patch
#selinux ptrace child permissions
@@ -1434,6 +1437,9 @@ ApplyPatch x86-mm-Fix-vmalloc_fault-oops-during-lazy-MMU-updates.patch
#rhbz 916833
ApplyPatch intel-pstate-do-not-load-on-VM-that-do-not-report-max-P-state.patch
+#rhbz 917984
+ApplyPatch efi-fixes.patch
+
# END OF PATCH APPLICATIONS
%endif
@@ -2275,6 +2281,9 @@ fi
# ||----w |
# || ||
%changelog
+* Tue Mar 05 2013 Josh Boyer <jwboyer at redhat.com>
+- Add 3 fixes for efi issues (rhbz 917984)
+
* Mon Mar 04 2013 Josh Boyer <jwboyer at redhat.com> - 3.9.0-0.rc1.git0.1
- Linux v3.9-rc1
- Add patch from Dirk Brandewie to fix intel pstate divide error (rhbz 916833)
More information about the scm-commits
mailing list