ACK: [PATCH 1/3] fwts_cpuinfo_x86: check for null pointers when using

[Alex Hung]

On 2016-07-09 07:42 AM, Ricardo Neri wrote:
> When successful, fwts_cpu_get_info returns a non-null pointer to a structure
> containing the CPU information. However, this does not imply that all the
> members of the structure are valid. This can happen if, for instance,  the
> requested CPU or one of its properties was not found.
>
> Thus, the sane thing to do is to have the callers of fwts_cpu_get_info to
> make sure that data is valid before using it. Otherwise, segmentation faults
> can occur if callers try to dereference NULL pointers from the CPU info
> structure.
>
> Signed-off-by: Ricardo Neri <ricardo.neri-calderon at linux.intel.com>
> ---
>   src/bios/mtrr/mtrr.c   |  4 ++++
>   src/cpu/nx/nx.c        | 16 ++++++++++++++++
>   src/cpu/virt/virt.c    | 10 ++++++++++
>   src/lib/src/fwts_cpu.c | 12 ++++++++++++
>   4 files changed, 42 insertions(+)
>
> diff --git a/src/bios/mtrr/mtrr.c b/src/bios/mtrr/mtrr.c
> index 3682ea1..9861fe0 100644
> --- a/src/bios/mtrr/mtrr.c
> +++ b/src/bios/mtrr/mtrr.c
> @@ -518,6 +518,10 @@ static int mtrr_test2(fwts_framework *fw)
>
>   static int mtrr_test3(fwts_framework *fw)
>   {
> +	if (fwts_cpuinfo->vendor_id == NULL) {
> +		fwts_log_error(fw, "Cannot get CPU vendor_id");
> +		return FWTS_ERROR;
> +	}
>   	if (strstr(fwts_cpuinfo->vendor_id, "AMD")) {
>   		if (klog != NULL) {
>   			if (fwts_klog_regex_find(fw, klog, "SYSCFG[MtrrFixDramModEn] not cleared by BIOS, clearing this bit") > 0)
> diff --git a/src/cpu/nx/nx.c b/src/cpu/nx/nx.c
> index b930d87..f5d74b3 100644
> --- a/src/cpu/nx/nx.c
> +++ b/src/cpu/nx/nx.c
> @@ -34,6 +34,12 @@ static int nx_test1(fwts_framework *fw)
>   		return FWTS_ERROR;
>   	}
>
> +	if (fwts_nx_cpuinfo->flags == NULL) {
> +		fwts_log_error(fw, "Cannot get CPU flags");
> +		fwts_cpu_free_info(fwts_nx_cpuinfo);
> +		return FWTS_ERROR;
> +	}
> +
>   	if (strstr(fwts_nx_cpuinfo->flags," nx")) {
>   		fwts_passed(fw, "CPU has NX flags, BIOS is not disabling it.");
>   		fwts_cpu_free_info(fwts_nx_cpuinfo);
> @@ -105,6 +111,11 @@ static int nx_test2(fwts_framework *fw)
>   			fwts_cpu_free_info(fwts_nx_cpuinfo);
>   			return FWTS_ERROR;
>   		}
> +		if (fwts_nx_cpuinfo->flags == NULL) {
> +			fwts_failed(fw, LOG_LEVEL_MEDIUM, "NXCPUInfoRead", "Cannot get CPU%d flags", i);
> +			fwts_cpu_free_info(fwts_nx_cpuinfo);
> +			return FWTS_ERROR;
> +		}
>   		if (i == 0) {
>   			cpu0_has_nx = (strstr(fwts_nx_cpuinfo->flags," nx") != NULL);
>   		} else {
> @@ -148,6 +159,11 @@ static int nx_test3(fwts_framework *fw)
>   			fwts_log_error(fw, "Cannot get CPU info");
>   			return FWTS_ERROR;
>   		}
> +		if (fwts_nx_cpuinfo->vendor_id == NULL) {
> +			fwts_log_error(fw, "Cannot get CPU vendor ID");
> +			fwts_cpu_free_info(fwts_nx_cpuinfo);
> +			return FWTS_ERROR;
> +		}
>   		if (strstr(fwts_nx_cpuinfo->vendor_id, "Intel") == NULL) {
>   			fwts_log_info(fw, "Non-Intel CPU, skipping test.");
>   			fwts_cpu_free_info(fwts_nx_cpuinfo);
> diff --git a/src/cpu/virt/virt.c b/src/cpu/virt/virt.c
> index f782e17..ecc84d0 100644
> --- a/src/cpu/virt/virt.c
> +++ b/src/cpu/virt/virt.c
> @@ -48,6 +48,16 @@ static int virt_init(fwts_framework *fw)
>   		return FWTS_ERROR;
>   	}
>
> +	if (fwts_virt_cpuinfo->vendor_id == NULL) {
> +		fwts_log_error(fw, "Cannot get CPU vendor ID");
> +		return FWTS_ERROR;
> +	}
> +
> +	if (fwts_virt_cpuinfo->flags == NULL) {
> +		fwts_log_error(fw, "Cannot get CPU flags");
> +		return FWTS_ERROR;
> +	}
> +
>   	return FWTS_OK;
>   }
>
> diff --git a/src/lib/src/fwts_cpu.c b/src/lib/src/fwts_cpu.c
> index a72500a..053f850 100644
> --- a/src/lib/src/fwts_cpu.c
> +++ b/src/lib/src/fwts_cpu.c
> @@ -171,6 +171,10 @@ static int fwts_cpu_matches_vendor_id(const char *vendor_id, bool *matches)
>
>   	if ((cpu = fwts_cpu_get_info(0)) == NULL)
>   		return FWTS_ERROR;
> +	if (cpu->vendor_id == NULL) {
> +		fwts_cpu_free_info(cpu);
> +		return FWTS_ERROR;
> +	}
>
>           *matches = (strstr(cpu->vendor_id, vendor_id) != NULL);
>
> @@ -203,6 +207,14 @@ fwts_bool fwts_cpu_has_c1e(void)
>
>   	if ((cpu = fwts_cpu_get_info(0)) == NULL)
>   		return FWTS_BOOL_ERROR;
> +	if (cpu->flags == NULL) {
> +		rc FWTS_BOOL_ERROR;
> +		goto free_info;
> +	}
> +	if (cpu->vendor_id == NULL) {
> +		rc FWTS_BOOL_ERROR;
> +		goto free_info;
> +	}
>
>   	/* no C1E on AMD */
>           if (strstr(cpu->vendor_id, "AuthenticAMD") == NULL) {
>


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