Problem with broken /sys/firmware/efi/mok-variables/MokListRT

[jan-kiszka]

I had an apparently broken /sys/firmware/efi/mok-variables/MokListRT which prevented mokutil from list and deleting keys from the valid /sys/firmware/efi/vars/MokListRT-*. But shim was loading signed grub without issues, using certs from the efivar.

So I hacked on mokutil like this:

diff --git a/src/mokutil.c b/src/mokutil.c
index f9820b4..fbb40e7 100644
--- a/src/mokutil.c
+++ b/src/mokutil.c
@@ -203,7 +203,8 @@ list_keys_in_var (const char *var_name, const efi_guid_t guid)
    if (ret >= 0) {
        ret = list_keys (data, data_sz);
        free(data);
-       return ret;
+       if (ret >= 0)
+           return ret;
    }

    for (i = 0; i < SIZE_MAX; i++) {
@@ -545,9 +546,8 @@ is_one_duplicate (const efi_guid_t *type,
        return 0;

    list = build_mok_list (var_data, var_data_size, &node_num);
-   if (list == NULL) {
-       goto done;
-   }
+   if (list == NULL)
+       return -1;

    for (unsigned int i = 0; i < node_num; i++) {
        efi_guid_t sigtype = list[i].header->SignatureType;
@@ -571,9 +571,7 @@ is_one_duplicate (const efi_guid_t *type,
        }
    }

-done:
-   if (list)
-       free (list);
+   free (list);

    return ret;
 }

which allowed to run mokutil -l and then also mokutil --delete .... After the latter was applied by shim, my /sys/firmware/efi/mok-variables/MokListRT is now fine, and I cannot reproduce the issue. The first variable was containing some data pointing to original certs by the Microsoft and/or the OEM. Unfortunately, I made no backup of it.

What could have happened? And is that change possibly valid, provide my case is something that needs to be accounted for?

[lcp]

Thanks for reporting the bug!

It seems that list_keys() had problem with the data from your /sys/firmware/efi/mok-variables/MokListRT but it was fine with /sys/firmware/efi/efivars/MokListRT-*. It's hard to tell what's wrong in mok-variables/MokListRT but the return value of list_keys() is indeed not handled properly.

On the other hand, those functions with the prefix "is_" are supposed to return only 0 and 1, and is_one_duplicate() is not handled correctly in is_duplicate().

[lcp]

Ah, I got it. build_mok_list() returned NULL in your case, so is_one_duplicate() has to return -1. I should rename it to get rid of the "is_" prefix.