2022-12-28 07:38:29

by Russell Currey

[permalink] [raw]
Subject: [PATCH 0/6] pseries dynamic secure boot interface using secvar

This series exposes an interface to userspace for reading and writing
secure variables contained within the PowerVM LPAR Platform KeyStore
(PLPKS) for the purpose of configuring dynamic secure boot.

This series builds on past work by Nayna Jain[0] in exposing PLPKS
variables to userspace. Rather than being a generic interface for
interacting with the keystore, however, we use the existing powerpc
secvar infrastructure to only expose objects in the keystore used
for dynamic secure boot. This has the benefit of leveraging an
existing interface and making the implementation relatively minimal.

This series needs to be applied on top of Andrew's recent bugfix
series[1].

There are a few relevant details to note about the implementation:

* New additions to the secvar API, format() and max_size()
* New optional sysfs directory "config/" for arbitrary ASCII variables
* Some OPAL-specific code has been relocated from secvar-sysfs.c to
powernv platform code. Would appreciate any powernv testing!
* Variable names are fixed and only those used for secure boot are
exposed. This is not a generic PLPKS interface, but also
doesn't preclude one being added in future.

With this series, both powernv and pseries platforms support dynamic
secure boot through the same interface.

[0]: https://lore.kernel.org/linuxppc-dev/[email protected]/
[1]: https://lore.kernel.org/linuxppc-dev/[email protected]/

Russell Currey (6):
powerpc/pseries: Log hcall return codes for PLPKS debug
powerpc/secvar: WARN_ON_ONCE() if multiple secvar ops are set
powerpc/secvar: Handle format string in the consumer
powerpc/secvar: Handle max object size in the consumer
powerpc/secvar: Extend sysfs to include config vars
powerpc/pseries: Implement secvars for dynamic secure boot

Documentation/ABI/testing/sysfs-secvar | 8 +
arch/powerpc/include/asm/secvar.h | 5 +
arch/powerpc/kernel/secvar-ops.c | 4 +-
arch/powerpc/kernel/secvar-sysfs.c | 76 +++---
arch/powerpc/platforms/powernv/opal-secvar.c | 44 +++
arch/powerpc/platforms/pseries/Kconfig | 13 +
arch/powerpc/platforms/pseries/Makefile | 4 +-
arch/powerpc/platforms/pseries/plpks-secvar.c | 250 ++++++++++++++++++
arch/powerpc/platforms/pseries/plpks.c | 2 +
9 files changed, 365 insertions(+), 41 deletions(-)
create mode 100644 arch/powerpc/platforms/pseries/plpks-secvar.c

--
2.38.1


2022-12-28 07:54:17

by Russell Currey

[permalink] [raw]
Subject: [PATCH 5/6] powerpc/secvar: Extend sysfs to include config vars

The forthcoming pseries consumer of the secvar API wants to expose a
number of config variables. Allowing secvar implementations to provide
their own sysfs attributes makes it easy for consumers to expose what
they need to.

This is not being used by the OPAL secvar implementation at present, and
the config directory will not be created if no attributes are set.

Signed-off-by: Russell Currey <[email protected]>
---
I played around with adding an API call to facilitate a more generic
key/value interface for config variables and it seemed like unnecessary
complexity. I think this is cleaner. If there was ever a secvar
interface other than sysfs we'd have to rework it, though.

arch/powerpc/include/asm/secvar.h | 3 +++
arch/powerpc/kernel/secvar-sysfs.c | 40 ++++++++++++++++++++++++++----
2 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
index 92d2c051918b..250e7066b6da 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -10,6 +10,7 @@

#include <linux/types.h>
#include <linux/errno.h>
+#include <linux/sysfs.h>

extern const struct secvar_operations *secvar_ops;

@@ -27,10 +28,12 @@ struct secvar_operations {
#ifdef CONFIG_PPC_SECURE_BOOT

extern void set_secvar_ops(const struct secvar_operations *ops);
+extern void set_secvar_config_attrs(const struct attribute **attrs);

#else

static inline void set_secvar_ops(const struct secvar_operations *ops) { }
+static inline void set_secvar_config_attrs(const struct attribute **attrs) { }

#endif

diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c
index ea408763dc78..0c3790345403 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -15,9 +15,17 @@

#define NAME_MAX_SIZE 1024

+const struct attribute **secvar_config_attrs __ro_after_init = NULL;
+
static struct kobject *secvar_kobj;
static struct kset *secvar_kset;

+void set_secvar_config_attrs(const struct attribute **attrs)
+{
+ WARN_ON_ONCE(secvar_config_attrs);
+ secvar_config_attrs = attrs;
+}
+
static ssize_t format_show(struct kobject *kobj, struct kobj_attribute *attr,
char *buf)
{
@@ -134,6 +142,16 @@ static int update_kobj_size(void)
return 0;
}

+static int secvar_sysfs_config(struct kobject *kobj)
+{
+ struct attribute_group config_group = {
+ .name = "config",
+ .attrs = (struct attribute **)secvar_config_attrs,
+ };
+
+ return sysfs_create_group(kobj, &config_group);
+}
+
static int secvar_sysfs_load(void)
{
char *name;
@@ -196,26 +214,38 @@ static int secvar_sysfs_init(void)

rc = sysfs_create_file(secvar_kobj, &format_attr.attr);
if (rc) {
- kobject_put(secvar_kobj);
- return -ENOMEM;
+ pr_err("secvar: Failed to create format object\n");
+ rc = -ENOMEM;
+ goto err;
}

secvar_kset = kset_create_and_add("vars", NULL, secvar_kobj);
if (!secvar_kset) {
pr_err("secvar: sysfs kobject registration failed.\n");
- kobject_put(secvar_kobj);
- return -ENOMEM;
+ rc = -ENOMEM;
+ goto err;
}

rc = update_kobj_size();
if (rc) {
pr_err("Cannot read the size of the attribute\n");
- return rc;
+ goto err;
+ }
+
+ if (secvar_config_attrs) {
+ rc = secvar_sysfs_config(secvar_kobj);
+ if (rc) {
+ pr_err("secvar: Failed to create config directory\n");
+ goto err;
+ }
}

secvar_sysfs_load();

return 0;
+err:
+ kobject_put(secvar_kobj);
+ return rc;
}

late_initcall(secvar_sysfs_init);
--
2.38.1

2022-12-28 07:54:52

by Russell Currey

[permalink] [raw]
Subject: [PATCH 6/6] powerpc/pseries: Implement secvars for dynamic secure boot

The pseries platform can support dynamic secure boot (i.e. secure boot
using user-defined keys) using variables contained with the PowerVM LPAR
Platform KeyStore (PLPKS). Using the powerpc secvar API, expose the
relevant variables for pseries dynamic secure boot through the existing
secvar filesystem layout.

The relevant variables for dynamic secure boot are signed in the
keystore, and can only be modified using the H_PKS_SIGNED_UPDATE hcall.
Object labels in the keystore are encoded using ucs2 format. With our
fixed variable names we don't have to care about encoding outside of the
necessary byte padding.

When a user writes to a variable, the first 8 bytes of data must contain
the signed update flags as defined by the hypervisor.

When a user reads a variable, the first 4 bytes of data contain the
policies defined for the object.

Limitations exist due to the underlying implementation of sysfs binary
attributes, as is the case for the OPAL secvar implementation -
partial writes are unsupported and writes cannot be larger than PAGE_SIZE.

Co-developed-by: Nayna Jain <[email protected]>
Signed-off-by: Nayna Jain <[email protected]>
Co-developed-by: Andrew Donnellan <[email protected]>
Signed-off-by: Andrew Donnellan <[email protected]>
Signed-off-by: Russell Currey <[email protected]>
---
Documentation/ABI/testing/sysfs-secvar | 8 +
arch/powerpc/platforms/pseries/Kconfig | 13 +
arch/powerpc/platforms/pseries/Makefile | 4 +-
arch/powerpc/platforms/pseries/plpks-secvar.c | 250 ++++++++++++++++++
4 files changed, 273 insertions(+), 2 deletions(-)
create mode 100644 arch/powerpc/platforms/pseries/plpks-secvar.c

diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar
index feebb8c57294..e6fef664c9c8 100644
--- a/Documentation/ABI/testing/sysfs-secvar
+++ b/Documentation/ABI/testing/sysfs-secvar
@@ -44,3 +44,11 @@ Contact: Nayna Jain <[email protected]>
Description: A write-only file that is used to submit the new value for the
variable. The size of the file represents the maximum size of
the variable data that can be written.
+
+What: /sys/firmware/secvar/config
+Date: December 2022
+Contact: Nayna Jain <[email protected]>
+Description: This optional directory contains read-only config attributes as
+ defined by the secure variable implementation. All data is in
+ ASCII format. The directory is only created if the backing
+ implementation provides variables to populate it.
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index a3b4d99567cb..94e08c405d50 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -162,6 +162,19 @@ config PSERIES_PLPKS

If unsure, select N.

+config PSERIES_PLPKS_SECVAR
+ depends on PSERIES_PLPKS
+ depends on PPC_SECURE_BOOT
+ bool "Support for the PLPKS secvar interface"
+ help
+ PowerVM can support dynamic secure boot with user-defined keys
+ through the PLPKS. Keystore objects used in dynamic secure boot
+ can be exposed to the kernel and userspace through the powerpc
+ secvar infrastructure. Select this to enable the PLPKS backend
+ for secvars for use in pseries dynamic secure boot.
+
+ If unsure, select N.
+
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 92310202bdd7..807756991f9d 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -27,8 +27,8 @@ obj-$(CONFIG_PAPR_SCM) += papr_scm.o
obj-$(CONFIG_PPC_SPLPAR) += vphn.o
obj-$(CONFIG_PPC_SVM) += svm.o
obj-$(CONFIG_FA_DUMP) += rtas-fadump.o
-obj-$(CONFIG_PSERIES_PLPKS) += plpks.o
-
+obj-$(CONFIG_PSERIES_PLPKS) += plpks.o
+obj-$(CONFIG_PSERIES_PLPKS_SECVAR) += plpks-secvar.o
obj-$(CONFIG_SUSPEND) += suspend.o
obj-$(CONFIG_PPC_VAS) += vas.o vas-sysfs.o

diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c b/arch/powerpc/platforms/pseries/plpks-secvar.c
new file mode 100644
index 000000000000..3f9ff16c03c8
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/plpks-secvar.c
@@ -0,0 +1,250 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Secure variable implementation using the PowerVM LPAR Platform KeyStore (PLPKS)
+ *
+ * Copyright 2022, IBM Corporation
+ * Authors: Russell Currey
+ * Andrew Donnellan
+ * Nayna Jain
+ */
+
+#define pr_fmt(fmt) "secvar: "fmt
+
+#include <linux/printk.h>
+#include <linux/init.h>
+#include <linux/types.h>
+#include <linux/slab.h>
+#include <linux/string.h>
+#include <linux/kobject.h>
+#include <asm/secvar.h>
+#include "plpks.h"
+
+// Config attributes for sysfs
+#define PLPKS_CONFIG_ATTR(name, fmt, func) \
+ static ssize_t name##_show(struct kobject *kobj, \
+ struct kobj_attribute *attr, \
+ char *buf) \
+ { \
+ return sprintf(buf, fmt, func()); \
+ } \
+ static struct kobj_attribute attr_##name = __ATTR_RO(name)
+
+PLPKS_CONFIG_ATTR(version, "%u\n", plpks_get_version);
+PLPKS_CONFIG_ATTR(object_overhead, "%u\n", plpks_get_objoverhead);
+PLPKS_CONFIG_ATTR(max_password_size, "%u\n", plpks_get_maxpwsize);
+PLPKS_CONFIG_ATTR(max_object_size, "%u\n", plpks_get_maxobjectsize);
+PLPKS_CONFIG_ATTR(max_object_label_size, "%u\n", plpks_get_maxobjectlabelsize);
+PLPKS_CONFIG_ATTR(total_size, "%u\n", plpks_get_totalsize);
+PLPKS_CONFIG_ATTR(used_space, "%u\n", plpks_get_usedspace);
+PLPKS_CONFIG_ATTR(supported_policies, "%08x\n", plpks_get_supportedpolicies);
+PLPKS_CONFIG_ATTR(max_large_object_size, "%u\n", plpks_get_maxlargeobjectsize);
+PLPKS_CONFIG_ATTR(signed_update_algorithms, "%016llx\n", plpks_get_signedupdatealgorithms);
+
+static const struct attribute *config_attrs[] = {
+ &attr_version.attr,
+ &attr_object_overhead.attr,
+ &attr_max_password_size.attr,
+ &attr_max_object_size.attr,
+ &attr_max_object_label_size.attr,
+ &attr_total_size.attr,
+ &attr_used_space.attr,
+ &attr_supported_policies.attr,
+ &attr_max_large_object_size.attr,
+ &attr_signed_update_algorithms.attr,
+ NULL,
+};
+
+static u16 get_ucs2name(const char *name, uint8_t **ucs2_name)
+{
+ int namelen = strlen(name) * 2;
+ *ucs2_name = kzalloc(namelen, GFP_KERNEL);
+
+ if (!*ucs2_name)
+ return 0;
+
+ for (int i = 0; name[i]; i++) {
+ (*ucs2_name)[i * 2] = name[i];
+ (*ucs2_name)[i * 2 + 1] = '\0';
+ }
+
+ return namelen;
+}
+
+static u32 get_policy(const char *name)
+{
+ if ((strcmp(name, "db") == 0) ||
+ (strcmp(name, "dbx") == 0) ||
+ (strcmp(name, "grubdb") == 0) ||
+ (strcmp(name, "sbat") == 0))
+ return (WORLDREADABLE | SIGNEDUPDATE);
+ else
+ return SIGNEDUPDATE;
+}
+
+#define PLPKS_SECVAR_COUNT 8
+static char *var_names[PLPKS_SECVAR_COUNT] = {
+ "PK",
+ "KEK",
+ "db",
+ "dbx",
+ "grubdb",
+ "sbat",
+ "moduledb",
+ "trustedcadb",
+};
+
+static int plpks_get_variable(const char *key, uint64_t key_len,
+ u8 *data, uint64_t *data_size)
+{
+ struct plpks_var var = {0};
+ u16 ucs2_namelen;
+ u8 *ucs2_name;
+ int rc = 0;
+
+ ucs2_namelen = get_ucs2name(key, &ucs2_name);
+ if (!ucs2_namelen)
+ return -ENOMEM;
+
+ var.name = ucs2_name;
+ var.namelen = ucs2_namelen;
+ var.os = PLPKS_VAR_LINUX;
+ rc = plpks_read_os_var(&var);
+
+ if (rc)
+ goto err;
+
+ *data_size = var.datalen + sizeof(var.policy);
+
+ // We can be called with data = NULL to just get the object size.
+ if (data) {
+ memcpy(data, &var.policy, sizeof(var.policy));
+ memcpy(data + sizeof(var.policy), var.data, var.datalen);
+ }
+
+ kfree(var.data);
+err:
+ kfree(ucs2_name);
+ return rc;
+}
+
+static int plpks_set_variable(const char *key, uint64_t key_len,
+ u8 *data, uint64_t data_size)
+{
+ struct plpks_var var = {0};
+ u16 ucs2_namelen;
+ u8 *ucs2_name;
+ int rc = 0;
+ u64 flags;
+
+ // Secure variables need to be prefixed with 8 bytes of flags.
+ // We only want to perform the write if we have at least one byte of data.
+ if (data_size <= sizeof(flags))
+ return -EINVAL;
+
+ ucs2_namelen = get_ucs2name(key, &ucs2_name);
+ if (!ucs2_namelen)
+ return -ENOMEM;
+
+ memcpy(&flags, data, sizeof(flags));
+
+ var.datalen = data_size - sizeof(flags);
+ var.data = kzalloc(var.datalen, GFP_KERNEL);
+ if (!var.data) {
+ rc = -ENOMEM;
+ goto err;
+ }
+
+ memcpy(var.data, data + sizeof(flags), var.datalen);
+
+ var.name = ucs2_name;
+ var.namelen = ucs2_namelen;
+ var.os = PLPKS_VAR_LINUX;
+ var.policy = get_policy(key);
+
+ rc = plpks_signed_update_var(var, flags);
+
+ kfree(var.data);
+err:
+ kfree(ucs2_name);
+ return rc;
+}
+
+/*
+ * get_next() in the secvar API is designed for the OPAL API.
+ * If *key is 0, it returns the first variable in the keystore.
+ * Otherwise, you pass the name of a key and it returns next in line.
+ *
+ * We're going to cheat here - since we have fixed keys and don't care about
+ * key_len, we can just use it as an index.
+ */
+static int plpks_get_next_variable(const char *key, uint64_t *key_len, uint64_t keybufsize)
+{
+ if (!key || !key_len)
+ return -EINVAL;
+
+ if (*key_len >= PLPKS_SECVAR_COUNT)
+ return -ENOENT;
+
+ if (strscpy((char *)key, var_names[(*key_len)++], keybufsize) < 0)
+ return -E2BIG;
+
+ return 0;
+}
+
+// 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.
+static ssize_t plpks_secvar_format(char *buf)
+{
+ struct plpks_var var = {0};
+ ssize_t ret;
+
+ var.component = NULL;
+ // Only the signed variables have ucs2-encoded names, this one doesn't
+ var.name = "SB_VERSION";
+ var.namelen = 10;
+ var.datalen = 0;
+ var.data = NULL;
+
+ // 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)
+ return sprintf(buf, "ibm,plpks-sb-unknown\n");
+
+ pr_err("Error %ld reading SB_VERSION from firmware\n", ret);
+ return ret;
+ }
+
+ // Hypervisor defines SB_VERSION as a "1 byte unsigned integer value"
+ ret = sprintf(buf, "ibm,plpks-sb-%hhu\n", var.data[0]);
+
+ kfree(var.data);
+ return ret;
+}
+
+static int plpks_max_size(uint64_t *max_size)
+{
+ *max_size = (uint64_t)plpks_get_maxobjectsize();
+
+ return 0;
+}
+
+
+static const struct secvar_operations plpks_secvar_ops = {
+ .get = plpks_get_variable,
+ .get_next = plpks_get_next_variable,
+ .set = plpks_set_variable,
+ .format = plpks_secvar_format,
+ .max_size = plpks_max_size,
+};
+
+static int plpks_secvar_init(void)
+{
+ if (!plpks_is_available())
+ return -ENODEV;
+
+ set_secvar_ops(&plpks_secvar_ops);
+ set_secvar_config_attrs(config_attrs);
+ return 0;
+}
+device_initcall(plpks_secvar_init);
--
2.38.1

2022-12-28 07:59:05

by Russell Currey

[permalink] [raw]
Subject: [PATCH 1/6] powerpc/pseries: Log hcall return codes for PLPKS debug

The plpks code converts hypervisor return codes into their Linux
equivalents so that users can understand them. Having access to the
original return codes is really useful for debugging, so add a
pr_debug() so we don't lose information from the conversion.

Signed-off-by: Russell Currey <[email protected]>
---
arch/powerpc/platforms/pseries/plpks.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/powerpc/platforms/pseries/plpks.c b/arch/powerpc/platforms/pseries/plpks.c
index 9e4401aabf4f..820218eb894f 100644
--- a/arch/powerpc/platforms/pseries/plpks.c
+++ b/arch/powerpc/platforms/pseries/plpks.c
@@ -131,6 +131,8 @@ static int pseries_status_to_err(int rc)
err = -EINVAL;
}

+ pr_debug("Converted hypervisor code %d to Linux %d\n", rc, err);
+
return err;
}

--
2.38.1

2022-12-28 08:00:00

by Russell Currey

[permalink] [raw]
Subject: [PATCH 2/6] powerpc/secvar: WARN_ON_ONCE() if multiple secvar ops are set

The secvar code only supports one consumer at a time.

Multiple consumers aren't possible at this point in time, but we'd want
it to be obvious if it ever could happen.

