[PATCH] lib: fwts: add more sanity checks when fetching the RSDP (LP: #1372849)
Keng-Yu Lin
keng-yu.lin at canonical.com
Wed Sep 24 01:59:22 UTC 2014
On Tue, Sep 23, 2014 at 6:04 PM, Colin King <colin.king at canonical.com> wrote:
> 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
>
>
Acked-by: Keng-Yu Lin <kengyu at canonical.com>
More information about the fwts-devel
mailing list