aboutsummaryrefslogtreecommitdiff
path: root/drivers/firmware
diff options
context:
space:
mode:
authorMatt Fleming <matt.fleming@intel.com>2013-03-01 14:49:12 +0000
committerMatt Fleming <matt.fleming@intel.com>2013-03-21 12:43:46 +0000
commitec50bd32f1672d38ddce10fb1841cbfda89cfe9a (patch)
tree4e88f868a6ba2d8d4867225dbb290590ac34dfa7 /drivers/firmware
parentec0971ba5372a4dfa753f232449d23a8fd98490e (diff)
efivars: explicitly calculate length of VariableName
It's not wise to assume VariableNameSize represents the length of VariableName, as not all firmware updates VariableNameSize in the same way (some don't update it at all if EFI_SUCCESS is returned). There are even implementations out there that update VariableNameSize with values that are both larger than the string returned in VariableName and smaller than the buffer passed to GetNextVariableName(), which resulted in the following bug report from Michael Schroeder, > On HP z220 system (firmware version 1.54), some EFI variables are > incorrectly named : > > ls -d /sys/firmware/efi/vars/*8be4d* | grep -v -- -8be returns > /sys/firmware/efi/vars/dbxDefault-pport8be4df61-93ca-11d2-aa0d-00e098032b8c > /sys/firmware/efi/vars/KEKDefault-pport8be4df61-93ca-11d2-aa0d-00e098032b8c > /sys/firmware/efi/vars/SecureBoot-pport8be4df61-93ca-11d2-aa0d-00e098032b8c > /sys/firmware/efi/vars/SetupMode-Information8be4df61-93ca-11d2-aa0d-00e098032b8c The issue here is that because we blindly use VariableNameSize without verifying its value, we can potentially read garbage values from the buffer containing VariableName if VariableNameSize is larger than the length of VariableName. Since VariableName is a string, we can calculate its size by searching for the terminating NULL character. Reported-by: Frederic Crozat <fcrozat@suse.com> Cc: Matthew Garrett <mjg59@srcf.ucam.org> Cc: Josh Boyer <jwboyer@redhat.com> Cc: Michael Schroeder <mls@suse.com> Cc: Lee, Chun-Yi <jlee@suse.com> Cc: Lingzhu Xiang <lxiang@redhat.com> Cc: Seiji Aguchi <seiji.aguchi@hds.com> Signed-off-by: Matt Fleming <matt.fleming@intel.com>
Diffstat (limited to 'drivers/firmware')
-rw-r--r--drivers/firmware/efivars.c32
1 files changed, 31 insertions, 1 deletions
diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c
index 6607daf5a08..1e9d9b9d7a1 100644
--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -1705,6 +1705,31 @@ static bool variable_is_present(efi_char16_t *variable_name, efi_guid_t *vendor)
return found;
}
+/*
+ * Returns the size of variable_name, in bytes, including the
+ * terminating NULL character, or variable_name_size if no NULL
+ * character is found among the first variable_name_size bytes.
+ */
+static unsigned long var_name_strnsize(efi_char16_t *variable_name,
+ unsigned long variable_name_size)
+{
+ unsigned long len;
+ efi_char16_t c;
+
+ /*
+ * The variable name is, by definition, a NULL-terminated
+ * string, so make absolutely sure that variable_name_size is
+ * the value we expect it to be. If not, return the real size.
+ */
+ for (len = 2; len <= variable_name_size; len += sizeof(c)) {
+ c = variable_name[(len / sizeof(c)) - 1];
+ if (!c)
+ break;
+ }
+
+ return min(len, variable_name_size);
+}
+
static void efivar_update_sysfs_entries(struct work_struct *work)
{
struct efivars *efivars = &__efivars;
@@ -1745,10 +1770,13 @@ static void efivar_update_sysfs_entries(struct work_struct *work)
if (!found) {
kfree(variable_name);
break;
- } else
+ } else {
+ variable_name_size = var_name_strnsize(variable_name,
+ variable_name_size);
efivar_create_sysfs_entry(efivars,
variable_name_size,
variable_name, &vendor);
+ }
}
}
@@ -1995,6 +2023,8 @@ int register_efivars(struct efivars *efivars,
&vendor_guid);
switch (status) {
case EFI_SUCCESS:
+ variable_name_size = var_name_strnsize(variable_name,
+ variable_name_size);
efivar_create_sysfs_entry(efivars,
variable_name_size,
variable_name,