Signed-off-by: Russell Currey <[email protected]>
---
arch/powerpc/kernel/secvar-ops.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/powerpc/kernel/secvar-ops.c b/arch/powerpc/kernel/secvar-ops.c
index 6a29777d6a2d..aa1b2adc2710 100644
--- a/arch/powerpc/kernel/secvar-ops.c
+++ b/arch/powerpc/kernel/secvar-ops.c
@@ -8,10 +8,12 @@

#include <linux/cache.h>
#include <asm/secvar.h>
+#include <asm/bug.h>

-const struct secvar_operations *secvar_ops __ro_after_init;
+const struct secvar_operations *secvar_ops __ro_after_init = NULL;

void set_secvar_ops(const struct secvar_operations *ops)
{
+ WARN_ON_ONCE(secvar_ops);
secvar_ops = ops;
}
--
2.38.1

2022-12-28 08:19:13

by Russell Currey

[permalink] [raw]
Subject: [PATCH 4/6] powerpc/secvar: Handle max object size in the consumer

Currently the max object size is handled in the core secvar code with an
entirely OPAL-specific implementation, so create a new max_size() op and
move the existing implementation into the powernv platform. Should be
no functional change.

Signed-off-by: Russell Currey <[email protected]>
---
arch/powerpc/include/asm/secvar.h | 1 +
arch/powerpc/kernel/secvar-sysfs.c | 17 +++--------------
arch/powerpc/platforms/powernv/opal-secvar.c | 19 +++++++++++++++++++
3 files changed, 23 insertions(+), 14 deletions(-)

diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
index 3b7e5a3625bd..92d2c051918b 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -21,6 +21,7 @@ struct secvar_operations {
int (*set)(const char *key, uint64_t key_len, u8 *data,
uint64_t data_size);
ssize_t (*format)(char *buf);
+ int (*max_size)(uint64_t *max_size);
};

#ifdef CONFIG_PPC_SECURE_BOOT
diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c
index daf28b11866f..ea408763dc78 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -122,27 +122,16 @@ static struct kobj_type secvar_ktype = {
static int update_kobj_size(void)
{

- struct device_node *node;
u64 varsize;
- int rc = 0;
+ int rc = secvar_ops->max_size(&varsize);

- node = of_find_compatible_node(NULL, NULL, "ibm,secvar-backend");
- if (!of_device_is_available(node)) {
- rc = -ENODEV;
- goto out;
- }
-
- rc = of_property_read_u64(node, "max-var-size", &varsize);
if (rc)
- goto out;
+ return rc;

data_attr.size = varsize;
update_attr.size = varsize;

-out:
- of_node_put(node);
-
- return rc;
+ return 0;
}

static int secvar_sysfs_load(void)
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c b/arch/powerpc/platforms/powernv/opal-secvar.c
index cd5b5c06c091..3ef6b9afd129 100644
--- a/arch/powerpc/platforms/powernv/opal-secvar.c
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -125,11 +125,30 @@ static ssize_t opal_secvar_format(char *buf)
return rc;
}

