linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem
@ 2019-05-20  6:25 Daniel Axtens
  2019-05-20  6:25 ` [WIP RFC PATCH 1/6] kernfs: add create() and unlink() hooks Daniel Axtens
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Daniel Axtens @ 2019-05-20  6:25 UTC (permalink / raw)
  To: nayna, cclaudio, linux-fsdevel, greg, linuxppc-dev; +Cc: Daniel Axtens

Hi all,

As PowerNV moves towards secure boot, we need a place to put secure
variables. One option that has been canvassed is to make our secure
variables look like EFI variables. This is an early sketch of another
approach where we create a generic firmware variable file system,
fwvarfs, and an OPAL Secure Variable backend for it.

In short, platforms provide a simple backend that can interface with
the hardware, and fwvarfs deals with translating that into a
filesystem that you can use. Almost all of the hard work is done by
kernfs: fwvarfs provides a pretty thin layer on top of that to make
backends a simple as possible.

Behaviour and the API is documented in Documentation/filesystems/fwvarfs.txt

To demonstrate the concept, a fully functional memory-based backend is
provided, and a read-only but userspace-compatible EFI backend.

For OPAL secure variables, I have taken Claudio's commit, tweaked it
to apply to linux-next, replaced all the EFI support with a generic
API, and then written a backend against that. There's a coming version
from Claudio that moves the opal calls towards a simple key/value
interface rather than (name, vendor) pairs - I haven't waited for
that: this is really just to demonstrate that it could be done rather
than an attempt to get mergable code.  It is also compile tested only
as I haven't yet set myself up with a test machine.

The patches are a bit rough, and there are a number of outstanding
TODOs sprinkled in everywhere. The idea is just to do a proof of
concept to inform our discussions:

 - Is this the sort of approach you'd like (generic vs specific)?
 
 - Does the backend API make sense?
 
 - Is the use of kernfs the correct decision, or is it potentially too
   limiting? (e.g. no ability to do case-insensitivity like efivarfs)

 - Is assuming flat fwvars correct or is there a firmware with a
   hierarchical structure?

Regards,
Daniel

Claudio Carvalho (1):
  powerpc/powernv: Add support for OPAL secure variables

Daniel Axtens (5):
  kernfs: add create() and unlink() hooks
  fwvarfs: a generic firmware variable filesystem
  fwvarfs: efi backend
  powerpc/powernv: Remove EFI support for OPAL secure variables
  fwvarfs: Add opal_secvar backend

 Documentation/filesystems/fwvarfs.txt        | 154 ++++++++++
 arch/powerpc/include/asm/opal-api.h          |   6 +-
 arch/powerpc/include/asm/opal-secvar.h       |  58 ++++
 arch/powerpc/include/asm/opal.h              |  10 +
 arch/powerpc/platforms/powernv/Kconfig       |   8 +
 arch/powerpc/platforms/powernv/Makefile      |   1 +
 arch/powerpc/platforms/powernv/opal-call.c   |   4 +
 arch/powerpc/platforms/powernv/opal-secvar.c | 121 ++++++++
 fs/Kconfig                                   |   1 +
 fs/Makefile                                  |   1 +
 fs/fwvarfs/Kconfig                           |  47 +++
 fs/fwvarfs/Makefile                          |  10 +
 fs/fwvarfs/efi.c                             | 177 +++++++++++
 fs/fwvarfs/fwvarfs.c                         | 294 +++++++++++++++++++
 fs/fwvarfs/fwvarfs.h                         | 124 ++++++++
 fs/fwvarfs/mem.c                             | 113 +++++++
 fs/fwvarfs/opal_secvar.c                     | 218 ++++++++++++++
 fs/kernfs/dir.c                              |  54 ++++
 include/linux/kernfs.h                       |   3 +
 include/uapi/linux/magic.h                   |   1 +
 20 files changed, 1404 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/filesystems/fwvarfs.txt
 create mode 100644 arch/powerpc/include/asm/opal-secvar.h
 create mode 100644 arch/powerpc/platforms/powernv/opal-secvar.c
 create mode 100644 fs/fwvarfs/Kconfig
 create mode 100644 fs/fwvarfs/Makefile
 create mode 100644 fs/fwvarfs/efi.c
 create mode 100644 fs/fwvarfs/fwvarfs.c
 create mode 100644 fs/fwvarfs/fwvarfs.h
 create mode 100644 fs/fwvarfs/mem.c
 create mode 100644 fs/fwvarfs/opal_secvar.c

-- 
2.19.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [WIP RFC PATCH 1/6] kernfs: add create() and unlink() hooks
  2019-05-20  6:25 [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem Daniel Axtens
@ 2019-05-20  6:25 ` Daniel Axtens
  2019-05-20  6:25 ` [WIP RFC PATCH 2/6] fwvarfs: a generic firmware variable filesystem Daniel Axtens
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Daniel Axtens @ 2019-05-20  6:25 UTC (permalink / raw)
  To: nayna, cclaudio, linux-fsdevel, greg, linuxppc-dev; +Cc: Daniel Axtens

I'm building a generic firmware variable filesystem on top of kernfs
and I'd like to be able to create and unlink files.

The hooks are fairly straightforward. create() returns a kernfs_node*,
which is safe with regards to cleanup on error paths, because there
is no way that things can fail after that point in the current
implementation. However, currently O_EXCL is not implemented and
that may create failure paths, in which case we may need to revisit
this later.

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 fs/kernfs/dir.c        | 55 ++++++++++++++++++++++++++++++++++++++++++
 include/linux/kernfs.h |  3 +++
 2 files changed, 58 insertions(+)

diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 016ba88f7335..74fe51dbd027 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1175,6 +1175,59 @@ static int kernfs_iop_rename(struct inode *old_dir, struct dentry *old_dentry,
 	return ret;
 }
 
+static int kernfs_iop_create(struct inode *dir, struct dentry *dentry,
+			     umode_t mode, bool excl)
+{
+	struct kernfs_node *parent = dir->i_private;
+	struct kernfs_node *kn;
+	struct kernfs_syscall_ops *scops = kernfs_root(parent)->syscall_ops;
+
+	if (!scops || !scops->create)
+		return -EPERM;
+
+	if (!kernfs_get_active(parent))
+		return -ENODEV;
+
+	// TODO: add some locking to ensure that scops->create
+	// is called only once, and possibly to handle the O_EXCL case
+	WARN_ONCE(excl, "excl unimplemented");
+
+	kn = scops->create(parent, dentry->d_name.name, mode);
+
+	if (!kn)
+		return -EPERM;
+
+	if (IS_ERR(kn))
+		return PTR_ERR(kn);
+
+	d_instantiate(dentry, kernfs_get_inode(dir->i_sb, kn));
+
+	return 0;
+}
+
+static int kernfs_iop_unlink(struct inode *dir, struct dentry *dentry)
+{
+	struct kernfs_node *parent = dir->i_private;
+	struct kernfs_node *kn = d_inode(dentry)->i_private;
+	struct kernfs_syscall_ops *scops = kernfs_root(parent)->syscall_ops;
+	int ret;
+
+
+	if (!scops || !scops->unlink)
+		return -EPERM;
+
+	if (!kernfs_get_active(parent))
+		return -ENODEV;
+
+	ret = scops->unlink(kn);
+	if (ret)
+		return ret;
+
+	drop_nlink(d_inode(dentry));
+	dput(dentry);
+	return 0;
+};
+
 const struct inode_operations kernfs_dir_iops = {
 	.lookup		= kernfs_iop_lookup,
 	.permission	= kernfs_iop_permission,
@@ -1185,6 +1238,8 @@ const struct inode_operations kernfs_dir_iops = {
 	.mkdir		= kernfs_iop_mkdir,
 	.rmdir		= kernfs_iop_rmdir,
 	.rename		= kernfs_iop_rename,
+	.create		= kernfs_iop_create,
+	.unlink		= kernfs_iop_unlink,
 };
 
 static struct kernfs_node *kernfs_leftmost_descendant(struct kernfs_node *pos)
diff --git a/include/linux/kernfs.h b/include/linux/kernfs.h
index 2bf477f86eb1..282b96acbd7e 100644
--- a/include/linux/kernfs.h
+++ b/include/linux/kernfs.h
@@ -179,6 +179,9 @@ struct kernfs_syscall_ops {
 		      const char *new_name);
 	int (*show_path)(struct seq_file *sf, struct kernfs_node *kn,
 			 struct kernfs_root *root);
+	struct kernfs_node* (*create)(struct kernfs_node *parent,
+				      const char *name, umode_t mode);
+	int (*unlink)(struct kernfs_node *kn);
 };
 
 struct kernfs_root {
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [WIP RFC PATCH 2/6] fwvarfs: a generic firmware variable filesystem
  2019-05-20  6:25 [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem Daniel Axtens
  2019-05-20  6:25 ` [WIP RFC PATCH 1/6] kernfs: add create() and unlink() hooks Daniel Axtens
@ 2019-05-20  6:25 ` Daniel Axtens
  2019-05-20  6:25 ` [WIP RFC PATCH 3/6] fwvarfs: efi backend Daniel Axtens
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Daniel Axtens @ 2019-05-20  6:25 UTC (permalink / raw)
  To: nayna, cclaudio, linux-fsdevel, greg, linuxppc-dev; +Cc: Daniel Axtens

Sometimes it is helpful to be able to access firmware variables as
file, like efivarfs, but not all firmware is EFI. Create a framework
that allows generic access to firmware variables exposed by a
implementations of a simple backend API.

Add a demonstration memory-based backend.

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 Documentation/filesystems/fwvarfs.txt | 146 +++++++++++++
 fs/Kconfig                            |   1 +
 fs/Makefile                           |   1 +
 fs/fwvarfs/Kconfig                    |  25 +++
 fs/fwvarfs/Makefile                   |   8 +
 fs/fwvarfs/fwvarfs.c                  | 289 ++++++++++++++++++++++++++
 fs/fwvarfs/fwvarfs.h                  | 116 +++++++++++
 fs/fwvarfs/mem.c                      | 113 ++++++++++
 fs/kernfs/dir.c                       |   1 -
 include/uapi/linux/magic.h            |   1 +
 10 files changed, 700 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/filesystems/fwvarfs.txt
 create mode 100644 fs/fwvarfs/Kconfig
 create mode 100644 fs/fwvarfs/Makefile
 create mode 100644 fs/fwvarfs/fwvarfs.c
 create mode 100644 fs/fwvarfs/fwvarfs.h
 create mode 100644 fs/fwvarfs/mem.c

