ACK: [PATCH 1/3] fwts_cpuinfo_x86: check for null pointers when using
Colin Ian King
colin.king at canonical.com
Sun Jul 10 18:23:42 UTC 2016
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>
More information about the fwts-devel
mailing list