+static int opal_secvar_max_size(uint64_t *max_size)
+{
+ int rc;
+ struct device_node *node;
+
+ node = of_find_compatible_node(NULL, NULL, "ibm,secvar-backend");
+ if (!of_device_is_available(node)) {
+ rc = -ENODEV;
+ goto out;
+ }
+
+ rc = of_property_read_u64(node, "max-var-size", max_size);
+
+out:
+ of_node_put(node);
+ return rc;
+}
+
static const struct secvar_operations opal_secvar_ops = {
.get = opal_get_variable,
.get_next = opal_get_next_variable,
.set = opal_set_variable,
.format = opal_secvar_format,
+ .max_size = opal_secvar_max_size,
};

static int opal_secvar_probe(struct platform_device *pdev)
--
2.38.1

2022-12-28 08:41:43

by Russell Currey

[permalink] [raw]
Subject: [PATCH 3/6] powerpc/secvar: Handle format string in the consumer

The code that handles the format string in secvar-sysfs.c is entirely
OPAL specific, so create a new "format" op in secvar_operations to make
the secvar code more generic. No functional change.

Signed-off-by: Russell Currey <[email protected]>
---
arch/powerpc/include/asm/secvar.h | 1 +
arch/powerpc/kernel/secvar-sysfs.c | 21 +---------------
arch/powerpc/platforms/powernv/opal-secvar.c | 25 ++++++++++++++++++++
3 files changed, 27 insertions(+), 20 deletions(-)

diff --git a/arch/powerpc/include/asm/secvar.h b/arch/powerpc/include/asm/secvar.h
index 4cc35b58b986..3b7e5a3625bd 100644
--- a/arch/powerpc/include/asm/secvar.h
+++ b/arch/powerpc/include/asm/secvar.h
@@ -20,6 +20,7 @@ struct secvar_operations {
uint64_t keybufsize);
int (*set)(const char *key, uint64_t key_len, u8 *data,
uint64_t data_size);
+ ssize_t (*format)(char *buf);
};

#ifdef CONFIG_PPC_SECURE_BOOT
diff --git a/arch/powerpc/kernel/secvar-sysfs.c b/arch/powerpc/kernel/secvar-sysfs.c
index 1ee4640a2641..daf28b11866f 100644
--- a/arch/powerpc/kernel/secvar-sysfs.c
+++ b/arch/powerpc/kernel/secvar-sysfs.c
@@ -21,26 +21,7 @@ static struct kset *secvar_kset;
static ssize_t format_show(struct kobject *kobj, struct kobj_attribute *attr,
char *buf)
{
- ssize_t rc = 0;
- struct device_node *node;
- const char *format;
-
- node = of_find_compatible_node(NULL, NULL, "ibm,secvar-backend");
- if (!of_device_is_available(node)) {
- rc = -ENODEV;
- goto out;
- }
-
- rc = of_property_read_string(node, "format", &format);
- if (rc)
- goto out;
-
- rc = sprintf(buf, "%s\n", format);
-
-out:
- of_node_put(node);
-
- return rc;
+ return secvar_ops->format(buf);
}


diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c b/arch/powerpc/platforms/powernv/opal-secvar.c
index 14133e120bdd..cd5b5c06c091 100644
--- a/arch/powerpc/platforms/powernv/opal-secvar.c
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -101,10 +101,35 @@ static int opal_set_variable(const char *key, uint64_t ksize, u8 *data,
return opal_status_to_err(rc);
}

+static ssize_t opal_secvar_format(char *buf)
+{
+ ssize_t rc = 0;
+ struct device_node *node;
+ const char *format;
+
+ node = of_find_compatible_node(NULL, NULL, "ibm,secvar-backend");
+ if (!of_device_is_available(node)) {
+ rc = -ENODEV;
+ goto out;
+ }
+
+ rc = of_property_read_string(node, "format", &format);
+ if (rc)
+ goto out;
+
+ rc = sprintf(buf, "%s\n", format);
+
+out:
+ of_node_put(node);
+
+ return rc;
+}
+
static const struct secvar_operations opal_secvar_ops = {
.get = opal_get_variable,
.get_next = opal_get_next_variable,
.set = opal_set_variable,
+ .format = opal_secvar_format,
};

static int opal_secvar_probe(struct platform_device *pdev)
--
2.38.1

2022-12-28 08:41:49

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 6/6] powerpc/pseries: Implement secvars for dynamic secure boot

On Wed, Dec 28, 2022 at 06:29:43PM +1100, Russell Currey wrote:
> The pseries platform can support dynamic secure boot (i.e. secure boot
> using user-defined keys) using variables contained with the PowerVM LPAR
> Platform KeyStore (PLPKS). Using the powerpc secvar API, expose the
> relevant variables for pseries dynamic secure boot through the existing
> secvar filesystem layout.
>
> The relevant variables for dynamic secure boot are signed in the
> keystore, and can only be modified using the H_PKS_SIGNED_UPDATE hcall.
> Object labels in the keystore are encoded using ucs2 format. With our
> fixed variable names we don't have to care about encoding outside of the
> necessary byte padding.
>
> When a user writes to a variable, the first 8 bytes of data must contain
> the signed update flags as defined by the hypervisor.
>
> When a user reads a variable, the first 4 bytes of data contain the
> policies defined for the object.
>
> Limitations exist due to the underlying implementation of sysfs binary
> attributes, as is the case for the OPAL secvar implementation -
> partial writes are unsupported and writes cannot be larger than PAGE_SIZE.
>
> Co-developed-by: Nayna Jain <[email protected]>
> Signed-off-by: Nayna Jain <[email protected]>
> Co-developed-by: Andrew Donnellan <[email protected]>
> Signed-off-by: Andrew Donnellan <[email protected]>
> Signed-off-by: Russell Currey <[email protected]>
> ---
> Documentation/ABI/testing/sysfs-secvar | 8 +
> arch/powerpc/platforms/pseries/Kconfig | 13 +
> arch/powerpc/platforms/pseries/Makefile | 4 +-
> arch/powerpc/platforms/pseries/plpks-secvar.c | 250 ++++++++++++++++++
> 4 files changed, 273 insertions(+), 2 deletions(-)
> create mode 100644 arch/powerpc/platforms/pseries/plpks-secvar.c
>
> diff --git a/Documentation/ABI/testing/sysfs-secvar b/Documentation/ABI/testing/sysfs-secvar
> index feebb8c57294..e6fef664c9c8 100644
> --- a/Documentation/ABI/testing/sysfs-secvar
> +++ b/Documentation/ABI/testing/sysfs-secvar
> @@ -44,3 +44,11 @@ Contact: Nayna Jain <[email protected]>
> Description: A write-only file that is used to submit the new value for the
> variable. The size of the file represents the maximum size of
> the variable data that can be written.
> +
> +What: /sys/firmware/secvar/config
> +Date: December 2022
> +Contact: Nayna Jain <[email protected]>
> +Description: This optional directory contains read-only config attributes as
> + defined by the secure variable implementation. All data is in
> + ASCII format. The directory is only created if the backing
> + implementation provides variables to populate it.

That is _WAY_ too vague, sorry. You have to define the files that are
in here and what the contents of them are.

If you do not, you will get warnings from the tools that we have that
you can run on a system that wants to verify all sysfs files are
properly documented.

> diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
> index a3b4d99567cb..94e08c405d50 100644
> --- a/arch/powerpc/platforms/pseries/Kconfig
> +++ b/arch/powerpc/platforms/pseries/Kconfig
> @@ -162,6 +162,19 @@ config PSERIES_PLPKS
>
> If unsure, select N.
>
> +config PSERIES_PLPKS_SECVAR
> + depends on PSERIES_PLPKS
> + depends on PPC_SECURE_BOOT
> + bool "Support for the PLPKS secvar interface"
> + help
> + PowerVM can support dynamic secure boot with user-defined keys
> + through the PLPKS. Keystore objects used in dynamic secure boot
> + can be exposed to the kernel and userspace through the powerpc
> + secvar infrastructure. Select this to enable the PLPKS backend
> + for secvars for use in pseries dynamic secure boot.
> +
> + If unsure, select N.
> +
> 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 92310202bdd7..807756991f9d 100644
> --- a/arch/powerpc/platforms/pseries/Makefile
> +++ b/arch/powerpc/platforms/pseries/Makefile
> @@ -27,8 +27,8 @@ obj-$(CONFIG_PAPR_SCM) += papr_scm.o
> obj-$(CONFIG_PPC_SPLPAR) += vphn.o
> obj-$(CONFIG_PPC_SVM) += svm.o
> obj-$(CONFIG_FA_DUMP) += rtas-fadump.o
> -obj-$(CONFIG_PSERIES_PLPKS) += plpks.o
> -
> +obj-$(CONFIG_PSERIES_PLPKS) += plpks.o
> +obj-$(CONFIG_PSERIES_PLPKS_SECVAR) += plpks-secvar.o
> obj-$(CONFIG_SUSPEND) += suspend.o
> obj-$(CONFIG_PPC_VAS) += vas.o vas-sysfs.o
>
> diff --git a/arch/powerpc/platforms/pseries/plpks-secvar.c b/arch/powerpc/platforms/pseries/plpks-secvar.c
> new file mode 100644
> index 000000000000..3f9ff16c03c8
> --- /dev/null
> +++ b/arch/powerpc/platforms/pseries/plpks-secvar.c
> @@ -0,0 +1,250 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Secure variable implementation using the PowerVM LPAR Platform KeyStore (PLPKS)
> + *
> + * Copyright 2022, IBM Corporation
> + * Authors: Russell Currey
> + * Andrew Donnellan
> + * Nayna Jain
> + */
> +
> +#define pr_fmt(fmt) "secvar: "fmt
> +
> +#include <linux/printk.h>
> +#include <linux/init.h>
> +#include <linux/types.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <linux/kobject.h>
> +#include <asm/secvar.h>
> +#include "plpks.h"
> +
> +// Config attributes for sysfs
> +#define PLPKS_CONFIG_ATTR(name, fmt, func) \
> + static ssize_t name##_show(struct kobject *kobj, \
> + struct kobj_attribute *attr, \
> + char *buf) \
> + { \
> + return sprintf(buf, fmt, func()); \

sysfs_emit() please.

> + } \
> + static struct kobj_attribute attr_##name = __ATTR_RO(name)
> +
> +PLPKS_CONFIG_ATTR(version, "%u\n", plpks_get_version);
> +PLPKS_CONFIG_ATTR(object_overhead, "%u\n", plpks_get_objoverhead);
> +PLPKS_CONFIG_ATTR(max_password_size, "%u\n", plpks_get_maxpwsize);
> +PLPKS_CONFIG_ATTR(max_object_size, "%u\n", plpks_get_maxobjectsize);
> +PLPKS_CONFIG_ATTR(max_object_label_size, "%u\n", plpks_get_maxobjectlabelsize);
> +PLPKS_CONFIG_ATTR(total_size, "%u\n", plpks_get_totalsize);
> +PLPKS_CONFIG_ATTR(used_space, "%u\n", plpks_get_usedspace);
> +PLPKS_CONFIG_ATTR(supported_policies, "%08x\n", plpks_get_supportedpolicies);
> +PLPKS_CONFIG_ATTR(max_large_object_size, "%u\n", plpks_get_maxlargeobjectsize);
> +PLPKS_CONFIG_ATTR(signed_update_algorithms, "%016llx\n", plpks_get_signedupdatealgorithms);

Looks like you know what these files and values are going to be, so
again, please document them properly.

thanks,

greg k-h