diff --git a/Documentation/filesystems/fwvarfs.txt b/Documentation/filesystems/fwvarfs.txt
new file mode 100644
index 000000000000..bf1bccba6ab9
--- /dev/null
+++ b/Documentation/filesystems/fwvarfs.txt
@@ -0,0 +1,146 @@
+fwvarfs
+=======
+
+fwvarfs is a generic firmware variable file-system. A platform
+provides a backend implementing a few very simple callbacks, and
+fwvarfs handles all the details required to present the variables as a
+filesystem.
+
+The minimum functionality for a backend is the ability to enumerate
+existing variables. Backends can optionally also allow:
+ - reading of variables
+ - writing of variables
+ - creation of variables
+ - deletion of variables
+
+Key assumptions
+---------------
+
+ * Variables for each backend live in a single, flat directory -
+   there's no concept of subdirectories.
+
+ * Files are created with mode 600 if the backend provides a write
+   hook, and 400 otherwise, and are owned by root:root.
+
+ * The set of variables stored can be determined at boot time, and
+   nothing outside of fwvarfs creates new variables after boot.
+
+Supported backends
+------------------
+
+ * mem - a memory-backed example filesystem supporting all
+   operations. Files created persist across mount/unmount but as no
+   hardware is involved they do not persist across reboots.
+
+Usage
+-----
+
+mount -t fwvarfs <backend> <dir>
+
+For example:
+
+mount -t fwvarfs mem /fw/mem/
+
+API
+---
+
+A backend is installed by creating a fwvarfs_backend struct,
+containing the name of the backend and various callbacks. The backend
+must be registered by adding it to the list at the top of
+fs/fwvarfs/fwvarfs.c
+
+The fwvarfs infrastructure provides the following function to backends:
+
+  int fwvarfs_register_var(struct fwvarfs_backend *backend, const char *name, void *variable, size_t size);
+
+  Register a variable with fwvarfs to allow it to be seen by users.
+
+  backend: the backend to which to add this variable
+
+  name: the name of file representing the variable. Must be a valid
+        filename, so no nulls or slashes.
+
+  variable: data private to the backend representing the variable -
+            will be passed back to every callback
+
+  size: the initial size of the variable
+
+
+The backend must then provide the following functions:
+
+    int (*enumerate)(void);
+
+	Mandatory and called at init time, a backend must call
+	fwvarfs_register_var for all variables it wants to expose to
+	the user.
+
+    void (*destroy)(void *variable);
+
+	Mandatory if you provide a create or unlink hook, and may
+	become mandatory in the future for cleanup.
+
+	Free backend data associated with variable. It will not be
+	referenced after this point by fwvarfs.
+
+
+    ssize_t (*read)(void *variable, char *dest, size_t bytes, loff_t off);
+
+	Read from variable into the a kernel buffer. Similar semantics
+	to a usual read operation, except that off is not a pointer
+	(unlike the usual ppos).
+
+	variable: the variable to read
+	dest: kernel buffer to read into
+	bytes: maximum number of bytes to read
+	off: offset to read from
+
+	Returns the number of bytes read or an error.
+	If this hook is not provided, all reads will fail with -EPERM.
+
+    ssize_t (*write)(void *variable, const char *src, size_t bytes, loff_t off);
+
+	Write into the variable from the given kernel buffer.
+
+	variable: the variable to write
+	src: kernel buffer with contents
+	bytes: write at most this many bytes
+	off: offset into the file to write at.
+
+	Returns the number of bytes written or an error.
+	If this hook is not provided, all writes will fail with -EPERM.
+
+
+    void* (*create)(const char *name);
+
+	Create a variable with the supplied name, and return the
+	associated private data or an error pointer. Do not return
+	NULL on failure.
+
+	If the variable created cannot be registered for any reason,
+	destroy() will be called on the variable returned.
+
+	If the hook is not provided, all attempts to create a file will
+	fail with -EPERM.
+
+    int (*unlink)(void *variable);
+
+	Delete the variable supplied from the backing store. Do not
+	free it yet, if you return success destroy() will be called on
+	the variable.
+
+	If an error is returned, the unlink will be aborted and the file
+	will still be present in the filesystem.
+
+	If the hook is not provided, all attempts to unlink a file will
+	fail with -EPERM.
+
+TODOs
+-----
+
+Perhaps a different registration scheme?
+Currently size is not updated after write
+Should standardise on whether writes must cover the whole file if partial writes are supported.
+Various TODOs in the code
+Convert API documentation to kerndoc
+perhaps better cleanup/removal, although kernfs doesn't seem to provide anything for this so difficult to do with out leaking memory
+check error handling with kernfs create and O_EXCL
diff --git a/fs/Kconfig b/fs/Kconfig
index cbbffc8b9ef5..6fb6e6cbd7b6 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -219,6 +219,7 @@ config ARCH_HAS_GIGANTIC_PAGE
 
 source "fs/configfs/Kconfig"
 source "fs/efivarfs/Kconfig"
+source "fs/fwvarfs/Kconfig"
 
 endmenu
 
diff --git a/fs/Makefile b/fs/Makefile
index c9aea23aba56..2a0c593dfc0f 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -130,3 +130,4 @@ obj-$(CONFIG_F2FS_FS)		+= f2fs/
 obj-$(CONFIG_CEPH_FS)		+= ceph/
 obj-$(CONFIG_PSTORE)		+= pstore/
 obj-$(CONFIG_EFIVAR_FS)		+= efivarfs/
