[PATCH] uefirtvariable: fix decalred VLA have zero size (LP: #1526815)

[Colin Ian King]

On 17/12/15 08:11, Ivan Hu wrote:
> Static analysis from clang scan-build found a declared variable-length
> array(VLA) has zero size, setvariable_insertvariable() declares data[datasize],
> however, it is called when delete a variable as follows:
> ret = setvariable_insertvariable(fw, attributes, 0, variablenametest,
> &gtestguid1, datadiff);
> so got a declared data[0] declared.
> 
> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
> ---
>  src/uefi/uefirtvariable/uefirtvariable.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c
> index b3c7559..27aa9a1 100644
> --- a/src/uefi/uefirtvariable/uefirtvariable.c
> +++ b/src/uefi/uefirtvariable/uefirtvariable.c
> @@ -867,7 +867,11 @@ static int setvariable_insertvariable(
>  	uint64_t status;
>  	uint64_t dataindex;
>  
> -	uint8_t data[datasize];
> +	uint8_t *data = malloc (datasize);

C standard states:

"If the space cannot be allocated, a null pointer is returned. If the
size of the space requested is zero, the behavior is implementation
defined: either a null pointer is returned, or the behavior is as if the
size were some nonzero value, except that the returned pointer shall not
be used to access an object."

So, I'm not entirely sure of this is legitimate code or not as were
using a implementation specific behaviour of malloc(0) here.


> +	if ((datasize != 0) && !data) {
> +		fwts_skipped(fw, "Unable to alloc memory for data");
> +		return FWTS_SKIP;
> +	}
>  
>  	for (dataindex = 0; dataindex < datasize; dataindex++)
>  		data[dataindex] = (uint8_t)dataindex + datadiff;
> @@ -892,6 +896,8 @@ static int setvariable_insertvariable(
>  				"with EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS or "
>  				"EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS "
>  				"EFI_VARIABLE_APPEND_WRITE attributes is set.");
> +			if (data)
> +				free(data);
>  			return FWTS_SKIP;
>  		}
>  		if (datasize == 0)
> @@ -910,6 +916,8 @@ static int setvariable_insertvariable(
>  					"after rebooting. Reboot and test "
>  					"again may be helpful to continue "
>  					"the test.");
> +				if (data)
> +					free(data);
>  				return FWTS_SKIP;
>  			}
>  			fwts_failed(fw, LOG_LEVEL_HIGH,
> @@ -918,8 +926,13 @@ static int setvariable_insertvariable(
>  				"runtime service.");
>  		}
>  		fwts_uefi_print_status_info(fw, status);
> +		if (data)
> +			free(data);
>  		return FWTS_ERROR;
>  	}
> +
> +	if (data)
> +		free(data);
>  	return FWTS_OK;
>  }
>  
>