Re: [PATCH v5 11/25] powerpc/secvar: Allow backend to populate static list of variable names

Stefan Berger
2023-01-31T15:54:26Z

On 1/31/23 01:39, Andrew Donnellan wrote:
> Currently, the list of variables is populated by calling
> secvar_ops->get_next() repeatedly, which is explicitly modelled on the
> OPAL API (including the keylen parameter).
>
> For the upcoming PLPKS backend, we have a static list of variable names.
> It is messy to fit that into get_next(), so instead, let the backend put
> a NULL-terminated array of variable names into secvar_ops->var_names,
> which will be used if get_next() is undefined.
>
> Signed-off-by: Andrew Donnellan <ajd@linux.ibm.com>
> Signed-off-by: Russell Currey <ruscur@russell.cc>
>
> ---
>
> v3: New patch (ajd/mpe)
> ---
> arch/powerpc/include/asm/secvar.h | 4 ++
> arch/powerpc/kernel/secvar-sysfs.c | 67 ++++++++++++++++++++----------
> 2 files changed, 50 insertions(+), 21 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
> index 011a53a8076c..4828e0ab7e3c 100644
> --- a/arch/powerpc/include/asm/secvar.h
> +++ b/arch/powerpc/include/asm/secvar.h
> @@ -21,6 +21,10 @@ struct secvar_operations {
> ssize_t (*format)(char *buf, size_t bufsize);
> int (*max_size)(u64 *max_size);
> const struct attribute **config_attrs;
> +
> + // NULL-terminated array of fixed variable names
> + // Only used if get_next() isn't provided
> + const char * const *var_names;
> };
>
> #ifdef CONFIG_PPC_SECURE_BOOT
> diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c
> index 7df32be86507..2cbc60b37e4e 100644
> --- a/arch/powerpc/kernel/secvar-sysfs.c
> +++ b/arch/powerpc/kernel/secvar-sysfs.c
> @@ -157,9 +157,31 @@ static int secvar_sysfs_config(struct kobject *kobj)
> return 0;
> }
>
> -static int secvar_sysfs_load(void)
> +static int add_var(const char *name)
> {
> struct kobject *kobj;
> + int rc;
> +
> + kobj = kzalloc(sizeof(*kobj), GFP_KERNEL);
> + if (!kobj)
> + return -ENOMEM;
> +
> + kobject_init(kobj, &secvar_ktype);
> +
> + rc = kobject_add(kobj, &secvar_kset->kobj, "%s", name);
> + if (rc) {
> + pr_warn("kobject_add error %d for attribute: %s\n", rc,
> + name);
> + kobject_put(kobj);
> + return rc;
> + }
> +
> + kobject_uevent(kobj, KOBJ_ADD);
> + return 0;
> +}
> +
> +static int secvar_sysfs_load(void)
> +{
> u64 namesize = 0;
> char *name;
> int rc;
> @@ -179,31 +201,26 @@ static int secvar_sysfs_load(void)
> break;
> }
>
> - kobj = kzalloc(sizeof(*kobj), GFP_KERNEL);
> - if (!kobj) {
> - rc = -ENOMEM;
> - break;
> - }
> -
> - kobject_init(kobj, &secvar_ktype);
> -
> - rc = kobject_add(kobj, &secvar_kset->kobj, "%s", name);
> - if (rc) {
> - pr_warn("kobject_add error %d for attribute: %s\n", rc,
> - name);
> - kobject_put(kobj);
> - kobj = NULL;
> - }
> -
> - if (kobj)
> - kobject_uevent(kobj, KOBJ_ADD);
> -
> + rc = add_var(name);
> } while (!rc);
>
> kfree(name);
> return rc;
> }
>
> +static int secvar_sysfs_load_static(void)
> +{
> + const char * const *name_ptr = secvar_ops->var_names;
> + int rc;

Missing newline ?

> + while (*name_ptr) {
> + rc = add_var(*name_ptr);
> + if (rc)
> + return rc;
> + name_ptr++;
> + }
> + return 0;
> +}
> +
> static int secvar_sysfs_init(void)
> {
> int rc;
> @@ -245,7 +262,15 @@ static int secvar_sysfs_init(void)
> goto err;
> }
>
> - secvar_sysfs_load();
> + if (secvar_ops->get_next)
> + rc = secvar_sysfs_load();
> + else
> + rc = secvar_sysfs_load_static();
> +
> + if (rc) {
> + pr_err("Failed to create variable attributes\n");
> + goto err;
> + }
>
> return 0;
> err:

With the above change:

Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>