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: [ 36/47] efi: Validate UEFI boot variables
Date: Fri, 04 May 2012 13:43:20 -0700	[thread overview]
Message-ID: <20120504204247.957402972@linuxfoundation.org> (raw)
In-Reply-To: <20120504204307.GA13761@kroah.com>

3.0-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
@@ -166,6 +166,176 @@ utf16_strsize(efi_char16_t *data, unsign
 	return utf16_strnlen(data, maxlength/sizeof(efi_char16_t)) * sizeof(efi_char16_t);
 }
 
+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(struct efivars *efivars, struct efi_variable *var)
 {
@@ -289,6 +459,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,
@@ -414,6 +590,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:49 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-04 20:43 [ 00/47] 3.0.31-stable review Greg KH
2012-05-04 20:42 ` [ 01/47] nfs: Enclose hostname in brackets when needed in nfs_do_root_mount Greg KH
2012-05-04 20:42 ` [ 02/47] NFSv4: Ensure that the LOCK code sets exception->inode Greg KH
2012-05-04 20:42 ` [ 03/47] NFSv4: Ensure that we check lock exclusive/shared type against open modes Greg KH
2012-05-04 20:42 ` [ 04/47] x86, apic: APIC code touches invalid MSR on P5 class machines Greg KH
2012-05-04 20:42 ` [ 05/47] Revert "autofs: work around unhappy compat problem on x86-64" Greg KH
2012-05-04 20:42 ` [ 06/47] xen: correctly check for pending events when restoring irq flags Greg KH
2012-05-04 20:42 ` [ 07/47] xen/smp: Fix crash when booting with ACPI hotplug CPUs Greg KH
2012-05-04 20:42 ` [ 08/47] ASoC: dapm: Ensure power gets managed for line widgets Greg KH
2012-05-04 20:42 ` [ 09/47] dmaengine: at_hdmac: remove clear-on-read in atc_dostart() Greg KH
2012-05-04 20:42 ` [ 10/47] hwmon: fam15h_power: fix bogus values with current BIOSes Greg KH
2012-05-04 20:42 ` [ 11/47] hwmon: (fam15h_power) Fix pci_device_id array Greg KH
2012-05-04 20:42 ` [ 12/47] drm/i915: handle input/output sdvo timings separately in mode_set Greg KH
2012-05-04 20:42 ` [ 13/47] drm/i915: fix integer overflow in i915_gem_execbuffer2() Greg KH
2012-05-04 20:42 ` [ 14/47] drm/i915: fix integer overflow in i915_gem_do_execbuffer() Greg KH
2012-05-04 20:42 ` [ 15/47] nl80211: ensure interface is up in various APIs Greg KH
2012-05-04 20:43 ` [ 16/47] EHCI: fix criterion for resuming the root hub Greg KH
2012-05-04 20:43 ` [ 17/47] brcm80211: smac: resume transmit fifo upon receiving frames Greg KH
2012-05-04 20:43 ` [ 18/47] Fix modpost failures in fedora 17 Greg KH
2012-05-04 20:43 ` [ 19/47] KVM: unmap pages from the iommu when slots are removed Greg KH
2012-05-04 20:43 ` [ 20/47] mmc: unbreak sdhci-esdhc-imx on i.MX25 Greg KH
2012-05-04 20:43 ` [ 21/47] nfsd: fix b0rken error value for setattr on read-only mount Greg KH
2012-05-04 20:43 ` [ 22/47] nfsd: fix error values returned by nfsd4_lockt() when nfsd_open() fails Greg KH
2012-05-04 20:43 ` [ 23/47] [PATCH] Revert "usb: Fix build error due to dma_mask is not at pdev_archdata at ARM" Greg KH
2012-05-07  1:59   ` Ben Hutchings
2012-05-08  0:31     ` Greg KH
2012-05-04 20:43 ` [ 24/47] USB: cdc-wdm: fix race leading leading to memory corruption Greg KH
2012-05-04 20:43 ` [ 25/47] USB: EHCI: fix crash during suspend on ASUS computers Greg KH
2012-05-04 20:43 ` [ 26/47] USB: gadget: storage gadgets send wrong error code for unknown commands Greg KH
2012-05-04 20:43 ` [ 27/47] usb gadget: uvc: uvc_request_data::length field must be signed Greg KH
2012-05-04 20:43 ` [ 28/47] pipes: add a "packetized pipe" mode for writing Greg KH
2012-05-04 20:43 ` [ 29/47] autofs: make the autofsv5 packet file descriptor use a packetized pipe Greg KH
2012-05-04 20:43 ` [ 30/47] ARM: 7403/1: tls: remove covert channel via TPIDRURW Greg KH
2012-05-04 20:43 ` [ 31/47] SCSI: libsas: fix sas_find_bcast_phy() in the presence of vacant phys Greg KH
2012-05-04 20:43 ` [ 32/47] SCSI: libsas: fix false positive device attached conditions Greg KH
2012-05-04 20:43 ` [ 33/47] efi: Add new variable attributes Greg KH
2012-05-04 20:43 ` [ 34/47] efivars: String functions Greg KH
2012-05-04 20:43 ` [ 35/47] efivars: fix warnings when CONFIG_PSTORE=n Greg KH
2012-05-04 20:43 ` Greg KH [this message]
2012-05-04 20:43 ` [ 37/47] efivars: Improve variable validation Greg KH
2012-05-04 20:43 ` [ 38/47] hwmon: (coretemp) Increase CPU core limit Greg KH
2012-05-04 20:43 ` [ 39/47] hwmon: (coretemp) fix oops on cpu unplug Greg KH
2012-05-04 20:43 ` [ 40/47] libata: skip old error history when counting probe trials Greg KH
2012-05-04 20:43 ` [ 41/47] i2c: pnx: Disable clk in suspend Greg KH
2012-05-04 20:43 ` [ 42/47] ipw2200: Fix race condition in the command completion acknowledge Greg KH
2012-05-04 20:43 ` [ 43/47] mac80211: fix AP mode EAP tx for VLAN stations Greg KH
2012-05-04 20:43 ` [ 44/47] rtlwifi: Fix oops on unload Greg KH
2012-05-04 20:43 ` [ 45/47] wl1251: fix crash on remove due to premature kfree Greg KH
2012-05-04 20:43 ` [ 46/47] wl1251: fix crash on remove due to leftover work item Greg KH
2012-05-04 20:43 ` [ 47/47] sched: Fix nohz load accounting -- again! 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=20120504204247.957402972@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