ACK: [PATCH] lib: fwts_cpu: fix memory leak and incorrect return (LP: #1549723)

[Alex Hung]

On 02/25/2016 07:11 PM, Colin King wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> Latest CoverityScan run picked up a minor memory leak:
>
> if (fwts_cpu_readmsr(0, MSR_AMD64_OSVW_STATUS, &val) != FWTS_OK)
>     CID 1351543 (#2 of 2): Resource leak (RESOURCE_LEAK)
>     leaked_storage: Variable cpu going out of scope leaks the storage
>     it points to.
>
> While fixing this bug I noticed that the case when family 0x0f
> models < rev F do not have C1E the code was returning FWTS_TRUE and
> should be returning FWTS_FALSE.
>
> Also, for non-x86 builds, skip all the x86 specific checks and always
> return FWTS_FALSE.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>   src/lib/src/fwts_cpu.c | 32 +++++++++++++++++++++-----------
>   1 file changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/src/lib/src/fwts_cpu.c b/src/lib/src/fwts_cpu.c
> index 7a4b5b2..bcdb89d 100644
> --- a/src/lib/src/fwts_cpu.c
> +++ b/src/lib/src/fwts_cpu.c
> @@ -195,22 +195,25 @@ int fwts_cpu_is_AMD(bool *is_amd)
>    */
>   fwts_bool fwts_cpu_has_c1e(void)
>   {
> +#if FWTS_ARCH_X86
>   	uint64_t val;
> +	fwts_bool rc = FWTS_FALSE;
>
>   	fwts_cpuinfo_x86 *cpu;
>
>   	if ((cpu = fwts_cpu_get_info(0)) == NULL)
>   		return FWTS_BOOL_ERROR;
>
> +	/* no C1E on AMD */
>           if (strstr(cpu->vendor_id, "AuthenticAMD") == NULL) {
> -		fwts_cpu_free_info(cpu);
> -		return FWTS_FALSE;
> +		rc = FWTS_FALSE;
> +		goto free_info;
>   	}
>
>           /* Family 0x0f models < rev F do not have C1E */
>           if (cpu->x86 == 0x0F && cpu->x86_model >= 0x40) {
> -		fwts_cpu_free_info(cpu);
> -                return FWTS_TRUE;
> +                rc = FWTS_FALSE;
> +		goto free_info;
>   	}
>
>           if (cpu->x86 == 0x10) {
> @@ -219,23 +222,30 @@ fwts_bool fwts_cpu_has_c1e(void)
>                    * by erratum #400
>                    */
>   		if (strstr(cpu->flags, "osvw") != NULL) {
> -			if (fwts_cpu_readmsr(0, MSR_AMD64_OSVW_ID_LENGTH, &val) != FWTS_OK)
> -				return FWTS_BOOL_ERROR;
> +			if (fwts_cpu_readmsr(0, MSR_AMD64_OSVW_ID_LENGTH, &val) != FWTS_OK) {
> +				rc = FWTS_BOOL_ERROR;
> +				goto free_info;
> +			}
>
>                           if (val >= 2) {
>                                   if (fwts_cpu_readmsr(0, MSR_AMD64_OSVW_STATUS, &val) != FWTS_OK)
> -					return FWTS_BOOL_ERROR;
> +					rc = FWTS_BOOL_ERROR;
> +					goto free_info;
>                                   if (!(val & 2)) {
> -					fwts_cpu_free_info(cpu);
> -					return FWTS_FALSE;
> +					rc = FWTS_FALSE;
> +					goto free_info;
>   				}
>                           }
>                   }
> -		fwts_cpu_free_info(cpu);
> -                return FWTS_TRUE;
> +                rc = FWTS_TRUE;
>           }
> +
> +free_info:
>   	fwts_cpu_free_info(cpu);
> +	return rc;
> +#else
>   	return FWTS_FALSE;
> +#endif
>   }
>
>   /*
>

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