public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org, stable@vger.kernel.org
Cc: torvalds@linux-foundation.org, akpm@linux-foundation.org,
	alan@lxorguk.ukuu.org.uk, Matthew Garrett <mjg@redhat.com>
Subject: [ 46/75] efi: Validate UEFI boot variables
Date: Fri, 04 May 2012 13:43:10 -0700	[thread overview]
Message-ID: <20120504204228.302668175@linuxfoundation.org> (raw)
In-Reply-To: <20120504204258.GA12552@kroah.com>

3.3-stable review patch.  If anyone has any objections, please let me know.

------------------

From: Matthew Garrett <mjg@redhat.com>

commit fec6c20b570bcf541e581fc97f2e0cbdb9725b98 upstream.

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 <mjg@redhat.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

--- a/drivers/firmware/efivars.c
+++ b/drivers/firmware/efivars.c
@@ -191,6 +191,176 @@ utf16_strncmp(const efi_char16_t *a, con
 	}
 }
 
+static bool
+validate_device_path(struct efi_variable *var, int match, u8 *buffer, int len)
+{
+	struct efi_generic_dev_path *node;
+	int offset = 0;
+
+	node = (struct efi_generic_dev_path *)buffer;
+
+	while (offset < len) {
+		offset += node->length;
+
+		if (offset > len)
+			return false;
+
+		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, int 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, int len)
+{
+	u16 filepathlength;
+	int i, desclength = 0;
+
+	/* Either "Boot" or "Driver" followed by four digits of hex */
+	for (i = match; i < match+4; i++) {
+		if (hex_to_bin(var->VariableName[i] & 0xff) < 0)
+			return true;
+	}
+
+	/* A valid entry must be at least 6 bytes */
+	if (len < 6)
+		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) + 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, int 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, int 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,
+			 int 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, int 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 +494,12 @@ efivar_store_raw(struct efivar_entry *en
 		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 +802,12 @@ static ssize_t efivar_create(struct file
 	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);
 
 	/*



  parent reply	other threads:[~2012-05-04 20:46 UTC|newest]

Thread overview: 88+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-04 20:42 [ 00/75] 3.3.5-stable review Greg KH
2012-05-04 20:42 ` [ 01/75] nfs: Enclose hostname in brackets when needed in nfs_do_root_mount Greg KH
2012-05-04 20:42 ` [ 02/75] NFSv4: Ensure that the LOCK code sets exception->inode Greg KH
2012-05-04 20:42 ` [ 03/75] NFSv4: Ensure that we check lock exclusive/shared type against open modes Greg KH
2012-05-04 20:42 ` [ 04/75] NFS: put open context on error in nfs_pagein_multi Greg KH
2012-05-04 20:42 ` [ 05/75] NFS: put open context on error in nfs_flush_multi Greg KH
2012-05-04 20:42 ` [ 06/75] x86, microcode: Fix sysfs warning during module unload on unsupported CPUs Greg KH
2012-05-04 20:42 ` [ 07/75] x86, microcode: Ensure that module is only loaded on supported AMD CPUs Greg KH
2012-05-04 20:42 ` [ 08/75] x86, apic: APIC code touches invalid MSR on P5 class machines Greg KH
2012-05-04 20:42 ` [ 09/75] x86/platform: Remove incorrect error message in x86_default_fixup_cpu_id() Greg KH
2012-05-04 20:42 ` [ 10/75] Revert "autofs: work around unhappy compat problem on x86-64" Greg KH
2012-05-04 20:42 ` [ 11/75] xen: correctly check for pending events when restoring irq flags Greg KH
2012-05-04 20:42 ` [ 12/75] xen/smp: Fix crash when booting with ACPI hotplug CPUs Greg KH
2012-05-04 20:42 ` [ 13/75] ASoC: dapm: Ensure power gets managed for line widgets Greg KH
2012-05-04 20:42 ` [ 14/75] ASoC: wm8994: Improve sequencing of AIF channel enables Greg KH
2012-05-04 20:42 ` [ 15/75] dmaengine: at_hdmac: remove clear-on-read in atc_dostart() Greg KH
2012-05-04 20:42 ` [ 16/75] sched: Fix OOPS when build_sched_domains() percpu allocation fails Greg KH
2012-05-04 20:42 ` [ 17/75] tracing: Fix stacktrace of latency tracers (irqsoff and friends) Greg KH
2012-05-04 20:42 ` [ 18/75] hwmon: fam15h_power: fix bogus values with current BIOSes Greg KH
2012-05-04 20:42 ` [ 19/75] hwmon: (fam15h_power) Fix pci_device_id array Greg KH
2012-05-04 20:42 ` [ 20/75] dell-laptop: Terminate quirks list properly Greg KH
2012-05-04 20:42 ` [ 21/75] drm/radeon/kms: need to set up ss on DP bridges as well Greg KH
2012-05-04 20:42 ` [ 22/75] drm/i915: handle input/output sdvo timings separately in mode_set Greg KH
2012-05-04 20:42 ` [ 23/75] drm/i915: Set the Stencil Cache eviction policy to non-LRA mode Greg KH
2012-05-04 20:42 ` [ 24/75] drm/i915: fix integer overflow in i915_gem_execbuffer2() Greg KH
2012-05-04 20:42 ` [ 25/75] drm/i915: fix integer overflow in i915_gem_do_execbuffer() Greg KH
2012-05-04 20:42 ` [ 26/75] i387: ptrace breaks the lazy-fpu-restore logic Greg KH
2012-05-04 20:42 ` [ 27/75] nl80211: ensure interface is up in various APIs Greg KH
2012-05-04 20:42 ` [ 28/75] ALSA: HDA: Add external mic quirk for Asus Zenbook UX31E Greg KH
2012-05-04 20:42 ` [ 29/75] USB: cdc-wdm: fix race leading leading to memory corruption Greg KH
2012-05-04 20:42 ` [ 30/75] USB: EHCI: fix crash during suspend on ASUS computers Greg KH
2012-05-04 20:42 ` [ 31/75] USB: gadget: storage gadgets send wrong error code for unknown commands Greg KH
2012-05-04 20:42 ` [ 32/75] usb: gadget: dummy: do not call pullup() on udc_stop() Greg KH
2012-05-04 20:42 ` [ 33/75] usb gadget: uvc: uvc_request_data::length field must be signed Greg KH
2012-05-04 20:42 ` [ 34/75] pipes: add a "packetized pipe" mode for writing Greg KH
2012-05-04 20:42 ` [ 35/75] autofs: make the autofsv5 packet file descriptor use a packetized pipe Greg KH
2012-05-04 20:43 ` [ 36/75] crypto: talitos - properly lock access to global talitos registers Greg KH
2012-05-04 20:43 ` [ 37/75] Input: synaptics - fix regression with "image sensor" trackpads Greg KH
2012-05-04 20:43 ` [ 38/75] USB: ehci-tegra: remove redundant gpio_set_value Greg KH
2012-05-04 20:43 ` [ 39/75] ARM: 7396/1: errata: only handle ARM erratum #326103 on affected cores Greg KH
2012-05-04 20:43 ` [ 40/75] ARM: 7403/1: tls: remove covert channel via TPIDRURW Greg KH
2012-05-04 20:43 ` [ 41/75] ARM: 7406/1: hotplug: copy the affinity mask when forcefully migrating IRQs Greg KH
2012-05-04 20:43 ` [ 42/75] MIPS: ath79: fix AR933X WMAC reset code Greg KH
2012-05-04 20:43 ` [ 43/75] SCSI: libsas: fix sas_find_bcast_phy() in the presence of vacant phys Greg KH
2012-05-04 20:43 ` [ 44/75] SCSI: libsas: fix false positive device attached conditions Greg KH
2012-05-04 20:43 ` [ 45/75] efi: Add new variable attributes Greg KH
2012-05-04 20:43 ` Greg KH [this message]
2012-05-04 20:43 ` [ 47/75] x86, efi: Fix pointer math issue in handle_ramdisks() Greg KH
2012-05-04 20:43 ` [ 48/75] tools/include: Add byteshift headers for endian access Greg KH
2012-05-04 20:43 ` [ 49/75] x86, mkpiggy: Dont open code put_unaligned_le32() Greg KH
2012-05-04 20:43 ` [ 50/75] x86, boot: Restrict CFLAGS for hostprogs Greg KH
2012-05-04 20:43 ` [ 51/75] x86, efi: Fix endian issues and unaligned accesses Greg KH
2012-05-04 20:43 ` [ 52/75] x86, boot: Correct CFLAGS for hostprogs Greg KH
2012-05-04 20:43 ` [ 53/75] x86, efi: Add dedicated EFI stub entry point Greg KH
2012-05-04 20:43 ` [ 54/75] powerpc/85xx: dont call of_platform_bus_probe() twice Greg KH
2012-05-04 20:43 ` [ 55/75] PM / Hibernate: fix the number of pages used for hibernate/thaw buffering Greg KH
2012-05-04 20:43 ` [ 56/75] sched: Fix nohz load accounting -- again! Greg KH
2012-05-04 22:03   ` Doug Smythies
     [not found]     ` <002101cd2e38$3adfbc80$b09f3580$@net>
2012-05-22 16:39       ` Doug Smythies
2012-05-04 20:43 ` [ 57/75] exit_signal: simplify the "we have changed execution domain" logic Greg KH
2012-05-07  1:57   ` Ben Hutchings
2012-05-08  0:31     ` Greg KH
2012-05-09  3:07       ` Ben Hutchings
2012-05-04 20:43 ` [ 58/75] exit_signal: fix the "parent has changed security " Greg KH
2012-05-04 20:43 ` [ 59/75] md/raid5: Fix a bug about judging if the operation is syncing or replacing Greg KH
2012-05-04 20:43 ` [ 60/75] efivars: Improve variable validation Greg KH
2012-05-04 20:43 ` [ 61/75] hwmon: (coretemp) Increase CPU core limit Greg KH
2012-05-04 20:43 ` [ 62/75] nouveau: initialise has_optimus variable Greg KH
2012-05-04 20:43 ` [ 63/75] hwmon: (coretemp) fix oops on cpu unplug Greg KH
2012-05-04 20:43 ` [ 64/75] libata: skip old error history when counting probe trials Greg KH
2012-05-04 20:43 ` [ 65/75] b43: only reload config after successful initialization Greg KH
2012-05-04 20:43 ` [ 66/75] i2c: pnx: Disable clk in suspend Greg KH
2012-05-04 20:43 ` [ 67/75] ipw2200: Fix race condition in the command completion acknowledge Greg KH
2012-05-07 14:37   ` Ben Hutchings
2012-05-07 23:45     ` Stanislav Yakovlev
2012-05-08  0:14       ` Ben Hutchings
2012-05-04 20:43 ` [ 68/75] mac80211: fix AP mode EAP tx for VLAN stations Greg KH
2012-05-04 20:43 ` [ 69/75] rtlwifi: Fix oops on unload Greg KH
2012-05-04 20:43 ` [ 70/75] wl1251: fix crash on remove due to premature kfree Greg KH
2012-05-04 20:43 ` [ 71/75] wl1251: fix crash on remove due to leftover work item Greg KH
2012-05-04 20:43 ` [ 72/75] iwlwifi: do not nulify ctx->vif on reset Greg KH
2012-05-04 20:43 ` [ 73/75] iwlwifi: use correct released ucode version Greg KH
2012-05-07 14:41   ` Ben Hutchings
2012-05-07 19:24     ` Venkataraman, Meenakshi
2012-05-09  3:03       ` Ben Hutchings
2012-05-04 20:43 ` [ 74/75] iwlwifi: fix hardware queue programming Greg KH
2012-05-04 20:43 ` [ 75/75] iwlwifi: use 6000G2B for 6030 device series Greg KH
2012-05-05 19:39 ` [ 00/75] 3.3.5-stable review Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120504204228.302668175@linuxfoundation.org \
    --to=gregkh@linuxfoundation.org \
    --cc=akpm@linux-foundation.org \
    --cc=alan@lxorguk.ukuu.org.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mjg@redhat.com \
    --cc=stable@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox