Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756160Ab2EBSyc (ORCPT ); Wed, 2 May 2012 14:54:32 -0400 Received: from mx1.redhat.com ([209.132.183.28]:63878 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756039Ab2EBSya (ORCPT ); Wed, 2 May 2012 14:54:30 -0400 From: Matthew Garrett To: torvalds@linux-foundation.org Cc: linux-kernel@vger.kernel.org, Matthew Garrett , stable@vger.kernel.org Subject: [PATCH V2 2/2] efi: Validate UEFI boot variables Date: Wed, 2 May 2012 14:54:14 -0400 Message-Id: <1335984854-24690-2-git-send-email-mjg@redhat.com> In-Reply-To: <1335984854-24690-1-git-send-email-mjg@redhat.com> References: <1335984854-24690-1-git-send-email-mjg@redhat.com> X-SA-Do-Not-Run: Yes X-SA-Exim-Connect-IP: 66.187.233.206 X-SA-Exim-Mail-From: mjg@redhat.com X-SA-Exim-Scanned: No (on cavan.codon.org.uk); SAEximRunCond expanded to false Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6599 Lines: 245 A common flaw in UEFI systems is a refusal to POST triggered by a malformed boot variable. Once in this state, machines may only be restored by reflashing their firmware with an external hardware device. While this is obviously a firmware bug, the serious nature of the outcome suggests that operating systems should filter their variable writes in order to prevent a malicious user from rendering the machine unusable. Signed-off-by: Matthew Garrett Cc: stable@vger.kernel.org --- Updated with comments from Ben Hutchings drivers/firmware/efivars.c | 196 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 196 insertions(+) diff --git a/drivers/firmware/efivars.c b/drivers/firmware/efivars.c index d25599f..47408e8 100644 --- a/drivers/firmware/efivars.c +++ b/drivers/firmware/efivars.c @@ -191,6 +191,190 @@ utf16_strncmp(const efi_char16_t *a, const efi_char16_t *b, size_t len) } } +static bool +validate_device_path(struct efi_variable *var, int match, u8 *buffer, + unsigned long len) +{ + struct efi_generic_dev_path *node; + int offset = 0; + + node = (struct efi_generic_dev_path *)buffer; + + if (len < sizeof(*node)) + return false; + + while (offset <= len - sizeof(*node) && + node->length >= sizeof(*node) && + node->length <= len - offset) { + offset += node->length; + + if ((node->type == EFI_DEV_END_PATH || + node->type == EFI_DEV_END_PATH2) && + node->sub_type == EFI_DEV_END_ENTIRE) + return true; + + node = (struct efi_generic_dev_path *)(buffer + offset); + } + + /* + * If we're here then either node->length pointed past the end + * of the buffer or we reached the end of the buffer without + * finding a device path end node. + */ + return false; +} + +static bool +validate_boot_order(struct efi_variable *var, int match, u8 *buffer, + unsigned long len) +{ + /* An array of 16-bit integers */ + if ((len % 2) != 0) + return false; + + return true; +} + +static bool +validate_load_option(struct efi_variable *var, int match, u8 *buffer, + unsigned long len) +{ + u16 filepathlength; + int i, desclength = 0, namelen; + + namelen = utf16_strnlen(var->VariableName, sizeof(var->VariableName)); + + /* Either "Boot" or "Driver" followed by four digits of hex */ + for (i = match; i < match+4; i++) { + if (var->VariableName[i] > 127 || + hex_to_bin(var->VariableName[i] & 0xff) < 0) + return true; + } + + /* Reject it if there's 4 digits of hex and then further content */ + if (namelen > match + 4) + return false; + + /* A valid entry must be at least 8 bytes */ + if (len < 8) + return false; + + filepathlength = buffer[4] | buffer[5] << 8; + + /* + * There's no stored length for the description, so it has to be + * found by hand + */ + desclength = utf16_strsize((efi_char16_t *)(buffer + 6), len - 6) + 2; + + /* Each boot entry must have a descriptor */ + if (!desclength) + return false; + + /* + * If the sum of the length of the description, the claimed filepath + * length and the original header are greater than the length of the + * variable, it's malformed + */ + if ((desclength + filepathlength + 6) > len) + return false; + + /* + * And, finally, check the filepath + */ + return validate_device_path(var, match, buffer + desclength + 6, + filepathlength); +} + +static bool +validate_uint16(struct efi_variable *var, int match, u8 *buffer, + unsigned long len) +{ + /* A single 16-bit integer */ + if (len != 2) + return false; + + return true; +} + +static bool +validate_ascii_string(struct efi_variable *var, int match, u8 *buffer, + unsigned long len) +{ + int i; + + for (i = 0; i < len; i++) { + if (buffer[i] > 127) + return false; + + if (buffer[i] == 0) + return true; + } + + return false; +} + +struct variable_validate { + char *name; + bool (*validate)(struct efi_variable *var, int match, u8 *data, + unsigned long len); +}; + +static const struct variable_validate variable_validate[] = { + { "BootNext", validate_uint16 }, + { "BootOrder", validate_boot_order }, + { "DriverOrder", validate_boot_order }, + { "Boot*", validate_load_option }, + { "Driver*", validate_load_option }, + { "ConIn", validate_device_path }, + { "ConInDev", validate_device_path }, + { "ConOut", validate_device_path }, + { "ConOutDev", validate_device_path }, + { "ErrOut", validate_device_path }, + { "ErrOutDev", validate_device_path }, + { "Timeout", validate_uint16 }, + { "Lang", validate_ascii_string }, + { "PlatformLang", validate_ascii_string }, + { "", NULL }, +}; + +static bool +validate_var(struct efi_variable *var, u8 *data, unsigned long len) +{ + int i; + u16 *unicode_name = var->VariableName; + + for (i = 0; variable_validate[i].validate != NULL; i++) { + const char *name = variable_validate[i].name; + int match; + + for (match = 0; ; match++) { + char c = name[match]; + u16 u = unicode_name[match]; + + /* All special variables are plain ascii */ + if (u > 127) + return true; + + /* Wildcard in the matching name means we've matched */ + if (c == '*') + return variable_validate[i].validate(var, + match, data, len); + + /* Case sensitive match */ + if (c != u) + break; + + /* Reached the end of the string while matching */ + if (!c) + return variable_validate[i].validate(var, + match, data, len); + } + } + + return true; +} + static efi_status_t get_var_data_locked(struct efivars *efivars, struct efi_variable *var) { @@ -324,6 +508,12 @@ efivar_store_raw(struct efivar_entry *entry, const char *buf, size_t count) return -EINVAL; } + if ((new_var->Attributes & ~EFI_VARIABLE_MASK) != 0 || + validate_var(new_var, new_var->Data, new_var->DataSize) == false) { + printk(KERN_ERR "efivars: Malformed variable content\n"); + return -EINVAL; + } + spin_lock(&efivars->lock); status = efivars->ops->set_variable(new_var->VariableName, &new_var->VendorGuid, @@ -626,6 +816,12 @@ static ssize_t efivar_create(struct file *filp, struct kobject *kobj, if (!capable(CAP_SYS_ADMIN)) return -EACCES; + if ((new_var->Attributes & ~EFI_VARIABLE_MASK) != 0 || + validate_var(new_var, new_var->Data, new_var->DataSize) == false) { + printk(KERN_ERR "efivars: Malformed variable content\n"); + return -EINVAL; + } + spin_lock(&efivars->lock); /* -- 1.7.10 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/