Re: [PATCH v6 24/26] powerpc/pseries: Implement secvars for dynamic secure boot

On 2/10/23 16:23, Stefan Berger wrote:

quoted

+
+// PLPKS dynamic secure boot doesn't give us a format string in the same way OPAL does.
+// Instead, report the format using the SB_VERSION variable in the keystore.
+// The string is made up by us, and takes the form "ibm,plpks-sb-v<n>" (or "ibm,plpks-sb-unknown"
+// if the SB_VERSION variable doesn't exist). Hypervisor defines the SB_VERSION variable as a
+// "1 byte unsigned integer value".
+static ssize_t plpks_secvar_format(char *buf, size_t bufsize)
+{
+    struct plpks_var var = {0};
+    ssize_t ret;
+    u8 version;
+
+    var.component = NULL;

Since it's initialized with {0} this is not necessary.

quoted

+    // Only the signed variables have null bytes in their names, this one doesn't
+    var.name = "SB_VERSION";
+    var.namelen = strlen(var.name);
+    var.datalen = 1;
+    var.data = &version;
+
+    // Unlike the other vars, SB_VERSION is owned by firmware instead of the OS
+    ret = plpks_read_fw_var(&var);
+    if (ret) {
+        if (ret == -ENOENT) {
+            ret = snprintf(buf, bufsize, "ibm,plpks-sb-unknown");
+        } else {
+            pr_err("Error %ld reading SB_VERSION from firmware\n", ret);
+            ret = -EIO;
+        }
+        goto err;
+    }
+
+    ret = snprintf(buf, bufsize, "ibm,plpks-sb-v%hhu", version);
+
+err:
+    kfree(var.data);

remove the kfree()

Actually don't remove it but it should probably be

if (var.data != &version)
     kfree(var.data);

quoted

+    return ret;
+}
+