diff mbox

[3/8] acpi: mcfg: add safe memory read check on mmap'd PCI config memory

Message ID 20170714095225.1259-3-colin.king@canonical.com
State Accepted
Headers show

Commit Message

Colin Ian King July 14, 2017, 9:52 a.m. UTC
From: Colin Ian King <colin.king@canonical.com>

Ensure we can read the mmap'd memory.

Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
 src/acpi/mcfg/mcfg.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Alex Hung July 17, 2017, 7:20 a.m. UTC | #1
On 2017-07-14 02:52 AM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Ensure we can read the mmap'd memory.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/acpi/mcfg/mcfg.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/src/acpi/mcfg/mcfg.c b/src/acpi/mcfg/mcfg.c
> index 43c747f2..08a90e0c 100644
> --- a/src/acpi/mcfg/mcfg.c
> +++ b/src/acpi/mcfg/mcfg.c
> @@ -66,9 +66,19 @@ static int compare_config_space(
>   	}
>   
>   	if ((mapped_config_space = fwts_mmap(config->base_address, page_size)) == FWTS_MAP_FAILED) {
> -		fwts_log_error(fw, "Cannot mmap PCI config space at 0x%" PRIx64 ".", config->base_address);
> +		fwts_log_error(fw, "Cannot mmap PCI config space at 0x%" PRIx64 ".",
> +			config->base_address);
>   		return FWTS_ERROR;
>   	}
> +
> +	/* We only need to check if just the config space is readable */
> +	if (fwts_safe_memread(mapped_config_space, sizeof(config_space)) != FWTS_OK) {
> +		fwts_log_error(fw, "Cannot read PCI config space at 0x%" PRIx64 ".",
> +			config->base_address);
> +		(void)fwts_munmap(mapped_config_space, page_size);
> +		return FWTS_ERROR;
> +	}
> +
>   	/*
>   	 * Need to explicitly do byte comparisons on region
>   	 * memcmp() fails as this can do 64 bit reads
> 

Acked-by: Alex Hung <alex.hung@canonical.com>
Ivan Hu July 17, 2017, 9:48 a.m. UTC | #2
On 07/14/2017 05:52 PM, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
> 
> Ensure we can read the mmap'd memory.
> 
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
>   src/acpi/mcfg/mcfg.c | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/src/acpi/mcfg/mcfg.c b/src/acpi/mcfg/mcfg.c
> index 43c747f2..08a90e0c 100644
> --- a/src/acpi/mcfg/mcfg.c
> +++ b/src/acpi/mcfg/mcfg.c
> @@ -66,9 +66,19 @@ static int compare_config_space(
>   	}
>   
>   	if ((mapped_config_space = fwts_mmap(config->base_address, page_size)) == FWTS_MAP_FAILED) {
> -		fwts_log_error(fw, "Cannot mmap PCI config space at 0x%" PRIx64 ".", config->base_address);
> +		fwts_log_error(fw, "Cannot mmap PCI config space at 0x%" PRIx64 ".",
> +			config->base_address);
>   		return FWTS_ERROR;
>   	}
> +
> +	/* We only need to check if just the config space is readable */
> +	if (fwts_safe_memread(mapped_config_space, sizeof(config_space)) != FWTS_OK) {
> +		fwts_log_error(fw, "Cannot read PCI config space at 0x%" PRIx64 ".",
> +			config->base_address);
> +		(void)fwts_munmap(mapped_config_space, page_size);
> +		return FWTS_ERROR;
> +	}
> +
>   	/*
>   	 * Need to explicitly do byte comparisons on region
>   	 * memcmp() fails as this can do 64 bit reads
> 

Acked-by: Ivan Hu <ivan.hu@canonical.com>
diff mbox

Patch

diff --git a/src/acpi/mcfg/mcfg.c b/src/acpi/mcfg/mcfg.c
index 43c747f2..08a90e0c 100644
--- a/src/acpi/mcfg/mcfg.c
+++ b/src/acpi/mcfg/mcfg.c
@@ -66,9 +66,19 @@  static int compare_config_space(
 	}
 
 	if ((mapped_config_space = fwts_mmap(config->base_address, page_size)) == FWTS_MAP_FAILED) {
-		fwts_log_error(fw, "Cannot mmap PCI config space at 0x%" PRIx64 ".", config->base_address);
+		fwts_log_error(fw, "Cannot mmap PCI config space at 0x%" PRIx64 ".",
+			config->base_address);
 		return FWTS_ERROR;
 	}
+
+	/* We only need to check if just the config space is readable */
+	if (fwts_safe_memread(mapped_config_space, sizeof(config_space)) != FWTS_OK) {
+		fwts_log_error(fw, "Cannot read PCI config space at 0x%" PRIx64 ".",
+			config->base_address);
+		(void)fwts_munmap(mapped_config_space, page_size);
+		return FWTS_ERROR;
+	}
+
 	/*
 	 * Need to explicitly do byte comparisons on region
 	 * memcmp() fails as this can do 64 bit reads