ACK: [PATCH 1/3] acpica: add some extra run time verification to FACP (FADT)

[Alex Hung]

On 2017-03-25 12:10 AM, Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> While running fwts through some regression tests from random broken
> ACPI data dumps I found that some files have an FACP (FADT) that
> point to the FACS or DSDT which actually don't exist in the data
> dump. This causes ACPICA to segfault because we have pointers to
> non-existent tables. Add a level of verification on these tables
> to ensure we are not passing garbage to the ACPICA execution engine.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>  src/acpica/fwts_acpica.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 70 insertions(+)
>
> diff --git a/src/acpica/fwts_acpica.c b/src/acpica/fwts_acpica.c
> index 726623cc..c26e3e4d 100644
> --- a/src/acpica/fwts_acpica.c
> +++ b/src/acpica/fwts_acpica.c
> @@ -957,6 +957,73 @@ static int fwtsInstallLateHandlers(fwts_framework *fw)
>  }
>
>  /*
> + *  fwts_acpica_verify_table_get()
> + *	fetch a named table with some sanity checks, helper
> + *	function for fwts_acpica_verify_facp_table_pointers().
> + *	Note: table is never NULL
> + */
> +static int fwts_acpica_verify_table_get(
> +	fwts_framework *fw,
> +	const char *name,
> +	fwts_acpi_table_info **table)
> +{
> +	if (fwts_acpi_find_table(fw, name, 0, table) != FWTS_OK) {
> +		fwts_log_error(fw, "Failed to find ACPI table '%s'.", name);
> +		return FWTS_ERROR;
> +	}
> +	if (!(*table)) {
> +		/* The load may have failed to find this table */
> +		fwts_log_error(fw, "Missing ACPI table '%s' and  "
> +			"the FACP points to it.", name);
> +		return FWTS_ERROR;
> +	}
> +	if ((*table)->data == NULL) {
> +		/* This really should not happen */
> +		fwts_log_error(fw, "ACPI table %s had a NULL address "
> +			"which is unexpected.", name);
> +		return FWTS_ERROR;
> +	}
> +	return FWTS_OK;
> +}
> +
> +/*
> + *  fwts_acpica_verify_facp_table_pointers()
> + *	verify facp points to tables that actually exist
> + *	Note: if they don't exist then ACPICA segfaults
> + */
> +static int fwts_acpica_verify_facp_table_pointers(fwts_framework *fw)
> +{
> +	fwts_acpi_table_info *table;
> +	fwts_acpi_table_fadt *fadt;
> +
> +	if (fwts_acpica_verify_table_get(fw, "FACP", &table) != FWTS_OK)
> +		return FWTS_ERROR;
> +	fadt = (fwts_acpi_table_fadt *)table->data;
> +
> +	if (fadt->dsdt || fadt->x_dsdt) {
> +		if (fwts_acpica_verify_table_get(fw, "DSDT", &table) != FWTS_OK)
> +			return FWTS_ERROR;
> +	printf("FADT DSDT -> %" PRIx32 " %" PRIx64 " %" PRIx64 "\n",
> +		fadt->dsdt, fadt->x_dsdt,  table->addr);
> +		if (((uint64_t)fadt->dsdt != table->addr) &&
> +		    (fadt->x_dsdt != table->addr)) {
> +			fwts_log_error(fw, "FACP points to non-existent DSDT.");
> +			return FWTS_ERROR;
> +		}
> +	}
> +	if (fadt->firmware_control || fadt->x_firmware_ctrl) {
> +		if (fwts_acpica_verify_table_get(fw, "FACS", &table) != FWTS_OK)
> +			return FWTS_ERROR;
> +		if (((uint64_t)fadt->firmware_control != table->addr) &&
> +		    (fadt->x_firmware_ctrl != table->addr)) {
> +			fwts_log_error(fw, "FACP points to non-existent FACS.");
> +			return FWTS_ERROR;
> +		}
> +	}
> +	return FWTS_OK;
> +}
> +
> +/*
>   *  fwts_acpcia_set_fwts_framework()
>   *	set fwts_acpica_fw ptr
>   */
> @@ -1048,6 +1115,9 @@ int fwts_acpica_init(fwts_framework *fw)
>  		fwts_acpica_FADT = NULL;
>  	}
>
> +	if (fwts_acpica_verify_facp_table_pointers(fw) != FWTS_OK)
> +		return FWTS_ERROR;
> +
>  	/* Clone XSDT, make it point to tables in user address space */
>  	if (fwts_acpi_find_table(fw, "XSDT", 0, &table) != FWTS_OK)
>  		return FWTS_ERROR;
>

Acked-by: Alex Hung <alex.hung at canonical.com>