+obj-$(CONFIG_FWVAR_FS)		+= fwvarfs/
diff --git a/fs/fwvarfs/Kconfig b/fs/fwvarfs/Kconfig
new file mode 100644
index 000000000000..62a47cddd4b5
--- /dev/null
+++ b/fs/fwvarfs/Kconfig
@@ -0,0 +1,25 @@
+# SPDX-License-Identifier: GPL-2.0
+
+config FWVAR_FS
+	bool "Generic Firmware Variable Filesystem"
+	help
+	  fwvarfs is a generic file system for access to firmware
+	  variables.
+
+	  It is pluggable: you will need to select a backend below in
+	  order to actually access anything.
+
+	  This cannot currently be built as a module. (TODO: see if
+	  kernfs can be exported or if there are technical obstacles.)
+
+	  If unsure, say N.
+
+config FWVAR_FS_MEM_BACKEND
+	bool "In-memory testing backend"
+	depends on FWVAR_FS
+	help
+	  Include a backend where firmware variables are just
+	  elements of an in-memory list. This is helpful mostly as a
+	  demonstration of fwvarfs.
+
+	  You can safely say N here unless you're exploring fwvarfs.
diff --git a/fs/fwvarfs/Makefile b/fs/fwvarfs/Makefile
new file mode 100644
index 000000000000..f1585baccabe
--- /dev/null
+++ b/fs/fwvarfs/Makefile
@@ -0,0 +1,8 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for the fwvarfs filesytem
+#
+
+obj-$(CONFIG_FWVAR_FS)		+= fwvarfs.o
+
+obj-$(CONFIG_FWVAR_FS_MEM_BACKEND)		+= mem.o
diff --git a/fs/fwvarfs/fwvarfs.c b/fs/fwvarfs/fwvarfs.c
new file mode 100644
index 000000000000..99b7f2fd0f14
--- /dev/null
+++ b/fs/fwvarfs/fwvarfs.c
@@ -0,0 +1,289 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Daniel Axtens
+ *
+ * Thanks to efivarfs, rdt, and cgroupfs for the kernfs example.
+ */
+
+#include <linux/fs.h>
+#include <linux/fs_context.h>
+#include <linux/kernfs.h>
+#include <linux/module.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include <linux/user_namespace.h>
+#include <uapi/linux/magic.h>
+#include "fwvarfs.h"
+
+#define pr_fmt(fmt)	KBUILD_MODNAME ": " fmt
+
+static struct fwvarfs_backend *fwvarfs_backends[] = {
+#if CONFIG_FWVAR_FS_MEM_BACKEND
+	&fwvarfs_mem_backend,
+#endif
+
+	NULL,
+};
+
+struct fwvarfs_file {
+	struct kernfs_node *kn;
+	struct fwvarfs_backend *backend;
+	void *backend_data;
+};
+
+static ssize_t fwvarfs_file_read(struct kernfs_open_file *of, char *buf,
+				 size_t bytes, loff_t off)
+{
+	struct fwvarfs_file *file_data = of->kn->priv;
+
+	if (file_data->backend->read)
+		return file_data->backend->read(file_data->backend_data, buf,
+						bytes, off);
+	else
+		return -EPERM;
+}
+
+static ssize_t fwvarfs_file_write(struct kernfs_open_file *of, char *buf,
+				  size_t bytes, loff_t off)
+{
+	struct fwvarfs_file *file_data = of->kn->priv;
+
+	if (file_data->backend->write)
+		return file_data->backend->write(file_data->backend_data, buf,
+						 bytes, off);
+	else
+		return -EPERM;
+}
+
+
+static struct kernfs_ops fwvarfs_kf_ops = {
+	.atomic_write_len	= PAGE_SIZE,
+	.read			= fwvarfs_file_read,
+	.write			= fwvarfs_file_write,
+};
+
+struct kernfs_node *fwvarfs_create(struct kernfs_node *parent,
+				   const char *name, umode_t mode)
+{
+	struct kernfs_node *kn;
+	struct fwvarfs_file *parent_file = parent->priv;
+	struct fwvarfs_file *file_data;
+	void *backend_data;
+
+	if (!parent_file->backend->create)
+		return ERR_PTR(-EPERM);
+
+	file_data = kzalloc(sizeof(struct fwvarfs_file), GFP_KERNEL);
+	if (!file_data)
+		return ERR_PTR(-ENOMEM);
+
+	file_data->backend = parent_file->backend;
+
+	backend_data = parent_file->backend->create(name);
+
+	if (IS_ERR(backend_data)) {
+		kfree(file_data);
+		return backend_data;
+	}
+
+	file_data->backend_data = backend_data;
+
+	kn = kernfs_create_file(parent, name,
+		(!!parent_file->backend->write ? 0600 : 0400), 0,
+		&fwvarfs_kf_ops, file_data);
+
+	if (IS_ERR(kn)) {
+		parent_file->backend->destroy(backend_data);
+		kfree(file_data);
+		return kn;
+	}
+
+	file_data->kn = kn;
+
+	return kn;
+}
+
+int fwvarfs_unlink(struct kernfs_node *kn)
+{
+
+	struct fwvarfs_file *file_data = kn->priv;
+	int ret;
+
+	if (!file_data->backend->unlink)
+		return -EPERM;
+
+	ret = file_data->backend->unlink(file_data->backend_data);
+
+	if (ret)
+		return ret;
+
+	kernfs_remove(file_data->kn);
+
+	file_data->backend->destroy(file_data->backend_data);
+
+	kfree(file_data);
+	return 0;
+}
+
+static struct kernfs_syscall_ops fwvarfs_scops = {
+	.create = fwvarfs_create,
+	.unlink = fwvarfs_unlink,
+};
+
+int fwvarfs_register_var(struct fwvarfs_backend *backend, const char *name,
+			 void *variable, size_t size)
+{
+	struct fwvarfs_file *file_data;
+	struct kernfs_node *kn;
+
+	file_data = kzalloc(sizeof(struct fwvarfs_file), GFP_KERNEL);
+	if (!file_data)
+		return -ENOMEM;
+
+	file_data->backend = backend;
+	file_data->backend_data = variable;
+
+	kn = kernfs_create_file(backend->kf_root->kn, name,
+		(!!backend->write ? 0600 : 0400), size,
+		&fwvarfs_kf_ops, file_data);
+
+	if (IS_ERR(kn)) {
+		kfree(file_data);
+		return PTR_ERR(kn);
+	}
+
+	file_data->kn = kn;
+
+	return 0;
+
+}
+
+static int fwvarfs_get_tree(struct fs_context *fc)
+{
+	int ret = -ENODEV;
+	struct fwvarfs_backend *backend;
+	struct kernfs_fs_context *kfc = fc->fs_private;
+	int i;
+
+	for (i = 0; (backend = fwvarfs_backends[i]); i++) {
+		if (!backend->is_active)
+			continue;
+
+		if (strcasecmp(fc->source, backend->name) == 0) {
+			kfc->root = backend->kf_root;
+			ret = 0;
+		}
+	}
+	if (ret)
+		return ret;
+
+	return kernfs_get_tree(fc);
+}
+
+static void fwvarfs_free_fs_context(struct fs_context *fc)
+{
+	kernfs_free_fs_context(fc);
+	kfree(fc->fs_private);
+}
+
+static const struct fs_context_operations fwvarfs_fs_context_ops = {
+	.get_tree	= fwvarfs_get_tree,
+	.free		= fwvarfs_free_fs_context,
+};
+
+static int fwvarfs_init_fs_context(struct fs_context *fc)
+{
+	struct kernfs_fs_context *ctx;
+
+	ctx = kzalloc(sizeof(struct kernfs_fs_context), GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
+
+	ctx->magic = FWVARFS_SUPER_MAGIC;
+	fc->fs_private = ctx;
+
+	fc->ops = &fwvarfs_fs_context_ops;
+	if (fc->user_ns)
+		put_user_ns(fc->user_ns);
+	fc->user_ns = get_user_ns(&init_user_ns);
+	fc->global = true;
+	return 0;
+}
+
+static struct file_system_type fwvarfs_type = {
+	.owner   = THIS_MODULE,
+	.name    = "fwvarfs",
+	.init_fs_context = fwvarfs_init_fs_context,
+	.kill_sb = kernfs_kill_sb,
+};
+
+static __init int fwvarfs_init(void)
+{
+	struct fwvarfs_backend *backend;
+	struct fwvarfs_file *file_data;
+	int ret, i;
+
+	for (i = 0; (backend = fwvarfs_backends[i]); i++) {
+		file_data = kzalloc(sizeof(struct fwvarfs_file), GFP_KERNEL);
+		if (!file_data)
+			return -ENOMEM;
+
+		file_data->backend = backend;
+
+		backend->kf_root = kernfs_create_root(&fwvarfs_scops,
+					KERNFS_ROOT_EXTRA_OPEN_PERM_CHECK,
+					file_data);
+
+		if (IS_ERR(backend->kf_root)) {
+			pr_err("kernfs_create_root failed for %s: %ld",
+				backend->name, PTR_ERR(backend->kf_root));
+			kfree(file_data);
+			continue;
+		}
+
+		file_data->kn = backend->kf_root->kn;
+
+		ret = backend->enumerate();
+		if (ret) {
+			pr_err("enumerate failed for %s: %d",
+			       backend->name, ret);
+
+			/*
+			 * TODO: we make no attempt to clean up partially
+			 * created files at this point
+			 */
+			kernfs_destroy_root(backend->kf_root);
+			kfree(file_data);
+			continue;
+		}
+
+		backend->is_active = true;
+	}
+
+	return register_filesystem(&fwvarfs_type);
+}
+
+
+/*
+ * kernfs doesn't support being called from a module atm
+ * and we also have no obvious way to remove all the created variables, so
+ * atm even if you did this you would leak memory. TODO
+ * static __exit void fwvarfs_exit(void)
+ * {
+ *	unregister_filesystem(&fwvarfs_type);
+ * }
+ */
+
+
+/*
+ * again, kernfs blocks module-ising this atm but it's still a neat way
+ * to handle initialisation
+ */
+MODULE_AUTHOR("Daniel Axtens");
+MODULE_DESCRIPTION("Generic Firmware Variable Filesystem");
+MODULE_LICENSE("GPL");
+MODULE_ALIAS_FS("fwvarfs");
+
+module_init(fwvarfs_init);
+/* module_exit(fwvarfs_exit); */
diff --git a/fs/fwvarfs/fwvarfs.h b/fs/fwvarfs/fwvarfs.h
new file mode 100644
index 000000000000..b2944a3baaf7
--- /dev/null
+++ b/fs/fwvarfs/fwvarfs.h
@@ -0,0 +1,116 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Daniel Axtens
+ */
+
+
+#ifndef FWVARFS_H
+#define FWVARFS_H
+
+#include <linux/kernfs.h>
+
+struct fwvarfs_backend {
+	/* name of backend */
+	const char *name;
+
+	/*
+	 * Mandatory and called at init time, a backend must call
+	 * fwvarfs_register_var for all variables it wants to expose to
+	 * the user.
+	 */
+	int (*enumerate)(void);
+
+	/*
+	 * Mandatory if you provide a create or unlink hook, and may
+	 * become mandatory in the future for cleanup.
+	 *
+	 * Free backend data associated with variable. It will not be
+	 * referenced after this point by fwvarfs.
+	 */
+	void (*destroy)(void *variable);
+
+	/*
+	 * Read from variable into the a kernel buffer. Similar semantics
+	 * to a usual read operation, except that off is not a pointer
+	 * (unlike the usual ppos).
+	 *
+	 * variable: the variable to read
+	 * dest: kernel buffer to read into
+	 * bytes: maximum number of bytes to read
+	 * off: offset to read from
+	 *
+	 * Returns the number of bytes read or an error.
+	 * If this hook is not provided, all reads will fail with -EPERM.
+	 */
+	ssize_t (*read)(void *variable, char *dest, size_t bytes, loff_t off);
+
+	/*
+	 * Write into the variable from the given kernel buffer.
+	 *
+	 * variable: the variable to write
+	 * src: kernel buffer with contents
+	 * bytes: write at most this many bytes
+	 * off: offset into the file to write at.
+	 *
+	 * Returns the number of bytes written or an error.
+	 * If this hook is not provided, all writes will fail with -EPERM.
+	 */
+	ssize_t (*write)(void *variable, const char *src, size_t bytes,
+			 loff_t off);
+
+	/*
+	 * Create a variable with the supplied name, and return the
+	 * associated private data or an error pointer. Do not return
+	 * NULL on failure.
+	 *
+	 * If the variable created cannot be registered for any reason,
+	 * destroy() will be called on the variable returned.
+	 *
+	 * If the hook is not provided, all attempts to create a file will
+	 * fail with -EPERM.
+	 */
+	void* (*create)(const char *name);
+
+	/*
+	 * Delete the variable supplied from the backing store. Do not
+	 * free it yet, if you return success destroy() will be called on
+	 * the variable.
+	 *
+	 * If an error is returned, the unlink will be aborted and the file
+	 * will still be present in the filesystem.
+	 *
+	 * If the hook is not provided, all attempts to unlink a file will
+	 * fail with -EPERM.
+	 */
+	int (*unlink)(void *variable);
+
+	/* private to fwvarfs generic code */
+	struct kernfs_root *kf_root;
+	/* did enumerate succeed? */
+	bool is_active;
+};
+
+/*
+ * Register a variable with fwvarfs to allow it to be seen by users.
+ *
+ * backend: the backend to which to add this variable
+ *
+ * name: the name of file representing the variable. Must be a valid
+ *       filename, so no nulls or slashes.
+ *
+ * variable: data private to the backend representing the variable -
+ *           will be passed back to every callback
+ *
+ * size: the initial size of the variable
+ */
+int fwvarfs_register_var(struct fwvarfs_backend *backend, const char *name,
+			 void *variable, size_t size);
+
+
+/* Backends go here */
+#if defined(CONFIG_FWVAR_FS_MEM_BACKEND)
+extern struct fwvarfs_backend fwvarfs_mem_backend;
+#endif
+
+#endif /* FWVARFS_H */
diff --git a/fs/fwvarfs/mem.c b/fs/fwvarfs/mem.c
new file mode 100644
index 000000000000..5c90ea856f8e
--- /dev/null
+++ b/fs/fwvarfs/mem.c
@@ -0,0 +1,113 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Daniel Axtens
+ *
+ * Thanks to efivarfs, and cgroupfs for the kernfs example.
+ */
+
+#include <linux/fs.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include "fwvarfs.h"
+
+static LIST_HEAD(mem_file_list);
+
+struct fwvarfs_mem_file {
+	size_t length;
+	char data[PAGE_SIZE];
+	struct list_head list;
+};
+
+static ssize_t fwvarfs_mem_file_read(void *var, char *buf, size_t bytes,
+				     loff_t off)
+{
+	struct fwvarfs_mem_file *file_data = var;
+	loff_t ppos = off;
+
+	return memory_read_from_buffer(buf, bytes, &ppos, file_data->data,
+				       file_data->length);
+}
+
+static ssize_t simple_write_to_kernel_buffer(void *to, size_t available,
+					     loff_t *ppos, const void *from,
+					     size_t count)
+{
+	loff_t pos = *ppos;
+
+	if (pos < 0)
+		return -EINVAL;
+	if (pos >= available)
+		return -ENOSPC;
+	if (!count)
+		return 0;
+	if (count > available - pos)
+		count = available - pos;
+	memcpy(to, from, count);
+	*ppos = pos + count;
+	return count;
+}
+
+static ssize_t fwvarfs_mem_file_write(void *var, const char *buf,
+				      size_t bytes, loff_t off)
+{
+	struct fwvarfs_mem_file *file_data = var;
+	loff_t ppos = off;
+	int rc;
+
+	// todo - update size of file
+	rc = simple_write_to_kernel_buffer(file_data->data, PAGE_SIZE, &ppos,
+					   buf, bytes);
+	if (rc)
+		file_data->length = ppos;
+	return rc;
+}
+
+
+static void *fwvarfs_mem_create(const char *name)
+{
+	struct fwvarfs_mem_file *file_data;
+
+	file_data = kzalloc(sizeof(struct fwvarfs_mem_file), GFP_KERNEL);
+	if (!file_data)
+		return ERR_PTR(-ENOMEM);
+
+	INIT_LIST_HEAD(&file_data->list);
+	list_add(&mem_file_list, &file_data->list);
+
+	return file_data;
+}
+
+static void fwvarfs_mem_destroy(void *var)
+{
+	struct fwvarfs_mem_file *file_data = var;
+
+	list_del(&file_data->list);
+	kfree(file_data);
+}
+
+static int fwvarfs_mem_unlink(void *var)
+{
+	/*
+	 * This always succeeds and there's nothing we need to do.
+	 * We free the memory in destroy() which is called after
+	 * this by fwvarfs.
+	 */
+	return 0;
+}
+
+static int fwvarfs_mem_enumerate(void)
+{
+	/* Nothing to do, we always start from a blank slate */
+	return 0;
+}
+
+struct fwvarfs_backend fwvarfs_mem_backend = {
+	.name = "mem",
+	.read = fwvarfs_mem_file_read,
+	.write = fwvarfs_mem_file_write,
+	.create = fwvarfs_mem_create,
+	.destroy = fwvarfs_mem_destroy,
+	.unlink = fwvarfs_mem_unlink,
+	.enumerate = fwvarfs_mem_enumerate,
+};
diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
index 74fe51dbd027..211366ecf5a8 100644
--- a/fs/kernfs/dir.c
+++ b/fs/kernfs/dir.c
@@ -1201,7 +1201,6 @@ static int kernfs_iop_create(struct inode *dir, struct dentry *dentry,
 		return PTR_ERR(kn);
 
 	d_instantiate(dentry, kernfs_get_inode(dir->i_sb, kn));
-
 	return 0;
 }
 
diff --git a/include/uapi/linux/magic.h b/include/uapi/linux/magic.h
index f8c00045d537..61f2f5532366 100644
--- a/include/uapi/linux/magic.h
+++ b/include/uapi/linux/magic.h
@@ -34,6 +34,7 @@
 #define EFIVARFS_MAGIC		0xde5e81e4
 #define HOSTFS_SUPER_MAGIC	0x00c0ffee
 #define OVERLAYFS_SUPER_MAGIC	0x794c7630
+#define FWVARFS_SUPER_MAGIC	0x66777672	/* "fwvr" */
 
 #define MINIX_SUPER_MAGIC	0x137F		/* minix v1 fs, 14 char names */
 #define MINIX_SUPER_MAGIC2	0x138F		/* minix v1 fs, 30 char names */
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [WIP RFC PATCH 3/6] fwvarfs: efi backend
  2019-05-20  6:25 [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem Daniel Axtens
  2019-05-20  6:25 ` [WIP RFC PATCH 1/6] kernfs: add create() and unlink() hooks Daniel Axtens
  2019-05-20  6:25 ` [WIP RFC PATCH 2/6] fwvarfs: a generic firmware variable filesystem Daniel Axtens
@ 2019-05-20  6:25 ` Daniel Axtens
  2019-05-20  6:25 ` [WIP RFC PATCH 4/6] powerpc/powernv: Add support for OPAL secure variables Daniel Axtens
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Daniel Axtens @ 2019-05-20  6:25 UTC (permalink / raw)
  To: nayna, cclaudio, linux-fsdevel, greg, linuxppc-dev; +Cc: Daniel Axtens

Add a read-only EFI backend. This does not rely on efivarfs at all
(although it does borrow heavily from the code).

It only supports reading the variables, but it supports the same
format as efivarfs so tools like efivar continue to work if you
mount this filesystem in the same place.

Two small quirks:
 - efivarfs (at least as configured on Ubuntu) allows users to
   access the files, here only root can.
 - efivarfs makes GUID comparison case-insensitive, this does not.

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 Documentation/filesystems/fwvarfs.txt |   5 +
 fs/fwvarfs/Kconfig                    |  11 ++
 fs/fwvarfs/Makefile                   |   1 +
 fs/fwvarfs/efi.c                      | 177 ++++++++++++++++++++++++++
 fs/fwvarfs/fwvarfs.c                  |   4 +-
 fs/fwvarfs/fwvarfs.h                  |   4 +
 6 files changed, 201 insertions(+), 1 deletion(-)
 create mode 100644 fs/fwvarfs/efi.c

diff --git a/Documentation/filesystems/fwvarfs.txt b/Documentation/filesystems/fwvarfs.txt
index bf1bccba6ab9..7c1e921e5c50 100644
--- a/Documentation/filesystems/fwvarfs.txt
+++ b/Documentation/filesystems/fwvarfs.txt
@@ -32,6 +32,10 @@ Supported backends
    operations. Files created persist across mount/unmount but as no
    hardware is involved they do not persist across reboots.
 
+ * efi - a partial reimplementation of efivarfs against the fwvarfs
+   backend. Read-only with no creation or deletion, but sufficient that
+   efivar --print --name <whatever> works the same as efivarfs.
+
 Usage
 -----
 
@@ -40,6 +44,7 @@ mount -t fwvarfs <backend> <dir>
 For example:
 
 mount -t fwvarfs mem /fw/mem/
+mount -t fwvarfs efi /sys/firmware/efi/efivars
 
 API
 ---
diff --git a/fs/fwvarfs/Kconfig b/fs/fwvarfs/Kconfig
index 62a47cddd4b5..e4474da11dbc 100644
--- a/fs/fwvarfs/Kconfig
+++ b/fs/fwvarfs/Kconfig
@@ -23,3 +23,14 @@ config FWVAR_FS_MEM_BACKEND
 	  demonstration of fwvarfs.
 
 	  You can safely say N here unless you're exploring fwvarfs.
+
+config FWVAR_FS_EFI_BACKEND
+	bool "EFI backend"
+	depends on FWVAR_FS
+	help
+	  Include a read-only EFI backend, largely cribbed from
+	  efivarfs. This is handy for demonstrating that the same
+	  userspace tools can read from EFI variables over fwvarfs
+	  in the same way the do with efivarfs.
+
+	  Say N here unless you're exploring fwvarfs.
diff --git a/fs/fwvarfs/Makefile b/fs/fwvarfs/Makefile
index f1585baccabe..2ab9dfd650ca 100644
--- a/fs/fwvarfs/Makefile
+++ b/fs/fwvarfs/Makefile
@@ -6,3 +6,4 @@
 obj-$(CONFIG_FWVAR_FS)		+= fwvarfs.o
 
 obj-$(CONFIG_FWVAR_FS_MEM_BACKEND)		+= mem.o
+obj-$(CONFIG_FWVAR_FS_EFI_BACKEND)		+= efi.o
diff --git a/fs/fwvarfs/efi.c b/fs/fwvarfs/efi.c
new file mode 100644
index 000000000000..7aa5c186d0c9
--- /dev/null
+++ b/fs/fwvarfs/efi.c
@@ -0,0 +1,177 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Daniel Axtens
+ *
+ * Based on efivarfs:
+ * Copyright (C) 2012 Red Hat, Inc.
+ * Copyright (C) 2012 Jeremy Kerr <jeremy.kerr@canonical.com>
+ *
+ * We cheat by not allowing for case-insensitivity.
+ */
+
+#include <linux/fs.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include "fwvarfs.h"
+
+#include <linux/efi.h>
+#include <linux/uuid.h>
+#include <linux/ucs2_string.h>
+
+static LIST_HEAD(efivar_list);
+static LIST_HEAD(efivar_file_list);
+
+struct fwvarfs_efi_file {
+	bool is_removable;
+	struct list_head list;
+	struct efivar_entry *entry;
+};
+
+// need a forward decl to pass down to register
+struct fwvarfs_backend fwvarfs_efi_backend;
+
+
+static ssize_t fwvarfs_efi_read(void *variable, char *buf,
+		size_t count, loff_t off)
+{
+	struct fwvarfs_efi_file *file_data = variable;
+	struct efivar_entry *var = file_data->entry;
+	unsigned long datasize = 0;
+	u32 attributes;
+	void *data;
+	ssize_t size = 0;
+	loff_t ppos = off;
+	int err;
+
+	err = efivar_entry_size(var, &datasize);
+
+	/*
+	 * efivarfs represents uncommitted variables with
+	 * zero-length files. Reading them should return EOF.
+	 */
+	if (err == -ENOENT)
+		return 0;
+	else if (err)
+		return err;
+
+	data = kmalloc(datasize + sizeof(attributes), GFP_KERNEL);
+
+	if (!data)
+		return -ENOMEM;
+
+	size = efivar_entry_get(var, &attributes, &datasize,
+				data + sizeof(attributes));
+	if (size)
+		goto out_free;
+
+	memcpy(data, &attributes, sizeof(attributes));
+	size = memory_read_from_buffer(buf, count, &ppos,
+				       data, datasize + sizeof(attributes));
+out_free:
+	kfree(data);
+
+	return size;
+}
+
+static int fwvarfs_efi_callback(efi_char16_t *name16, efi_guid_t vendor,
+				unsigned long name_size, void *data)
+{
+	struct efivar_entry *entry;
+	struct fwvarfs_efi_file *file_data;
+	unsigned long size = 0;
+	char *name;
+	int len;
+	int err = -ENOMEM;
+
+	file_data = kzalloc(sizeof(*file_data), GFP_KERNEL);
+	if (!file_data)
+		return err;
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry)
+		goto fail;
+
+	memcpy(entry->var.VariableName, name16, name_size);
+	memcpy(&(entry->var.VendorGuid), &vendor, sizeof(efi_guid_t));
+
+	len = ucs2_utf8size(entry->var.VariableName);
+
+	/* name, plus '-', plus GUID, plus NUL */
+	name = kmalloc(len + 1 + EFI_VARIABLE_GUID_LEN + 1, GFP_KERNEL);
+	if (!name)
+		goto fail_entry;
+
+	ucs2_as_utf8(name, entry->var.VariableName, len);
+
+	if (efivar_variable_is_removable(entry->var.VendorGuid, name, len))
+		file_data->is_removable = true;
+
+	name[len] = '-';
+
+	efi_guid_to_str(&entry->var.VendorGuid, name + len + 1);
+
+	name[len + EFI_VARIABLE_GUID_LEN+1] = '\0';
+
+	efivar_entry_size(entry, &size);
+	err = efivar_entry_add(entry, &efivar_list);
+	if (err)
+		goto fail_name;
+
+	err = fwvarfs_register_var(&fwvarfs_efi_backend, name, file_data, size);
+	if (err)
+		goto fail_name;
+
+	INIT_LIST_HEAD(&file_data->list);
+	list_add(&efivar_file_list, &file_data->list);
+	file_data->entry = entry;
+
+	/* copied by the above, I think */
+	kfree(name);
+
+	return 0;
+fail_name:
+	kfree(name);
+fail_entry:
+	kfree(entry);
+fail:
+	kfree(file_data);
+	return err;
+}
+
+
+static void fwvarfs_efi_destroy(void *var)
+{
+	struct fwvarfs_efi_file *file_data = var;
+	struct efivar_entry *entry = file_data->entry;
+
+	// ignore error, eek.
+	efivar_entry_remove(entry);
+	kfree(entry);
+
+	list_del(&file_data->list);
+	kfree(file_data);
+}
+
+
+static int fwvarfs_efi_enumerate(void)
+{
+	int err;
+	struct fwvarfs_efi_file *pos, *tmp;
+
+	err = efivar_init(fwvarfs_efi_callback, NULL, true, &efivar_list);
+	if (err) {
+		list_for_each_entry_safe(pos, tmp, &efivar_file_list, list) {
+			fwvarfs_efi_destroy(pos);
+		}
+	}
+
+	return err;
+}
+
+struct fwvarfs_backend fwvarfs_efi_backend = {
+	.name = "efi",
+	.destroy = fwvarfs_efi_destroy,
+	.enumerate = fwvarfs_efi_enumerate,
+	.read = fwvarfs_efi_read,
+};
diff --git a/fs/fwvarfs/fwvarfs.c b/fs/fwvarfs/fwvarfs.c
index 99b7f2fd0f14..643ec6585b4d 100644
--- a/fs/fwvarfs/fwvarfs.c
+++ b/fs/fwvarfs/fwvarfs.c
@@ -22,7 +22,9 @@ static struct fwvarfs_backend *fwvarfs_backends[] = {
 #if CONFIG_FWVAR_FS_MEM_BACKEND
 	&fwvarfs_mem_backend,
 #endif
-
+#ifdef CONFIG_FWVAR_FS_EFI_BACKEND
+	&fwvarfs_efi_backend,
+#endif
 	NULL,
 };
 
diff --git a/fs/fwvarfs/fwvarfs.h b/fs/fwvarfs/fwvarfs.h
index b2944a3baaf7..49bde268401f 100644
--- a/fs/fwvarfs/fwvarfs.h
+++ b/fs/fwvarfs/fwvarfs.h
@@ -113,4 +113,8 @@ int fwvarfs_register_var(struct fwvarfs_backend *backend, const char *name,
 extern struct fwvarfs_backend fwvarfs_mem_backend;
 #endif
 
+#if defined(CONFIG_FWVAR_FS_EFI_BACKEND)
+extern struct fwvarfs_backend fwvarfs_efi_backend;
+#endif
+
 #endif /* FWVARFS_H */
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [WIP RFC PATCH 4/6] powerpc/powernv: Add support for OPAL secure variables
  2019-05-20  6:25 [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem Daniel Axtens
                   ` (2 preceding siblings ...)
  2019-05-20  6:25 ` [WIP RFC PATCH 3/6] fwvarfs: efi backend Daniel Axtens
@ 2019-05-20  6:25 ` Daniel Axtens
  2019-05-20  6:25 ` [WIP RFC PATCH 5/6] powerpc/powernv: Remove EFI " Daniel Axtens
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Daniel Axtens @ 2019-05-20  6:25 UTC (permalink / raw)
  To: nayna, cclaudio, linux-fsdevel, greg, linuxppc-dev; +Cc: Daniel Axtens

From: Claudio Carvalho <cclaudio@linux.ibm.com>

[dja: this is a WIP version - a new version is coming that changes
the interface. I also had to renumber the opal calls to get this
to apply. Basically, this is an illustration of the concept: more
work would be required to get this to actually function.]

The X.509 certificates trusted by the platform and other information
required to secure boot the host OS kernel are wrapped in secure
variables, which are controlled by OPAL.

The OPAL secure variables can be handled through the following OPAL
calls.

OPAL_SECVAR_GET:
Returns the data for a given secure variable name and vendor GUID.

OPAL_SECVAR_GET_NEXT:
For a given secure variable, it returns the name and vendor GUID
of the next variable.

OPAL_SECVAR_ENQUEUE:
Enqueue the supplied secure variable update so that it can be processed
by OPAL in the next boot. Variable updates cannot be be processed right
away because the variable storage is write locked at runtime.

OPAL_SECVAR_INFO:
Returns size information about the variable.

This patch adds support for OPAL secure variables by setting up the EFI
runtime variable services to make OPAL calls.

This patch also introduces CONFIG_OPAL_SECVAR for enabling the OPAL
secure variables support in the kernel. Since CONFIG_OPAL_SECVAR selects
CONFIG_EFI, it also allow us to manage the OPAL secure variables from
userspace via efivarfs.

Signed-off-by: Claudio Carvalho <cclaudio@linux.ibm.com>
Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 arch/powerpc/include/asm/opal-api.h          |   6 +-
 arch/powerpc/include/asm/opal.h              |  10 ++
 arch/powerpc/platforms/Kconfig               |   3 +
 arch/powerpc/platforms/powernv/Kconfig       |   9 +
 arch/powerpc/platforms/powernv/Makefile      |   1 +
 arch/powerpc/platforms/powernv/opal-call.c   |   4 +
 arch/powerpc/platforms/powernv/opal-secvar.c | 179 +++++++++++++++++++
 7 files changed, 211 insertions(+), 1 deletion(-)
 create mode 100644 arch/powerpc/platforms/powernv/opal-secvar.c

diff --git a/arch/powerpc/include/asm/opal-api.h b/arch/powerpc/include/asm/opal-api.h
index e1577cfa7186..8054e1e983ff 100644
--- a/arch/powerpc/include/asm/opal-api.h
+++ b/arch/powerpc/include/asm/opal-api.h
@@ -212,7 +212,11 @@
 #define OPAL_HANDLE_HMI2			166
 #define	OPAL_NX_COPROC_INIT			167
 #define OPAL_XIVE_GET_VP_STATE			170
-#define OPAL_LAST				170
+#define OPAL_SECVAR_GET				171
+#define OPAL_SECVAR_GET_NEXT			172
+#define OPAL_SECVAR_ENQUEUE			173
+#define OPAL_SECVAR_INFO			174
+#define OPAL_LAST				174
 
 #define QUIESCE_HOLD			1 /* Spin all calls at entry */
 #define QUIESCE_REJECT			2 /* Fail all calls with OPAL_BUSY */
diff --git a/arch/powerpc/include/asm/opal.h b/arch/powerpc/include/asm/opal.h
index 4cc37e708bc7..4b8046caaf4f 100644
--- a/arch/powerpc/include/asm/opal.h
+++ b/arch/powerpc/include/asm/opal.h
@@ -394,6 +394,16 @@ void opal_powercap_init(void);
 void opal_psr_init(void);
 void opal_sensor_groups_init(void);
 
+extern int opal_secvar_get(uint64_t name, uint64_t vendor, uint64_t attr,
+			   uint64_t data_size, uint64_t data);
+extern int opal_secvar_get_next(uint64_t name_size, uint64_t name,
+				uint64_t vendor);
+extern int opal_secvar_enqueue(uint64_t name, uint64_t vendor, uint64_t attr,
+			       uint64_t data_size, uint64_t data);
+extern int opal_secvar_info(uint64_t attr, uint64_t storage_space,
+			    uint64_t remaining_space,
+			    uint64_t max_variable_size);
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_POWERPC_OPAL_H */
diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index f3fb79fccc72..8e30510bc0c1 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -326,4 +326,7 @@ config XILINX_PCI
 	bool "Xilinx PCI host bridge support"
 	depends on PCI && XILINX_VIRTEX
 
+config EFI
+	bool
+
 endmenu
diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig
index 850eee860cf2..879f8e766098 100644
--- a/arch/powerpc/platforms/powernv/Kconfig
+++ b/arch/powerpc/platforms/powernv/Kconfig
@@ -47,3 +47,12 @@ config PPC_VAS
 	  VAS adapters are found in POWER9 based systems.
 
 	  If unsure, say N.
+
+config OPAL_SECVAR
+	bool "OPAL Secure Variables"
+	depends on PPC_POWERNV && !CPU_BIG_ENDIAN
+	select UCS2_STRING
+	select EFI
+	help
+	  This enables the kernel to access OPAL secure variables via EFI
+	  runtime variable services.
diff --git a/arch/powerpc/platforms/powernv/Makefile b/arch/powerpc/platforms/powernv/Makefile
index da2e99efbd04..1511d836fd19 100644
--- a/arch/powerpc/platforms/powernv/Makefile
+++ b/arch/powerpc/platforms/powernv/Makefile
@@ -16,3 +16,4 @@ obj-$(CONFIG_PERF_EVENTS) += opal-imc.o
 obj-$(CONFIG_PPC_MEMTRACE)	+= memtrace.o
 obj-$(CONFIG_PPC_VAS)	+= vas.o vas-window.o vas-debug.o
 obj-$(CONFIG_OCXL_BASE)	+= ocxl.o
+obj-$(CONFIG_OPAL_SECVAR)	+= opal-secvar.o
diff --git a/arch/powerpc/platforms/powernv/opal-call.c b/arch/powerpc/platforms/powernv/opal-call.c
index 36c8fa3647a2..1a2e080dd027 100644
--- a/arch/powerpc/platforms/powernv/opal-call.c
+++ b/arch/powerpc/platforms/powernv/opal-call.c
@@ -288,3 +288,7 @@ OPAL_CALL(opal_pci_set_pbcq_tunnel_bar,		OPAL_PCI_SET_PBCQ_TUNNEL_BAR);
 OPAL_CALL(opal_sensor_read_u64,			OPAL_SENSOR_READ_U64);
 OPAL_CALL(opal_sensor_group_enable,		OPAL_SENSOR_GROUP_ENABLE);
 OPAL_CALL(opal_nx_coproc_init,			OPAL_NX_COPROC_INIT);
+OPAL_CALL(opal_secvar_get,			OPAL_SECVAR_GET);
+OPAL_CALL(opal_secvar_get_next,			OPAL_SECVAR_GET_NEXT);
+OPAL_CALL(opal_secvar_enqueue,			OPAL_SECVAR_ENQUEUE);
+OPAL_CALL(opal_secvar_info,			OPAL_SECVAR_INFO)
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c b/arch/powerpc/platforms/powernv/opal-secvar.c
new file mode 100644
index 000000000000..e333828bd0bc
--- /dev/null
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -0,0 +1,179 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * PowerNV code for secure variables
+ *
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Claudio Carvalho <cclaudio@linux.ibm.com>
+ *
+ */
+#define pr_fmt(fmt) "secvar: "fmt
+
+#include <linux/efi.h>
+#include <asm/machdep.h>
+#include <asm/opal.h>
+
+static bool opal_secvar_supported;
+
+static efi_status_t opal_to_efi_status_log(int rc, const char *func_name)
+{
+	efi_status_t status;
+
+	switch (rc) {
+	case OPAL_EMPTY:
+		status = EFI_NOT_FOUND;
+		break;
+	case OPAL_HARDWARE:
+		status = EFI_DEVICE_ERROR;
+		break;
+	case OPAL_NO_MEM:
+		pr_err("%s: No space in the volatile storage\n", func_name);
+		status = EFI_OUT_OF_RESOURCES;
+		break;
+	case OPAL_PARAMETER:
+		status = EFI_INVALID_PARAMETER;
+		break;
+	case OPAL_PARTIAL:
+		status = EFI_BUFFER_TOO_SMALL;
+		break;
+	case OPAL_PERMISSION:
+		status = EFI_WRITE_PROTECTED;
+		break;
+	case OPAL_RESOURCE:
+		pr_err("%s: No space in the non-volatile storage\n", func_name);
+		status = EFI_OUT_OF_RESOURCES;
+		break;
+	case OPAL_SUCCESS:
+		status = EFI_SUCCESS;
+		break;
+	default:
+		pr_err("%s: Unknown OPAL error %d\n", func_name, rc);
+		status = EFI_DEVICE_ERROR;
+		break;
+	}
+
+	return status;
+}
+
+#define opal_to_efi_status(rc) opal_to_efi_status_log(rc, __func__)
+
+static efi_status_t
+opal_get_variable(efi_char16_t *name, efi_guid_t *vendor, u32 *attr,
+		  unsigned long *data_size, void *data)
+{
+	int rc;
+
+	if (!opal_secvar_supported)
+		return EFI_UNSUPPORTED;
+
+	*data_size = cpu_to_be64(*data_size);
+
+	rc = opal_secvar_get(__pa(name), __pa(vendor), __pa(attr),
+			     __pa(data_size), __pa(data));
+	/*
+	 * The @attr is an optional output parameter. It is returned in
+	 * big-endian.
+	 */
+	if (attr)
+		*attr = be32_to_cpup(attr);
+	*data_size = be64_to_cpu(*data_size);
+
+	return opal_to_efi_status(rc);
+}
+
+static efi_status_t
+opal_get_next_variable(unsigned long *name_size, efi_char16_t *name,
+		       efi_guid_t *vendor)
+{
+	int rc;
+
+	if (!opal_secvar_supported)
+		return EFI_UNSUPPORTED;
+
+	*name_size = cpu_to_be64(*name_size);
+
+	rc = opal_secvar_get_next(__pa(name_size), __pa(name),
+				  __pa(vendor));
+
+	*name_size = be64_to_cpu(*name_size);
+
+	return opal_to_efi_status(rc);
+}
+
+static efi_status_t
+opal_set_variable(efi_char16_t *name, efi_guid_t *vendor, u32 attr,
+		  unsigned long data_size, void *data)
+{
+	int rc;
+
+	if (!opal_secvar_supported)
+		return EFI_UNSUPPORTED;
+	/*
+	 * The secure variable update must be enqueued in order to be processed
+	 * in the next boot by firmware. The secure variable storage is write
+	 * locked at runtime.
+	 */
+	rc = opal_secvar_enqueue(__pa(name), __pa(vendor), attr,
+				 data_size, __pa(data));
+	return opal_to_efi_status(rc);
+}
+
+static efi_status_t
+opal_query_variable_info(u32 attr, u64 *storage_space, u64 *remaining_space,
+			 u64 *max_variable_size)
+{
+	int rc;
+
+	if (!opal_secvar_supported)
+		return EFI_UNSUPPORTED;
+
+	*storage_space = cpu_to_be64p(storage_space);
+	*remaining_space = cpu_to_be64p(remaining_space);
+	*max_variable_size = cpu_to_be64p(max_variable_size);
+
+	rc = opal_secvar_info(attr, __pa(storage_space), __pa(remaining_space),
+			      __pa(max_variable_size));
+
+	*storage_space = be64_to_cpup(storage_space);
+	*remaining_space = be64_to_cpup(remaining_space);
+	*max_variable_size = be64_to_cpup(max_variable_size);
+
+	return opal_to_efi_status(rc);
+}
+
+static void pnv_efi_runtime_setup(void)
+{
+	/*
+	 * The opal wrappers below treat the @name, @vendor, and @data
+	 * parameters as little endian blobs.
+	 * @name is a ucs2 string
+	 * @vendor is the vendor GUID. It is converted to LE in the kernel
+	 * @data variable data, which layout may be different for each variable
+	 */
+	efi.get_variable = opal_get_variable;
+	efi.get_next_variable = opal_get_next_variable;
+	efi.set_variable = opal_set_variable;
+	efi.query_variable_info = opal_query_variable_info;
+
+	if (!opal_check_token(OPAL_SECVAR_GET) ||
+	    !opal_check_token(OPAL_SECVAR_GET_NEXT) ||
+	    !opal_check_token(OPAL_SECVAR_ENQUEUE) ||
+	    !opal_check_token(OPAL_SECVAR_INFO)) {
+		pr_err("OPAL doesn't support secure variables\n");
+		opal_secvar_supported = false;
+	} else {
+		opal_secvar_supported = true;
+	}
+}
+
+static int __init pnv_efi_init(void)
+{
+	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
+	set_bit(EFI_BOOT, &efi.flags);
+
+	if (IS_ENABLED(CONFIG_64BIT))
+		set_bit(EFI_64BIT, &efi.flags);
+
+	pnv_efi_runtime_setup();
+	return 0;
+}
+machine_arch_initcall(powernv, pnv_efi_init);
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [WIP RFC PATCH 5/6] powerpc/powernv: Remove EFI support for OPAL secure variables
  2019-05-20  6:25 [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem Daniel Axtens
                   ` (3 preceding siblings ...)
  2019-05-20  6:25 ` [WIP RFC PATCH 4/6] powerpc/powernv: Add support for OPAL secure variables Daniel Axtens
@ 2019-05-20  6:25 ` Daniel Axtens
  2019-05-20  6:25 ` [WIP RFC PATCH 6/6] fwvarfs: Add opal_secvar backend Daniel Axtens
  2019-05-31  4:04 ` [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem Nayna
  6 siblings, 0 replies; 16+ messages in thread
From: Daniel Axtens @ 2019-05-20  6:25 UTC (permalink / raw)
  To: nayna, cclaudio, linux-fsdevel, greg, linuxppc-dev; +Cc: Daniel Axtens

Replace it with a generic API.

Compile tested only.

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 arch/powerpc/include/asm/opal-secvar.h       |  58 +++++++++++
 arch/powerpc/platforms/Kconfig               |   3 -
 arch/powerpc/platforms/powernv/Kconfig       |   5 +-
 arch/powerpc/platforms/powernv/opal-secvar.c | 104 ++++---------------
 4 files changed, 83 insertions(+), 87 deletions(-)
 create mode 100644 arch/powerpc/include/asm/opal-secvar.h

diff --git a/arch/powerpc/include/asm/opal-secvar.h b/arch/powerpc/include/asm/opal-secvar.h
new file mode 100644
index 000000000000..ba9bd52138d9
--- /dev/null
+++ b/arch/powerpc/include/asm/opal-secvar.h
@@ -0,0 +1,58 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * PowerNV code for secure variables
+ *
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Claudio Carvalho <cclaudio@linux.ibm.com>
+ * Author: Daniel Axtens <dja@axtens.net>
+ *
+ */
+
+#ifndef __OPAL_SECVAR_H
+#define __OPAL_SECVAR_H
+
+#include <linux/types.h>
+#include <linux/uuid.h>
+
+#ifdef CONFIG_OPAL_SECVAR
+int opal_get_secure_variable(u16 *name, guid_t *vendor, u32 *attr,
+			     unsigned long *data_size, void *data);
+
+int opal_get_next_secure_variable(unsigned long *name_size, u16 *name,
+				  guid_t *vendor);
+
+int opal_set_secure_variable(u16 *name, guid_t *vendor, u32 attr,
+			     unsigned long data_size, void *data);
+
+int opal_query_secure_variable_info(u32 attr, u64 *storage_space,
+				    u64 *remaining_space,
+				    u64 *max_variable_size);
+#else
+
+static inline int opal_get_secure_variable(u16 *name, guid_t *vendor,
+			u32 *attr, unsigned long *data_size, void *data)
+{
+	return OPAL_UNSUPPORTED;
+}
+
+static inline int opal_get_next_secure_variable(unsigned long *name_size,
+			u16 *name, guid_t *vendor)
+{
+	return OPAL_UNSUPPORTED;
+}
+
+static inline int opal_set_secure_variable(u16 *name, guid_t *vendor,
+			u32 attr, unsigned long data_size, void *data)
+{
+	return OPAL_UNSUPPORTED;
+}
+
+static inline int opal_query_secure_variable_info(u32 attr,
+			u64 *storage_space, u64 *remaining_space,
+			u64 *max_variable_size)
+{
+	return OPAL_UNSUPPORTED;
+}
+
+#endif
+#endif
diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig
index 8e30510bc0c1..f3fb79fccc72 100644
--- a/arch/powerpc/platforms/Kconfig
+++ b/arch/powerpc/platforms/Kconfig
@@ -326,7 +326,4 @@ config XILINX_PCI
 	bool "Xilinx PCI host bridge support"
 	depends on PCI && XILINX_VIRTEX
 
-config EFI
-	bool
-
 endmenu
diff --git a/arch/powerpc/platforms/powernv/Kconfig b/arch/powerpc/platforms/powernv/Kconfig
index 879f8e766098..a71fc5daa60a 100644
--- a/arch/powerpc/platforms/powernv/Kconfig
+++ b/arch/powerpc/platforms/powernv/Kconfig
@@ -52,7 +52,6 @@ config OPAL_SECVAR
 	bool "OPAL Secure Variables"
 	depends on PPC_POWERNV && !CPU_BIG_ENDIAN
 	select UCS2_STRING
-	select EFI
 	help
-	  This enables the kernel to access OPAL secure variables via EFI
-	  runtime variable services.
+	  This enables the kernel to access OPAL secure variables via
+	  an API.
diff --git a/arch/powerpc/platforms/powernv/opal-secvar.c b/arch/powerpc/platforms/powernv/opal-secvar.c
index e333828bd0bc..af753b94cceb 100644
--- a/arch/powerpc/platforms/powernv/opal-secvar.c
+++ b/arch/powerpc/platforms/powernv/opal-secvar.c
@@ -8,62 +8,20 @@
  */
 #define pr_fmt(fmt) "secvar: "fmt
 
-#include <linux/efi.h>
 #include <asm/machdep.h>
 #include <asm/opal.h>
+#include <asm/opal-secvar.h>
+#include <linux/uuid.h>
 
 static bool opal_secvar_supported;
 
-static efi_status_t opal_to_efi_status_log(int rc, const char *func_name)
-{
-	efi_status_t status;
-
-	switch (rc) {
-	case OPAL_EMPTY:
-		status = EFI_NOT_FOUND;
-		break;
-	case OPAL_HARDWARE:
-		status = EFI_DEVICE_ERROR;
-		break;
-	case OPAL_NO_MEM:
-		pr_err("%s: No space in the volatile storage\n", func_name);
-		status = EFI_OUT_OF_RESOURCES;
-		break;
-	case OPAL_PARAMETER:
-		status = EFI_INVALID_PARAMETER;
-		break;
-	case OPAL_PARTIAL:
-		status = EFI_BUFFER_TOO_SMALL;
-		break;
-	case OPAL_PERMISSION:
-		status = EFI_WRITE_PROTECTED;
-		break;
-	case OPAL_RESOURCE:
-		pr_err("%s: No space in the non-volatile storage\n", func_name);
-		status = EFI_OUT_OF_RESOURCES;
-		break;
-	case OPAL_SUCCESS:
-		status = EFI_SUCCESS;
-		break;
-	default:
-		pr_err("%s: Unknown OPAL error %d\n", func_name, rc);
-		status = EFI_DEVICE_ERROR;
-		break;
-	}
-
-	return status;
-}
-
-#define opal_to_efi_status(rc) opal_to_efi_status_log(rc, __func__)
-
-static efi_status_t
-opal_get_variable(efi_char16_t *name, efi_guid_t *vendor, u32 *attr,
-		  unsigned long *data_size, void *data)
+int opal_get_secure_variable(u16 *name, guid_t *vendor, u32 *attr,
+			     unsigned long *data_size, void *data)
 {
 	int rc;
 
 	if (!opal_secvar_supported)
-		return EFI_UNSUPPORTED;
+		return OPAL_UNSUPPORTED;
 
 	*data_size = cpu_to_be64(*data_size);
 
@@ -77,17 +35,16 @@ opal_get_variable(efi_char16_t *name, efi_guid_t *vendor, u32 *attr,
 		*attr = be32_to_cpup(attr);
 	*data_size = be64_to_cpu(*data_size);
 
-	return opal_to_efi_status(rc);
+	return rc;
 }
 
-static efi_status_t
-opal_get_next_variable(unsigned long *name_size, efi_char16_t *name,
-		       efi_guid_t *vendor)
+int opal_get_next_secure_variable(unsigned long *name_size, u16 *name,
+				  guid_t *vendor)
 {
 	int rc;
 
 	if (!opal_secvar_supported)
-		return EFI_UNSUPPORTED;
+		return OPAL_UNSUPPORTED;
 
 	*name_size = cpu_to_be64(*name_size);
 
@@ -96,17 +53,16 @@ opal_get_next_variable(unsigned long *name_size, efi_char16_t *name,
 
 	*name_size = be64_to_cpu(*name_size);
 
-	return opal_to_efi_status(rc);
+	return rc;
 }
 
-static efi_status_t
-opal_set_variable(efi_char16_t *name, efi_guid_t *vendor, u32 attr,
-		  unsigned long data_size, void *data)
+int opal_set_secure_variable(u16 *name, guid_t *vendor, u32 attr,
+			     unsigned long data_size, void *data)
 {
 	int rc;
 
 	if (!opal_secvar_supported)
-		return EFI_UNSUPPORTED;
+		return OPAL_UNSUPPORTED;
 	/*
 	 * The secure variable update must be enqueued in order to be processed
 	 * in the next boot by firmware. The secure variable storage is write
@@ -114,17 +70,17 @@ opal_set_variable(efi_char16_t *name, efi_guid_t *vendor, u32 attr,
 	 */
 	rc = opal_secvar_enqueue(__pa(name), __pa(vendor), attr,
 				 data_size, __pa(data));
-	return opal_to_efi_status(rc);
+	return rc;
 }
 
-static efi_status_t
-opal_query_variable_info(u32 attr, u64 *storage_space, u64 *remaining_space,
-			 u64 *max_variable_size)
+int opal_query_secure_variable_info(u32 attr, u64 *storage_space,
+				    u64 *remaining_space,
+				    u64 *max_variable_size)
 {
 	int rc;
 
 	if (!opal_secvar_supported)
-		return EFI_UNSUPPORTED;
+		return OPAL_UNSUPPORTED;
 
 	*storage_space = cpu_to_be64p(storage_space);
 	*remaining_space = cpu_to_be64p(remaining_space);
@@ -137,22 +93,18 @@ opal_query_variable_info(u32 attr, u64 *storage_space, u64 *remaining_space,
 	*remaining_space = be64_to_cpup(remaining_space);
 	*max_variable_size = be64_to_cpup(max_variable_size);
 
-	return opal_to_efi_status(rc);
+	return rc;
 }
 
-static void pnv_efi_runtime_setup(void)
+static int __init pnv_opal_secvar_init(void)
 {
 	/*
 	 * The opal wrappers below treat the @name, @vendor, and @data
-	 * parameters as little endian blobs.
+	 * parameters as opaque blobs.
 	 * @name is a ucs2 string
-	 * @vendor is the vendor GUID. It is converted to LE in the kernel
+	 * @vendor is the vendor GUID in EFI format
 	 * @data variable data, which layout may be different for each variable
 	 */
-	efi.get_variable = opal_get_variable;
-	efi.get_next_variable = opal_get_next_variable;
-	efi.set_variable = opal_set_variable;
-	efi.query_variable_info = opal_query_variable_info;
 
 	if (!opal_check_token(OPAL_SECVAR_GET) ||
 	    !opal_check_token(OPAL_SECVAR_GET_NEXT) ||
@@ -163,17 +115,7 @@ static void pnv_efi_runtime_setup(void)
 	} else {
 		opal_secvar_supported = true;
 	}
-}
-
-static int __init pnv_efi_init(void)
-{
-	set_bit(EFI_RUNTIME_SERVICES, &efi.flags);
-	set_bit(EFI_BOOT, &efi.flags);
-
-	if (IS_ENABLED(CONFIG_64BIT))
-		set_bit(EFI_64BIT, &efi.flags);
 
-	pnv_efi_runtime_setup();
 	return 0;
 }
-machine_arch_initcall(powernv, pnv_efi_init);
+machine_arch_initcall(powernv, pnv_opal_secvar_init);
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [WIP RFC PATCH 6/6] fwvarfs: Add opal_secvar backend
  2019-05-20  6:25 [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem Daniel Axtens
                   ` (4 preceding siblings ...)
  2019-05-20  6:25 ` [WIP RFC PATCH 5/6] powerpc/powernv: Remove EFI " Daniel Axtens
@ 2019-05-20  6:25 ` Daniel Axtens
  2019-05-31  4:04 ` [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem Nayna
  6 siblings, 0 replies; 16+ messages in thread
From: Daniel Axtens @ 2019-05-20  6:25 UTC (permalink / raw)
  To: nayna, cclaudio, linux-fsdevel, greg, linuxppc-dev; +Cc: Daniel Axtens

COMPILE TESTED ONLY.

mount -t fwvarfs opal_secvar /fw

Signed-off-by: Daniel Axtens <dja@axtens.net>
---
 Documentation/filesystems/fwvarfs.txt |   3 +
 fs/fwvarfs/Kconfig                    |  11 ++
 fs/fwvarfs/Makefile                   |   1 +
 fs/fwvarfs/fwvarfs.c                  |   3 +
 fs/fwvarfs/fwvarfs.h                  |   4 +
 fs/fwvarfs/opal_secvar.c              | 218 ++++++++++++++++++++++++++
 6 files changed, 240 insertions(+)
 create mode 100644 fs/fwvarfs/opal_secvar.c

diff --git a/Documentation/filesystems/fwvarfs.txt b/Documentation/filesystems/fwvarfs.txt
index 7c1e921e5c50..3ecc4b4428a5 100644
--- a/Documentation/filesystems/fwvarfs.txt
+++ b/Documentation/filesystems/fwvarfs.txt
@@ -36,6 +36,9 @@ Supported backends
    backend. Read-only with no creation or deletion, but sufficient that
    efivar --print --name <whatever> works the same as efivarfs.
 
+ * opal_secvar - COMPILE-TESTED ONLY implementation against PowerNV
+   OPAL Secure Variable storage.
+
 Usage
 -----
 
diff --git a/fs/fwvarfs/Kconfig b/fs/fwvarfs/Kconfig
index e4474da11dbc..cb9cbc6f8fb3 100644
--- a/fs/fwvarfs/Kconfig
+++ b/fs/fwvarfs/Kconfig
@@ -34,3 +34,14 @@ config FWVAR_FS_EFI_BACKEND
 	  in the same way the do with efivarfs.
 
 	  Say N here unless you're exploring fwvarfs.
+
+config FWVAR_FS_OPAL_SECVAR_BACKEND
+	bool "OPAL Secure Variable backend"
+	depends on FWVAR_FS
+	depends on OPAL_SECVAR
+	help
+	  Include a read-only, compile-tested-only, not up-to-date
+	  backend for OPAL Secure Variables. This is really just
+	  designed to show how the code would work, and you should
+	  only select Y here if you are developing OPAL secure
+	  variables.
diff --git a/fs/fwvarfs/Makefile b/fs/fwvarfs/Makefile
index 2ab9dfd650ca..8d258acdfef7 100644
--- a/fs/fwvarfs/Makefile
+++ b/fs/fwvarfs/Makefile
@@ -7,3 +7,4 @@ obj-$(CONFIG_FWVAR_FS)		+= fwvarfs.o
 
 obj-$(CONFIG_FWVAR_FS_MEM_BACKEND)		+= mem.o
 obj-$(CONFIG_FWVAR_FS_EFI_BACKEND)		+= efi.o
+obj-$(CONFIG_FWVAR_FS_OPAL_SECVAR_BACKEND)	+= opal_secvar.o
diff --git a/fs/fwvarfs/fwvarfs.c b/fs/fwvarfs/fwvarfs.c
index 643ec6585b4d..3c93b6442e95 100644
--- a/fs/fwvarfs/fwvarfs.c
+++ b/fs/fwvarfs/fwvarfs.c
@@ -24,6 +24,9 @@ static struct fwvarfs_backend *fwvarfs_backends[] = {
 #endif
 #ifdef CONFIG_FWVAR_FS_EFI_BACKEND
 	&fwvarfs_efi_backend,
+#endif
+#ifdef CONFIG_FWVAR_FS_OPAL_SECVAR_BACKEND
+	&fwvarfs_opal_secvar_backend,
 #endif
 	NULL,
 };
diff --git a/fs/fwvarfs/fwvarfs.h b/fs/fwvarfs/fwvarfs.h
index 49bde268401f..5780046dafae 100644
--- a/fs/fwvarfs/fwvarfs.h
+++ b/fs/fwvarfs/fwvarfs.h
@@ -117,4 +117,8 @@ extern struct fwvarfs_backend fwvarfs_mem_backend;
 extern struct fwvarfs_backend fwvarfs_efi_backend;
 #endif
 
+#if defined(CONFIG_FWVAR_FS_OPAL_SECVAR_BACKEND)
+extern struct fwvarfs_backend fwvarfs_opal_secvar_backend;
+#endif
+
 #endif /* FWVARFS_H */
diff --git a/fs/fwvarfs/opal_secvar.c b/fs/fwvarfs/opal_secvar.c
new file mode 100644
index 000000000000..4a1749317ed9
--- /dev/null
+++ b/fs/fwvarfs/opal_secvar.c
@@ -0,0 +1,218 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 IBM Corporation
+ * Author: Daniel Axtens
+ *
+ * Loosely based on efivarfs:
+ * Copyright (C) 2012 Red Hat, Inc.
+ * Copyright (C) 2012 Jeremy Kerr <jeremy.kerr@canonical.com>
+ * and drivers/firmware/efi/vars.c:
+ * Copyright (C) 2001,2003,2004 Dell <Matt_Domsch@dell.com>
+ * Copyright (C) 2004 Intel Corporation <matthew.e.tolentino@intel.com>
+ *
+ * We cheat by not allowing for case-insensitivity.
+ */
+
+#include <linux/fs.h>
+#include <linux/list.h>
+#include <linux/slab.h>
+#include "fwvarfs.h"
+
+#include <linux/uuid.h>
+#include <linux/ucs2_string.h>
+#include <asm/opal-api.h>
+#include <asm/opal-secvar.h>
+
+#define pr_fmt(fmt)	"fwvarfs: opal_secvar: " fmt
+
+static LIST_HEAD(opal_secvar_file_list);
+
+struct fwvarfs_opal_secvar_file {
+	struct list_head list;
+	u16 *name;
+	guid_t vendor;
+};
+
+// stolen from efi.h
+static inline char *
+guid_to_str(guid_t *guid, char *out)
+{
+	sprintf(out, "%pUl", guid->b);
+	return out;
+}
+
+// need a forward decl to pass down to register
+struct fwvarfs_backend fwvarfs_opal_secvar_backend;
+
+
+static ssize_t fwvarfs_opal_secvar_read(void *variable, char *buf,
+		size_t count, loff_t off)
+{
+	struct fwvarfs_opal_secvar_file *file_data = variable;
+	unsigned long datasize = 0;
+	u32 attributes;
+	void *data;
+	ssize_t size = 0;
+	loff_t ppos = off;
+	int rc;
+
+	// get size
+	rc = opal_get_secure_variable(file_data->name, &file_data->vendor,
+				      NULL, &datasize, NULL);
+	if (rc != OPAL_SUCCESS && rc != OPAL_PARTIAL)
+		return -EIO;
+
+	data = kmalloc(datasize + sizeof(attributes), GFP_KERNEL);
+
+	if (!data)
+		return -ENOMEM;
+
+	rc = opal_get_secure_variable(file_data->name, &file_data->vendor,
+		&attributes, &datasize, data + sizeof(attributes));
+	if (rc != OPAL_SUCCESS) {
+		size = -EIO;
+		goto out_free;
+	}
+
+	memcpy(data, &attributes, sizeof(attributes));
+	size = memory_read_from_buffer(buf, count, &ppos, data,
+				       datasize + sizeof(attributes));
+out_free:
+	kfree(data);
+
+	return size;
+}
+
+static int fwvarfs_opal_secvar_callback(u16 *name16, guid_t vendor,
+				unsigned long name_size)
+{
+	struct fwvarfs_opal_secvar_file *file_data;
+	char *name;
+	int len;
+	int err = -ENOMEM;
+
+	file_data = kzalloc(sizeof(*file_data), GFP_KERNEL);
+	if (!file_data)
+		return err;
+
+	file_data->name = kmemdup(name16, name_size, GFP_KERNEL);
+	if (!file_data->name)
+		goto fail;
+
+	file_data->vendor = vendor;
+
+	len = ucs2_utf8size(name16);
+
+	/* name, plus '-', plus GUID, plus NUL */
+	name = kmalloc(len + 1 + UUID_STRING_LEN + 1, GFP_KERNEL);
+	if (!name)
+		goto fail_embedded_name;
+
+	ucs2_as_utf8(name, name16, len);
+
+	name[len] = '-';
+
+	guid_to_str(&vendor, name + len + 1);
+
+	name[len + UUID_STRING_LEN + 1] = '\0';
+
+	// no convenient way to get size without reading the whole thing,
+	// present size as 0 for now.
+
+	err = fwvarfs_register_var(&fwvarfs_opal_secvar_backend, name,
+				   file_data, 0);
+	if (err)
+		goto fail_name;
+
+	INIT_LIST_HEAD(&file_data->list);
+	list_add(&opal_secvar_file_list, &file_data->list);
+
+	/* copied by the above, I think */
+	kfree(name);
+
+	return 0;
+fail_name:
+	kfree(name);
+fail_embedded_name:
+	kfree(file_data->name);
+fail:
+	kfree(file_data);
+	return err;
+}
+
+
+static void fwvarfs_opal_secvar_destroy(void *var)
+{
+	struct fwvarfs_opal_secvar_file *file_data = var;
+
+	kfree(file_data->name);
+	list_del(&file_data->list);
+	kfree(file_data);
+}
+
+
+static int fwvarfs_opal_secvar_enumerate(void)
+{
+	int err;
+	struct fwvarfs_opal_secvar_file *pos, *tmp;
+	unsigned long variable_name_size = 1024;
+	u16 *variable_name;
+	unsigned long status;
+	guid_t vendor_guid;
+
+	variable_name = kzalloc(variable_name_size, GFP_KERNEL);
+	if (!variable_name)
+		return -ENOMEM;
+
+	/*
+	 * Assume that as per EFI spec, the maximum storage allocated for both
+	 * the variable name and variable data is 1024 bytes.
+	 */
+
+	do {
+		variable_name_size = 1024;
+
+		status = opal_get_next_secure_variable(&variable_name_size,
+						variable_name,
+						&vendor_guid);
+		switch (status) {
+		case OPAL_SUCCESS:
+			err = fwvarfs_opal_secvar_callback(variable_name,
+					vendor_guid, variable_name_size);
+			if (err)
+				status = OPAL_EMPTY;
+
+			break;
+		case OPAL_EMPTY:
+			break;
+		case OPAL_UNSUPPORTED:
+			status = OPAL_EMPTY;
+			err = -ENODEV;
+		default:
+			pr_warn("opal_get_next_secure_variable: status=%lx\n",
+				status);
+			status = OPAL_EMPTY;
+			err = -EIO;
+			break;
+		}
+
+	} while (status != OPAL_EMPTY);
+
+	kfree(variable_name);
+
+	if (err) {
+		list_for_each_entry_safe(pos, tmp, &opal_secvar_file_list,
+					 list) {
+			fwvarfs_opal_secvar_destroy(pos);
+		}
+	}
+
+	return err;
+}
+
+struct fwvarfs_backend fwvarfs_opal_secvar_backend = {
+	.name = "opal_secvar",
+	.destroy = fwvarfs_opal_secvar_destroy,
+	.enumerate = fwvarfs_opal_secvar_enumerate,
+	.read = fwvarfs_opal_secvar_read,
+};
-- 
2.19.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem
  2019-05-20  6:25 [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem Daniel Axtens
                   ` (5 preceding siblings ...)
  2019-05-20  6:25 ` [WIP RFC PATCH 6/6] fwvarfs: Add opal_secvar backend Daniel Axtens
@ 2019-05-31  4:04 ` Nayna
  2019-06-03  6:04   ` Daniel Axtens
  6 siblings, 1 reply; 16+ messages in thread
From: Nayna @ 2019-05-31  4:04 UTC (permalink / raw)
  To: Daniel Axtens, nayna, cclaudio, linux-fsdevel, greg, linuxppc-dev



On 05/20/2019 02:25 AM, Daniel Axtens wrote:
> Hi all,
>
> As PowerNV moves towards secure boot, we need a place to put secure
> variables. One option that has been canvassed is to make our secure
> variables look like EFI variables. This is an early sketch of another
> approach where we create a generic firmware variable file system,
> fwvarfs, and an OPAL Secure Variable backend for it.

Is there a need of new filesystem ? I am wondering why can't these be 
exposed via sysfs / securityfs ?
Probably, something like... /sys/firmware/secureboot or 
/sys/kernel/security/secureboot/  ?

Also, it sounds like this is needed only for secure firmware variables 
and does not include
other firmware variables which are not security relevant ? Is that 
correct understanding ?

Thanks & Regards,
       - Nayna



^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem
  2019-05-31  4:04 ` [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem Nayna
@ 2019-06-03  6:04   ` Daniel Axtens
  2019-06-03  7:29     ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Daniel Axtens @ 2019-06-03  6:04 UTC (permalink / raw)
  To: Nayna, nayna, cclaudio, linux-fsdevel, greg, linuxppc-dev

Hi Nayna,

>> As PowerNV moves towards secure boot, we need a place to put secure
>> variables. One option that has been canvassed is to make our secure
>> variables look like EFI variables. This is an early sketch of another
>> approach where we create a generic firmware variable file system,
>> fwvarfs, and an OPAL Secure Variable backend for it.
>
> Is there a need of new filesystem ? I am wondering why can't these be 
> exposed via sysfs / securityfs ?
> Probably, something like... /sys/firmware/secureboot or 
> /sys/kernel/security/secureboot/  ?

I suppose we could put secure variables in sysfs, but I'm not sure
that's what sysfs was intended for. I understand sysfs as "a
filesystem-based view of kernel objects" (from
Documentation/filesystems/configfs/configfs.txt), and I don't think a
secure variable is really a kernel object in the same way most other
things in sysfs are... but I'm open to being convinced.

securityfs seems to be reserved for LSMs, I don't think we can put
things there.

My hope with fwvarfs is to provide a generic place for firmware
variables so that we don't need to expand the list of firmware-specific
filesystems beyond efivarfs. I am also aiming to make things simple to
use so that people familiar with firmware don't also have to become
familiar with filesystem code in order to expose firmware variables to
userspace.

> Also, it sounds like this is needed only for secure firmware variables 
> and does not include
> other firmware variables which are not security relevant ? Is that 
> correct understanding ?

The primary use case at the moment - OPAL secure variables - is security
focused because the current OPAL secure variable design stores and
manipulates secure variables separately from the rest of nvram. This
isn't an inherent feature of fwvarfs.

fwvarfs can also be used for variables that are not security relevant as
well. For example, with the EFI backend (patch 3), both secure and
insecure variables can be read.

Regards,
Daniel

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem
  2019-06-03  6:04   ` Daniel Axtens
@ 2019-06-03  7:29     ` Greg KH
  2019-06-03 23:56       ` Daniel Axtens
  2019-06-04 20:33       ` Nayna
  0 siblings, 2 replies; 16+ messages in thread
From: Greg KH @ 2019-06-03  7:29 UTC (permalink / raw)
  To: Daniel Axtens; +Cc: Nayna, nayna, cclaudio, linux-fsdevel, linuxppc-dev

On Mon, Jun 03, 2019 at 04:04:32PM +1000, Daniel Axtens wrote:
> Hi Nayna,
> 
> >> As PowerNV moves towards secure boot, we need a place to put secure
> >> variables. One option that has been canvassed is to make our secure
> >> variables look like EFI variables. This is an early sketch of another
> >> approach where we create a generic firmware variable file system,
> >> fwvarfs, and an OPAL Secure Variable backend for it.
> >
> > Is there a need of new filesystem ? I am wondering why can't these be 
> > exposed via sysfs / securityfs ?
> > Probably, something like... /sys/firmware/secureboot or 
> > /sys/kernel/security/secureboot/  ?
> 
> I suppose we could put secure variables in sysfs, but I'm not sure
> that's what sysfs was intended for. I understand sysfs as "a
> filesystem-based view of kernel objects" (from
> Documentation/filesystems/configfs/configfs.txt), and I don't think a
> secure variable is really a kernel object in the same way most other
> things in sysfs are... but I'm open to being convinced.

What makes them more "secure" than anything else that is in sysfs today?
I didn't see anything in this patchset that provided "additional
security", did I miss it?

> securityfs seems to be reserved for LSMs, I don't think we can put
> things there.

Yeah, I wouldn't mess with that.

I would just recommend putting this in sysfs.  Make a new subsystem
(i.e. class) and away you go.

> My hope with fwvarfs is to provide a generic place for firmware
> variables so that we don't need to expand the list of firmware-specific
> filesystems beyond efivarfs. I am also aiming to make things simple to
> use so that people familiar with firmware don't also have to become
> familiar with filesystem code in order to expose firmware variables to
> userspace.

Why would anyone need to be writing new code to firmware variables that
makes it any different from any other kernel change?

> > Also, it sounds like this is needed only for secure firmware variables 
> > and does not include
> > other firmware variables which are not security relevant ? Is that 
> > correct understanding ?
> 
> The primary use case at the moment - OPAL secure variables - is security
> focused because the current OPAL secure variable design stores and
> manipulates secure variables separately from the rest of nvram. This
> isn't an inherent feature of fwvarfs.

Again, why not just put it in sysfs please?

> fwvarfs can also be used for variables that are not security relevant as
> well. For example, with the EFI backend (patch 3), both secure and
> insecure variables can be read.

I don't remember why efi variables were not put in sysfs, I think there
was some reasoning behind it originally.  Perhaps look in the linux-efi
archives.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem
  2019-06-03  7:29     ` Greg KH
@ 2019-06-03 23:56       ` Daniel Axtens
  2019-06-04 20:01         ` Nayna
  2019-06-04 20:33       ` Nayna
  1 sibling, 1 reply; 16+ messages in thread
From: Daniel Axtens @ 2019-06-03 23:56 UTC (permalink / raw)
  To: Greg KH; +Cc: Nayna, nayna, cclaudio, linux-fsdevel, linuxppc-dev

Hi Greg,

>> >> As PowerNV moves towards secure boot, we need a place to put secure
>> >> variables. One option that has been canvassed is to make our secure
>> >> variables look like EFI variables. This is an early sketch of another
>> >> approach where we create a generic firmware variable file system,
>> >> fwvarfs, and an OPAL Secure Variable backend for it.
>> >
>> > Is there a need of new filesystem ? I am wondering why can't these be 
>> > exposed via sysfs / securityfs ?
>> > Probably, something like... /sys/firmware/secureboot or 
>> > /sys/kernel/security/secureboot/  ?
>> 
>> I suppose we could put secure variables in sysfs, but I'm not sure
>> that's what sysfs was intended for. I understand sysfs as "a
>> filesystem-based view of kernel objects" (from
>> Documentation/filesystems/configfs/configfs.txt), and I don't think a
>> secure variable is really a kernel object in the same way most other
>> things in sysfs are... but I'm open to being convinced.
>
> What makes them more "secure" than anything else that is in sysfs today?
> I didn't see anything in this patchset that provided "additional
> security", did I miss it?

You're right, there's no additional security. What I should have said
was that I didn't think that _firmware_ variables were kernel objects in
the same way that other things in sysfs are. Having read the rest of
your reply it seems I'm mistaken on this.

> I would just recommend putting this in sysfs.  Make a new subsystem
> (i.e. class) and away you go.
>
>> My hope with fwvarfs is to provide a generic place for firmware
>> variables so that we don't need to expand the list of firmware-specific
>> filesystems beyond efivarfs. I am also aiming to make things simple to
>> use so that people familiar with firmware don't also have to become
>> familiar with filesystem code in order to expose firmware variables to
>> userspace.

>> fwvarfs can also be used for variables that are not security relevant as
>> well. For example, with the EFI backend (patch 3), both secure and
>> insecure variables can be read.
>
> I don't remember why efi variables were not put in sysfs, I think there
> was some reasoning behind it originally.  Perhaps look in the linux-efi
> archives.

I'll have a look: I suspect the appeal of efivarfs is that it allows for
things like non-case-sensitive matching on the GUID part of the filename
while retaining case-sensitivity on the part of the filename
representing the variable name.

As suggested, I'll try a sysfs class. I think that will allow me to
kill off most of the abstraction layer too. Thanks for the input.

Regards,
Daniel

>
> thanks,
>
> greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem
  2019-06-03 23:56       ` Daniel Axtens
@ 2019-06-04 20:01         ` Nayna
  2019-06-04 20:05           ` Matthew Garrett
  0 siblings, 1 reply; 16+ messages in thread
From: Nayna @ 2019-06-04 20:01 UTC (permalink / raw)
  To: Daniel Axtens, Greg KH
  Cc: linux-fsdevel, nayna, linuxppc-dev, cclaudio, Matthew Garrett,
	George Wilson, Mimi Zohar, Elaine Palmer



On 06/03/2019 07:56 PM, Daniel Axtens wrote:
>
>> I would just recommend putting this in sysfs.  Make a new subsystem
>> (i.e. class) and away you go.
>>
>>> My hope with fwvarfs is to provide a generic place for firmware
>>> variables so that we don't need to expand the list of firmware-specific
>>> filesystems beyond efivarfs. I am also aiming to make things simple to
>>> use so that people familiar with firmware don't also have to become
>>> familiar with filesystem code in order to expose firmware variables to
>>> userspace.
>>> fwvarfs can also be used for variables that are not security relevant as
>>> well. For example, with the EFI backend (patch 3), both secure and
>>> insecure variables can be read.
>> I don't remember why efi variables were not put in sysfs, I think there
>> was some reasoning behind it originally.  Perhaps look in the linux-efi
>> archives.
> I'll have a look: I suspect the appeal of efivarfs is that it allows for
> things like non-case-sensitive matching on the GUID part of the filename
> while retaining case-sensitivity on the part of the filename
> representing the variable name.

It seems efivars were first implemented in sysfs and then later 
separated out as efivarfs.
Refer - Documentation/filesystems/efivarfs.txt.

So, the reason wasn't that sysfs should not be used for exposing 
firmware variables,
but for the size limitations which seems to come from UEFI Specification.

Is this limitation valid for the new requirement of secure variables ?

Copying Matthew who can give us more insights...

Thanks & Regards,
      - Nayna


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem
  2019-06-04 20:01         ` Nayna
@ 2019-06-04 20:05           ` Matthew Garrett
  2019-06-05  8:13             ` Greg KH
  0 siblings, 1 reply; 16+ messages in thread
From: Matthew Garrett @ 2019-06-04 20:05 UTC (permalink / raw)
  To: Nayna
  Cc: Daniel Axtens, Greg KH, linux-fsdevel, Nayna Jain, linuxppc-dev,
	Claudio Carvalho, George Wilson, Mimi Zohar, Elaine Palmer

On Tue, Jun 4, 2019 at 1:01 PM Nayna <nayna@linux.vnet.ibm.com> wrote:
> It seems efivars were first implemented in sysfs and then later
> separated out as efivarfs.
> Refer - Documentation/filesystems/efivarfs.txt.
>
> So, the reason wasn't that sysfs should not be used for exposing
> firmware variables,
> but for the size limitations which seems to come from UEFI Specification.
>
> Is this limitation valid for the new requirement of secure variables ?

I don't think the size restriction is an issue now, but there's a lot
of complex semantics around variable deletion and immutability that
need to be represented somehow.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem
  2019-06-03  7:29     ` Greg KH
  2019-06-03 23:56       ` Daniel Axtens
@ 2019-06-04 20:33       ` Nayna
  2019-06-05  6:14         ` Greg KH
  1 sibling, 1 reply; 16+ messages in thread
From: Nayna @ 2019-06-04 20:33 UTC (permalink / raw)
  To: Greg KH, Daniel Axtens
  Cc: nayna, cclaudio, linux-fsdevel, linuxppc-dev, George Wilson,
	Mimi Zohar, Matthew Garrett, Elaine Palmer



On 06/03/2019 03:29 AM, Greg KH wrote:
> On Mon, Jun 03, 2019 at 04:04:32PM +1000, Daniel Axtens wrote:
>> Hi Nayna,
>>
>>>> As PowerNV moves towards secure boot, we need a place to put secure
>>>> variables. One option that has been canvassed is to make our secure
>>>> variables look like EFI variables. This is an early sketch of another
>>>> approach where we create a generic firmware variable file system,
>>>> fwvarfs, and an OPAL Secure Variable backend for it.
>>> Is there a need of new filesystem ? I am wondering why can't these be
>>> exposed via sysfs / securityfs ?
>>> Probably, something like... /sys/firmware/secureboot or
>>> /sys/kernel/security/secureboot/  ?
>> I suppose we could put secure variables in sysfs, but I'm not sure
>> that's what sysfs was intended for. I understand sysfs as "a
>> filesystem-based view of kernel objects" (from
>> Documentation/filesystems/configfs/configfs.txt), and I don't think a
>> secure variable is really a kernel object in the same way most other
>> things in sysfs are... but I'm open to being convinced.
> What makes them more "secure" than anything else that is in sysfs today?
> I didn't see anything in this patchset that provided "additional
> security", did I miss it?
>
>> securityfs seems to be reserved for LSMs, I don't think we can put
>> things there.
> Yeah, I wouldn't mess with that.

Thanks Greg for clarifying!! I am curious, the TPM exposes the BIOS event log to userspace via securityfs. Is there a reason for not exposing these security variables to userspace via securityfs as well?

Thanks & Regards,
    - Nayna


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem
  2019-06-04 20:33       ` Nayna
@ 2019-06-05  6:14         ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2019-06-05  6:14 UTC (permalink / raw)
  To: Nayna
  Cc: Daniel Axtens, nayna, cclaudio, linux-fsdevel, linuxppc-dev,
	George Wilson, Mimi Zohar, Matthew Garrett, Elaine Palmer

On Tue, Jun 04, 2019 at 04:33:14PM -0400, Nayna wrote:
> 
> 
> On 06/03/2019 03:29 AM, Greg KH wrote:
> > On Mon, Jun 03, 2019 at 04:04:32PM +1000, Daniel Axtens wrote:
> > > Hi Nayna,
> > > 
> > > > > As PowerNV moves towards secure boot, we need a place to put secure
> > > > > variables. One option that has been canvassed is to make our secure
> > > > > variables look like EFI variables. This is an early sketch of another
> > > > > approach where we create a generic firmware variable file system,
> > > > > fwvarfs, and an OPAL Secure Variable backend for it.
> > > > Is there a need of new filesystem ? I am wondering why can't these be
> > > > exposed via sysfs / securityfs ?
> > > > Probably, something like... /sys/firmware/secureboot or
> > > > /sys/kernel/security/secureboot/  ?
> > > I suppose we could put secure variables in sysfs, but I'm not sure
> > > that's what sysfs was intended for. I understand sysfs as "a
> > > filesystem-based view of kernel objects" (from
> > > Documentation/filesystems/configfs/configfs.txt), and I don't think a
> > > secure variable is really a kernel object in the same way most other
> > > things in sysfs are... but I'm open to being convinced.
> > What makes them more "secure" than anything else that is in sysfs today?
> > I didn't see anything in this patchset that provided "additional
> > security", did I miss it?
> > 
> > > securityfs seems to be reserved for LSMs, I don't think we can put
> > > things there.
> > Yeah, I wouldn't mess with that.
> 
> Thanks Greg for clarifying!! I am curious, the TPM exposes the BIOS
> event log to userspace via securityfs. Is there a reason for not
> exposing these security variables to userspace via securityfs as well?

securityfs is for LSMs to use.  If the TPM drivers also use it, well,
that's between those authors and the securityfs developers.

BIOS/firmware variables are a much different thing than a TPM log.

thanks,

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem
  2019-06-04 20:05           ` Matthew Garrett
@ 2019-06-05  8:13             ` Greg KH
  0 siblings, 0 replies; 16+ messages in thread
From: Greg KH @ 2019-06-05  8:13 UTC (permalink / raw)
  To: Matthew Garrett
  Cc: Nayna, Daniel Axtens, linux-fsdevel, Nayna Jain, linuxppc-dev,
	Claudio Carvalho, George Wilson, Mimi Zohar, Elaine Palmer

On Tue, Jun 04, 2019 at 01:05:45PM -0700, Matthew Garrett wrote:
> On Tue, Jun 4, 2019 at 1:01 PM Nayna <nayna@linux.vnet.ibm.com> wrote:
> > It seems efivars were first implemented in sysfs and then later
> > separated out as efivarfs.
> > Refer - Documentation/filesystems/efivarfs.txt.
> >
> > So, the reason wasn't that sysfs should not be used for exposing
> > firmware variables,
> > but for the size limitations which seems to come from UEFI Specification.
> >
> > Is this limitation valid for the new requirement of secure variables ?
> 
> I don't think the size restriction is an issue now, but there's a lot
> of complex semantics around variable deletion and immutability that
> need to be represented somehow.

Ah, yeah, that's the reason it would not work in sysfs, forgot all about
that, thanks.

greg k-h

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-06-05  8:13 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-20  6:25 [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem Daniel Axtens
2019-05-20  6:25 ` [WIP RFC PATCH 1/6] kernfs: add create() and unlink() hooks Daniel Axtens
2019-05-20  6:25 ` [WIP RFC PATCH 2/6] fwvarfs: a generic firmware variable filesystem Daniel Axtens
2019-05-20  6:25 ` [WIP RFC PATCH 3/6] fwvarfs: efi backend Daniel Axtens
2019-05-20  6:25 ` [WIP RFC PATCH 4/6] powerpc/powernv: Add support for OPAL secure variables Daniel Axtens
2019-05-20  6:25 ` [WIP RFC PATCH 5/6] powerpc/powernv: Remove EFI " Daniel Axtens
2019-05-20  6:25 ` [WIP RFC PATCH 6/6] fwvarfs: Add opal_secvar backend Daniel Axtens
2019-05-31  4:04 ` [WIP RFC PATCH 0/6] Generic Firmware Variable Filesystem Nayna
2019-06-03  6:04   ` Daniel Axtens
2019-06-03  7:29     ` Greg KH
2019-06-03 23:56       ` Daniel Axtens
2019-06-04 20:01         ` Nayna
2019-06-04 20:05           ` Matthew Garrett
2019-06-05  8:13             ` Greg KH
2019-06-04 20:33       ` Nayna
2019-06-05  6:14         ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).