[RFC PATCH v2 0/3] powerpc/pseries: add support for local secure storage called Platform KeyStore(PKS)

[RFC PATCH v2 0/3] powerpc/pseries: add support for local secure storage called Platform KeyStore(PKS)

[Nayna Jain]

PowerVM provides an isolated Platform KeyStore(PKS)[1] storage allocation
for each partition(LPAR) with individually managed access controls to store
sensitive information securely. Linux Kernel can access this storage by
interfacing with hypervisor using a new set of hypervisor calls. 

PowerVM guest secure boot feature intend to use Platform KeyStore for
the purpose of storing public keys. Secure boot requires public keys to
be able to verify the grub and boot kernel. To allow authenticated
 manipulation of keys, it supports variables to store key authorities
- PK/KEK. Other variables are used to store code signing keys - db/grubdb.
It also supports denied list to disallow booting even if signed with
valid key. This is done via denied list database - dbx or sbat. These
variables would be stored in PKS, and are managed and controlled by
firmware.

The purpose of this patchset is to add userspace interface to manage
these variables.

For v1[2] version, we received following feedback
"Ok, this is like the 3rd or 4th different platform-specific proposal for
this type of functionality.  I think we need to give up on
platform-specific user/kernel apis on this (random sysfs/securityfs
files scattered around the tree), and come up with a standard place for
all of this."

Currently, OpenPOWER exposes variables via sysfs, while EFI platforms
have used sysfs and then moved to their own efivarfs filesystem.
Recently, coco feature is using securityfs to expose their
secrets. All of these environments are different both syntactically and
semantically.

securityfs is meant for linux security subsystems to expose policies/logs
or any other information, and do not interact with firmware for managing
these variables. However, there are various firmware security
features which expose their variables for user management via kernel as
discussed above. There is currently no single place to expose these
variables. Different platforms use sysfs/platform specific
filesystem(efivarfs)/securityfs interface as find appropriate. This has
resulted in interfaces scattered around the tree.

This resulted in demand of a need for a common single place for new
platform interfaces to expose their variables for firmware security
features. This would simplify the interface for users of these platforms.
This patchset proposes firmware security filesystem(fwsecurityfs). Any
platform can expose the variables which are required by firmware security
features via this interface. Going forward, this would give a common place
for exposing variables managed by firmware while still allowing platforms
to implement their own underlying semantics.

This design consists of two parts:
1. firmware security filesystem(fwsecurityfs) that provides platforms with
APIs to create their own underlying directory and file structure. It is
recommended to establish a well known mount point:
i.e. /sys/firmware/security/

2. platform specific implementation for these variables which implements
underlying semantics. Platforms can expose their variables as files
allowing read/write/add/delete operations by defining their own inode and
file operations.

This patchset defines:
1. pseries driver to access LPAR Platform Key Store(PLPKS)
2. firmware security filesystem named fwsecurityfs
3. Interface to expose secure variables stored in LPAR PKS via fwsecurityfs

[1] https://community.ibm.com/community/user/power/blogs/chris-engel1/2020/11/20/powervm-introduces-the-platform-keystore
[2] https://lore.kernel.org/linuxppc-dev/20220122005637.28199-1-nayna@linux.ibm.com/

Changelog:

v1:

* Defined unified interface(firmware security filesystem) for all platforms
to expose their variables used for security features. 
* Expose secvars using firmware security fileystem.
* Renamed PKS driver to PLPKS to avoid naming conflict as mentioned by
Dave Hanson.

Nayna Jain (3):
  powerpc/pseries: define driver for Platform KeyStore
  fs: define a firmware security filesystem named fwsecurityfs
  powerpc/pseries: expose authenticated variables stored in LPAR PKS

 arch/powerpc/include/asm/hvcall.h             |  12 +-
 arch/powerpc/include/asm/plpks.h              |  92 ++++
 arch/powerpc/platforms/pseries/Kconfig        |  27 +
 arch/powerpc/platforms/pseries/Makefile       |   2 +
 arch/powerpc/platforms/pseries/plpks/Makefile |   9 +
 .../pseries/plpks/fwsecurityfs_arch.c         |  16 +
 .../platforms/pseries/plpks/internal.h        |  18 +
 arch/powerpc/platforms/pseries/plpks/plpks.c  | 517 ++++++++++++++++++
 .../powerpc/platforms/pseries/plpks/secvars.c | 239 ++++++++
 fs/Kconfig                                    |   1 +
 fs/Makefile                                   |   1 +
 fs/fwsecurityfs/Kconfig                       |  14 +
 fs/fwsecurityfs/Makefile                      |  10 +
 fs/fwsecurityfs/inode.c                       | 159 ++++++
 fs/fwsecurityfs/internal.h                    |  13 +
 fs/fwsecurityfs/super.c                       | 154 ++++++
 include/linux/fwsecurityfs.h                  |  33 ++
 include/uapi/linux/magic.h                    |   1 +
 18 files changed, 1317 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/include/asm/plpks.h
 create mode 100644 arch/powerpc/platforms/pseries/plpks/Makefile
 create mode 100644 arch/powerpc/platforms/pseries/plpks/fwsecurityfs_arch.c
 create mode 100644 arch/powerpc/platforms/pseries/plpks/internal.h
 create mode 100644 arch/powerpc/platforms/pseries/plpks/plpks.c
 create mode 100644 arch/powerpc/platforms/pseries/plpks/secvars.c
 create mode 100644 fs/fwsecurityfs/Kconfig
 create mode 100644 fs/fwsecurityfs/Makefile
 create mode 100644 fs/fwsecurityfs/inode.c
 create mode 100644 fs/fwsecurityfs/internal.h
 create mode 100644 fs/fwsecurityfs/super.c
 create mode 100644 include/linux/fwsecurityfs.h

-- 
2.27.0

[RFC PATCH v2 1/3] powerpc/pseries: define driver for Platform KeyStore

[Nayna Jain]

PowerVM provides an isolated Platform Keystore(PKS) storage allocation
for each LPAR with individually managed access controls to store
sensitive information securely. It provides a new set of hypervisor
calls for Linux kernel to access PKS storage.

Define PLPKS driver using H_CALL interface to access PKS storage.

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 arch/powerpc/include/asm/hvcall.h             |  12 +-
 arch/powerpc/include/asm/plpks.h              |  92 ++++
 arch/powerpc/platforms/pseries/Kconfig        |  10 +
 arch/powerpc/platforms/pseries/Makefile       |   2 +
 arch/powerpc/platforms/pseries/plpks/Makefile |   7 +
 arch/powerpc/platforms/pseries/plpks/plpks.c  | 517 ++++++++++++++++++
 6 files changed, 639 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/include/asm/plpks.h
 create mode 100644 arch/powerpc/platforms/pseries/plpks/Makefile
 create mode 100644 arch/powerpc/platforms/pseries/plpks/plpks.c

diff --git a/arch/powerpc/include/asm/hvcall.h b/arch/powerpc/include/asm/hvcall.h
index d92a20a85395..1da429235632 100644
--- a/arch/powerpc/include/asm/hvcall.h
+++ b/arch/powerpc/include/asm/hvcall.h
@@ -97,6 +97,7 @@
 #define H_OP_MODE	-73
 #define H_COP_HW	-74
 #define H_STATE		-75
+#define H_IN_USE	-77
 #define H_UNSUPPORTED_FLAG_START	-256
 #define H_UNSUPPORTED_FLAG_END		-511
 #define H_MULTI_THREADS_ACTIVE	-9005
@@ -321,10 +322,19 @@
 #define H_SCM_UNBIND_ALL        0x3FC
 #define H_SCM_HEALTH            0x400
 #define H_SCM_PERFORMANCE_STATS 0x418
+#define H_PKS_GET_CONFIG	0x41C
+#define H_PKS_SET_PASSWORD	0x420
+#define H_PKS_GEN_PASSWORD	0x424
+#define H_PKS_WRITE_OBJECT	0x42C
+#define H_PKS_GEN_KEY		0x430
+#define H_PKS_READ_OBJECT	0x434
+#define H_PKS_REMOVE_OBJECT	0x438
+#define H_PKS_CONFIRM_OBJECT_FLUSHED	0x43C
 #define H_RPT_INVALIDATE	0x448
 #define H_SCM_FLUSH		0x44C
 #define H_GET_ENERGY_SCALE_INFO	0x450
-#define MAX_HCALL_OPCODE	H_GET_ENERGY_SCALE_INFO
+#define H_PKS_SIGNED_UPDATE	0x454
+#define MAX_HCALL_OPCODE	H_PKS_SIGNED_UPDATE
 
 /* Scope args for H_SCM_UNBIND_ALL */
 #define H_UNBIND_SCOPE_ALL (0x1)
diff --git a/arch/powerpc/include/asm/plpks.h b/arch/powerpc/include/asm/plpks.h
new file mode 100644
index 000000000000..c3b544bdbcb6
--- /dev/null
+++ b/arch/powerpc/include/asm/plpks.h
@@ -0,0 +1,92 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ * Platform keystore for pseries LPAR(PLPKS).
+ */
+
+#ifndef _PSERIES_PLPKS_H
+#define _PSERIES_PLPKS_H
+
+
+#include <linux/types.h>
+#include <linux/list.h>
+
+
+#define OSSECBOOTAUDIT 0x40000000
+#define OSSECBOOTENFORCE 0x20000000
+#define WORLDREADABLE 0x08000000
+#define SIGNEDUPDATE 0x01000000
+
+struct plpks_var {
+	char *component;
+	u8 *name;
+	u16 namelen;
+	u32 policy;
+	u16 datalen;
+	u8 *data;
+};
+
+struct plpks_var_name {
+	u16 namelen;
+	u8  *name;
+};
+
+struct plpks_var_name_list {
+	u32 varcount;
+	struct plpks_var_name varlist[];
+};
+
+struct plpks_config {
+	u8 version;
+	u8 flags;
+	u32 rsvd0;
+	u16 maxpwsize;
+	u16 maxobjlabelsize;
+	u16 maxobjsize;
+	u32 totalsize;
+	u32 usedspace;
+	u32 supportedpolicies;
+	u64 rsvd1;
+} __packed;
+
+/**
+ * Successful return from this API  implies PKS is available.
+ * This is used to initialize kernel driver and user interfaces.
+ */
+extern struct plpks_config *plpks_get_config(void);
+
+/**
+ * Updates the authenticated variable. It expects NULL as the component.
+ */
+extern int plpks_signed_update_var(struct plpks_var var);
+
+/**
+ * Writes the specified var and its data to PKS.
+ * Any caller of PKS driver should present a valid component type for
+ * their variable.
+ */
+extern int plpks_write_var(struct plpks_var var);
+
+/**
+ * Removes the specified var and its data from PKS.
+ */
+extern int plpks_remove_var(char *component, struct plpks_var_name vname);
+
+/**
+ * Returns the data for the specified os variable.
+ */
+extern int plpks_read_os_var(struct plpks_var *var);
+
+/**
+ * Returns the data for the specified firmware variable.
+ */
+extern int plpks_read_fw_var(struct plpks_var *var);
+
+/**
+ * Returns the data for the specified bootloader variable.
+ */
+extern int plpks_read_bootloader_var(struct plpks_var *var);
+
+#endif
diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index f7fd91d153a4..6c1ca487103f 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -142,6 +142,16 @@ config IBMEBUS
 	help
 	  Bus device driver for GX bus based adapters.
 
