[PATCH] uefirtvariable: remove the invalid attribute tests after ExitBootServices() is performed

Colin Ian King colin.king at canonical.com
Fri Dec 14 10:52:39 UTC 2012 

On 14/12/12 09:48, Ivan Hu wrote:
> In the UEFI spec Version 2.3.1, Errata C, section 7.2, it mentions that attributes have some usage rules:
> * Attributes that have EFI_VARIABLE_RUNTIME_ACCESS set must also have
>    EFI_VARIABLE_BOOTSERVICE_ACCESS set.
>
> * Once ExitBootServices() is performed, only variables that have
>    EFI_VARIABLE_RUNTIME_ACCESS and EFI_VARIABLE_NON_VOLATILE set can be
>    set with SetVariable().

I'm still a bit uncertain about this.

So what happens if we try to set variables with the wrong attributes? 
Should the firmware return an error?  In which case, isn't it valid to 
test with invalid attributes to make sure the firmware rejects this?

Colin

>
> Once power on to Ubuntu, the attribute which sets EFI_VARIABLE_BOOTSERVICE_ACCESS,
> EFI_VARIABLE_RUNTIME_ACCESS and EFI_VARIABLE_NON_VOLATILE, should only be tests.
>
> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
> ---
>   src/uefi/uefirtvariable/uefirtvariable.c |   92 ++++++++++++------------------
>   1 file changed, 35 insertions(+), 57 deletions(-)
>
> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c
> index d87253e..d9e91d2 100644
> --- a/src/uefi/uefirtvariable/uefirtvariable.c
> +++ b/src/uefi/uefirtvariable/uefirtvariable.c
> @@ -48,11 +48,8 @@
>   static int fd;
>   EFI_GUID gtestguid1 = TEST_GUID1;
>   EFI_GUID gtestguid2 = TEST_GUID2;
> -uint32_t attributesarray[] = { FWTS_UEFI_VAR_BOOTSERVICE_ACCESS,
> -			       FWTS_UEFI_VAR_NON_VOLATILE | FWTS_UEFI_VAR_BOOTSERVICE_ACCESS,
> -			       FWTS_UEFI_VAR_BOOTSERVICE_ACCESS | FWTS_UEFI_VAR_RUNTIME_ACCESS,
> -			       FWTS_UEFI_VAR_NON_VOLATILE | FWTS_UEFI_VAR_BOOTSERVICE_ACCESS | FWTS_UEFI_VAR_RUNTIME_ACCESS};
>
> +uint32_t attributes = FWTS_UEFI_VAR_NON_VOLATILE | FWTS_UEFI_VAR_BOOTSERVICE_ACCESS | FWTS_UEFI_VAR_RUNTIME_ACCESS;
>   uint16_t variablenametest[] = {'T', 'e', 's', 't', 'v', 'a', 'r', '\0'};
>
>   static int uefirtvariable_init(fwts_framework *fw)
> @@ -86,14 +83,14 @@ static int uefirtvariable_deinit(fwts_framework *fw)
>   	return FWTS_OK;
>   }
>
> -static int getvariable_test(fwts_framework *fw, uint32_t attributes, uint64_t datasize, uint16_t *varname)
> +static int getvariable_test(fwts_framework *fw, uint64_t datasize, uint16_t *varname)
>   {
>   	long ioret;
>   	struct efi_getvariable getvariable;
>   	struct efi_setvariable setvariable;
>
>   	uint64_t status;
> -	uint8_t testdata[datasize+1];
> +	uint8_t testdata[MAX_DATA_LENGTH];
>   	uint64_t dataindex;
>   	uint64_t getdatasize;
>   	uint32_t attributestest;
> @@ -101,7 +98,7 @@ static int getvariable_test(fwts_framework *fw, uint32_t attributes, uint64_t da
>   	uint8_t data[datasize+1];
>   	for (dataindex = 0; dataindex < datasize; dataindex++)
>   		data[dataindex] = (uint8_t)dataindex;
> -	data[dataindex] = '0';
> +	data[dataindex] = '\0';
>
>   	setvariable.VariableName = varname;
>   	setvariable.VendorGuid = &gtestguid1;
> @@ -172,7 +169,6 @@ static int getvariable_test(fwts_framework *fw, uint32_t attributes, uint64_t da
>   	return FWTS_OK;
>   }
>
> -
>   static bool compare_guid(EFI_GUID *guid1, EFI_GUID *guid2)
>   {
>   	bool ident = true;
> @@ -205,7 +201,7 @@ static bool compare_name(uint16_t *name1, uint16_t *name2)
>   	return ident;
>   }
>
> -static int getnextvariable_test(fwts_framework *fw, uint32_t attributes)
> +static int getnextvariable_test(fwts_framework *fw)
>   {
>   	long ioret;
>   	uint64_t status;
> @@ -223,6 +219,7 @@ static int getnextvariable_test(fwts_framework *fw, uint32_t attributes)
>
>   	for (dataindex = 0; dataindex < datasize; dataindex++)
>   		data[dataindex] = (uint8_t)dataindex;
> +	data[dataindex] = '\0';
>
>   	setvariable.VariableName = variablenametest;
>   	setvariable.VendorGuid = &gtestguid1;
> @@ -306,7 +303,7 @@ static int setvariable_insertvariable(fwts_framework *fw, uint32_t attributes, u
>
>   	for (dataindex = 0; dataindex < datasize; dataindex++)
>   		data[dataindex] = (uint8_t)dataindex + datadiff;
> -	data[dataindex] = '0';
> +	data[dataindex] = '\0';
>
>   	setvariable.VariableName = varname;
>   	setvariable.VendorGuid = gtestguid;
> @@ -325,7 +322,7 @@ static int setvariable_insertvariable(fwts_framework *fw, uint32_t attributes, u
>   	return FWTS_OK;
>   }
>
> -static int setvariable_checkvariable(fwts_framework *fw, uint32_t attributes, uint64_t datasize,
> +static int setvariable_checkvariable(fwts_framework *fw, uint64_t datasize,
>   					uint16_t *varname, EFI_GUID *gtestguid, uint8_t datadiff)
>   {
>   	long ioret;
> @@ -404,7 +401,7 @@ static int setvariable_checkvariable_notfound(fwts_framework *fw, uint16_t *varn
>   	return FWTS_ERROR;
>   }
>
> -static int setvariable_test1(fwts_framework *fw, uint32_t attributes, uint64_t datasize1,
> +static int setvariable_test1(fwts_framework *fw, uint64_t datasize1,
>   							uint64_t datasize2, uint16_t *varname)
>   {
>   	uint8_t datadiff_g2 = 2, datadiff_g1 = 0;
> @@ -417,11 +414,11 @@ static int setvariable_test1(fwts_framework *fw, uint32_t attributes, uint64_t d
>   					&gtestguid1, datadiff_g1) == FWTS_ERROR)
>   		return FWTS_ERROR;
>
> -	if (setvariable_checkvariable(fw, attributes, datasize2, varname,
> +	if (setvariable_checkvariable(fw, datasize2, varname,
>   					&gtestguid2, datadiff_g2) == FWTS_ERROR)
>   		return FWTS_ERROR;
>
> -	if (setvariable_checkvariable(fw, attributes, datasize1, varname,
> +	if (setvariable_checkvariable(fw, datasize1, varname,
>   					&gtestguid1, datadiff_g1) == FWTS_ERROR)
>   		return FWTS_ERROR;
>
> @@ -436,7 +433,7 @@ static int setvariable_test1(fwts_framework *fw, uint32_t attributes, uint64_t d
>   	return FWTS_OK;
>   }
>
> -static int setvariable_test2(fwts_framework *fw, uint32_t attributes, uint16_t *varname)
> +static int setvariable_test2(fwts_framework *fw, uint16_t *varname)
>   {
>   	uint64_t datasize = 10;
>   	uint8_t datadiff1 = 0, datadiff2 = 2, datadiff3 = 4;
> @@ -450,7 +447,7 @@ static int setvariable_test2(fwts_framework *fw, uint32_t attributes, uint16_t *
>   					&gtestguid1, datadiff1) == FWTS_ERROR)
>   		return FWTS_ERROR;
>
> -	if (setvariable_checkvariable(fw, attributes, datasize, varname,
> +	if (setvariable_checkvariable(fw, datasize, varname,
>   					&gtestguid1, datadiff1) == FWTS_ERROR)
>   		return FWTS_ERROR;
>
> @@ -466,7 +463,7 @@ static int setvariable_test2(fwts_framework *fw, uint32_t attributes, uint16_t *
>   					&gtestguid1, datadiff2) == FWTS_ERROR)
>   		return FWTS_ERROR;
>
> -	if (setvariable_checkvariable(fw, attributes, datasize, varname,
> +	if (setvariable_checkvariable(fw, datasize, varname,
>   					&gtestguid1, datadiff2) == FWTS_ERROR)
>   		return FWTS_ERROR;
>
> @@ -480,7 +477,7 @@ static int setvariable_test2(fwts_framework *fw, uint32_t attributes, uint16_t *
>   					&gtestguid1, datadiff3) == FWTS_ERROR)
>   		return FWTS_ERROR;
>
> -	if (setvariable_checkvariable(fw, attributes, datasize, varname,
> +	if (setvariable_checkvariable(fw, datasize, varname,
>   					&gtestguid1, datadiff3) == FWTS_ERROR)
>   		return FWTS_ERROR;
>
> @@ -491,7 +488,7 @@ static int setvariable_test2(fwts_framework *fw, uint32_t attributes, uint16_t *
>   	return FWTS_OK;
>   }
>
> -static int setvariable_test3(fwts_framework *fw, uint32_t attributes)
> +static int setvariable_test3(fwts_framework *fw)
>   {
>   	uint64_t datasize = 10;
>   	uint8_t datadiff1 = 0, datadiff2 = 1, datadiff3 = 2;
> @@ -510,15 +507,15 @@ static int setvariable_test3(fwts_framework *fw, uint32_t attributes)
>   						&gtestguid1, datadiff1) == FWTS_ERROR)
>   		return FWTS_ERROR;
>
> -	if (setvariable_checkvariable(fw, attributes, datasize, variablenametest2,
> +	if (setvariable_checkvariable(fw, datasize, variablenametest2,
>   						&gtestguid1, datadiff2) == FWTS_ERROR)
>   		return FWTS_ERROR;
>
> -	if (setvariable_checkvariable(fw, attributes, datasize, variablenametest3,
> +	if (setvariable_checkvariable(fw, datasize, variablenametest3,
>   						&gtestguid1, datadiff3) == FWTS_ERROR)
>   		return FWTS_ERROR;
>
> -	if (setvariable_checkvariable(fw, attributes, datasize, variablenametest,
> +	if (setvariable_checkvariable(fw, datasize, variablenametest,
>   						&gtestguid1, datadiff1) == FWTS_ERROR)
>   		return FWTS_ERROR;
>
> @@ -537,7 +534,7 @@ static int setvariable_test3(fwts_framework *fw, uint32_t attributes)
>   	return FWTS_OK;
>   }
>
> -static int setvariable_test4(fwts_framework *fw, uint32_t attributes)
> +static int setvariable_test4(fwts_framework *fw)
>   {
>   	uint64_t datasize = 10;
>   	uint8_t datadiff = 0;
> @@ -556,7 +553,7 @@ static int setvariable_test4(fwts_framework *fw, uint32_t attributes)
>   	return FWTS_OK;
>   }
>
> -static int setvariable_test5(fwts_framework *fw, uint32_t attributes)
> +static int setvariable_test5(fwts_framework *fw)
>   {
>   	uint64_t datasize = 10;
>   	uint8_t datadiff = 0;
> @@ -577,13 +574,10 @@ static int setvariable_test5(fwts_framework *fw, uint32_t attributes)
>
>   static int uefirtvariable_test1(fwts_framework *fw)
>   {
> -	uint64_t index;
>   	uint64_t datasize = 10;
>
> -	for (index = 0; index < (sizeof(attributesarray)/(sizeof attributesarray[0])); index++) {
> -		if (getvariable_test(fw, attributesarray[index], datasize, variablenametest) == FWTS_ERROR)
> -			return FWTS_ERROR;
> -	}
> +	if (getvariable_test(fw, datasize, variablenametest) == FWTS_ERROR)
> +		return FWTS_ERROR;
>
>   	fwts_passed(fw, "UEFI runtime service GetVariable interface test passed.");
>
> @@ -592,12 +586,8 @@ static int uefirtvariable_test1(fwts_framework *fw)
>
>   static int uefirtvariable_test2(fwts_framework *fw)
>   {
> -	uint64_t index;
> -
> -	for (index = 0; index < (sizeof(attributesarray)/(sizeof attributesarray[0])); index++) {
> -		if (getnextvariable_test(fw, attributesarray[index]) == FWTS_ERROR)
> -			return FWTS_ERROR;
> -	}
> +	if (getnextvariable_test(fw) == FWTS_ERROR)
> +		return FWTS_ERROR;
>
>   	fwts_passed(fw, "UEFI runtime service GetNextVariableName interface test passed.");
>
> @@ -606,43 +596,31 @@ static int uefirtvariable_test2(fwts_framework *fw)
>
>   static int uefirtvariable_test3(fwts_framework *fw)
>   {
> -	uint64_t index;
>   	uint64_t datasize1 = 10, datasize2 = 20;
>
>   	fwts_log_info(fw, "Testing SetVariable on two different GUIDs and the same variable name.");
> -	for (index = 0; index < (sizeof(attributesarray)/(sizeof attributesarray[0])); index++) {
> -		if (setvariable_test1(fw, attributesarray[index], datasize1, datasize2,
> -								variablenametest) == FWTS_ERROR)
> -			return FWTS_ERROR;
> -	}
> +	if (setvariable_test1(fw, datasize1, datasize2, variablenametest) == FWTS_ERROR)
> +		return FWTS_ERROR;
>   	fwts_passed(fw, "SetVariable on two different GUIDs and the same variable name passed.");
>
>   	fwts_log_info(fw, "Testing SetVariable on the same and different variable data.");
> -	for (index = 0; index < (sizeof(attributesarray)/(sizeof attributesarray[0])); index++) {
> -		if (setvariable_test2(fw, attributesarray[index], variablenametest) == FWTS_ERROR)
> -			return FWTS_ERROR;
> -	}
> +	if (setvariable_test2(fw, variablenametest) == FWTS_ERROR)
> +		return FWTS_ERROR;
>   	fwts_passed(fw, "SetVariable on the same and different variable data passed.");
>
>   	fwts_log_info(fw, "Testing SetVariable on similar variable name.");
> -	for (index = 0; index < (sizeof(attributesarray)/(sizeof attributesarray[0])); index++) {
> -		if (setvariable_test3(fw, attributesarray[index]) == FWTS_ERROR)
> -			return FWTS_ERROR;
> -	}
> +	if (setvariable_test3(fw) == FWTS_ERROR)
> +		return FWTS_ERROR;
>   	fwts_passed(fw, "SetVariable on similar variable name passed.");
>
>   	fwts_log_info(fw, "Testing SetVariable on DataSize is 0.");
> -	for (index = 0; index < (sizeof(attributesarray)/(sizeof attributesarray[0])); index++) {
> -		if (setvariable_test4(fw, attributesarray[index]) == FWTS_ERROR)
> -			return FWTS_ERROR;
> -	}
> +	if (setvariable_test4(fw) == FWTS_ERROR)
> +		return FWTS_ERROR;
>   	fwts_passed(fw, "SetVariable on DataSize is 0 passed.");
>
>   	fwts_log_info(fw, "Testing SetVariable on Attributes is 0.");
> -	for (index = 0; index < (sizeof(attributesarray)/(sizeof attributesarray[0])); index++) {
> -		if (setvariable_test5(fw, attributesarray[index]) == FWTS_ERROR)
> -			return FWTS_ERROR;
> -	}
> +	if (setvariable_test5(fw) == FWTS_ERROR)
> +		return FWTS_ERROR;
>   	fwts_passed(fw, "SetVariable on Attributes is 0 passed.");
>
>   	return FWTS_OK;
>

More information about the fwts-devel mailing list