[PATCH v2 0/3] Enhancements to the secvar interface in static key management mode

[PATCH v2 0/3] Enhancements to the secvar interface in static key management mode

[Srish Srinivasan]

The PLPKS enabled Power LPAR sysfs exposes all of the secure boot secure
variables irrespective of the key management mode. There is support for
both static and dynamic key management and the key management mode can
be updated using the management console. The user can modify the secure
boot secvars db, dbx, grubdb, grubdbx, and sbat only in the dynamic key
mode. But the sysfs interface exposes these secvars even in static key
mode. This could lead to errors when reading them or writing to them in
the static key mode.

Update the secvar format property based on the key management mode and
expose only the secure variables relevant to the key management mode.
Enable loading of signed third-party kernel modules in the static key
mode when the platform keystore is enabled.

Changelog:

v2:

* Patch 1:

  - Updated plpks_get_sb_keymgmt_mode to handle -ENOENT and -EPERM in
    the case of static key management mode, based on feedback from
    Andrew.
  - Moved the documentation changes relevant to the secvar format
    property from Patch 2 to Patch 1.
  - Added reviewed-by from Nayna.

* Patch 2:

  - Moved the documentaton changes relevant to secure variables from
    /sys/firmware/secvar/format to
    /sys/firmware/secvar/vars/<variable name>.
  - Added reviewed-by from Nayna and Andrew.

* Patch 3:
  - Added reviewed-by from Nayna and Andrew.


Srish Srinivasan (3):
  powerpc/pseries: Correct secvar format representation for static key
    management
  powerpc/secvar: Expose secvars relevant to the key management mode
  integrity/platform_certs: Allow loading of keys in the static key
    management mode

 Documentation/ABI/testing/sysfs-secvar        |  15 ++-
 arch/powerpc/platforms/pseries/plpks-secvar.c | 104 ++++++++++++------
 .../integrity/platform_certs/load_powerpc.c   |   5 +-
 3 files changed, 85 insertions(+), 39 deletions(-)

-- 
2.47.1

[PATCH v2 1/3] powerpc/pseries: Correct secvar format representation for static key management

[Srish Srinivasan]

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@imap.linux.ibm.com>
Signed-off-by: Souradeep <soura@imap.linux.ibm.com>
Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Nayna Jain <nayna@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.
 
 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)
-- 
2.47.1

[PATCH v2 2/3] powerpc/secvar: Expose secvars relevant to the key management mode

[Srish Srinivasan]

The PLPKS enabled PowerVM LPAR sysfs exposes all of the secure boot
secvars irrespective of the key management mode.

The PowerVM LPAR supports static and dynamic key management for secure
boot. The key management option can be updated in the management
console. Only in the dynamic key mode can the user modify the secure
boot secvars db, dbx, grubdb, grubdbx, and sbat, which are exposed via
the sysfs interface. But the sysfs interface exposes these secvars even
in the static key mode. This could lead to errors when reading them or
writing to them in the static key mode.

Expose only PK, trustedcadb, and moduledb in the static key mode to
enable loading of signed third-party kernel modules.

Co-developed-by: Souradeep <soura@imap.linux.ibm.com>
Signed-off-by: Souradeep <soura@imap.linux.ibm.com>
Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
---
 Documentation/ABI/testing/sysfs-secvar        |  6 ++++
 arch/powerpc/platforms/pseries/plpks-secvar.c | 28 ++++++++++++++++---
 2 files changed, 30 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar
index 45281888e520..948df3446a03 100644
--- a/Documentation/ABI/testing/sysfs-secvar
+++ b/Documentation/ABI/testing/sysfs-secvar
@@ -37,6 +37,12 @@ Description:	Each secure variable is represented as a directory named as
 		representation. The data and size can be determined by reading
 		their respective attribute files.
 
+		Only secvars relevant to the key management mode are exposed.
+		Only in the dynamic key mode can the user modify the secure boot
+		secvars db, dbx, grubdb, grubdbx, and sbat. PK, trustedcadb and
+		moduledb are the secvars common to both static and dynamic key
+		management modes.
+
 What:		/sys/firmware/secvar/vars/<variable_name>/size
 Date:		August 2019
 Contact:	Nayna Jain <nayna@linux.ibm.com>
diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c b/arch/powerpc/platforms/pseries/plpks-secvar.c
index 767e5e8c6990..f9e9cc40c9d0 100644
--- a/arch/powerpc/platforms/pseries/plpks-secvar.c
+++ b/arch/powerpc/platforms/pseries/plpks-secvar.c
@@ -59,7 +59,14 @@ static u32 get_policy(const char *name)
 		return PLPKS_SIGNEDUPDATE;
 }
 
-static const char * const plpks_var_names[] = {
+static const char * const plpks_var_names_static[] = {
+	"PK",
+	"moduledb",
+	"trustedcadb",
+	NULL,
+};
+
+static const char * const plpks_var_names_dynamic[] = {
 	"PK",
 	"KEK",
 	"db",
@@ -213,21 +220,34 @@ static int plpks_max_size(u64 *max_size)
 	return 0;
 }
 
+static const struct secvar_operations plpks_secvar_ops_static = {
+	.get = plpks_get_variable,
+	.set = plpks_set_variable,
+	.format = plpks_secvar_format,
+	.max_size = plpks_max_size,
+	.config_attrs = config_attrs,
+	.var_names = plpks_var_names_static,
+};
 
-static const struct secvar_operations plpks_secvar_ops = {
+static const struct secvar_operations plpks_secvar_ops_dynamic = {
 	.get = plpks_get_variable,
 	.set = plpks_set_variable,
 	.format = plpks_secvar_format,
 	.max_size = plpks_max_size,
 	.config_attrs = config_attrs,
-	.var_names = plpks_var_names,
+	.var_names = plpks_var_names_dynamic,
 };
 
 static int plpks_secvar_init(void)
 {
+	u8 mode;
+
 	if (!plpks_is_available())
 		return -ENODEV;
 
-	return set_secvar_ops(&plpks_secvar_ops);
+	mode = plpks_get_sb_keymgmt_mode();
+	if (mode)
+		return set_secvar_ops(&plpks_secvar_ops_dynamic);
+	return set_secvar_ops(&plpks_secvar_ops_static);
 }
 machine_device_initcall(pseries, plpks_secvar_init);
-- 
2.47.1

[PATCH v2 3/3] integrity/platform_certs: Allow loading of keys in the static key management mode

[Srish Srinivasan]

On PLPKS enabled PowerVM LPAR, there is no provision to load signed
third-party kernel modules when the key management mode is static. This
is because keys from secure boot secvars are only loaded when the key
management mode is dynamic.

Allow loading of the trustedcadb and moduledb keys even in the static
key management mode, where the secvar format string takes the form
"ibm,plpks-sb-v0".

Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
---
 security/integrity/platform_certs/load_powerpc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c
index c85febca3343..714c961a00f5 100644
--- a/security/integrity/platform_certs/load_powerpc.c
+++ b/security/integrity/platform_certs/load_powerpc.c
@@ -75,12 +75,13 @@ static int __init load_powerpc_certs(void)
 		return -ENODEV;
 
 	// Check for known secure boot implementations from OPAL or PLPKS
-	if (strcmp("ibm,edk2-compat-v1", buf) && strcmp("ibm,plpks-sb-v1", buf)) {
+	if (strcmp("ibm,edk2-compat-v1", buf) && strcmp("ibm,plpks-sb-v1", buf) &&
+	    strcmp("ibm,plpks-sb-v0", buf)) {
 		pr_err("Unsupported secvar implementation \"%s\", not loading certs\n", buf);
 		return -ENODEV;
 	}
 
-	if (strcmp("ibm,plpks-sb-v1", buf) == 0)
+	if (strcmp("ibm,plpks-sb-v1", buf) == 0 || strcmp("ibm,plpks-sb-v0", buf) == 0)
 		/* PLPKS authenticated variables ESL data is prefixed with 8 bytes of timestamp */
 		offset = 8;
 
-- 
2.47.1

Re: [PATCH v2 1/3] powerpc/pseries: Correct secvar format representation for static key management

[Andrew Donnellan]

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@imap.linux.ibm.com>
> Signed-off-by: Souradeep <soura@imap.linux.ibm.com>
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Nayna Jain <nayna@linux.ibm.com>

Thanks for the fixes, minor comment about the docs below.

Reviewed-by: Andrew Donnellan <ajd@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?

>  
>  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)

-- 
Andrew Donnellan    OzLabs, ADL Canberra
ajd@linux.ibm.com   IBM Australia Limited

Re: [PATCH v2 2/3] powerpc/secvar: Expose secvars relevant to the key management mode

[Michal Suchánek]

Hello,

On Wed, May 21, 2025 at 04:27:58PM +0530, Srish Srinivasan wrote:
> The PLPKS enabled PowerVM LPAR sysfs exposes all of the secure boot
> secvars irrespective of the key management mode.
> 
> The PowerVM LPAR supports static and dynamic key management for secure
> boot. The key management option can be updated in the management
> console. Only in the dynamic key mode can the user modify the secure
> boot secvars db, dbx, grubdb, grubdbx, and sbat, which are exposed via
> the sysfs interface. But the sysfs interface exposes these secvars even
> in the static key mode. This could lead to errors when reading them or
> writing to them in the static key mode.

would it cause an error when reading these variables or only when
writing them?

Thanks

Michal


> 
> Expose only PK, trustedcadb, and moduledb in the static key mode to
> enable loading of signed third-party kernel modules.
> 
> Co-developed-by: Souradeep <soura@imap.linux.ibm.com>
> Signed-off-by: Souradeep <soura@imap.linux.ibm.com>
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
> Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
> ---
>  Documentation/ABI/testing/sysfs-secvar        |  6 ++++
>  arch/powerpc/platforms/pseries/plpks-secvar.c | 28 ++++++++++++++++---
>  2 files changed, 30 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar
> index 45281888e520..948df3446a03 100644
> --- a/Documentation/ABI/testing/sysfs-secvar
> +++ b/Documentation/ABI/testing/sysfs-secvar
> @@ -37,6 +37,12 @@ Description:	Each secure variable is represented as a directory named as
>  		representation. The data and size can be determined by reading
>  		their respective attribute files.
>  
> +		Only secvars relevant to the key management mode are exposed.
> +		Only in the dynamic key mode can the user modify the secure boot
> +		secvars db, dbx, grubdb, grubdbx, and sbat. PK, trustedcadb and
> +		moduledb are the secvars common to both static and dynamic key
> +		management modes.
> +
>  What:		/sys/firmware/secvar/vars/<variable_name>/size
>  Date:		August 2019
>  Contact:	Nayna Jain <nayna@linux.ibm.com>
> diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c b/arch/powerpc/platforms/pseries/plpks-secvar.c
> index 767e5e8c6990..f9e9cc40c9d0 100644
> --- a/arch/powerpc/platforms/pseries/plpks-secvar.c
> +++ b/arch/powerpc/platforms/pseries/plpks-secvar.c
> @@ -59,7 +59,14 @@ static u32 get_policy(const char *name)
>  		return PLPKS_SIGNEDUPDATE;
>  }
>  
> -static const char * const plpks_var_names[] = {
> +static const char * const plpks_var_names_static[] = {
> +	"PK",
> +	"moduledb",
> +	"trustedcadb",
> +	NULL,
> +};
> +
> +static const char * const plpks_var_names_dynamic[] = {
>  	"PK",
>  	"KEK",
>  	"db",
> @@ -213,21 +220,34 @@ static int plpks_max_size(u64 *max_size)
>  	return 0;
>  }
>  
> +static const struct secvar_operations plpks_secvar_ops_static = {
> +	.get = plpks_get_variable,
> +	.set = plpks_set_variable,
> +	.format = plpks_secvar_format,
> +	.max_size = plpks_max_size,
> +	.config_attrs = config_attrs,
> +	.var_names = plpks_var_names_static,
> +};
>  
> -static const struct secvar_operations plpks_secvar_ops = {
> +static const struct secvar_operations plpks_secvar_ops_dynamic = {
>  	.get = plpks_get_variable,
>  	.set = plpks_set_variable,
>  	.format = plpks_secvar_format,
>  	.max_size = plpks_max_size,
>  	.config_attrs = config_attrs,
> -	.var_names = plpks_var_names,
> +	.var_names = plpks_var_names_dynamic,
>  };
>  
>  static int plpks_secvar_init(void)
>  {
> +	u8 mode;
> +
>  	if (!plpks_is_available())
>  		return -ENODEV;
>  
> -	return set_secvar_ops(&plpks_secvar_ops);
> +	mode = plpks_get_sb_keymgmt_mode();
> +	if (mode)
> +		return set_secvar_ops(&plpks_secvar_ops_dynamic);
> +	return set_secvar_ops(&plpks_secvar_ops_static);
>  }
>  machine_device_initcall(pseries, plpks_secvar_init);
> -- 
> 2.47.1
> 
> 

Re: [PATCH v2 2/3] powerpc/secvar: Expose secvars relevant to the key management mode

[Srish Srinivasan]

On 5/23/25 11:49 AM, Michal Suchánek wrote:
> Hello,
>
> On Wed, May 21, 2025 at 04:27:58PM +0530, Srish Srinivasan wrote:
>> The PLPKS enabled PowerVM LPAR sysfs exposes all of the secure boot
>> secvars irrespective of the key management mode.
>>
>> The PowerVM LPAR supports static and dynamic key management for secure
>> boot. The key management option can be updated in the management
>> console. Only in the dynamic key mode can the user modify the secure
>> boot secvars db, dbx, grubdb, grubdbx, and sbat, which are exposed via
>> the sysfs interface. But the sysfs interface exposes these secvars even
>> in the static key mode. This could lead to errors when reading them or
>> writing to them in the static key mode.
> would it cause an error when reading these variables or only when
> writing them?
>
> Thanks
>
> Michal

Hi Michal,
Thanks for taking a look.


Yes, when PKS is enabled without enabling dynamic key secure boot, the 
secvars
are NOT yet initialized with the default keys built into the binaries, 
and therefore
reading them will result in an error.


Now, while in static key management mode with PKS enabled, if one tries to
populate secvars that are relevant to dynamic key management, the write does
not fail as long as the "Platform KeyStore Signed Update Infrastructure" 
flag on
the HMC is enabled and the signed updates are authorized by valid PK/KEK 
keys.
However, secvars like db and grubdb populated while in static key management
mode are not used by the Partition Firmware or grub as SB_VERSION is not 
present,
i.e dynamic key secure boot has not been enabled yet. In this case, when 
there is a
transition from static key management to dynamic key management, secvars 
with
the signed update policy bit set will not be overwritten by the 
hypervisor with the
default keys. Now, if the keys written into these secvars were not the 
ones that were
used to sign the grub and kernel, it would fail to verify them.


These are the reasons behind the decision to expose only those secvars 
that are
relevant to the key management mode.

>
>
>> Expose only PK, trustedcadb, and moduledb in the static key mode to
>> enable loading of signed third-party kernel modules.
>>
>> Co-developed-by: Souradeep <soura@imap.linux.ibm.com>
>> Signed-off-by: Souradeep <soura@imap.linux.ibm.com>
>> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
>> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>> Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
>> Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
>> ---
>>   Documentation/ABI/testing/sysfs-secvar        |  6 ++++
>>   arch/powerpc/platforms/pseries/plpks-secvar.c | 28 ++++++++++++++++---
>>   2 files changed, 30 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar
>> index 45281888e520..948df3446a03 100644
>> --- a/Documentation/ABI/testing/sysfs-secvar
>> +++ b/Documentation/ABI/testing/sysfs-secvar
>> @@ -37,6 +37,12 @@ Description:	Each secure variable is represented as a directory named as
>>   		representation. The data and size can be determined by reading
>>   		their respective attribute files.
>>   
>> +		Only secvars relevant to the key management mode are exposed.
>> +		Only in the dynamic key mode can the user modify the secure boot
>> +		secvars db, dbx, grubdb, grubdbx, and sbat. PK, trustedcadb and
>> +		moduledb are the secvars common to both static and dynamic key
>> +		management modes.
>> +
>>   What:		/sys/firmware/secvar/vars/<variable_name>/size
>>   Date:		August 2019
>>   Contact:	Nayna Jain <nayna@linux.ibm.com>
>> diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c b/arch/powerpc/platforms/pseries/plpks-secvar.c
>> index 767e5e8c6990..f9e9cc40c9d0 100644
>> --- a/arch/powerpc/platforms/pseries/plpks-secvar.c
>> +++ b/arch/powerpc/platforms/pseries/plpks-secvar.c
>> @@ -59,7 +59,14 @@ static u32 get_policy(const char *name)
>>   		return PLPKS_SIGNEDUPDATE;
>>   }
>>   
>> -static const char * const plpks_var_names[] = {
>> +static const char * const plpks_var_names_static[] = {
>> +	"PK",
>> +	"moduledb",
>> +	"trustedcadb",
>> +	NULL,
>> +};
>> +
>> +static const char * const plpks_var_names_dynamic[] = {
>>   	"PK",
>>   	"KEK",
>>   	"db",
>> @@ -213,21 +220,34 @@ static int plpks_max_size(u64 *max_size)
>>   	return 0;
>>   }
>>   
>> +static const struct secvar_operations plpks_secvar_ops_static = {
>> +	.get = plpks_get_variable,
>> +	.set = plpks_set_variable,
>> +	.format = plpks_secvar_format,
>> +	.max_size = plpks_max_size,
>> +	.config_attrs = config_attrs,
>> +	.var_names = plpks_var_names_static,
>> +};
>>   
>> -static const struct secvar_operations plpks_secvar_ops = {
>> +static const struct secvar_operations plpks_secvar_ops_dynamic = {
>>   	.get = plpks_get_variable,
>>   	.set = plpks_set_variable,
>>   	.format = plpks_secvar_format,
>>   	.max_size = plpks_max_size,
>>   	.config_attrs = config_attrs,
>> -	.var_names = plpks_var_names,
>> +	.var_names = plpks_var_names_dynamic,
>>   };
>>   
>>   static int plpks_secvar_init(void)
>>   {
>> +	u8 mode;
>> +
>>   	if (!plpks_is_available())
>>   		return -ENODEV;
>>   
>> -	return set_secvar_ops(&plpks_secvar_ops);
>> +	mode = plpks_get_sb_keymgmt_mode();
>> +	if (mode)
>> +		return set_secvar_ops(&plpks_secvar_ops_dynamic);
>> +	return set_secvar_ops(&plpks_secvar_ops_static);
>>   }
>>   machine_device_initcall(pseries, plpks_secvar_init);
>> -- 
>> 2.47.1
>>
>>

Re: [PATCH v2 1/3] powerpc/pseries: Correct secvar format representation for static key management

[Srish Srinivasan]

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@imap.linux.ibm.com>
>> Signed-off-by: Souradeep <soura@imap.linux.ibm.com>
>> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
>> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>> Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
> Thanks for the fixes, minor comment about the docs below.
>
> Reviewed-by: Andrew Donnellan <ajd@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)

Re: [PATCH v2 2/3] powerpc/secvar: Expose secvars relevant to the key management mode

[Michal Suchánek]

On Thu, May 29, 2025 at 10:39:58PM +0530, Srish Srinivasan wrote:
> 
> On 5/23/25 11:49 AM, Michal Suchánek wrote:
> > Hello,
> > 
> > On Wed, May 21, 2025 at 04:27:58PM +0530, Srish Srinivasan wrote:
> > > The PLPKS enabled PowerVM LPAR sysfs exposes all of the secure boot
> > > secvars irrespective of the key management mode.
> > > 
> > > The PowerVM LPAR supports static and dynamic key management for secure
> > > boot. The key management option can be updated in the management
> > > console. Only in the dynamic key mode can the user modify the secure
> > > boot secvars db, dbx, grubdb, grubdbx, and sbat, which are exposed via
> > > the sysfs interface. But the sysfs interface exposes these secvars even
> > > in the static key mode. This could lead to errors when reading them or
> > > writing to them in the static key mode.
> > would it cause an error when reading these variables or only when
> > writing them?
> > 
> > Thanks
> > 
> > Michal
> 
> Hi Michal,
> Thanks for taking a look.
> 
> 
> Yes, when PKS is enabled without enabling dynamic key secure boot, the
> secvars
> are NOT yet initialized with the default keys built into the binaries, and
> therefore
> reading them will result in an error.

That suggests that 'cannot be written' as said in the documentation and
commit message, which would imply readonly, is misleading. The value is
not accessible at all.

> Now, while in static key management mode with PKS enabled, if one tries to
> populate secvars that are relevant to dynamic key management, the write does
> not fail as long as the "Platform KeyStore Signed Update Infrastructure"
> flag on
> the HMC is enabled and the signed updates are authorized by valid PK/KEK
> keys.

Which suggests that some variables can if fact be written

> However, secvars like db and grubdb populated while in static key management
> mode are not used by the Partition Firmware or grub as SB_VERSION is not
> present,

but are not used until the key management is switched to dynamic

> i.e dynamic key secure boot has not been enabled yet. In this case, when
> there is a
> transition from static key management to dynamic key management, secvars
> with
> the signed update policy bit set will not be overwritten by the hypervisor
> with the
> default keys. Now, if the keys written into these secvars were not the ones
> that were
> used to sign the grub and kernel, it would fail to verify them.

Which is the case even for the case the system is already in dynamic key
mode, unless the variables are append-only.

Thanks

Michal

> These are the reasons behind the decision to expose only those secvars that
> are
> relevant to the key management mode.
> 
> > 
> > 
> > > Expose only PK, trustedcadb, and moduledb in the static key mode to
> > > enable loading of signed third-party kernel modules.
> > > 
> > > Co-developed-by: Souradeep <soura@imap.linux.ibm.com>
> > > Signed-off-by: Souradeep <soura@imap.linux.ibm.com>
> > > Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> > > Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
> > > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
> > > Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
> > > Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
> > > ---
> > >   Documentation/ABI/testing/sysfs-secvar        |  6 ++++
> > >   arch/powerpc/platforms/pseries/plpks-secvar.c | 28 ++++++++++++++++---
> > >   2 files changed, 30 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar
> > > index 45281888e520..948df3446a03 100644
> > > --- a/Documentation/ABI/testing/sysfs-secvar
> > > +++ b/Documentation/ABI/testing/sysfs-secvar
> > > @@ -37,6 +37,12 @@ Description:	Each secure variable is represented as a directory named as
> > >   		representation. The data and size can be determined by reading
> > >   		their respective attribute files.
> > > +		Only secvars relevant to the key management mode are exposed.
> > > +		Only in the dynamic key mode can the user modify the secure boot
> > > +		secvars db, dbx, grubdb, grubdbx, and sbat. PK, trustedcadb and
> > > +		moduledb are the secvars common to both static and dynamic key
> > > +		management modes.
> > > +
> > >   What:		/sys/firmware/secvar/vars/<variable_name>/size
> > >   Date:		August 2019
> > >   Contact:	Nayna Jain <nayna@linux.ibm.com>
> > > diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c b/arch/powerpc/platforms/pseries/plpks-secvar.c
> > > index 767e5e8c6990..f9e9cc40c9d0 100644
> > > --- a/arch/powerpc/platforms/pseries/plpks-secvar.c
> > > +++ b/arch/powerpc/platforms/pseries/plpks-secvar.c
> > > @@ -59,7 +59,14 @@ static u32 get_policy(const char *name)
> > >   		return PLPKS_SIGNEDUPDATE;
> > >   }
> > > -static const char * const plpks_var_names[] = {
> > > +static const char * const plpks_var_names_static[] = {
> > > +	"PK",
> > > +	"moduledb",
> > > +	"trustedcadb",
> > > +	NULL,
> > > +};
> > > +
> > > +static const char * const plpks_var_names_dynamic[] = {
> > >   	"PK",
> > >   	"KEK",
> > >   	"db",
> > > @@ -213,21 +220,34 @@ static int plpks_max_size(u64 *max_size)
> > >   	return 0;
> > >   }
> > > +static const struct secvar_operations plpks_secvar_ops_static = {
> > > +	.get = plpks_get_variable,
> > > +	.set = plpks_set_variable,
> > > +	.format = plpks_secvar_format,
> > > +	.max_size = plpks_max_size,
> > > +	.config_attrs = config_attrs,
> > > +	.var_names = plpks_var_names_static,
> > > +};
> > > -static const struct secvar_operations plpks_secvar_ops = {
> > > +static const struct secvar_operations plpks_secvar_ops_dynamic = {
> > >   	.get = plpks_get_variable,
> > >   	.set = plpks_set_variable,
> > >   	.format = plpks_secvar_format,
> > >   	.max_size = plpks_max_size,
> > >   	.config_attrs = config_attrs,
> > > -	.var_names = plpks_var_names,
> > > +	.var_names = plpks_var_names_dynamic,
> > >   };
> > >   static int plpks_secvar_init(void)
> > >   {
> > > +	u8 mode;
> > > +
> > >   	if (!plpks_is_available())
> > >   		return -ENODEV;
> > > -	return set_secvar_ops(&plpks_secvar_ops);
> > > +	mode = plpks_get_sb_keymgmt_mode();
> > > +	if (mode)
> > > +		return set_secvar_ops(&plpks_secvar_ops_dynamic);
> > > +	return set_secvar_ops(&plpks_secvar_ops_static);
> > >   }
> > >   machine_device_initcall(pseries, plpks_secvar_init);
> > > -- 
> > > 2.47.1
> > > 
> > > 
> 

Re: [PATCH v2 2/3] powerpc/secvar: Expose secvars relevant to the key management mode

[Srish Srinivasan]

On 6/4/25 10:11 PM, Michal Suchánek wrote:
> On Thu, May 29, 2025 at 10:39:58PM +0530, Srish Srinivasan wrote:
>> On 5/23/25 11:49 AM, Michal Suchánek wrote:
>>> Hello,
>>>
>>> On Wed, May 21, 2025 at 04:27:58PM +0530, Srish Srinivasan wrote:
>>>> The PLPKS enabled PowerVM LPAR sysfs exposes all of the secure boot
>>>> secvars irrespective of the key management mode.
>>>>
>>>> The PowerVM LPAR supports static and dynamic key management for secure
>>>> boot. The key management option can be updated in the management
>>>> console. Only in the dynamic key mode can the user modify the secure
>>>> boot secvars db, dbx, grubdb, grubdbx, and sbat, which are exposed via
>>>> the sysfs interface. But the sysfs interface exposes these secvars even
>>>> in the static key mode. This could lead to errors when reading them or
>>>> writing to them in the static key mode.
>>> would it cause an error when reading these variables or only when
>>> writing them?
>>>
>>> Thanks
>>>
>>> Michal
>> Hi Michal,
>> Thanks for taking a look.
>>
>>
>> Yes, when PKS is enabled without enabling dynamic key secure boot, the
>> secvars
>> are NOT yet initialized with the default keys built into the binaries, and
>> therefore
>> reading them will result in an error.
> That suggests that 'cannot be written' as said in the documentation and
> commit message, which would imply readonly, is misleading. The value is
> not accessible at all.

Hi Michal.

Yes, this seems to be misleading.

Will address this.

>
>> Now, while in static key management mode with PKS enabled, if one tries to
>> populate secvars that are relevant to dynamic key management, the write does
>> not fail as long as the "Platform KeyStore Signed Update Infrastructure"
>> flag on
>> the HMC is enabled and the signed updates are authorized by valid PK/KEK
>> keys.
> Which suggests that some variables can if fact be written
>
>> However, secvars like db and grubdb populated while in static key management
>> mode are not used by the Partition Firmware or grub as SB_VERSION is not
>> present,
> but are not used until the key management is switched to dynamic
>
>> i.e dynamic key secure boot has not been enabled yet. In this case, when
>> there is a
>> transition from static key management to dynamic key management, secvars
>> with
>> the signed update policy bit set will not be overwritten by the hypervisor
>> with the
>> default keys. Now, if the keys written into these secvars were not the ones
>> that were
>> used to sign the grub and kernel, it would fail to verify them.
> Which is the case even for the case the system is already in dynamic key
> mode, unless the variables are append-only.

Yes, that is correct. The main intention of this patch is to not expose 
secvars that are

to be consumed only in the dynamic key management mode while in static key

management mode.


I will post v4 with the updated patch description and documentation.

> Thanks
>
> Michal
>
>> These are the reasons behind the decision to expose only those secvars that
>> are
>> relevant to the key management mode.
>>
>>>
>>>> Expose only PK, trustedcadb, and moduledb in the static key mode to
>>>> enable loading of signed third-party kernel modules.
>>>>
>>>> Co-developed-by: Souradeep <soura@imap.linux.ibm.com>
>>>> Signed-off-by: Souradeep <soura@imap.linux.ibm.com>
>>>> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
>>>> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
>>>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>>>> Reviewed-by: Nayna Jain <nayna@linux.ibm.com>
>>>> Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>
>>>> ---
>>>>    Documentation/ABI/testing/sysfs-secvar        |  6 ++++
>>>>    arch/powerpc/platforms/pseries/plpks-secvar.c | 28 ++++++++++++++++---
>>>>    2 files changed, 30 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar
>>>> index 45281888e520..948df3446a03 100644
>>>> --- a/Documentation/ABI/testing/sysfs-secvar
>>>> +++ b/Documentation/ABI/testing/sysfs-secvar
>>>> @@ -37,6 +37,12 @@ Description:	Each secure variable is represented as a directory named as
>>>>    		representation. The data and size can be determined by reading
>>>>    		their respective attribute files.
>>>> +		Only secvars relevant to the key management mode are exposed.
>>>> +		Only in the dynamic key mode can the user modify the secure boot
>>>> +		secvars db, dbx, grubdb, grubdbx, and sbat. PK, trustedcadb and
>>>> +		moduledb are the secvars common to both static and dynamic key
>>>> +		management modes.
>>>> +
>>>>    What:		/sys/firmware/secvar/vars/<variable_name>/size
>>>>    Date:		August 2019
>>>>    Contact:	Nayna Jain <nayna@linux.ibm.com>
>>>> diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c b/arch/powerpc/platforms/pseries/plpks-secvar.c
>>>> index 767e5e8c6990..f9e9cc40c9d0 100644
>>>> --- a/arch/powerpc/platforms/pseries/plpks-secvar.c
>>>> +++ b/arch/powerpc/platforms/pseries/plpks-secvar.c
>>>> @@ -59,7 +59,14 @@ static u32 get_policy(const char *name)
>>>>    		return PLPKS_SIGNEDUPDATE;
>>>>    }
>>>> -static const char * const plpks_var_names[] = {
>>>> +static const char * const plpks_var_names_static[] = {
>>>> +	"PK",
>>>> +	"moduledb",
>>>> +	"trustedcadb",
>>>> +	NULL,
>>>> +};
>>>> +
>>>> +static const char * const plpks_var_names_dynamic[] = {
>>>>    	"PK",
>>>>    	"KEK",
>>>>    	"db",
>>>> @@ -213,21 +220,34 @@ static int plpks_max_size(u64 *max_size)
>>>>    	return 0;
>>>>    }
>>>> +static const struct secvar_operations plpks_secvar_ops_static = {
>>>> +	.get = plpks_get_variable,
>>>> +	.set = plpks_set_variable,
>>>> +	.format = plpks_secvar_format,
>>>> +	.max_size = plpks_max_size,
>>>> +	.config_attrs = config_attrs,
>>>> +	.var_names = plpks_var_names_static,
>>>> +};
>>>> -static const struct secvar_operations plpks_secvar_ops = {
>>>> +static const struct secvar_operations plpks_secvar_ops_dynamic = {
>>>>    	.get = plpks_get_variable,
>>>>    	.set = plpks_set_variable,
>>>>    	.format = plpks_secvar_format,
>>>>    	.max_size = plpks_max_size,
>>>>    	.config_attrs = config_attrs,
>>>> -	.var_names = plpks_var_names,
>>>> +	.var_names = plpks_var_names_dynamic,
>>>>    };
>>>>    static int plpks_secvar_init(void)
>>>>    {
>>>> +	u8 mode;
>>>> +
>>>>    	if (!plpks_is_available())
>>>>    		return -ENODEV;
>>>> -	return set_secvar_ops(&plpks_secvar_ops);
>>>> +	mode = plpks_get_sb_keymgmt_mode();
>>>> +	if (mode)
>>>> +		return set_secvar_ops(&plpks_secvar_ops_dynamic);
>>>> +	return set_secvar_ops(&plpks_secvar_ops_static);
>>>>    }
>>>>    machine_device_initcall(pseries, plpks_secvar_init);
>>>> -- 
>>>> 2.47.1
>>>>
>>>>