Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

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>

+31 -1
+31 -1
drivers/firmware/efivars.c
··· 1705 1705 return found; 1706 1706 } 1707 1707 1708 + /* 1709 + * Returns the size of variable_name, in bytes, including the 1710 + * terminating NULL character, or variable_name_size if no NULL 1711 + * character is found among the first variable_name_size bytes. 1712 + */ 1713 + static unsigned long var_name_strnsize(efi_char16_t *variable_name, 1714 + unsigned long variable_name_size) 1715 + { 1716 + unsigned long len; 1717 + efi_char16_t c; 1718 + 1719 + /* 1720 + * The variable name is, by definition, a NULL-terminated 1721 + * string, so make absolutely sure that variable_name_size is 1722 + * the value we expect it to be. If not, return the real size. 1723 + */ 1724 + for (len = 2; len <= variable_name_size; len += sizeof(c)) { 1725 + c = variable_name[(len / sizeof(c)) - 1]; 1726 + if (!c) 1727 + break; 1728 + } 1729 + 1730 + return min(len, variable_name_size); 1731 + } 1732 + 1708 1733 static void efivar_update_sysfs_entries(struct work_struct *work) 1709 1734 { 1710 1735 struct efivars *efivars = &__efivars; ··· 1770 1745 if (!found) { 1771 1746 kfree(variable_name); 1772 1747 break; 1773 - } else 1748 + } else { 1749 + variable_name_size = var_name_strnsize(variable_name, 1750 + variable_name_size); 1774 1751 efivar_create_sysfs_entry(efivars, 1775 1752 variable_name_size, 1776 1753 variable_name, &vendor); 1754 + } 1777 1755 } 1778 1756 } 1779 1757 ··· 2023 1995 &vendor_guid); 2024 1996 switch (status) { 2025 1997 case EFI_SUCCESS: 1998 + variable_name_size = var_name_strnsize(variable_name, 1999 + variable_name_size); 2026 2000 efivar_create_sysfs_entry(efivars, 2027 2001 variable_name_size, 2028 2002 variable_name,