[PATCH v2 1/3] powerpc/pseries: Correct secvar format representation for static key management
Srish Srinivasan
ssrish at linux.ibm.com
Fri May 30 03:57:23 AEST 2025
On 5/23/25 11:27 AM, Andrew Donnellan wrote:
> On Wed, 2025-05-21 at 16:27 +0530, Srish Srinivasan wrote:
>> On a PLPKS enabled PowerVM LPAR, the secvar format property for
>> static
>> key management is misrepresented as "ibm,plpks-sb-unknown", creating
>> reason for confusion.
>>
>> Static key management mode uses fixed, built-in keys. Dynamic key
>> management mode allows keys to be updated in production to handle
>> security updates without firmware rebuilds.
>>
>> Define a function named plpks_get_sb_keymgmt_mode() to retrieve the
>> key management mode based on the existence of the SB_VERSION property
>> in the firmware.
>>
>> Set the secvar format property to either "ibm,plpks-sb-v<version>" or
>> "ibm,plpks-sb-v0" based on the key management mode, and return the
>> length of the secvar format property.
>>
>> Co-developed-by: Souradeep <soura at imap.linux.ibm.com>
>> Signed-off-by: Souradeep <soura at imap.linux.ibm.com>
>> Signed-off-by: Srish Srinivasan <ssrish at linux.ibm.com>
>> Reviewed-by: Mimi Zohar <zohar at linux.ibm.com>
>> Reviewed-by: Stefan Berger <stefanb at linux.ibm.com>
>> Reviewed-by: Nayna Jain <nayna at linux.ibm.com>
> Thanks for the fixes, minor comment about the docs below.
>
> Reviewed-by: Andrew Donnellan <ajd at linux.ibm.com>
>
>> ---
>> Documentation/ABI/testing/sysfs-secvar | 9 ++-
>> arch/powerpc/platforms/pseries/plpks-secvar.c | 76 +++++++++++------
>> --
>> 2 files changed, 52 insertions(+), 33 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-secvar
>> b/Documentation/ABI/testing/sysfs-secvar
>> index 857cf12b0904..45281888e520 100644
>> --- a/Documentation/ABI/testing/sysfs-secvar
>> +++ b/Documentation/ABI/testing/sysfs-secvar
>> @@ -22,9 +22,12 @@ Description: A string indicating which backend is
>> in use by the firmware.
>> and is expected to be "ibm,edk2-compat-v1".
>>
>> On pseries/PLPKS, this is generated by the kernel
>> based on the
>> - version number in the SB_VERSION variable in the
>> keystore, and
>> - has the form "ibm,plpks-sb-v<version>", or
>> - "ibm,plpks-sb-unknown" if there is no SB_VERSION
>> variable.
>> + version number in the SB_VERSION variable in the
>> keystore. The
>> + version numbering in the SB_VERSION variable starts
>> from 1. The
>> + format string takes the form "ibm,plpks-sb-
>> v<version>" in the
>> + case of dynamic key management mode. Otherwise, for
>> any error in
>> + reading SB_VERSION it takes the form "ibm,plpks-sb-
>> v0",
>> + indicating that the key management mode is static.
> This last sentence is true, but makes it sound like static mode is only
> used in case of errors.
>
> Something like:
>
> "If the SB_VERSION variable does not exist (or there is an error while
> reading it), it takes the form "ibm,plpks-sb-v0", indicating that the
> key management mode is static."
>
> might be slightly clearer?
Hi Andrew,
Sure, will take this up in v3.
Thank You.
>
>>
>> What: /sys/firmware/secvar/vars/<variable name>
>> Date: August 2019
>> diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c
>> b/arch/powerpc/platforms/pseries/plpks-secvar.c
>> index 257fd1f8bc19..767e5e8c6990 100644
>> --- a/arch/powerpc/platforms/pseries/plpks-secvar.c
>> +++ b/arch/powerpc/platforms/pseries/plpks-secvar.c
>> @@ -152,39 +152,55 @@ static int plpks_set_variable(const char *key,
>> u64 key_len, u8 *data,
>> return rc;
>> }
>>
>> -// 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)
>> +/*
>> + * Return the key management mode.
>> + *
>> + * SB_VERSION is defined as a "1 byte unsigned integer value",
>> taking values
>> + * starting from 1. It is owned by the Partition Firmware and its
>> presence
>> + * indicates that the key management mode is dynamic. Any failure in
>> + * reading SB_VERSION defaults the key management mode to static.
>> The error
>> + * codes -ENOENT or -EPERM are expected in static key management
>> mode. An
>> + * unexpected error code will have to be investigated. Only signed
>> variables
>> + * have null bytes in their names, SB_VERSION does not.
>> + *
>> + * Return 0 to indicate that the key management mode is static.
>> Otherwise
>> + * return the SB_VERSION value to indicate that the key management
>> mode is
>> + * dynamic.
>> + */
>> +static u8 plpks_get_sb_keymgmt_mode(void)
>> {
>> - struct plpks_var var = {0};
>> - ssize_t ret;
>> - u8 version;
>> -
>> - var.component = NULL;
>> - // 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;
>> + u8 mode;
>> + ssize_t rc;
>> + struct plpks_var var = {
>> + .component = NULL,
>> + .name = "SB_VERSION",
>> + .namelen = 10,
>> + .datalen = 1,
>> + .data = &mode,
>> + };
>> +
>> + rc = plpks_read_fw_var(&var);
>> + if (rc) {
>> + if (rc != -ENOENT && rc != -EPERM)
>> + pr_info("Error %ld reading SB_VERSION from
>> firmware\n", rc);
>> + mode = 0;
>> }
>> + return mode;
>> +}
>>
>> - ret = snprintf(buf, bufsize, "ibm,plpks-sb-v%hhu", version);
>> -err:
>> - return ret;
>> +/*
>> + * 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, made up by us, takes the form of either
>> + * "ibm,plpks-sb-v<n>" or "ibm,plpks-sb-v0", based on the key
>> management mode,
>> + * and return the length of the secvar format property.
>> + */
>> +static ssize_t plpks_secvar_format(char *buf, size_t bufsize)
>> +{
>> + u8 mode;
>> +
>> + mode = plpks_get_sb_keymgmt_mode();
>> + return snprintf(buf, bufsize, "ibm,plpks-sb-v%hhu", mode);
>> }
>>
>> static int plpks_max_size(u64 *max_size)
More information about the Linuxppc-dev
mailing list