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

[Colin Ian King]

On 09/07/16 00:42, 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) {
> 
Thanks

Acked-by: Colin Ian King <colin.king at canonical.com>