[PATCH] lib: fwts: add more sanity checks when fetching the RSDP (LP: #1372849)

Colin King colin.king at canonical.com
Tue Sep 23 10:04:46 UTC 2014


From: Colin Ian King <colin.king at canonical.com>

The --rsdp option allows a user to specify the RSDP address, which
could be erroneous. Also, it is possible (but very unlikely) that
the RSDP in set incorrectly at boot time. Currently fwts_acpi_get_rsdp()
does some checking but fails to return NULL on failure so fwts segfaults.
Also fwts assumes that the RSDP address is readable, but we should also
catch any segv faults in case it is not.

This patch adds some "safe" memory helpers that can catch segfaults and
uses these in the table loading mmap'd reads and copies.  The patch also
ensures NULL is returned if the RSDP is fails any sanity checks.

Signed-off-by: Colin Ian King <colin.king at canonical.com>
---
 src/lib/include/fwts.h          |  1 +
 src/lib/include/fwts_safe_mem.h | 26 +++++++++++++++++
 src/lib/src/Makefile.am         |  5 ++--
 src/lib/src/fwts_acpi_tables.c  | 37 +++++++++++++++++++-----
 src/lib/src/fwts_safe_mem.c     | 63 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 123 insertions(+), 9 deletions(-)
 create mode 100644 src/lib/include/fwts_safe_mem.h
 create mode 100644 src/lib/src/fwts_safe_mem.c

diff --git a/src/lib/include/fwts.h b/src/lib/include/fwts.h
index 84075f7..730967d 100644
--- a/src/lib/include/fwts.h
+++ b/src/lib/include/fwts.h
@@ -80,5 +80,6 @@
 #include "fwts_ioport.h"
 #include "fwts_release.h"
 #include "fwts_pci.h"
+#include "fwts_safe_mem.h"
 
 #endif
diff --git a/src/lib/include/fwts_safe_mem.h b/src/lib/include/fwts_safe_mem.h
new file mode 100644
index 0000000..4490026
--- /dev/null
+++ b/src/lib/include/fwts_safe_mem.h
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2014 Canonical
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ */
+
+#ifndef __FWTS_SAFE_MEMCPY_H__
+#define __FWTS_SAFE_MEMCPY_H__
+
+int fwts_safe_memcpy(void *dst, const void *src, const size_t n);
+int fwts_safe_memread(const void *src, const size_t n);
+
+#endif
diff --git a/src/lib/src/Makefile.am b/src/lib/src/Makefile.am
index 1385c68..fca9c06 100644
--- a/src/lib/src/Makefile.am
+++ b/src/lib/src/Makefile.am
@@ -72,5 +72,6 @@ libfwts_la_SOURCES = 		\
 	fwts_text_list.c 	\
 	fwts_tty.c 		\
 	fwts_uefi.c 		\
-	fwts_wakealarm.c \
-	fwts_pm_method.c
+	fwts_wakealarm.c 	\
+	fwts_pm_method.c	\
+	fwts_safe_mem.c
diff --git a/src/lib/src/fwts_acpi_tables.c b/src/lib/src/fwts_acpi_tables.c
index 343f849..56498e0 100644
--- a/src/lib/src/fwts_acpi_tables.c
+++ b/src/lib/src/fwts_acpi_tables.c
@@ -112,9 +112,16 @@ static void *fwts_acpi_find_rsdp_bios(void)
 	/* Scan BIOS for RSDP, ACPI spec states it is aligned on 16 byte intervals */
 	for (ptr = bios; ptr < (bios+BIOS_LENGTH); ptr += 16) {
 		rsdp = (fwts_acpi_table_rsdp*)ptr;
+
+		/* Can we read this memory w/o segfaulting? */
+		if (fwts_safe_memread(rsdp, 8) != FWTS_OK)
+			continue;
+
 		/* Look for RSD PTR string */
 		if (strncmp(rsdp->signature, "RSD PTR ",8) == 0) {
 			int length = (rsdp->revision < 1) ? 20 : 36;
+			if (fwts_safe_memread(ptr, length) != FWTS_OK)
+				continue;
 			if (fwts_checksum(ptr, length) == 0) {
 				addr = (void*)(BIOS_START+(ptr - bios));
 				break;
@@ -134,7 +141,7 @@ static void *fwts_acpi_find_rsdp_bios(void)
  *	given the address of the rsdp, map in the region, copy it and
  *	return the rsdp table. Return NULL if fails.
  */
-static fwts_acpi_table_rsdp *fwts_acpi_get_rsdp(void *addr, size_t *rsdp_len)
+static fwts_acpi_table_rsdp *fwts_acpi_get_rsdp(fwts_framework *fw, void *addr, size_t *rsdp_len)
 {
 	uint8_t *mem;
 	fwts_acpi_table_rsdp *rsdp = NULL;
@@ -143,17 +150,27 @@ static fwts_acpi_table_rsdp *fwts_acpi_get_rsdp(void *addr, size_t *rsdp_len)
 	if ((mem = fwts_mmap((off_t)addr, sizeof(fwts_acpi_table_rsdp))) == FWTS_MAP_FAILED)
 		return NULL;
 
+	if (fwts_safe_memread(mem, sizeof(fwts_acpi_table_rsdp)) != FWTS_OK) {
+		fwts_log_error(fw, "Cannot safely read RSDP from address %p.", mem);
+		goto out;
+	}
+
 	rsdp = (fwts_acpi_table_rsdp*)mem;
 	/* Determine original RSDP size from revision. */
 	*rsdp_len = (rsdp->revision < 1) ? 20 : 36;
 
 	/* Must have the correct signature */
-	if (strncmp(rsdp->signature, "RSD PTR ", 8))
+	if (strncmp(rsdp->signature, "RSD PTR ", 8)) {
+		fwts_log_error(fw, "RSDP did not have expected signature.");
+		rsdp = NULL;
 		goto out;
+	}
 
 	/* Assume version 2.0 size, we don't care about a few bytes over allocation if it's version 1.0 */
-	if ((rsdp = (fwts_acpi_table_rsdp*)fwts_low_calloc(1, sizeof(fwts_acpi_table_rsdp))) == NULL)
+	if ((rsdp = (fwts_acpi_table_rsdp*)fwts_low_calloc(1, sizeof(fwts_acpi_table_rsdp))) == NULL) {
+		rsdp = NULL;
 		goto out;
+	}
 
 	memcpy(rsdp, mem, *rsdp_len);
 out:
@@ -177,6 +194,11 @@ static void *fwts_acpi_load_table(const off_t addr)
 	if ((hdr = fwts_mmap((off_t)addr, sizeof(fwts_acpi_table_header))) == FWTS_MAP_FAILED)
 		return NULL;
 
+	if (fwts_safe_memread(hdr, sizeof(fwts_acpi_table_header)) != FWTS_OK) {
+		(void)fwts_munmap(hdr, sizeof(fwts_acpi_table_header));
+		return NULL;
+	}
+
 	len = hdr->length;
 	if (len < (int)sizeof(fwts_acpi_table_header)) {
 		(void)fwts_munmap(hdr, sizeof(fwts_acpi_table_header));
@@ -187,11 +209,12 @@ static void *fwts_acpi_load_table(const off_t addr)
 
 	if ((table = fwts_low_calloc(1, len)) == NULL)
 		return NULL;
-
 	if ((mem = fwts_mmap((off_t)addr, len)) == FWTS_MAP_FAILED)
 		return NULL;
-
-	memcpy(table, mem, len);
+	if (fwts_safe_memcpy(table, mem, len) != FWTS_OK) {
+		(void)fwts_munmap(mem, len);
+		return NULL;
+	}
 	(void)fwts_munmap(mem, len);
 
 	return table;
@@ -392,7 +415,7 @@ static int fwts_acpi_load_tables_from_firmware(fwts_framework *fw)
 		return FWTS_ERROR;
 
 	/* Load and save cached RSDP */
-	if ((rsdp = fwts_acpi_get_rsdp(rsdp_addr, &rsdp_len)) == NULL)
+	if ((rsdp = fwts_acpi_get_rsdp(fw, rsdp_addr, &rsdp_len)) == NULL)
 		return FWTS_ERROR;
 
 	fwts_acpi_add_table("RSDP", rsdp, (uint64_t)(off_t)rsdp_addr, rsdp_len, FWTS_ACPI_TABLE_FROM_FIRMWARE);
diff --git a/src/lib/src/fwts_safe_mem.c b/src/lib/src/fwts_safe_mem.c
new file mode 100644
index 0000000..6fcfe1f
--- /dev/null
+++ b/src/lib/src/fwts_safe_mem.c
@@ -0,0 +1,63 @@
+/*
+ * Copyright (C) 2014 Canonical
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
+ *
+ */
+#include <string.h>
+#include <signal.h>
+#include <setjmp.h>
+
+#include "fwts.h"
+
+static sigjmp_buf jmpbuf;
+
+/*
+ *  If we hit a SIGSEGV then the port read
+ *  failed and we longjmp back and return
+ *  FWTS_ERROR
+ */
+static void segv_handler(int dummy)
+{
+	FWTS_UNUSED(dummy);
+
+	signal(SIGSEGV, SIG_DFL);
+	siglongjmp(jmpbuf, 1);
+}
+
+/*
+ *  fwts_safe_memcpy()
+ *	memcpy that catches segfaults. to be used when
+ *	attempting to read BIOS tables from memory which
+ *	may segfault if the src address is corrupt
+ */
+int fwts_safe_memcpy(void *dst, const void *src, const size_t n)
+{
+	if (sigsetjmp(jmpbuf, 1) != 0)
+		return FWTS_ERROR;
+	
+	signal(SIGSEGV, segv_handler);
+	memcpy(dst, src, n);
+	signal(SIGSEGV, SIG_DFL);
+
+	return FWTS_OK;
+}
+
+int fwts_safe_memread(const void *src, const size_t n)
+{
+	unsigned char buf[n];
+	
+	return fwts_safe_memcpy(buf, src, n);
+}
-- 
2.1.0




More information about the fwts-devel mailing list