+config PSERIES_PLPKS
+	depends on PPC_PSERIES
+	tristate "Support for the Platform Key Storage"
+	help
+	  PowerVM provides an isolated Platform Keystore(PKS) storage
+	  allocation for each LPAR with individually managed access
+	  controls to store sensitive information securely. Select this
+	  config to enable operating system interface to hypervisor to
+	  access this space.
+
 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 7aaff5323544..d6a9209e08c0 100644
--- a/arch/powerpc/platforms/pseries/Makefile
+++ b/arch/powerpc/platforms/pseries/Makefile
@@ -37,3 +37,5 @@ obj-$(CONFIG_ARCH_HAS_CC_PLATFORM)	+= cc_platform.o
 # nothing that operates in real mode is safe for KASAN
 KASAN_SANITIZE_ras.o := n
 KASAN_SANITIZE_kexec.o := n
+
+obj-$(CONFIG_PSERIES_PLPKS)      += plpks/
diff --git a/arch/powerpc/platforms/pseries/plpks/Makefile b/arch/powerpc/platforms/pseries/plpks/Makefile
new file mode 100644
index 000000000000..e651ace920db
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/plpks/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Copyright (C) 2022 IBM Corporation
+# Author: Nayna Jain <nayna@linux.ibm.com>
+#
+
+obj-$(CONFIG_PSERIES_PLPKS)  += plpks.o
diff --git a/arch/powerpc/platforms/pseries/plpks/plpks.c b/arch/powerpc/platforms/pseries/plpks/plpks.c
new file mode 100644
index 000000000000..1edb5905bbef
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/plpks/plpks.c
@@ -0,0 +1,517 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * POWER LPAR Platform KeyStore (PLPKS)
+ * Copyright (C) 2022 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ * Provides access to variables stored in Power LPAR Platform KeyStore(PLPKS).
+ */
+
+#include <linux/module.h>
+#include <linux/types.h>
+#include <linux/errno.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+#include <asm/hvcall.h>
+#include <asm/plpks.h>
+#include <asm/unaligned.h>
+#include <asm/machdep.h>
+
+#define MODULE_VERS "1.0"
+#define MODULE_NAME "pseries-plpks"
+
+#define PKS_FW_OWNER   0x1
+#define PKS_BOOTLOADER_OWNER   0x2
+#define PKS_OS_OWNER   0x3
+
+#define PKS_VAR_LINUX	0x01
+#define PKS_VAR_COMMON	0x04
+
+#define MAX_LABEL_ATTR_SIZE 16
+#define MAX_NAME_SIZE 240
+#define MAX_DATA_SIZE 4000
+
+static bool configset;
+static struct plpks_config *config;
+static u8 *ospassword;
+static u16 ospasswordlength;
+
+struct plpks_auth {
+	u8 version;
+	u8 consumer;
+	__be64 rsvd0;
+	__be32 rsvd1;
+	__be16 passwordlength;
+	u8 password[];
+} __attribute__ ((packed, aligned(16)));
+
+struct label_attr {
+	u8 prefix[8];
+	u8 version;
+	u8 os;
+	u8 length;
+	u8 reserved[5];
+};
+
+struct label {
+	struct label_attr attr;
+	u8 name[MAX_NAME_SIZE];
+};
+
+static int pseries_status_to_err(int rc)
+{
+	int err;
+
+	switch (rc) {
+	case H_SUCCESS:
+		err = 0;
+		break;
+	case H_FUNCTION:
+		err = -ENXIO;
+		break;
+	case H_P2:
+	case H_P3:
+	case H_P4:
+	case H_P5:
+	case H_P6:
+		err = -EINVAL;
+		break;
+	case H_NOT_FOUND:
+		err = -ENOENT;
+		break;
+	case H_BUSY:
+		err = -EBUSY;
+		break;
+	case H_AUTHORITY:
+		err = -EPERM;
+		break;
+	case H_NO_MEM:
+		err = -ENOMEM;
+		break;
+	case H_RESOURCE:
+		err = -EEXIST;
+		break;
+	case H_TOO_BIG:
+		err = -EFBIG;
+		break;
+	default:
+		err = -EINVAL;
+	}
+
+	return err;
+}
+
+static int plpks_gen_password(void)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+	u8 consumer = PKS_OS_OWNER;
+	int rc;
+
+	ospassword = kzalloc(config->maxpwsize, GFP_KERNEL);
+	if (!ospassword)
+		return -ENOMEM;
+
+	ospasswordlength = config->maxpwsize;
+	rc = plpar_hcall(H_PKS_GEN_PASSWORD,
+			retbuf,
+			consumer,
+			0,
+			virt_to_phys(ospassword),
+			config->maxpwsize);
+
+	return pseries_status_to_err(rc);
+}
+
+static int construct_auth(u8 consumer, struct plpks_auth **auth)
+{
+	pr_debug("max password size is %u\n", config->maxpwsize);
+
+	if (!auth || (consumer > 3))
+		return -EINVAL;
+
+	*auth = kmalloc(struct_size(*auth, password, config->maxpwsize),
+			GFP_KERNEL);
+	if (!*auth)
+		return -ENOMEM;
+
+	(*auth)->version = 1;
+	(*auth)->consumer = consumer;
+	(*auth)->rsvd0 = 0;
+	(*auth)->rsvd1 = 0;
+	if ((consumer == PKS_FW_OWNER) || (consumer == PKS_BOOTLOADER_OWNER)) {
+		pr_debug("consumer is bootloader or firmware\n");
+		(*auth)->passwordlength = 0;
+		return 0;
+	}
+
+	(*auth)->passwordlength = ospasswordlength;
+
+	memcpy((*auth)->password, ospassword, flex_array_size(*auth, password,
+					      (*auth)->passwordlength));
+	(*auth)->passwordlength = cpu_to_be16((*auth)->passwordlength);
+
+	return 0;
+}
+
+/**
+ * Label is combination of label attributes + name.
+ * Label attributes are used internally by kernel and not exposed to the user.
+ */
+static int construct_label(char *component, u8 *name, u16 namelen, u8 **label)
+{
+	int varlen;
+	int len = 0;
+	int llen = 0;
+	u8 anyos;
+	int i;
+	int rc = 0;
+	u8 labellength = MAX_LABEL_ATTR_SIZE;
+
+	if (!label)
+		return -EINVAL;
+
+	varlen = namelen + sizeof(struct label_attr);
+	*label = kzalloc(varlen, GFP_KERNEL);
+
+	if (!*label)
+		return -ENOMEM;
+
+	if (component) {
+		len = strlen(component);
+		memcpy(*label, component, len);
+	}
+	llen = len;
+
+	if (component)
+		len = 8 - strlen(component);
+	else
+		len = 8;
+
+	memset(*label + llen, 0, len);
+	llen = llen + len;
+
+	((*label)[llen]) = 0;
+	llen = llen + 1;
+
+	if (component)
+		if (memcmp(component, "sed-opal", 8) == 0)
+			anyos = PKS_VAR_COMMON;
+		else
+			anyos = PKS_VAR_LINUX;
+	else
+		anyos = PKS_VAR_LINUX;
+	memcpy(*label + llen, &anyos, 1);
+	llen = llen + 1;
+
+	memcpy(*label + llen, &labellength, 1);
+	llen = llen + 1;
+
+	memset(*label + llen, 0, 5);
+	llen = llen + 5;
+
+	memcpy(*label + llen, name, namelen);
+	llen = llen + namelen;
+
+	for (i = 0; i < llen; i++)
+		pr_debug("%c", (*label)[i]);
+
+	rc = llen;
+	return rc;
+}
+
+static int _plpks_get_config(void)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+	int rc;
+	size_t size = sizeof(struct plpks_config);
+
+	config = kzalloc(size, GFP_KERNEL);
+	if (!config)
+		return -ENOMEM;
+
+	rc = plpar_hcall(H_PKS_GET_CONFIG,
+			retbuf,
+			virt_to_phys(config),
+			size);
+
+	if (rc != H_SUCCESS)
+		return pseries_status_to_err(rc);
+
+	config->rsvd0 = be32_to_cpu(config->rsvd0);
+	config->maxpwsize = be16_to_cpu(config->maxpwsize);
+	config->maxobjlabelsize = be16_to_cpu(config->maxobjlabelsize);
+	config->maxobjsize = be16_to_cpu(config->maxobjsize);
+	config->totalsize = be32_to_cpu(config->totalsize);
+	config->usedspace = be32_to_cpu(config->usedspace);
+	config->supportedpolicies = be32_to_cpu(config->supportedpolicies);
+	config->rsvd1 = be64_to_cpu(config->rsvd1);
+
+	configset = true;
+
+	return 0;
+}
+
+int plpks_signed_update_var(struct plpks_var var)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+	int rc;
+	u8 *label;
+	u16 varlen;
+	u8 *data;
+	struct plpks_auth *auth;
+	u64 flags;
+	u64 continuetoken;
+
+	if (!var.data || (var.datalen <= 0)
+		  || (var.namelen > MAX_NAME_SIZE)
+		  || (var.datalen > MAX_DATA_SIZE))
+		return -EINVAL;
+
+	if (!(var.policy & SIGNEDUPDATE))
+		return -EINVAL;
+
+	rc = construct_auth(PKS_OS_OWNER, &auth);
+	if (rc)
+		return rc;
+
+	rc = construct_label(var.component, var.name, var.namelen, &label);
+	if (rc <= 0)
+		goto out;
+
+	varlen =  rc;
+	pr_debug("Name to be written is %s of label size %d\n", label, varlen);
+
+	memcpy(&flags, var.data, sizeof(u64));
+	data = kzalloc(var.datalen - sizeof(u64), GFP_KERNEL);
+	memcpy(data, var.data + sizeof(u64), var.datalen - sizeof(u64));
+
+	do {
+		rc = plpar_hcall(H_PKS_SIGNED_UPDATE,
+				retbuf,
+				virt_to_phys(auth),
+				virt_to_phys(label),
+				varlen,
+				var.policy,
+				flags,
+				virt_to_phys(data),
+				var.datalen);
+
+		continuetoken = retbuf[0];
+
+	} while (rc == H_BUSY);
+
+	rc = pseries_status_to_err(rc);
+
+	kfree(label);
+	kfree(data);
+
+out:
+	kfree(auth);
+
+	return rc;
+}
+EXPORT_SYMBOL(plpks_signed_update_var);
+
+int plpks_write_var(struct plpks_var var)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+	int rc;
+	u8 *label;
+	u16 varlen;
+	u8 *data = var.data;
+	struct plpks_auth *auth;
+
+	if (!var.component || !data || (var.datalen <= 0)
+			   || (var.namelen > MAX_NAME_SIZE)
+			   || (var.datalen > MAX_DATA_SIZE))
+		return -EINVAL;
+
+	if (var.policy & SIGNEDUPDATE)
+		return -EINVAL;
+
+	rc = construct_auth(PKS_OS_OWNER, &auth);
+	if (rc)
+		return rc;
+
+	rc = construct_label(var.component, var.name, var.namelen, &label);
+	if (rc <= 0)
+		goto out;
+
+	varlen =  rc;
+	pr_debug("Name to be written is %s of label size %d\n", label, varlen);
+	rc = plpar_hcall(H_PKS_WRITE_OBJECT,
+			retbuf,
+			virt_to_phys(auth),
+			virt_to_phys(label),
+			varlen,
+			var.policy,
+			virt_to_phys(data),
+			var.datalen);
+
+	rc = pseries_status_to_err(rc);
+	kfree(label);
+
+out:
+	kfree(auth);
+
+	return rc;
+}
+EXPORT_SYMBOL(plpks_write_var);
+
+int plpks_remove_var(char *component, struct plpks_var_name vname)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+	int rc;
+	u8 *label;
+	u16 varlen;
+	struct plpks_auth *auth;
+
+	if (!component || vname.namelen > MAX_NAME_SIZE)
+		return -EINVAL;
+
+	rc = construct_auth(PKS_OS_OWNER, &auth);
+	if (rc)
+		return rc;
+
+	rc = construct_label(component, vname.name, vname.namelen, &label);
+	if (rc <= 0)
+		goto out;
+
+	varlen = rc;
+	pr_debug("Name to be written is %s of label size %d\n", label, varlen);
+	rc = plpar_hcall(H_PKS_REMOVE_OBJECT,
+			retbuf,
+			virt_to_phys(auth),
+			virt_to_phys(label),
+			varlen);
+
+	rc = pseries_status_to_err(rc);
+	kfree(label);
+
+out:
+	kfree(auth);
+
+	return rc;
+}
+EXPORT_SYMBOL(plpks_remove_var);
+
+static int plpks_read_var(u8 consumer, struct plpks_var *var)
+{
+	unsigned long retbuf[PLPAR_HCALL_BUFSIZE] = {0};
+	int rc;
+	u16 outlen = config->maxobjsize;
+	u8 *label;
+	u8 *out;
+	u16 varlen;
+	struct plpks_auth *auth;
+
+	if (var->namelen > MAX_NAME_SIZE)
+		return -EINVAL;
+
+	rc = construct_auth(PKS_OS_OWNER, &auth);
+	if (rc)
+		return rc;
+
+	rc = construct_label(var->component, var->name, var->namelen, &label);
+	if (rc <= 0)
+		goto out;
+
+	varlen = rc;
+	pr_debug("Name to be written is %s of label size %d\n", label, varlen);
+	out = kzalloc(outlen, GFP_KERNEL);
+	if (!out)
+		goto out1;
+
+	rc = plpar_hcall(H_PKS_READ_OBJECT,
+			retbuf,
+			virt_to_phys(auth),
+			virt_to_phys(label),
+			varlen,
+			virt_to_phys(out),
+			outlen);
+
+	if (rc != H_SUCCESS) {
+		pr_err("Failed to read %d\n", rc);
+		rc = pseries_status_to_err(rc);
+		goto out2;
+	}
+
+	if ((var->datalen == 0) || (var->datalen > retbuf[0]))
+		var->datalen = retbuf[0];
+
+	var->data = kzalloc(var->datalen, GFP_KERNEL);
+	if (!var->data) {
+		rc = -ENOMEM;
+		goto out2;
+	}
+	var->policy = retbuf[1];
+
+	memcpy(var->data, out, var->datalen);
+
+out2:
+	kfree(out);
+
+out1:
+	kfree(label);
+
+out:
+	kfree(auth);
+
+	return rc;
+}
+
+int plpks_read_os_var(struct plpks_var *var)
+{
+	return plpks_read_var(PKS_OS_OWNER, var);
+}
+EXPORT_SYMBOL(plpks_read_os_var);
+
+int plpks_read_fw_var(struct plpks_var *var)
+{
+	return plpks_read_var(PKS_FW_OWNER, var);
+}
+EXPORT_SYMBOL(plpks_read_fw_var);
+
+int plpks_read_bootloader_var(struct plpks_var *var)
+{
+	return plpks_read_var(PKS_BOOTLOADER_OWNER, var);
+}
+EXPORT_SYMBOL(plpks_read_bootloader_var);
+
+struct plpks_config *plpks_get_config(void)
+{
+
+	if (!configset) {
+		if (_plpks_get_config())
+			return NULL;
+	}
+
+	return config;
+}
+EXPORT_SYMBOL(plpks_get_config);
+
+int __init pseries_plpks_init(void)
+{
+	int rc = 0;
+
+	rc = _plpks_get_config();
+
+	if (rc) {
+		pr_err("Error initializing plpks\n");
+		return rc;
+	}
+
+	rc = plpks_gen_password();
+	if (rc) {
+		if (rc == H_IN_USE) {
+			rc = 0;
+		} else {
+			pr_err("Failed setting password %d\n", rc);
+			rc = pseries_status_to_err(rc);
+			return rc;
+		}
+	}
+
+	return rc;
+}
+arch_initcall(pseries_plpks_init);
-- 
2.27.0

