Re: [PATCH 4/4] powerpc/pseries: update SED for PLPKS api changes

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

On Fri, 2023-05-05 at 14:44 -0500, gjoyce@linux.vnet.ibm.com wrote:
> From: Greg Joyce <gjoyce@linux.vnet.ibm.com>
> 
> Changes to the PLPKS API require minor updates to the SED Opal
> PLPKS keystore code.
> 
> Signed-off-by: Greg Joyce <gjoyce@linux.vnet.ibm.com>

[+ Nayna]

This patch will need to be squashed with patch 2.

> ---
>  arch/powerpc/platforms/pseries/Kconfig        |  6 +++++
>  arch/powerpc/platforms/pseries/Makefile       |  2 +-
>  .../powerpc/platforms/pseries/plpks_sed_ops.c | 22 +++++------------
> --
>  block/Kconfig                                 |  1 +
>  4 files changed, 13 insertions(+), 18 deletions(-)
> 
> diff --git a/arch/powerpc/platforms/pseries/Kconfig
> b/arch/powerpc/platforms/pseries/Kconfig
> index 21b22bf16ce6..c2f8a29e7b9b 100644
> --- a/arch/powerpc/platforms/pseries/Kconfig
> +++ b/arch/powerpc/platforms/pseries/Kconfig
> @@ -163,6 +163,12 @@ config PSERIES_PLPKS
>         # This option is selected by in-kernel consumers that require
>         # access to the PKS.
>  
> +config PSERIES_PLPKS_SED
> +       depends on PPC_PSERIES
> +       bool
> +       # This option is selected by in-kernel consumers that require
> +       # access to the SED PKS keystore.
> +
>  config PAPR_SCM
>         depends on PPC_PSERIES && MEMORY_HOTPLUG && LIBNVDIMM
>         tristate "Support for the PAPR Storage Class Memory
> interface"
> diff --git a/arch/powerpc/platforms/pseries/Makefile
> b/arch/powerpc/platforms/pseries/Makefile
> index 4242aed0d5d3..1476c5e4433c 100644
> --- a/arch/powerpc/platforms/pseries/Makefile
> +++ b/arch/powerpc/platforms/pseries/Makefile
> @@ -29,7 +29,7 @@ obj-$(CONFIG_PPC_SVM)         += svm.o
>  obj-$(CONFIG_FA_DUMP)          += rtas-fadump.o
>  obj-$(CONFIG_PSERIES_PLPKS)    += plpks.o
>  obj-$(CONFIG_PPC_SECURE_BOOT)  += plpks-secvar.o
> -obj-$(CONFIG_PSERIES_PLPKS_SED)        += plpks-sed.o
> +obj-$(CONFIG_PSERIES_PLPKS_SED)        += plpks_sed_ops.o

I think you could just use obj-$(CONFIG_BLK_SED_OPAL) and then there
wouldn't be a need to introduce a new option? Unless there's going to
be a second consumer.

>  obj-$(CONFIG_SUSPEND)          += suspend.o
>  obj-$(CONFIG_PPC_VAS)          += vas.o vas-sysfs.o
>  
> diff --git a/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> b/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> index 086934b319a9..c1d08075e850 100644
> --- a/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> +++ b/arch/powerpc/platforms/pseries/plpks_sed_ops.c
> @@ -14,7 +14,7 @@
>  #include <linux/string.h>
>  #include <linux/ioctl.h>
>  #include <linux/sed-opal-key.h>
> -#include "plpks.h"
> +#include <asm/plpks.h>
>  
>  /*
>   * structure that contains all SED data
> @@ -28,9 +28,6 @@ struct plpks_sed_object_data {
>         u_char key[32];
>  };
>  
> -#define PLPKS_PLATVAR_POLICY            WORLDREADABLE
> -#define PLPKS_PLATVAR_OS_COMMON         4
> -
>  #define PLPKS_SED_OBJECT_DATA_V0        0
>  #define PLPKS_SED_MANGLED_LABEL         "/default/pri"
>  #define PLPKS_SED_COMPONENT             "sed-opal"
> @@ -50,8 +47,8 @@ void plpks_init_var(struct plpks_var *var, char
> *keyname)
>                 var->name = PLPKS_SED_MANGLED_LABEL;
>                 var->namelen = strlen(keyname);
>         }
> -       var->policy = PLPKS_PLATVAR_POLICY;
> -       var->os = PLPKS_PLATVAR_OS_COMMON;
> +       var->policy = PLPKS_WORLDREADABLE;
> +       var->os = PLPKS_VAR_COMMON;
>         var->data = NULL;
>         var->datalen = 0;
>         var->component = PLPKS_SED_COMPONENT;
> @@ -64,28 +61,19 @@ int sed_read_key(char *keyname, char *key, u_int
> *keylen)
>  {
>         struct plpks_var var;
>         struct plpks_sed_object_data data;
> -       u_int offset;
>         int ret;
>         u_int len;
>  
>         plpks_init_var(&var, keyname);
> -       var.data = &data;
> +       var.data = (u8 *)&data;
>         var.datalen = sizeof(data);
>  
>         ret = plpks_read_os_var(&var);
>         if (ret != 0)
>                 return ret;
>  
> -       offset = offsetof(struct plpks_sed_object_data, key);
> -       if (offset > var.datalen) {
> -               return -EINVAL;
> -       }
> -
> -       len = min(be32_to_cpu(data.key_len), *keylen);
> -
> +       len = min_t(u16, be32_to_cpu(data.key_len), var.datalen);
>         memcpy(key, data.key, len);
> -       kfree(var.data);
> -
>         key[len] = '\0';
>         *keylen = len;
>  
> diff --git a/block/Kconfig b/block/Kconfig
> index 76b23114fdeb..75d4db34df5a 100644
> --- a/block/Kconfig
> +++ b/block/Kconfig
> @@ -182,6 +182,7 @@ config BLK_SED_OPAL
>         bool "Logic for interfacing with Opal enabled SEDs"
>         depends on KEYS
>         select PSERIES_PLPKS if PPC_PSERIES
> +       select PSERIES_PLPKS_SED if PPC_PSERIES
>         help
>         Builds Logic for interfacing with Opal enabled controllers.
>         Enabling this option enables users to setup/unlock/lock

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