[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <a63276d5-1be4-b140-6a4a-4ad4efa60eda@linux.ibm.com>
Date: Fri, 10 Feb 2023 16:28:40 -0500
From: Stefan Berger <stefanb@...ux.ibm.com>
To: Andrew Donnellan <ajd@...ux.ibm.com>,
linuxppc-dev@...ts.ozlabs.org, linux-integrity@...r.kernel.org
Cc: sudhakar@...ux.ibm.com, erichte@...ux.ibm.com,
gregkh@...uxfoundation.org, nayna@...ux.ibm.com, npiggin@...il.com,
linux-kernel@...r.kernel.org, zohar@...ux.ibm.com,
gjoyce@...ux.ibm.com, ruscur@...sell.cc, joel@....id.au,
bgray@...ux.ibm.com, brking@...ux.ibm.com, gcwilson@...ux.ibm.com
Subject: Re: [PATCH v6 24/26] powerpc/pseries: Implement secvars for dynamic
secure boot
On 2/10/23 16:23, Stefan Berger wrote:
>
>
>> +
>> +// 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.
>
>> + // 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);
>
>> + return ret;
>> +}
>> +
Powered by blists - more mailing lists