[RFC PATCH v2 2/3] fs: define a firmware security filesystem named fwsecurityfs

[Nayna Jain]

securityfs is meant for linux security subsystems to expose policies/logs
or any other information. However, there are various firmware security
features which expose their variables for user management via kernel.
There is currently no single place to expose these variables. Different
platforms use sysfs/platform specific filesystem(efivarfs)/securityfs
interface as find appropriate. Thus, there is a gap in kernel interfaces
to expose variables for security features.

Define a firmware security filesystem (fwsecurityfs) to be used for
exposing variables managed by firmware and to be used by firmware
enabled security features. These variables are platform specific.
Filesystem provides platforms to implement their own underlying
semantics by defining own inode and file operations.

Similar to securityfs, the firmware security filesystem is recommended
to be exposed on a well known mount point /sys/firmware/security.
Platforms can define their own directory or file structure under this path.

Example:

# mount -t fwsecurityfs fwsecurityfs /sys/firmware/security

# cd /sys/firmware/security/

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 fs/Kconfig                   |   1 +
 fs/Makefile                  |   1 +
 fs/fwsecurityfs/Kconfig      |  14 +++
 fs/fwsecurityfs/Makefile     |  10 +++
 fs/fwsecurityfs/inode.c      | 159 +++++++++++++++++++++++++++++++++++
 fs/fwsecurityfs/internal.h   |  13 +++
 fs/fwsecurityfs/super.c      | 154 +++++++++++++++++++++++++++++++++
 include/linux/fwsecurityfs.h |  29 +++++++
 include/uapi/linux/magic.h   |   1 +
 9 files changed, 382 insertions(+)
 create mode 100644 fs/fwsecurityfs/Kconfig
 create mode 100644 fs/fwsecurityfs/Makefile
 create mode 100644 fs/fwsecurityfs/inode.c
 create mode 100644 fs/fwsecurityfs/internal.h
 create mode 100644 fs/fwsecurityfs/super.c
 create mode 100644 include/linux/fwsecurityfs.h

diff --git a/fs/Kconfig b/fs/Kconfig
index 5976eb33535f..19ea28143428 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -276,6 +276,7 @@ config ARCH_HAS_GIGANTIC_PAGE
 
 source "fs/configfs/Kconfig"
 source "fs/efivarfs/Kconfig"
+source "fs/fwsecurityfs/Kconfig"
 
 endmenu
 
diff --git a/fs/Makefile b/fs/Makefile
index 208a74e0b00e..5792cd0443cb 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -137,6 +137,7 @@ obj-$(CONFIG_F2FS_FS)		+= f2fs/
 obj-$(CONFIG_CEPH_FS)		+= ceph/
 obj-$(CONFIG_PSTORE)		+= pstore/
 obj-$(CONFIG_EFIVAR_FS)		+= efivarfs/
+obj-$(CONFIG_FWSECURITYFS)		+= fwsecurityfs/
 obj-$(CONFIG_EROFS_FS)		+= erofs/
 obj-$(CONFIG_VBOXSF_FS)		+= vboxsf/
 obj-$(CONFIG_ZONEFS_FS)		+= zonefs/
