2023-01-18 07:30:35

by Andrew Donnellan

[permalink] [raw]
Subject: [PATCH v3 08/24] powerpc/secvar: Allow backend to populate static list of variable names

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 <[email protected]>
Signed-off-by: Russell Currey <[email protected]>

---

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 ebf95386d720..c8bee1834b54 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -23,6 +23,10 @@ struct secvar_operations {
ssize_t (*format)(char *buf);
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 b82e95a2e415..d9352d4be87b 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -153,9 +153,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;
@@ -173,31 +195,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;
+ while (*name_ptr) {
+ rc = add_var(*name_ptr);
+ if (rc)
+ return rc;
+ name_ptr++;
+ }
+ return 0;
+}
+
static int secvar_sysfs_init(void)
{
int rc;
@@ -239,7 +256,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:
--
2.39.0


2023-01-19 01:30:13

by Nicholas Piggin

[permalink] [raw]
Subject: Re: [PATCH v3 08/24] powerpc/secvar: Allow backend to populate static list of variable names

On Wed Jan 18, 2023 at 4:10 PM AEST, 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 <[email protected]>
> Signed-off-by: Russell Currey <[email protected]>
>
> ---
>
> 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 ebf95386d720..c8bee1834b54 100644
> --- a/arch/powerpc/include/asm/secvar.h
> +++ b/arch/powerpc/include/asm/secvar.h
> @@ -23,6 +23,10 @@ struct secvar_operations {
> ssize_t (*format)(char *buf);
> 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;

The other way you could go is provide a sysfs_init() ops call here,
and export the add_var as a library function that backends can use.

Thanks,
Nick

2023-01-20 06:36:11

by Andrew Donnellan

[permalink] [raw]
Subject: Re: [PATCH v3 08/24] powerpc/secvar: Allow backend to populate static list of variable names

On Thu, 2023-01-19 at 11:10 +1000, Nicholas Piggin wrote:
> > diff --git a/arch/powerpc/include/asm/secvar.h
> > b/arch/powerpc/include/asm/secvar.h
> > index ebf95386d720..c8bee1834b54 100644
> > --- a/arch/powerpc/include/asm/secvar.h
> > +++ b/arch/powerpc/include/asm/secvar.h
> > @@ -23,6 +23,10 @@ struct secvar_operations {
> >         ssize_t (*format)(char *buf);
> >         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;
>
> The other way you could go is provide a sysfs_init() ops call here,
> and export the add_var as a library function that backends can use.

True, I think I'll keep it as is for now but I'll have a think about
whether to do that in a later patch.

--
Andrew Donnellan OzLabs, ADL Canberra
[email protected] IBM Australia Limited