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

[Colin Ian King]

On 04/01/16 10:18, Colin Ian King wrote:
> 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.

uefi/uefirtvariable/uefirtvariable.c:870:18: warning: Call to 'malloc'
has an allocation size of 0 bytes
        uint8_t *data = malloc (datasize);
                        ^~~~~~~~~~~~~~~~~

clang scan-build also doesn't like it :-(
> 
> 
>> +	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;
>>  }
>>  
>>
>