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

[Andrew Donnellan <ajd@linux.ibm.com>]

On Wed, 2025-04-30 at 14:33 +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.
> 
> 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>

I'm assuming it's been determined that there's no value in letting
userspace see db/dbx/etc in a read-only way in static mode?

With one comment below:

Reviewed-by: Andrew Donnellan <ajd@linux.ibm.com>

> ---
>  Documentation/ABI/testing/sysfs-secvar        |  9 ++++--
>  arch/powerpc/platforms/pseries/plpks-secvar.c | 28 ++++++++++++++++-
> --
>  2 files changed, 30 insertions(+), 7 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/sysfs-secvar
> b/Documentation/ABI/testing/sysfs-secvar
> index 857cf12b0904..2bdc7d9c0c10 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.
> +		existence of the SB_VERSION property in firmware.
> This string
> +		takes the form "ibm,plpks-sb-v1" in the presence of
> SB_VERSION,
> +		indicating the key management mode is dynamic.
> Otherwise it
> +		takes the form "ibm,plpks-sb-v0" in the static key
> management
> +		mode. Only secvars relevant to the key management
> mode are
> +		exposed.

Everything except the last sentence here is relevant to the previous
patch in the series (noting my comments on the previous patch about the
string).

The last sentence is more related to the <variable name> entry than the
format entry, and perhaps worth including a list of what variables are
applicable to each mode.

>  
>  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 d57067a733ab..cbcb2c356f2a 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",
> @@ -207,21 +214,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);

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