ACK: [PATCH] uefivarinfo: allocate buffer rewrite to avoid realloc failure (LP: #1362540)

[Alex Hung]

On 14-09-15 02:34 PM, Ivan Hu wrote:
> The data buffer which was brought into firmware seems to be changed that caused
> the realloc function failed below. Rewrite the function that allocate buffer
> with the max allowed variable size without reallocing the memory.
>
> *** glibc detected *** fwts: realloc(): invalid next size: 0x00007f5e807a7080 **
>
> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
> ---
>   src/uefi/uefivarinfo/uefivarinfo.c |   55 +++++++++++++++++++-----------------
>   1 file changed, 29 insertions(+), 26 deletions(-)
>
> diff --git a/src/uefi/uefivarinfo/uefivarinfo.c b/src/uefi/uefivarinfo/uefivarinfo.c
> index 84e960b..a4d2783 100644
> --- a/src/uefi/uefivarinfo/uefivarinfo.c
> +++ b/src/uefi/uefivarinfo/uefivarinfo.c
> @@ -29,7 +29,6 @@
>   #include "fwts_efi_module.h"
>   
>   #define MAX_VARNAME_LENGTH	1024
> -#define DATA_LENGTH		1024
>   
>   static int fd;
>   
> @@ -66,7 +65,7 @@ static int do_checkvariables(
>   	fwts_framework *fw,
>   	uint64_t *usedvars,
>   	uint64_t *usedvarssize,
> -	uint64_t maxvarsize)
> +	const uint64_t maxvarsize)
>   {
>   	long ioret;
>   	uint64_t status;
> @@ -114,46 +113,50 @@ static int do_checkvariables(
>   
>   		(*usedvars)++;
>   
> -		data = malloc(DATA_LENGTH);
> +		data = malloc(maxvarsize);
>   		if (!data) {
>   			fwts_log_info(fw, "Failed to allocate memory for test.");
>   			return FWTS_ERROR;
>   		}
>   
> -		getdatasize = DATA_LENGTH;
> +		getdatasize = maxvarsize;
>   		getvariable.VariableName = variablename;
>   		getvariable.VendorGuid = &vendorguid;
>   		getvariable.DataSize = &getdatasize;
>   		getvariable.Data = data;
> -		while (true) {
> -			ioret = ioctl(fd, EFI_RUNTIME_GET_VARIABLE, &getvariable);
> -			if (ioret == -1) {
> -				if (status != EFI_BUFFER_TOO_SMALL) {
> -					fwts_log_info(fw, "Failed to get variable with UEFI runtime service.");
> -					fwts_uefi_print_status_info(fw, status);
> -					free(data);
> +
> +		ioret = ioctl(fd, EFI_RUNTIME_GET_VARIABLE, &getvariable);
> +		if (ioret == -1) {
> +			free(data);
> +			if (status != EFI_BUFFER_TOO_SMALL) {
> +				fwts_log_info(fw, "Failed to get variable with UEFI runtime service.");
> +				fwts_uefi_print_status_info(fw, status);
> +				return FWTS_ERROR;
> +			} else if (getdatasize > maxvarsize) {
> +				fwts_log_info(fw, "Variable is larger than maximum variable length.");
> +				fwts_uefi_print_status_info(fw, status);
> +
> +				/*
> +				 * Although the variable is larger than maximum variable length,
> +				 * still try to calculate the total sizes of the used variables.
> +				 */
> +				data = malloc(getdatasize);
> +				if (!data) {
> +					fwts_log_info(fw, "Failed to allocate memory for test.");
>   					return FWTS_ERROR;
>   				}
> -				if (getdatasize == maxvarsize) {
> -					fwts_log_info(fw, "Variable is larger than maximum variable length.");
> +
> +				getvariable.Data = data;
> +				ioret = ioctl(fd, EFI_RUNTIME_GET_VARIABLE, &getvariable);
> +				if (ioret == -1) {
> +					fwts_log_info(fw, "Failed to get variable with variable larger than maximum variable length.");
>   					fwts_uefi_print_status_info(fw, status);
>   					free(data);
>   					return FWTS_ERROR;
>   				}
> -				getdatasize += DATA_LENGTH;
> -				getdatasize = (getdatasize > maxvarsize) ? maxvarsize : getdatasize;
> -				data = realloc(data, getdatasize);
> -				if (data) {
> -					getvariable.DataSize = &getdatasize;
> -					getvariable.Data = data;
> -					continue;
> -				} else {
> -					fwts_log_info(fw, "Failed to allocate memory for test.");
> -					return FWTS_ERROR;
> -				}
>   			}
> -			break;
> -		};
> +		}
> +
>   		free(data);
>   
>   		(*usedvarssize) += getdatasize;

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