ACK: [PATCH 1/2] uefi: clean and check status with magic value ~0ULL (LP: #1784365)
Colin Ian King
colin.king at canonical.com
Tue Aug 7 11:24:02 UTC 2018
On 07/08/18 12:18, Ivan Hu wrote:
> This patch add the clean the status value to ~0ULL, which is not define on UEFI
> Spec., can safely be used to avoid some wrong staus reports and false alarms.
> Also add the checking of staus of value ~0ULL on
> fwts_uefi_print_status_info function.
>
> Signed-off-by: Ivan Hu <ivan.hu at canonical.com>
> ---
> src/lib/src/fwts_uefi.c | 7 ++++++
> src/uefi/securebootcert/securebootcert.c | 2 +-
> src/uefi/uefirtauthvar/uefirtauthvar.c | 2 ++
> src/uefi/uefirtmisc/uefirtmisc.c | 5 ++--
> src/uefi/uefirttime/uefirttime.c | 29 ++++++++++++++--------
> src/uefi/uefirtvariable/uefirtvariable.c | 42 ++++++++++++++++++++++++--------
> src/uefi/uefivarinfo/uefivarinfo.c | 5 ++++
> 7 files changed, 69 insertions(+), 23 deletions(-)
>
> diff --git a/src/lib/src/fwts_uefi.c b/src/lib/src/fwts_uefi.c
> index e200904..f719466 100644
> --- a/src/lib/src/fwts_uefi.c
> +++ b/src/lib/src/fwts_uefi.c
> @@ -27,6 +27,8 @@
> #include <dirent.h>
> #include <stdint.h>
> #include <inttypes.h>
> +#include <errno.h>
> +#include <string.h>
>
> #include "fwts.h"
> #include "fwts_uefi.h"
> @@ -468,6 +470,11 @@ void fwts_uefi_print_status_info(fwts_framework *fw, const uint64_t status)
>
> const uefistatus_info *info;
>
> + if (status == ~0ULL) {
> + fwts_log_info(fw, "fwts test ioctl() failed, errno=%d (%s)", errno, strerror(errno));
> + return;
> + }
> +
> for (info = uefistatus_info_table; info->mnemonic != NULL; info++) {
> if (status == info->statusvalue) {
> fwts_log_info(fw, "Return status: %s. %s", info->mnemonic, info->description);
> diff --git a/src/uefi/securebootcert/securebootcert.c b/src/uefi/securebootcert/securebootcert.c
> index f1161d6..41712a7 100644
> --- a/src/uefi/securebootcert/securebootcert.c
> +++ b/src/uefi/securebootcert/securebootcert.c
> @@ -525,7 +525,7 @@ static int securebootcert_setvar(
> long ioret;
> struct efi_setvariable setvariable;
>
> - uint64_t status;
> + uint64_t status = ~0ULL;
> uint64_t datasize = 1;
>
> setvariable.VariableName = varname;
> diff --git a/src/uefi/uefirtauthvar/uefirtauthvar.c b/src/uefi/uefirtauthvar/uefirtauthvar.c
> index 7384ae3..c59b087 100644
> --- a/src/uefi/uefirtauthvar/uefirtauthvar.c
> +++ b/src/uefi/uefirtauthvar/uefirtauthvar.c
> @@ -70,6 +70,7 @@ static long setvar(
> setvariable.DataSize = datasize;
> setvariable.Data = data;
> setvariable.status = status;
> + *status = ~0ULL;
> ioret = ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
>
> return ioret;
> @@ -91,6 +92,7 @@ static long getvar(
> getvariable.DataSize = getdatasize;
> getvariable.Data = data;
> getvariable.status = status;
> + *status = ~0ULL;
> ioret = ioctl(fd, EFI_RUNTIME_GET_VARIABLE, &getvariable);
>
> return ioret;
> diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c
> index 7faa910..5679d26 100644
> --- a/src/uefi/uefirtmisc/uefirtmisc.c
> +++ b/src/uefi/uefirtmisc/uefirtmisc.c
> @@ -85,6 +85,7 @@ static int getnexthighmonotoniccount_test(fwts_framework *fw, uint32_t multitest
> getnexthighmonotoniccount.status = &status;
>
> for (i = 0; i < multitesttime; i++) {
> + status = ~0ULL;
> long ioret = ioctl(fd, EFI_RUNTIME_GET_NEXTHIGHMONOTONICCOUNT, &getnexthighmonotoniccount);
>
> if (ioret == -1) {
> @@ -124,8 +125,8 @@ static int querycapsulecapabilities_test(fwts_framework *fw, uint32_t multitestt
> querycapsulecapabilities.ResetType = &resettype;
>
> for (i = 0; i < multitesttime; i++) {
> + status = ~0ULL;
> long ioret = ioctl(fd, EFI_RUNTIME_QUERY_CAPSULECAPABILITIES, &querycapsulecapabilities);
> -
> if (ioret == -1) {
> if (status == EFI_UNSUPPORTED) {
> fwts_skipped(fw, "Not support the UEFI QueryCapsuleCapabilities runtime interface"
> @@ -219,7 +220,7 @@ static int uefirtmisc_test2(fwts_framework *fw)
>
> static int uefirtmisc_test3(fwts_framework *fw)
> {
> - uint64_t status;
> + uint64_t status = ~0ULL;
> long ioret;
> struct efi_getnexthighmonotoniccount getnexthighmonotoniccount;
>
> diff --git a/src/uefi/uefirttime/uefirttime.c b/src/uefi/uefirttime/uefirttime.c
> index ee013a7..be824e6 100644
> --- a/src/uefi/uefirttime/uefirttime.c
> +++ b/src/uefi/uefirttime/uefirttime.c
> @@ -206,7 +206,7 @@ static int uefirttime_test1(fwts_framework *fw)
> EFI_TIME efi_time;
>
> EFI_TIME_CAPABILITIES efi_time_cap;
> - uint64_t status;
> + uint64_t status = ~0ULL;
>
> gettime.Capabilities = &efi_time_cap;
> gettime.Time = &efi_time;
> @@ -236,7 +236,7 @@ static int uefirttime_test_gettime_invalid(
> {
> long ioret;
> struct efi_gettime gettime;
> - uint64_t status;
> + uint64_t status = ~0ULL;
>
> gettime.Capabilities = efi_time_cap;
> gettime.Time = efi_time;
> @@ -277,7 +277,7 @@ static int uefirttime_test4(fwts_framework *fw)
>
> long ioret;
> struct efi_settime settime;
> - uint64_t status;
> + uint64_t status = ~0ULL;
> struct efi_gettime gettime;
>
> EFI_TIME oldtime;
> @@ -325,6 +325,7 @@ static int uefirttime_test4(fwts_framework *fw)
> time.TimeZone = 2047;
>
> settime.Time = &time;
> + status = ~0ULL;
> settime.status = &status;
>
> ioret = ioctl(fd, EFI_RUNTIME_SET_TIME, &settime);
> @@ -338,6 +339,7 @@ static int uefirttime_test4(fwts_framework *fw)
> sleep(1);
>
> gettime.Time = &newtime;
> + status = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_GET_TIME, &gettime);
>
> @@ -380,6 +382,7 @@ static int uefirttime_test4(fwts_framework *fw)
>
> /* restore the previous time. */
> settime.Time = &oldtime;
> + status = ~0ULL;
> ioret = ioctl(fd, EFI_RUNTIME_SET_TIME, &settime);
> if (ioret == -1) {
> fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeSetTime",
> @@ -398,7 +401,7 @@ static int uefirttime_test_settime_invalid(
> struct efi_settime *settime)
> {
> long ioret;
> - uint64_t status;
> + uint64_t status = ~0ULL;
>
> settime->status = &status;
>
> @@ -427,7 +430,7 @@ static int uefirttime_test_settime_invalid_time(
> struct efi_gettime gettime;
> struct efi_settime settime;
> EFI_TIME oldtime, newtime;
> - uint64_t status;
> + uint64_t status = ~0ULL;
> int ret, ioret;
>
> gettime.Time = &oldtime;
> @@ -467,6 +470,7 @@ static int uefirttime_test_settime_invalid_time(
>
> /* Restore original time */
> settime.Time = &oldtime;
> + status = ~0ULL;
> settime.status = &status;
> ioret = ioctl(fd, EFI_RUNTIME_SET_TIME, &settime);
> if (ioret == -1) {
> @@ -609,7 +613,7 @@ static int uefirttime_test18(fwts_framework *fw)
> {
> long ioret;
> struct efi_getwakeuptime getwakeuptime;
> - uint64_t status;
> + uint64_t status = ~0ULL;
> uint8_t enabled, pending;
> EFI_TIME efi_time;
>
> @@ -644,7 +648,7 @@ static int uefirttime_test_getwaketime_invalid(
> struct efi_getwakeuptime *getwakeuptime)
> {
> long ioret;
> - uint64_t status;
> + uint64_t status = ~0ULL;
> getwakeuptime->status = &status;
>
> ioret = ioctl(fd, EFI_RUNTIME_GET_WAKETIME, getwakeuptime);
> @@ -722,7 +726,7 @@ static int uefirttime_test23(fwts_framework *fw)
> {
> long ioret;
> struct efi_setwakeuptime setwakeuptime;
> - uint64_t status;
> + uint64_t status = ~0ULL;
> EFI_TIME oldtime;
> EFI_TIME newtime;
>
> @@ -749,6 +753,7 @@ static int uefirttime_test23(fwts_framework *fw)
> addonehour(&oldtime);
>
> setwakeuptime.Time = &oldtime;
> + status = ~0ULL;
> setwakeuptime.status = &status;
> setwakeuptime.Enabled = true;
>
> @@ -770,6 +775,7 @@ static int uefirttime_test23(fwts_framework *fw)
> getwakeuptime.Enabled = &enabled;
> getwakeuptime.Pending = &pending;
> getwakeuptime.Time = &newtime;
> + status = ~0ULL;
> getwakeuptime.status = &status;
>
> ioret = ioctl(fd, EFI_RUNTIME_GET_WAKETIME, &getwakeuptime);
> @@ -805,6 +811,7 @@ static int uefirttime_test23(fwts_framework *fw)
> }
>
> setwakeuptime.Enabled = false;
> + status = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_SET_WAKETIME, &setwakeuptime);
> if (ioret == -1) {
> @@ -815,6 +822,7 @@ static int uefirttime_test23(fwts_framework *fw)
> }
>
> sleep(1);
> + status = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_GET_WAKETIME, &getwakeuptime);
> if (ioret == -1) {
> @@ -846,7 +854,7 @@ static int uefirttime_test_setwakeuptime_invalid(
> )
> {
> long ioret;
> - uint64_t status;
> + uint64_t status = ~0ULL;
>
> setwakeuptime->status = &status;
>
> @@ -890,7 +898,7 @@ static int uefirttime_test_setwakeuptime_invalid_time(
> struct efi_getwakeuptime getwakeuptime;
> struct efi_setwakeuptime setwakeuptime;
> EFI_TIME oldtime, newtime;
> - uint64_t status;
> + uint64_t status = ~0ULL;
> uint8_t pending, enabled;
> int ret, ioret;
>
> @@ -938,6 +946,7 @@ static int uefirttime_test_setwakeuptime_invalid_time(
>
> /* Restore original time */
> setwakeuptime.Time = &oldtime;
> + status = ~0ULL;
> setwakeuptime.status = &status;
> setwakeuptime.Enabled = true;
> ioret = ioctl(fd, EFI_RUNTIME_SET_WAKETIME, &setwakeuptime);
> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c
> index b42240e..552790e 100644
> --- a/src/uefi/uefirtvariable/uefirtvariable.c
> +++ b/src/uefi/uefirtvariable/uefirtvariable.c
> @@ -75,15 +75,19 @@ static void uefirtvariable_env_cleanup(void)
> setvariable.Attributes = 0;
> setvariable.DataSize = 0;
> setvariable.Data = &data;
> + status = ~0ULL;
> setvariable.status = &status;
> (void)ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
>
> + status = ~0ULL;
> setvariable.VariableName = variablenametest2;
> (void)ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
>
> + status = ~0ULL;
> setvariable.VariableName = variablenametest3;
> (void)ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
>
> + status = ~0ULL;
> setvariable.VariableName = variablenametest;
> setvariable.VendorGuid = >estguid2;
> (void)ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
> @@ -130,7 +134,7 @@ static int getvariable_test(
> struct efi_getvariable getvariable;
> struct efi_setvariable setvariable;
>
> - uint64_t status;
> + uint64_t status = ~0ULL;
> uint8_t testdata[MAX_DATA_LENGTH];
> uint64_t dataindex;
> uint64_t getdatasize = sizeof(testdata);
> @@ -177,6 +181,7 @@ static int getvariable_test(
> getvariable.status = &status;
>
> for (i = 0; i < multitesttime; i++) {
> + status = ~0ULL;
> ioret = ioctl(fd, EFI_RUNTIME_GET_VARIABLE, &getvariable);
> if (ioret == -1) {
> fwts_failed(fw, LOG_LEVEL_HIGH,
> @@ -221,6 +226,7 @@ static int getvariable_test(
>
> /* delete the variable */
> setvariable.DataSize = 0;
> + status = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
>
> @@ -236,6 +242,7 @@ static int getvariable_test(
> err_restore_env:
>
> setvariable.DataSize = 0;
> + status = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
>
> @@ -288,7 +295,7 @@ static bool compare_name(const uint16_t *name1, const uint16_t *name2)
> static int getnextvariable_test1(fwts_framework *fw)
> {
> long ioret;
> - uint64_t status;
> + uint64_t status = ~0ULL;
>
> struct efi_setvariable setvariable;
>
> @@ -351,6 +358,7 @@ static int getnextvariable_test1(fwts_framework *fw)
> variablename[0] = '\0';
> while (true) {
> variablenamesize = maxvariablenamesize;
> + status = ~0ULL;
> ioret = ioctl(fd, EFI_RUNTIME_GET_NEXTVARIABLENAME, &getnextvariablename);
>
> if (ioret == -1) {
> @@ -416,6 +424,7 @@ static int getnextvariable_test1(fwts_framework *fw)
>
> /* delete the variable */
> setvariable.DataSize = 0;
> + status = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
>
> @@ -431,6 +440,7 @@ static int getnextvariable_test1(fwts_framework *fw)
> err_restore_env:
>
> setvariable.DataSize = 0;
> + status = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
>
> @@ -497,6 +507,7 @@ static int getnextvariable_test2(fwts_framework *fw)
> while (true) {
> long ioret;
>
> + status = ~0ULL;
> variablenamesize = maxvariablenamesize;
> ioret = ioctl(fd, EFI_RUNTIME_GET_NEXTVARIABLENAME, &getnextvariablename);
>
> @@ -657,6 +668,7 @@ static int getnextvariable_test3(fwts_framework *fw)
> while (true) {
> struct efi_var_item *item;
>
> + status = ~0ULL;
> variablenamesize = maxvariablenamesize;
> ioret = ioctl(fd, EFI_RUNTIME_GET_NEXTVARIABLENAME, &getnextvariablename);
>
> @@ -758,7 +770,7 @@ err:
> static int getnextvariable_test4(fwts_framework *fw)
> {
> long ioret;
> - uint64_t status;
> + uint64_t status = ~0ULL;
> uint64_t i;
>
> struct efi_getnextvariablename getnextvariablename;
> @@ -787,6 +799,7 @@ static int getnextvariable_test4(fwts_framework *fw)
>
> getnextvariablename.VariableName = variablename;
> getnextvariablename.VendorGuid = NULL;
> + status = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_GET_NEXTVARIABLENAME, &getnextvariablename);
>
> @@ -800,6 +813,7 @@ static int getnextvariable_test4(fwts_framework *fw)
>
> getnextvariablename.VendorGuid = &vendorguid;
> getnextvariablename.VariableNameSize = NULL;
> + status = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_GET_NEXTVARIABLENAME, &getnextvariablename);
>
> @@ -821,6 +835,7 @@ static int getnextvariable_test4(fwts_framework *fw)
> * string in VariableName
> */
> variablename[0] = '\0';
> + status = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_GET_NEXTVARIABLENAME, &getnextvariablename);
>
> @@ -867,7 +882,7 @@ static int setvariable_insertvariable(
> long ioret;
> struct efi_setvariable setvariable;
>
> - uint64_t status;
> + uint64_t status = ~0ULL;
> uint64_t dataindex;
>
> uint8_t data[datasize + 1];
> @@ -936,7 +951,7 @@ static int setvariable_checkvariable(
> long ioret;
> struct efi_getvariable getvariable;
>
> - uint64_t status;
> + uint64_t status = ~0ULL;
> uint8_t testdata[datasize];
> uint64_t dataindex;
> uint64_t getdatasize = sizeof(testdata);
> @@ -992,7 +1007,7 @@ static int setvariable_checkvariable_notfound(
> long ioret;
> struct efi_getvariable getvariable;
>
> - uint64_t status;
> + uint64_t status = ~0ULL;
> uint8_t testdata[MAX_DATA_LENGTH];
> uint64_t getdatasize = sizeof(testdata);
> uint32_t attributestest;
> @@ -1028,7 +1043,7 @@ static int setvariable_invalidattr(
> {
> long ioret;
> struct efi_setvariable setvariable;
> - uint64_t status;
> + uint64_t status = ~0ULL;
> uint64_t dataindex;
> uint8_t data[datasize];
>
> @@ -1374,7 +1389,7 @@ static int setvariable_test8(fwts_framework *fw)
> uint32_t attr = attributes | FWTS_UEFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS;
> uint64_t datasize = 1;
> uint8_t data = 1;
> - uint64_t status;
> + uint64_t status = ~0ULL;
>
> setvariable.VariableName = variablenametest;
> setvariable.VendorGuid = >estguid1;
> @@ -1411,6 +1426,7 @@ static int do_queryvariableinfo(
> queryvariableinfo.RemainingVariableStorageSize = remvarstoragesize;
> queryvariableinfo.MaximumVariableSize = maxvariablesize;
> queryvariableinfo.status = status;
> + *status = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_QUERY_VARIABLEINFO, &queryvariableinfo);
>
> @@ -1425,7 +1441,7 @@ static int getnextvariable_multitest(
> const uint32_t multitesttime)
> {
> long ioret;
> - uint64_t status;
> + uint64_t status = ~0ULL;
> uint32_t i;
>
> struct efi_setvariable setvariable;
> @@ -1475,6 +1491,7 @@ static int getnextvariable_multitest(
> for (i = 0; i < multitesttime; i++) {
> variablename[0] = '\0';
> variablenamesize = MAX_DATA_LENGTH;
> + status = ~0ULL;
> ioret = ioctl(fd, EFI_RUNTIME_GET_NEXTVARIABLENAME, &getnextvariablename);
>
> if (ioret == -1) {
> @@ -1487,6 +1504,7 @@ static int getnextvariable_multitest(
> };
>
> setvariable.DataSize = 0;
> + status = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
>
> @@ -1501,6 +1519,7 @@ static int getnextvariable_multitest(
> err_restore_env:
>
> setvariable.DataSize = 0;
> + status = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
>
> @@ -1816,6 +1835,7 @@ static void getvariable_test_invalid(
> long ioret;
>
> fwts_log_info(fw, "Testing GetVariable with %s.", test);
> + *(getvariable->status) = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_GET_VARIABLE, getvariable);
> if (ioret == -1) {
> @@ -1845,7 +1865,8 @@ static int uefirtvariable_test8(fwts_framework *fw)
> struct efi_getvariable getvariable;
> struct efi_setvariable setvariable;
> uint8_t data[16];
> - uint64_t status, dataindex;
> + uint64_t status= ~0ULL;
> + uint64_t dataindex;
> uint64_t getdatasize = sizeof(data);
> uint32_t attr;
> int ioret;
> @@ -1920,6 +1941,7 @@ static int uefirtvariable_test8(fwts_framework *fw)
>
> /* delete the variable */
> setvariable.DataSize = 0;
> + status = ~0ULL;
> ioret = ioctl(fd, EFI_RUNTIME_SET_VARIABLE, &setvariable);
> if (ioret == -1) {
> fwts_failed(fw, LOG_LEVEL_HIGH, "UEFIRuntimeSetVariable",
> diff --git a/src/uefi/uefivarinfo/uefivarinfo.c b/src/uefi/uefivarinfo/uefivarinfo.c
> index f8f1f1b..a54a636 100644
> --- a/src/uefi/uefivarinfo/uefivarinfo.c
> +++ b/src/uefi/uefivarinfo/uefivarinfo.c
> @@ -99,6 +99,7 @@ static int do_checkvariables(
> variablename[0] = '\0';
> while (true) {
> long ioret;
> + status = ~0ULL;
>
> variablenamesize = MAX_VARNAME_LENGTH;
> ioret = ioctl(fd, EFI_RUNTIME_GET_NEXTVARIABLENAME, &getnextvariablename);
> @@ -127,6 +128,7 @@ static int do_checkvariables(
> getvariable.VendorGuid = &vendorguid;
> getvariable.DataSize = &getdatasize;
> getvariable.Data = data;
> + status = ~0ULL;
>
> ioret = ioctl(fd, EFI_RUNTIME_GET_VARIABLE, &getvariable);
> if (ioret == -1) {
> @@ -151,6 +153,8 @@ static int do_checkvariables(
> }
>
> getvariable.Data = data;
> + status = ~0ULL;
> +
> 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.");
> @@ -185,6 +189,7 @@ static int do_queryvariableinfo(
> queryvariableinfo.MaximumVariableStorageSize = maxvarstoragesize;
> queryvariableinfo.RemainingVariableStorageSize = remvarstoragesize;
> queryvariableinfo.MaximumVariableSize = maxvariablesize;
> + *status = ~0ULL;
> queryvariableinfo.status = status;
>
> ioret = ioctl(fd, EFI_RUNTIME_QUERY_VARIABLEINFO, &queryvariableinfo);
>
Thanks Ivan, much appreciated!
Acked-by: Colin Ian King <colin.king at canonical.com>
More information about the fwts-devel
mailing list