diff --git a/fs/fwsecurityfs/Kconfig b/fs/fwsecurityfs/Kconfig
new file mode 100644
index 000000000000..f1665511eeb9
--- /dev/null
+++ b/fs/fwsecurityfs/Kconfig
@@ -0,0 +1,14 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Copyright (C) 2022 IBM Corporation
+# Author: Nayna Jain <nayna@linux.ibm.com>
+#
+
+config FWSECURITYFS
+	bool "Enable the fwsecurityfs filesystem"
+	help
+	  This will build the fwsecurityfs file system which is recommended
+	  to be mounted on /sys/firmware/security. This can be used by
+	  platforms to expose their variables which are managed by firmware.
+
+	  If you are unsure how to answer this question, answer N.
diff --git a/fs/fwsecurityfs/Makefile b/fs/fwsecurityfs/Makefile
new file mode 100644
index 000000000000..b9931d180178
--- /dev/null
+++ b/fs/fwsecurityfs/Makefile
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Copyright (C) 2022 IBM Corporation
+# Author: Nayna Jain <nayna@linux.ibm.com>
+#
+# Makefile for the firmware security filesystem
+
+obj-$(CONFIG_FWSECURITYFS)		+= fwsecurityfs.o
+
+fwsecurityfs-objs			:= inode.o super.o
diff --git a/fs/fwsecurityfs/inode.c b/fs/fwsecurityfs/inode.c
new file mode 100644
index 000000000000..5d06dc0de059
--- /dev/null
+++ b/fs/fwsecurityfs/inode.c
@@ -0,0 +1,159 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ */
+
+#include <linux/sysfs.h>
+#include <linux/kobject.h>
+#include <linux/fs.h>
+#include <linux/fs_context.h>
+#include <linux/mount.h>
+#include <linux/pagemap.h>
+#include <linux/init.h>
+#include <linux/namei.h>
+#include <linux/security.h>
+#include <linux/lsm_hooks.h>
+#include <linux/magic.h>
+#include <linux/ctype.h>
+#include <linux/fwsecurityfs.h>
+
+#include "internal.h"
+
+int fwsecurityfs_remove_file(struct dentry *dentry)
+{
+	drop_nlink(d_inode(dentry));
+	dput(dentry);
+	return 0;
+};
+EXPORT_SYMBOL_GPL(fwsecurityfs_remove_file);
+
+int fwsecurityfs_create_file(const char *name, umode_t mode,
+					u16 filesize, struct dentry *parent,
+					struct dentry *dentry,
+					const struct file_operations *fops)
+{
+	struct inode *inode;
+	int error;
+	struct inode *dir;
+
+	if (!parent)
+		return -EINVAL;
+
+	dir = d_inode(parent);
+	pr_debug("securityfs: creating file '%s'\n", name);
+
+	inode = new_inode(dir->i_sb);
+	if (!inode) {
+		error = -ENOMEM;
+		goto out1;
+	}
+
+	inode->i_ino = get_next_ino();
+	inode->i_mode = mode;
+	inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
+
+	if (fops)
+		inode->i_fop = fops;
+	else
+		inode->i_fop = &simple_dir_operations;
+
+	if (!dentry) {
+		dentry = fwsecurityfs_alloc_dentry(parent, name);
+		if (IS_ERR(dentry)) {
+			error = PTR_ERR(dentry);
+			goto out;
+		}
+	}
+
+	inode_lock(inode);
+	i_size_write(inode, filesize);
+	d_instantiate(dentry, inode);
+	dget(dentry);
+	d_add(dentry, inode);
+	inode_unlock(inode);
+	return 0;
+
+out1:
+	if (dentry)
+		dput(dentry);
+out:
+	return error;
+}
+EXPORT_SYMBOL_GPL(fwsecurityfs_create_file);
+
+struct dentry *fwsecurityfs_create_dir(const char *name, umode_t mode,
+				       struct dentry *parent,
+				       const struct inode_operations *iops)
+{
+	struct dentry *dentry;
+	struct inode *inode;
+	int error;
+	struct inode *dir;
+	struct super_block *fwsecsb;
+
+	if (!parent) {
+		fwsecsb = fwsecurityfs_get_superblock();
+		if (!fwsecsb)
+			return ERR_PTR(-EIO);
+		parent = fwsecsb->s_root;
+	}
+
+	dir = d_inode(parent);
+
+	inode_lock(dir);
+	dentry = lookup_one_len(name, parent, strlen(name));
+	if (IS_ERR(dentry))
+		goto out;
+
+	inode = new_inode(dir->i_sb);
+	if (!inode) {
+		error = -ENOMEM;
+		goto out1;
+	}
+
+	inode->i_ino = get_next_ino();
+	inode->i_mode = mode;
+	inode->i_atime = inode->i_mtime = inode->i_ctime = current_time(inode);
+	if (iops)
+		inode->i_op = iops;
+	else
+		inode->i_op = &simple_dir_inode_operations;
+	inode->i_fop = &simple_dir_operations;
+	inc_nlink(inode);
+	inc_nlink(dir);
+	d_instantiate(dentry, inode);
+	dget(dentry);
+	inode_unlock(dir);
+	return dentry;
+
+out1:
+	dput(dentry);
+	dentry = ERR_PTR(error);
+out:
+	inode_unlock(dir);
+	return dentry;
+}
+EXPORT_SYMBOL_GPL(fwsecurityfs_create_dir);
+
+int fwsecurityfs_remove_dir(struct dentry *dentry)
+{
+	struct inode *dir;
+
+	if (!dentry || IS_ERR(dentry))
+		return -EINVAL;
+
+	if (!d_is_dir(dentry))
+		return -EPERM;
+
+	dir = d_inode(dentry->d_parent);
+	inode_lock(dir);
+	if (simple_positive(dentry)) {
+		simple_rmdir(dir, dentry);
+		dput(dentry);
+	}
+	inode_unlock(dir);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(fwsecurityfs_remove_dir);
diff --git a/fs/fwsecurityfs/internal.h b/fs/fwsecurityfs/internal.h
new file mode 100644
index 000000000000..b73f6d4b9504
--- /dev/null
+++ b/fs/fwsecurityfs/internal.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ */
+
+#ifndef __FWSECURITYFS_INTERNAL_H
+#define __FWSECURITYFS_INTERNAL_H
+
+struct dentry *fwsecurityfs_alloc_dentry(struct dentry *parent,
+					 const char *name);
+
+#endif
diff --git a/fs/fwsecurityfs/super.c b/fs/fwsecurityfs/super.c
new file mode 100644
index 000000000000..9930889c22a5
--- /dev/null
+++ b/fs/fwsecurityfs/super.c
@@ -0,0 +1,154 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2022 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ */
+
+#include <linux/sysfs.h>
+#include <linux/kobject.h>
+#include <linux/fs.h>
+#include <linux/fs_context.h>
+#include <linux/mount.h>
+#include <linux/pagemap.h>
+#include <linux/init.h>
+#include <linux/namei.h>
+#include <linux/security.h>
+#include <linux/lsm_hooks.h>
+#include <linux/magic.h>
+#include <linux/ctype.h>
+#include <linux/fwsecurityfs.h>
+
+#include "internal.h"
+
+static struct super_block *fwsecsb;
+
+struct super_block *fwsecurityfs_get_superblock(void)
+{
+	return fwsecsb;
+}
+
+static int fwsecurityfs_d_hash(const struct dentry *dir, struct qstr *this)
+{
+	unsigned long hash;
+	int i;
+
+	hash = init_name_hash(dir);
+	for (i = 0; i < this->len; i++)
+		hash = partial_name_hash(tolower(this->name[i]), hash);
+	this->hash = end_name_hash(hash);
+
+	return 0;
+}
+
+static int fwsecurityfs_d_compare(const struct dentry *dentry,
+				  unsigned int len, const char *str,
+				  const struct qstr *name)
+{
+	int i;
+	int result = 1;
+
+	if (len != name->len)
+		goto out;
+	for (i = 0; i < len; i++) {
+		if (tolower(str[i]) != tolower(name->name[i]))
+			goto out;
+	}
+	result = 0;
+out:
+	return result;
+}
+
+struct dentry *fwsecurityfs_alloc_dentry(struct dentry *parent, const char *name)
+{
+	struct dentry *d;
+	struct qstr q;
+	int err;
+
+	q.name = name;
+	q.len = strlen(name);
+
+	err = fwsecurityfs_d_hash(parent, &q);
+	if (err)
+		return ERR_PTR(err);
+
+	d = d_alloc(parent, &q);
+	if (d)
+		return d;
+
+	return ERR_PTR(-ENOMEM);
+}
+
+static const struct dentry_operations fwsecurityfs_d_ops = {
+	.d_compare = fwsecurityfs_d_compare,
+	.d_hash = fwsecurityfs_d_hash,
+	.d_delete = always_delete_dentry,
+};
+
+static const struct super_operations securityfs_super_operations = {
+	.statfs = simple_statfs,
+	.drop_inode = generic_delete_inode,
+};
+
+static int fwsecurityfs_fill_super(struct super_block *sb,
+				   struct fs_context *fc)
+{
+	static const struct tree_descr files[] = {{""}};
+	int error;
+
+	error = simple_fill_super(sb, FWSECURITYFS_MAGIC, files);
+	if (error)
+		return error;
+
+	sb->s_d_op = &fwsecurityfs_d_ops;
+
+	fwsecsb = sb;
+
+	error = arch_fwsecurity_init();
+	if (error)
+		pr_err("arch specific firmware initialization failed\n");
+
+	return 0;
+}
+
+static int fwsecurityfs_get_tree(struct fs_context *fc)
+{
+	return get_tree_single(fc, fwsecurityfs_fill_super);
+}
+
+static const struct fs_context_operations fwsecurityfs_context_ops = {
+	.get_tree	= fwsecurityfs_get_tree,
+};
+
+static int fwsecurityfs_init_fs_context(struct fs_context *fc)
+{
+	fc->ops = &fwsecurityfs_context_ops;
+	return 0;
+}
+
+static struct file_system_type fs_type = {
+	.owner =	THIS_MODULE,
+	.name =		"fwsecurityfs",
+	.init_fs_context = fwsecurityfs_init_fs_context,
+	.kill_sb =	kill_litter_super,
+};
+
+static int __init fwsecurityfs_init(void)
+{
+	int retval;
+
+	retval = sysfs_create_mount_point(firmware_kobj, "security");
+	if (retval)
+		return retval;
+
+	retval = register_filesystem(&fs_type);
+	if (retval) {
+		sysfs_remove_mount_point(firmware_kobj, "security");
+		return retval;
+	}
+
+	return 0;
+}
+core_initcall(fwsecurityfs_init);
+MODULE_DESCRIPTION("Firmware Security Filesystem");
+MODULE_AUTHOR("Nayna Jain");
+MODULE_LICENSE("GPL");
diff --git a/include/linux/fwsecurityfs.h b/include/linux/fwsecurityfs.h
new file mode 100644
index 000000000000..c079ce939f42
--- /dev/null
+++ b/include/linux/fwsecurityfs.h
@@ -0,0 +1,29 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ */
+
+#ifndef _FWSECURITYFS_H_
+#define _FWSECURITYFS_H_
+
+#include <linux/ctype.h>
+#include <linux/fs.h>
+
+struct super_block *fwsecurityfs_get_superblock(void);
+int fwsecurityfs_create_file(const char *name, umode_t mode,
+					u16 filesize, struct dentry *parent,
+					struct dentry *dentry,
+					const struct file_operations *fops);
+int fwsecurityfs_remove_file(struct dentry *dentry);
+struct dentry *fwsecurityfs_create_dir(const char *name, umode_t mode,
+				       struct dentry *parent,
+				       const struct inode_operations *iops);
+int fwsecurityfs_remove_dir(struct dentry *dentry);
+
+static int arch_fwsecurity_init(void)
+{
+	return 0;
+}
+
+#endif /* _FWSECURITYFS_H_ */
diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
index f724129c0425..3c6754937e15 100644
--- a/include/uapi/linux/magic.h
+++ b/include/uapi/linux/magic.h
@@ -53,6 +53,7 @@
 #define QNX4_SUPER_MAGIC	0x002f		/* qnx4 fs detection */
 #define QNX6_SUPER_MAGIC	0x68191122	/* qnx6 fs detection */
 #define AFS_FS_MAGIC		0x6B414653
+#define FWSECURITYFS_MAGIC         0x5345434e      /* "SECM" */
 
 
 #define REISERFS_SUPER_MAGIC	0x52654973	/* used by gcc */
-- 
2.27.0

[RFC PATCH v2 3/3] powerpc/pseries: expose authenticated variables stored in LPAR PKS

[Nayna Jain]

PowerVM Guest Secure boot feature need to expose firmware managed
secure variables for user management. These variables store keys for
grub/kernel verification and also corresponding denied list.

Expose these variables to the userpace via fwsecurityfs.

Example:

Example:

# cd /sys/firmware/security/secvars

# pwd
/sys/firmware/security/secvars

# cat /tmp/PK.bin > PK

# ls -l
total 0
-rw-r--r-- 1 root root 2497 Jun 22 08:34 PK

# hexdump -C PK
00000000  00 00 00 00 00 08 00 00  a1 59 c0 a5 e4 94 a7 4a  |.........Y.....J|
00000010  87 b5 ab 15 5c 2b f0 72  3f 03 00 00 00 00 00 00  |....\+.r?.......|
00000020  23 03 00 00 ca 18 1d 1c  01 7d eb 11 9a 71 08 94  |#........}...q..|
00000030  ef 31 fb e4 30 82 03 0f  30 82 01 f7 a0 03 02 01  |.1..0...0.......|
00000040  02 02 14 22 ab 18 2f d5  aa dd c5 ba 98 27 60 26  |..."../......'`&|
00000050  f1 63 89 54 4c 52 d9 30  0d 06 09 2a 86 48 86 f7  |.c.TLR.0...*.H..|
...

Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
---
 arch/powerpc/platforms/pseries/Kconfig        |  17 ++
 arch/powerpc/platforms/pseries/plpks/Makefile |   2 +
 .../pseries/plpks/fwsecurityfs_arch.c         |  16 ++
 .../platforms/pseries/plpks/internal.h        |  18 ++
 .../powerpc/platforms/pseries/plpks/secvars.c | 239 ++++++++++++++++++
 include/linux/fwsecurityfs.h                  |   4 +
 6 files changed, 296 insertions(+)
 create mode 100644 arch/powerpc/platforms/pseries/plpks/fwsecurityfs_arch.c
 create mode 100644 arch/powerpc/platforms/pseries/plpks/internal.h
 create mode 100644 arch/powerpc/platforms/pseries/plpks/secvars.c

diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
index 6c1ca487103f..9c52095e20c4 100644
--- a/arch/powerpc/platforms/pseries/Kconfig
+++ b/arch/powerpc/platforms/pseries/Kconfig
@@ -152,6 +152,23 @@ config PSERIES_PLPKS
 	  config to enable operating system interface to hypervisor to
 	  access this space.
 
+config PSERIES_FWSECURITYFS_ARCH
+	depends on FWSECURITYFS
+	bool "Support fwsecurityfs for pseries"
+	help
+	  Enable fwsecuirtyfs arch specific code. This would initialize
+	  the firmware security filesystem with initial platform specific
+	  structure.
+
+config PSERIES_PLPKS_SECVARS
+	depends on PSERIES_PLPKS
+	select PSERIES_FWSECURITYFS_ARCH
+	tristate "Support for secvars"
+	help
+	  This interface exposes authenticated variables stored in the LPAR
+	  Platform KeyStore using fwsecurityfs interface.
+	  If you are unsure how to use it, say 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/plpks/Makefile b/arch/powerpc/platforms/pseries/plpks/Makefile
index e651ace920db..ff3d4b4cd3d7 100644
--- a/arch/powerpc/platforms/pseries/plpks/Makefile
+++ b/arch/powerpc/platforms/pseries/plpks/Makefile
@@ -5,3 +5,5 @@
 #
 
 obj-$(CONFIG_PSERIES_PLPKS)  += plpks.o
+obj-$(CONFIG_PSERIES_FWSECURITYFS_ARCH) += fwsecurityfs_arch.o
+obj-$(CONFIG_PSERIES_PLPKS_SECVARS) += secvars.o
diff --git a/arch/powerpc/platforms/pseries/plpks/fwsecurityfs_arch.c b/arch/powerpc/platforms/pseries/plpks/fwsecurityfs_arch.c
new file mode 100644
index 000000000000..6ccdfe4000a6
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/plpks/fwsecurityfs_arch.c
@@ -0,0 +1,16 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * POWER LPAR Platform KeyStore (PLPKS)
+ * Copyright (C) 2022 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ */
+
+#include <linux/fwsecurityfs.h>
+
+#include "internal.h"
+
+int arch_fwsecurity_init(void)
+{
+	return plpks_secvars_init();
+}
diff --git a/arch/powerpc/platforms/pseries/plpks/internal.h b/arch/powerpc/platforms/pseries/plpks/internal.h
new file mode 100644
index 000000000000..6061ffd37677
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/plpks/internal.h
@@ -0,0 +1,18 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2022 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ */
+#ifndef PKS_FWSEC_INTERNAL
+#define PKS_FWSEC_INTERNAL
+
+#ifdef CONFIG_PSERIES_PLPKS_SECVARS
+int plpks_secvars_init(void);
+#else
+int plpks_secvars_init(void)
+{
+	return 0;
+}
+#endif
+#endif
diff --git a/arch/powerpc/platforms/pseries/plpks/secvars.c b/arch/powerpc/platforms/pseries/plpks/secvars.c
new file mode 100644
index 000000000000..8852cb8f2f3c
--- /dev/null
+++ b/arch/powerpc/platforms/pseries/plpks/secvars.c
@@ -0,0 +1,239 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * POWER LPAR Platform KeyStore (PLPKS)
+ * Copyright (C) 2022 IBM Corporation
+ * Author: Nayna Jain <nayna@linux.ibm.com>
+ *
+ */
+
+#include <linux/fs.h>
+#include <linux/fs_context.h>
+#include <linux/mount.h>
+#include <linux/pagemap.h>
+#include <linux/init.h>
+#include <linux/namei.h>
+#include <linux/ctype.h>
+#include <linux/fwsecurityfs.h>
+#include <asm/plpks.h>
+
+#include "internal.h"
+
+static struct dentry *secvar_dir;
+
+static const char * const names[] = {
+	"PK",
+	"KEK",
+	"db",
+	"dbx",
+	"grubdb",
+	"sbat",
+	"moduledb",
+	"trustedcadb",
+	NULL
+};
+
+static int validate_name(char *name)
+{
+	int i = 0;
+
+	while (names[i] != NULL) {
+		if ((strlen(names[i]) == strlen(name))
+		&& (strncmp(name, names[i], strlen(names[i])) == 0))
+			return 0;
+		i++;
+	}
+	pr_err("Invalid name, allowed ones are (dbx,grubdb,sbat,moduledb,trustedcadb)\n");
+
+	return -EINVAL;
+}
+
+static u32 get_policy(char *name)
+{
+	if ((strncmp(name, "PK", 2) == 0)
+	|| (strncmp(name, "KEK", 3) == 0)
+	|| (strncmp(name, "db", 2) == 0)
+	|| (strncmp(name, "dbx", 3) == 0)
+	|| (strncmp(name, "grubdb", 6) == 0)
+	|| (strncmp(name, "sbat", 4) == 0))
+		return (WORLDREADABLE & SIGNEDUPDATE);
+	else
+		return SIGNEDUPDATE;
+}
+
+static ssize_t plpks_secvar_file_write(struct file *file,
+				     const char __user *userbuf,
+				     size_t count, loff_t *ppos)
+{
+	struct plpks_var var;
+	void *data;
+	struct inode *inode = file->f_mapping->host;
+	u16 datasize = count;
+	ssize_t bytes;
+
+	data = memdup_user(userbuf, datasize);
+	if (IS_ERR(data))
+		return PTR_ERR(data);
+
+	var.component = NULL;
+	var.name = file->f_path.dentry->d_iname;
+	var.namelen = strlen(var.name);
+	var.policy = get_policy(var.name);
+	var.datalen = datasize;
+	var.data = data;
+	bytes = plpks_signed_update_var(var);
+
+	if (bytes) {
+		pr_err("Update of the variable failed with error %ld\n", bytes);
+		goto out;
+	}
+
+	inode_lock(inode);
+	i_size_write(inode, datasize);
+	inode->i_mtime = current_time(inode);
+	inode_unlock(inode);
+
+	bytes = count;
+out:
+	kfree(data);
+
+	return bytes;
+
+}
+
+static ssize_t plpks_secvar_file_read(struct file *file, char __user *userbuf,
+				    size_t count, loff_t *ppos)
+{
+	struct plpks_var var;
+	char *out;
+	u32 outlen;
+	int rc;
+	size_t size;
+
+	var.name = file->f_path.dentry->d_iname;
+	var.namelen = strlen(var.name);
+	var.component = NULL;
+	rc = plpks_read_os_var(&var);
+	if (rc) {
+		pr_err("Error reading object %d\n", rc);
+		return rc;
+	}
+
+	outlen = sizeof(var.policy) + var.datalen;
+	out = kzalloc(outlen, GFP_KERNEL);
+	memcpy(out, &var.policy, sizeof(var.policy));
+
+	memcpy(out + sizeof(var.policy), var.data, var.datalen);
+
+	size = simple_read_from_buffer(userbuf, count, ppos,
+				       out, outlen);
+	kfree(out);
+	return size;
+}
+
+
+static const struct file_operations plpks_secvar_file_operations = {
+	.open   = simple_open,
+	.read   = plpks_secvar_file_read,
+	.write  = plpks_secvar_file_write,
+	.llseek = no_llseek,
+};
+
+static int plpks_secvar_create(struct user_namespace *mnt_userns, struct inode *dir,
+			     struct dentry *dentry, umode_t mode, bool excl)
+{
+	int namelen, i = 0;
+	char *varname;
+	int rc = 0;
+
+	namelen = dentry->d_name.len;
+
+	varname = kzalloc(namelen + 1, GFP_KERNEL);
+	if (!varname)
+		return -ENOMEM;
+
+	for (i = 0; i < namelen; i++)
+		varname[i] = dentry->d_name.name[i];
+	varname[i] = '\0';
+
+	rc = validate_name(varname);
+	if (rc)
+		goto out;
+
+	rc = validate_name(varname);
+	if (rc)
+		goto out;
+
+	rc = fwsecurityfs_create_file(varname, S_IFREG|0644, 0, secvar_dir,
+				      dentry, &plpks_secvar_file_operations);
+	if (rc)
+		pr_err("Error creating file\n");
+
+out:
+	kfree(varname);
+
+	return rc;
+}
+
+static const struct inode_operations plpks_secvar_dir_inode_operations = {
+	.lookup = simple_lookup,
+	.create = plpks_secvar_create,
+};
+
+static int plpks_fill_secvars(struct super_block *sb)
+{
+	struct plpks_var *var = NULL;
+	int err;
+	int i = 0;
+
+	while (names[i] != NULL) {
+		var = kzalloc(sizeof(struct plpks_var), GFP_KERNEL);
+		var->name = (char *)names[i];
+		var->namelen = strlen(names[i]);
+		pr_debug("name is %s\n", var->name);
+		var->component = NULL;
+		i++;
+		err = plpks_read_os_var(var);
+		if (err) {
+			kfree(var);
+			continue;
+		}
+
+		err = fwsecurityfs_create_file(var->name, S_IFREG|0644,
+					       var->datalen, secvar_dir,
+					       NULL,
+					       &plpks_secvar_file_operations);
+
+		kfree(var);
+		if (err) {
+			pr_err("Error creating file\n");
+			break;
+		}
+	}
+	return  err;
+};
+
+int plpks_secvars_init(void)
+{
+	int error;
+
+	struct super_block *sb;
+
+	secvar_dir = fwsecurityfs_create_dir("secvars", S_IFDIR | 0755, NULL,
+					     &plpks_secvar_dir_inode_operations);
+	if (IS_ERR(secvar_dir)) {
+		int ret = PTR_ERR(secvar_dir);
+
+		if (ret != -ENODEV)
+			pr_err("Unable to create integrity sysfs dir: %d\n",
+					ret);
+		secvar_dir = NULL;
+		return ret;
+	}
+
+	sb = fwsecurityfs_get_superblock();
+	error = plpks_fill_secvars(sb);
+	if (error)
+		pr_err("Filling secvars failed\n");
+
+	return 0;
+};
diff --git a/include/linux/fwsecurityfs.h b/include/linux/fwsecurityfs.h
index c079ce939f42..d5fd51aa6322 100644
--- a/include/linux/fwsecurityfs.h
+++ b/include/linux/fwsecurityfs.h
@@ -21,9 +21,13 @@ struct dentry *fwsecurityfs_create_dir(const char *name, umode_t mode,
 				       const struct inode_operations *iops);
 int fwsecurityfs_remove_dir(struct dentry *dentry);
 
+#ifdef CONFIG_PSERIES_FWSECURITYFS_ARCH
+int arch_fwsecurity_init(void);
+#else
 static int arch_fwsecurity_init(void)
 {
 	return 0;
 }
+#endif
 
 #endif /* _FWSECURITYFS_H_ */
-- 
2.27.0

Re: [RFC PATCH v2 2/3] fs: define a firmware security filesystem named fwsecurityfs

[Casey Schaufler]

On 6/22/2022 2:56 PM, Nayna Jain wrote:
> securityfs is meant for linux security subsystems to expose policies/logs
> or any other information. However, there are various firmware security
> features which expose their variables for user management via kernel.
> There is currently no single place to expose these variables. Different
> platforms use sysfs/platform specific filesystem(efivarfs)/securityfs
> interface as find appropriate. Thus, there is a gap in kernel interfaces
> to expose variables for security features.

Why not put the firmware entries under /sys/kernel/security/firmware?

Re: [RFC PATCH v2 2/3] fs: define a firmware security filesystem named fwsecurityfs

[Nayna]

On 6/22/22 18:29, Casey Schaufler wrote:
> On 6/22/2022 2:56 PM, Nayna Jain wrote:
>> securityfs is meant for linux security subsystems to expose 
>> policies/logs
>> or any other information. However, there are various firmware security
>> features which expose their variables for user management via kernel.
>> There is currently no single place to expose these variables. Different
>> platforms use sysfs/platform specific filesystem(efivarfs)/securityfs
>> interface as find appropriate. Thus, there is a gap in kernel interfaces
>> to expose variables for security features.
>
> Why not put the firmware entries under /sys/kernel/security/firmware?

 From man 5 sysfs page:

/sys/firmware: This subdirectory contains interfaces for viewing and 
manipulating firmware-specific objects and attributes.

/sys/kernel: This subdirectory contains various files and subdirectories 
that provide information about the running kernel.

The security variables which are supposed to be exposed via fwsecurityfs 
are managed by firmware, stored in firmware managed space and also often 
consumed by firmware for enabling various security features.

 From git commit b67dbf9d4c1987c370fd18fdc4cf9d8aaea604c2, the purpose 
of securityfs(/sys/kernel/security) is to provide a common place for all 
kernel LSMs to use a common place. The idea of 
fwsecurityfs(/sys/firmware/security) is to similarly provide a common 
place for all firmware security objects.

By having another firmware directory within /sys/kernel/security would 
mean scattering firmware objects at multiple places and confusing the 
purpose of /sys/kernel and /sys/firmware.

Thanks & Regards,

      - Nayna

Re: [RFC PATCH v2 3/3] powerpc/pseries: expose authenticated variables stored in LPAR PKS

[Randy Dunlap]

On 6/22/22 14:56, Nayna Jain wrote:
> diff --git a/arch/powerpc/platforms/pseries/Kconfig b/arch/powerpc/platforms/pseries/Kconfig
> index 6c1ca487103f..9c52095e20c4 100644
> --- a/arch/powerpc/platforms/pseries/Kconfig
> +++ b/arch/powerpc/platforms/pseries/Kconfig
> @@ -152,6 +152,23 @@ config PSERIES_PLPKS
>  	  config to enable operating system interface to hypervisor to
>  	  access this space.
>  
> +config PSERIES_FWSECURITYFS_ARCH
> +	depends on FWSECURITYFS
> +	bool "Support fwsecurityfs for pseries"
> +	help
> +	  Enable fwsecuirtyfs arch specific code. This would initialize

	         fwsecurityfs                   . This initializes

> +	  the firmware security filesystem with initial platform specific
> +	  structure.
> +
> +config PSERIES_PLPKS_SECVARS
> +	depends on PSERIES_PLPKS
> +	select PSERIES_FWSECURITYFS_ARCH
> +	tristate "Support for secvars"
> +	help
> +	  This interface exposes authenticated variables stored in the LPAR
> +	  Platform KeyStore using fwsecurityfs interface.
> +	  If you are unsure how to use it, say N.

-- 
~Randy

Re: [RFC PATCH v2 2/3] fs: define a firmware security filesystem named fwsecurityfs

[Greg Kroah-Hartman]

On Wed, Jun 22, 2022 at 05:56:47PM -0400, Nayna Jain wrote:
> securityfs is meant for linux security subsystems to expose policies/logs
> or any other information. However, there are various firmware security
> features which expose their variables for user management via kernel.
> There is currently no single place to expose these variables. Different
> platforms use sysfs/platform specific filesystem(efivarfs)/securityfs
> interface as find appropriate. Thus, there is a gap in kernel interfaces
> to expose variables for security features.
> 
> Define a firmware security filesystem (fwsecurityfs) to be used for
> exposing variables managed by firmware and to be used by firmware
> enabled security features. These variables are platform specific.
> Filesystem provides platforms to implement their own underlying
> semantics by defining own inode and file operations.
> 
> Similar to securityfs, the firmware security filesystem is recommended
> to be exposed on a well known mount point /sys/firmware/security.
> Platforms can define their own directory or file structure under this path.
> 
> Example:
> 
> # mount -t fwsecurityfs fwsecurityfs /sys/firmware/security
> 
> # cd /sys/firmware/security/
> 
> Signed-off-by: Nayna Jain <nayna@linux.ibm.com>
> ---
>  fs/Kconfig                   |   1 +
>  fs/Makefile                  |   1 +
>  fs/fwsecurityfs/Kconfig      |  14 +++
>  fs/fwsecurityfs/Makefile     |  10 +++
>  fs/fwsecurityfs/inode.c      | 159 +++++++++++++++++++++++++++++++++++
>  fs/fwsecurityfs/internal.h   |  13 +++
>  fs/fwsecurityfs/super.c      | 154 +++++++++++++++++++++++++++++++++
>  include/linux/fwsecurityfs.h |  29 +++++++
>  include/uapi/linux/magic.h   |   1 +
>  9 files changed, 382 insertions(+)
>  create mode 100644 fs/fwsecurityfs/Kconfig
>  create mode 100644 fs/fwsecurityfs/Makefile
>  create mode 100644 fs/fwsecurityfs/inode.c
>  create mode 100644 fs/fwsecurityfs/internal.h
>  create mode 100644 fs/fwsecurityfs/super.c
>  create mode 100644 include/linux/fwsecurityfs.h
> 
> diff --git a/fs/Kconfig b/fs/Kconfig
> index 5976eb33535f..19ea28143428 100644
> --- a/fs/Kconfig
> +++ b/fs/Kconfig
> @@ -276,6 +276,7 @@ config ARCH_HAS_GIGANTIC_PAGE
>  
>  source "fs/configfs/Kconfig"
>  source "fs/efivarfs/Kconfig"
> +source "fs/fwsecurityfs/Kconfig"
>  
>  endmenu
>  
> diff --git a/fs/Makefile b/fs/Makefile
> index 208a74e0b00e..5792cd0443cb 100644
> --- a/fs/Makefile
> +++ b/fs/Makefile
> @@ -137,6 +137,7 @@ obj-$(CONFIG_F2FS_FS)		+= f2fs/
>  obj-$(CONFIG_CEPH_FS)		+= ceph/
>  obj-$(CONFIG_PSTORE)		+= pstore/
>  obj-$(CONFIG_EFIVAR_FS)		+= efivarfs/
> +obj-$(CONFIG_FWSECURITYFS)		+= fwsecurityfs/
>  obj-$(CONFIG_EROFS_FS)		+= erofs/
>  obj-$(CONFIG_VBOXSF_FS)		+= vboxsf/
>  obj-$(CONFIG_ZONEFS_FS)		+= zonefs/
> diff --git a/fs/fwsecurityfs/Kconfig b/fs/fwsecurityfs/Kconfig
> new file mode 100644
> index 000000000000..f1665511eeb9
> --- /dev/null
> +++ b/fs/fwsecurityfs/Kconfig
> @@ -0,0 +1,14 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Copyright (C) 2022 IBM Corporation
> +# Author: Nayna Jain <nayna@linux.ibm.com>
> +#
> +
> +config FWSECURITYFS
> +	bool "Enable the fwsecurityfs filesystem"
> +	help
> +	  This will build the fwsecurityfs file system which is recommended
> +	  to be mounted on /sys/firmware/security. This can be used by
> +	  platforms to expose their variables which are managed by firmware.
> +
> +	  If you are unsure how to answer this question, answer N.
> diff --git a/fs/fwsecurityfs/Makefile b/fs/fwsecurityfs/Makefile
> new file mode 100644
> index 000000000000..b9931d180178
> --- /dev/null
> +++ b/fs/fwsecurityfs/Makefile
> @@ -0,0 +1,10 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Copyright (C) 2022 IBM Corporation
> +# Author: Nayna Jain <nayna@linux.ibm.com>
> +#
> +# Makefile for the firmware security filesystem
> +
> +obj-$(CONFIG_FWSECURITYFS)		+= fwsecurityfs.o
> +
> +fwsecurityfs-objs			:= inode.o super.o
> diff --git a/fs/fwsecurityfs/inode.c b/fs/fwsecurityfs/inode.c
> new file mode 100644
> index 000000000000..5d06dc0de059
> --- /dev/null
> +++ b/fs/fwsecurityfs/inode.c
> @@ -0,0 +1,159 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2022 IBM Corporation
> + * Author: Nayna Jain <nayna@linux.ibm.com>
> + */
> +
> +#include <linux/sysfs.h>
> +#include <linux/kobject.h>
> +#include <linux/fs.h>
> +#include <linux/fs_context.h>
> +#include <linux/mount.h>
> +#include <linux/pagemap.h>
> +#include <linux/init.h>
> +#include <linux/namei.h>
> +#include <linux/security.h>
> +#include <linux/lsm_hooks.h>
> +#include <linux/magic.h>
> +#include <linux/ctype.h>
> +#include <linux/fwsecurityfs.h>
> +
> +#include "internal.h"
> +
> +int fwsecurityfs_remove_file(struct dentry *dentry)
> +{
> +	drop_nlink(d_inode(dentry));
> +	dput(dentry);
> +	return 0;
> +};
> +EXPORT_SYMBOL_GPL(fwsecurityfs_remove_file);
> +
> +int fwsecurityfs_create_file(const char *name, umode_t mode,
> +					u16 filesize, struct dentry *parent,
> +					struct dentry *dentry,
> +					const struct file_operations *fops)
> +{
> +	struct inode *inode;
> +	int error;
> +	struct inode *dir;
> +
> +	if (!parent)
> +		return -EINVAL;
> +
> +	dir = d_inode(parent);
> +	pr_debug("securityfs: creating file '%s'\n", name);

Did you forget to call simple_pin_fs() here or anywhere else?

And this can be just one function with the directory creation file, just
check the mode and you will be fine.  Look at securityfs as an example
of how to make this simpler.

> diff --git a/fs/fwsecurityfs/super.c b/fs/fwsecurityfs/super.c

super.c and inode.c can be in the same file, these are tiny, just make
one file for the filesystem logic.

thanks,

greg k-h

Re: [RFC PATCH v2 2/3] fs: define a firmware security filesystem named fwsecurityfs

[James Bottomley]

On Thu, 2022-06-23 at 10:54 +0200, Greg Kroah-Hartman wrote:
[...]
> > diff --git a/fs/fwsecurityfs/inode.c b/fs/fwsecurityfs/inode.c
> > new file mode 100644
> > index 000000000000..5d06dc0de059
> > --- /dev/null
> > +++ b/fs/fwsecurityfs/inode.c
> > @@ -0,0 +1,159 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Copyright (C) 2022 IBM Corporation
> > + * Author: Nayna Jain <nayna@linux.ibm.com>
> > + */
> > +
> > +#include <linux/sysfs.h>
> > +#include <linux/kobject.h>
> > +#include <linux/fs.h>
> > +#include <linux/fs_context.h>
> > +#include <linux/mount.h>
> > +#include <linux/pagemap.h>
> > +#include <linux/init.h>
> > +#include <linux/namei.h>
> > +#include <linux/security.h>
> > +#include <linux/lsm_hooks.h>
> > +#include <linux/magic.h>
> > +#include <linux/ctype.h>
> > +#include <linux/fwsecurityfs.h>
> > +
> > +#include "internal.h"
> > +
> > +int fwsecurityfs_remove_file(struct dentry *dentry)
> > +{
> > +	drop_nlink(d_inode(dentry));
> > +	dput(dentry);
> > +	return 0;
> > +};
> > +EXPORT_SYMBOL_GPL(fwsecurityfs_remove_file);
> > +
> > +int fwsecurityfs_create_file(const char *name, umode_t mode,
> > +					u16 filesize, struct dentry
> > *parent,
> > +					struct dentry *dentry,
> > +					const struct file_operations
> > *fops)
> > +{
> > +	struct inode *inode;
> > +	int error;
> > +	struct inode *dir;
> > +
> > +	if (!parent)
> > +		return -EINVAL;
> > +
> > +	dir = d_inode(parent);
> > +	pr_debug("securityfs: creating file '%s'\n", name);
> 
> Did you forget to call simple_pin_fs() here or anywhere else?
> 
> And this can be just one function with the directory creation file,
> just check the mode and you will be fine.  Look at securityfs as an
> example of how to make this simpler.

Actually, before you go down this route can you consider the namespace
ramifications.  In fact we're just having to rework securityfs to pull
out all the simple_pin_... calls because simple_pin_... is completely
inimical to namespaces.

The first thing to consider is if you simply use securityfs you'll
inherit all the simple_pin_... removal work and be namespace ready.  It
could be that creating a new filesystem that can't be namespaced is the
right thing to do here, but at least ask the question: would we ever
want any of these files to be presented selectively inside containers? 
If the answer is "yes" then simple_pin_... is the wrong interface.

James

Re: [RFC PATCH v2 2/3] fs: define a firmware security filesystem named fwsecurityfs

[Mimi Zohar]

On Thu, 2022-06-23 at 09:23 -0400, James Bottomley wrote:
> On Thu, 2022-06-23 at 10:54 +0200, Greg Kroah-Hartman wrote:
> [...]
> > > diff --git a/fs/fwsecurityfs/inode.c b/fs/fwsecurityfs/inode.c
> > > new file mode 100644
> > > index 000000000000..5d06dc0de059
> > > --- /dev/null
> > > +++ b/fs/fwsecurityfs/inode.c
> > > @@ -0,0 +1,159 @@
> > > +// SPDX-License-Identifier: GPL-2.0-only
> > > +/*
> > > + * Copyright (C) 2022 IBM Corporation
> > > + * Author: Nayna Jain <nayna@linux.ibm.com>
> > > + */
> > > +
> > > +#include <linux/sysfs.h>
> > > +#include <linux/kobject.h>
> > > +#include <linux/fs.h>
> > > +#include <linux/fs_context.h>
> > > +#include <linux/mount.h>
> > > +#include <linux/pagemap.h>
> > > +#include <linux/init.h>
> > > +#include <linux/namei.h>
> > > +#include <linux/security.h>
> > > +#include <linux/lsm_hooks.h>
> > > +#include <linux/magic.h>
> > > +#include <linux/ctype.h>
> > > +#include <linux/fwsecurityfs.h>
> > > +
> > > +#include "internal.h"
> > > +
> > > +int fwsecurityfs_remove_file(struct dentry *dentry)
> > > +{
> > > +	drop_nlink(d_inode(dentry));
> > > +	dput(dentry);
> > > +	return 0;
> > > +};
> > > +EXPORT_SYMBOL_GPL(fwsecurityfs_remove_file);
> > > +
> > > +int fwsecurityfs_create_file(const char *name, umode_t mode,
> > > +					u16 filesize, struct dentry
> > > *parent,
> > > +					struct dentry *dentry,
> > > +					const struct file_operations
> > > *fops)
> > > +{
> > > +	struct inode *inode;
> > > +	int error;
> > > +	struct inode *dir;
> > > +
> > > +	if (!parent)
> > > +		return -EINVAL;
> > > +
> > > +	dir = d_inode(parent);
> > > +	pr_debug("securityfs: creating file '%s'\n", name);
> > 
> > Did you forget to call simple_pin_fs() here or anywhere else?
> > 
> > And this can be just one function with the directory creation file,
> > just check the mode and you will be fine.  Look at securityfs as an
> > example of how to make this simpler.
> 
> Actually, before you go down this route can you consider the namespace
> ramifications.  In fact we're just having to rework securityfs to pull
> out all the simple_pin_... calls because simple_pin_... is completely
> inimical to namespaces.
> 
> The first thing to consider is if you simply use securityfs you'll
> inherit all the simple_pin_... removal work and be namespace ready.  It
> could be that creating a new filesystem that can't be namespaced is the
> right thing to do here, but at least ask the question: would we ever
> want any of these files to be presented selectively inside containers? 
> If the answer is "yes" then simple_pin_... is the wrong interface.

Greg, the securityfs changes James is referring to are part of the IMA
namespacing patch set:
https://lore.kernel.org/linux-integrity/20220420140633.753772-1-stefanb@linux.ibm.com/

I'd really appreciate your reviewing the first two patches:
[PATCH v12 01/26] securityfs: rework dentry creation
[PATCH v12 02/26] securityfs: Extend securityfs with namespacing
support

thanks,

Mimi

Re: [RFC PATCH v2 2/3] fs: define a firmware security filesystem named fwsecurityfs

[Greg Kroah-Hartman]

On Sun, Jun 26, 2022 at 11:48:06AM -0400, Mimi Zohar wrote:
> On Thu, 2022-06-23 at 09:23 -0400, James Bottomley wrote:
> > On Thu, 2022-06-23 at 10:54 +0200, Greg Kroah-Hartman wrote:
> > [...]
> > > > diff --git a/fs/fwsecurityfs/inode.c b/fs/fwsecurityfs/inode.c
> > > > new file mode 100644
> > > > index 000000000000..5d06dc0de059
> > > > --- /dev/null
> > > > +++ b/fs/fwsecurityfs/inode.c
> > > > @@ -0,0 +1,159 @@
> > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > +/*
> > > > + * Copyright (C) 2022 IBM Corporation
> > > > + * Author: Nayna Jain <nayna@linux.ibm.com>
> > > > + */
> > > > +
> > > > +#include <linux/sysfs.h>
> > > > +#include <linux/kobject.h>
> > > > +#include <linux/fs.h>
> > > > +#include <linux/fs_context.h>
> > > > +#include <linux/mount.h>
> > > > +#include <linux/pagemap.h>
> > > > +#include <linux/init.h>
> > > > +#include <linux/namei.h>
> > > > +#include <linux/security.h>
> > > > +#include <linux/lsm_hooks.h>
> > > > +#include <linux/magic.h>
> > > > +#include <linux/ctype.h>
> > > > +#include <linux/fwsecurityfs.h>
> > > > +
> > > > +#include "internal.h"
> > > > +
> > > > +int fwsecurityfs_remove_file(struct dentry *dentry)
> > > > +{
> > > > +	drop_nlink(d_inode(dentry));
> > > > +	dput(dentry);
> > > > +	return 0;
> > > > +};
> > > > +EXPORT_SYMBOL_GPL(fwsecurityfs_remove_file);
> > > > +
> > > > +int fwsecurityfs_create_file(const char *name, umode_t mode,
> > > > +					u16 filesize, struct dentry
> > > > *parent,
> > > > +					struct dentry *dentry,
> > > > +					const struct file_operations
> > > > *fops)
> > > > +{
> > > > +	struct inode *inode;
> > > > +	int error;
> > > > +	struct inode *dir;
> > > > +
> > > > +	if (!parent)
> > > > +		return -EINVAL;
> > > > +
> > > > +	dir = d_inode(parent);
> > > > +	pr_debug("securityfs: creating file '%s'\n", name);
> > > 
> > > Did you forget to call simple_pin_fs() here or anywhere else?
> > > 
> > > And this can be just one function with the directory creation file,
> > > just check the mode and you will be fine.  Look at securityfs as an
> > > example of how to make this simpler.
> > 
> > Actually, before you go down this route can you consider the namespace
> > ramifications.  In fact we're just having to rework securityfs to pull
> > out all the simple_pin_... calls because simple_pin_... is completely
> > inimical to namespaces.
> > 
> > The first thing to consider is if you simply use securityfs you'll
> > inherit all the simple_pin_... removal work and be namespace ready.  It
> > could be that creating a new filesystem that can't be namespaced is the
> > right thing to do here, but at least ask the question: would we ever
> > want any of these files to be presented selectively inside containers? 
> > If the answer is "yes" then simple_pin_... is the wrong interface.
> 
> Greg, the securityfs changes James is referring to are part of the IMA
> namespacing patch set:
> https://lore.kernel.org/linux-integrity/20220420140633.753772-1-stefanb@linux.ibm.com/
> 
> I'd really appreciate your reviewing the first two patches:
> [PATCH v12 01/26] securityfs: rework dentry creation
> [PATCH v12 02/26] securityfs: Extend securityfs with namespacing
> support

Looks like others have already reviewed them, they seem sane to me if
they past testing.

thanks,

greg k-h

Re: [RFC PATCH v2 0/3] powerpc/pseries: add support for local secure storage called Platform KeyStore(PKS)

[Dave Hansen]

On 6/22/22 14:56, Nayna Jain wrote:
> * Renamed PKS driver to PLPKS to avoid naming conflict as mentioned by
> Dave Hanson.

Thank you for doing this!  The new naming looks much less likely to
cause confusion.

Re: [RFC PATCH v2 2/3] fs: define a firmware security filesystem named fwsecurityfs

[Christian Brauner]

On Mon, Jun 27, 2022 at 09:37:28AM +0200, Greg Kroah-Hartman wrote:
> On Sun, Jun 26, 2022 at 11:48:06AM -0400, Mimi Zohar wrote:
> > On Thu, 2022-06-23 at 09:23 -0400, James Bottomley wrote:
> > > On Thu, 2022-06-23 at 10:54 +0200, Greg Kroah-Hartman wrote:
> > > [...]
> > > > > diff --git a/fs/fwsecurityfs/inode.c b/fs/fwsecurityfs/inode.c
> > > > > new file mode 100644
> > > > > index 000000000000..5d06dc0de059
> > > > > --- /dev/null
> > > > > +++ b/fs/fwsecurityfs/inode.c
> > > > > @@ -0,0 +1,159 @@
> > > > > +// SPDX-License-Identifier: GPL-2.0-only
> > > > > +/*
> > > > > + * Copyright (C) 2022 IBM Corporation
> > > > > + * Author: Nayna Jain <nayna@linux.ibm.com>
> > > > > + */
> > > > > +
> > > > > +#include <linux/sysfs.h>
> > > > > +#include <linux/kobject.h>
> > > > > +#include <linux/fs.h>
> > > > > +#include <linux/fs_context.h>
> > > > > +#include <linux/mount.h>
> > > > > +#include <linux/pagemap.h>
> > > > > +#include <linux/init.h>
> > > > > +#include <linux/namei.h>
> > > > > +#include <linux/security.h>
> > > > > +#include <linux/lsm_hooks.h>
> > > > > +#include <linux/magic.h>
> > > > > +#include <linux/ctype.h>
> > > > > +#include <linux/fwsecurityfs.h>
> > > > > +
> > > > > +#include "internal.h"
> > > > > +
> > > > > +int fwsecurityfs_remove_file(struct dentry *dentry)
> > > > > +{
> > > > > +	drop_nlink(d_inode(dentry));
> > > > > +	dput(dentry);
> > > > > +	return 0;
> > > > > +};
> > > > > +EXPORT_SYMBOL_GPL(fwsecurityfs_remove_file);
> > > > > +
> > > > > +int fwsecurityfs_create_file(const char *name, umode_t mode,
> > > > > +					u16 filesize, struct dentry
> > > > > *parent,
> > > > > +					struct dentry *dentry,
> > > > > +					const struct file_operations
> > > > > *fops)
> > > > > +{
> > > > > +	struct inode *inode;
> > > > > +	int error;
> > > > > +	struct inode *dir;
> > > > > +
> > > > > +	if (!parent)
> > > > > +		return -EINVAL;
> > > > > +
> > > > > +	dir = d_inode(parent);
> > > > > +	pr_debug("securityfs: creating file '%s'\n", name);
> > > > 
> > > > Did you forget to call simple_pin_fs() here or anywhere else?
> > > > 
> > > > And this can be just one function with the directory creation file,
> > > > just check the mode and you will be fine.  Look at securityfs as an
> > > > example of how to make this simpler.
> > > 
> > > Actually, before you go down this route can you consider the namespace
> > > ramifications.  In fact we're just having to rework securityfs to pull
> > > out all the simple_pin_... calls because simple_pin_... is completely
> > > inimical to namespaces.

I described this at length in the securityfs namespacing thread at
various points. simple_pin_*() should be avoided if possible. Ideally
the filesystem will just be cleaned up on umount. There might be a
reason to make it survive umounts if you have state that stays around
and somehow is intimately tied to that filesystem.

> > > 
> > > The first thing to consider is if you simply use securityfs you'll
> > > inherit all the simple_pin_... removal work and be namespace ready.  It
> > > could be that creating a new filesystem that can't be namespaced is the
> > > right thing to do here, but at least ask the question: would we ever
> > > want any of these files to be presented selectively inside containers? 
> > > If the answer is "yes" then simple_pin_... is the wrong interface.
> > 
> > Greg, the securityfs changes James is referring to are part of the IMA
> > namespacing patch set:
> > https://lore.kernel.org/linux-integrity/20220420140633.753772-1-stefanb@linux.ibm.com/
> > 
> > I'd really appreciate your reviewing the first two patches:
> > [PATCH v12 01/26] securityfs: rework dentry creation
> > [PATCH v12 02/26] securityfs: Extend securityfs with namespacing
> > support
> 
> Looks like others have already reviewed them, they seem sane to me if
> they past testing.

Thanks for taking a look.