* [PATCH v2 1/4] rpmb: add Replay Protected Memory Block (RPMB) subsystem
2022-04-05 9:37 [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver Alex Bennée
@ 2022-04-05 9:37 ` Alex Bennée
2022-04-05 21:59 ` Bart Van Assche
2022-04-05 9:37 ` [PATCH v2 2/4] char: rpmb: provide a user space interface Alex Bennée
` (5 subsequent siblings)
6 siblings, 1 reply; 29+ messages in thread
From: Alex Bennée @ 2022-04-05 9:37 UTC (permalink / raw)
To: linux-kernel
Cc: maxim.uvarov, joakim.bech, ulf.hansson, ilias.apalodimas, arnd,
ruchika.gupta, tomas.winkler, yang.huang, bing.zhu, Matti.Moell,
hmo, linux-mmc, linux-scsi, Alex Bennée, Linus Walleij,
Arnd Bergmann
A number of storage technologies support a specialised hardware
partition designed to be resistant to replay attacks. The underlying
HW protocols differ but the operations are common. The RPMB partition
cannot be accessed via standard block layer, but by a set of specific
commands: WRITE, READ, GET_WRITE_COUNTER, and PROGRAM_KEY. Such a
partition provides authenticated and replay protected access, hence
suitable as a secure storage.
The RPMB layer aims to provide in-kernel API for Trusted Execution
Environment (TEE) devices that are capable to securely compute block
frame signature. In case a TEE device wishes to store a replay
protected data, requests the storage device via RPMB layer to store
the data.
A TEE device driver can claim the RPMB interface, for example, via
class_interface_register(). The RPMB layer provides a series of
operations for interacting with the device.
* program_key - a one time operation for setting up a new device
* get_capacity - introspect the device capacity
* get_write_count - check the write counter
* write_blocks - write a series of blocks to the RPMB device
* read_blocks - read a series of blocks from the RPMB device
The detailed operation of implementing the access is left to the TEE
device driver itself.
[This is based-on Thomas Winkler's proposed API from:
https://lore.kernel.org/linux-mmc/1478548394-8184-2-git-send-email-tomas.winkler@intel.com/
The principle difference is the framing details and HW specific
bits (JDEC vs NVME frames) are left to the lower level TEE driver to
worry about. The eventual userspace ioctl interface will aim to be
similarly generic. This is an RFC to follow up on:
Subject: RPMB user space ABI
Date: Thu, 11 Feb 2021 14:07:00 +0000
Message-ID: <87mtwashi4.fsf@linaro.org>]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Tomas Winkler <tomas.winkler@intel.com>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Arnd Bergmann <arnd.bergmann@linaro.org>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
---
v2
- dropped keyid stuff
- moved from char to its own subdirectory driver/rpmb
- fixed compile errors for bisectability
---
MAINTAINERS | 7 +
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/rpmb/Kconfig | 11 ++
drivers/rpmb/Makefile | 7 +
drivers/rpmb/core.c | 434 ++++++++++++++++++++++++++++++++++++++++++
include/linux/rpmb.h | 172 +++++++++++++++++
7 files changed, 634 insertions(+)
create mode 100644 drivers/rpmb/Kconfig
create mode 100644 drivers/rpmb/Makefile
create mode 100644 drivers/rpmb/core.c
create mode 100644 include/linux/rpmb.h
diff --git a/MAINTAINERS b/MAINTAINERS
index cd0f68d4a34a..9ab02b589005 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16744,6 +16744,13 @@ T: git git://linuxtv.org/media_tree.git
F: Documentation/devicetree/bindings/media/allwinner,sun8i-a83t-de2-rotate.yaml
F: drivers/media/platform/sunxi/sun8i-rotate/
+RPMB SUBSYSTEM
+M: ?
+L: linux-kernel@vger.kernel.org
+S: Supported
+F: drivers/rpmb/*
+F: include/linux/rpmb.h
+
RPMSG TTY DRIVER
M: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
L: linux-remoteproc@vger.kernel.org
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 0d399ddaa185..90d18a8a0e72 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -236,4 +236,6 @@ source "drivers/interconnect/Kconfig"
source "drivers/counter/Kconfig"
source "drivers/most/Kconfig"
+
+source "drivers/rpmb/Kconfig"
endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index a110338c860c..ade465c4589f 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -187,3 +187,4 @@ obj-$(CONFIG_GNSS) += gnss/
obj-$(CONFIG_INTERCONNECT) += interconnect/
obj-$(CONFIG_COUNTER) += counter/
obj-$(CONFIG_MOST) += most/
+obj-$(CONFIG_RPMB) += rpmb/
diff --git a/drivers/rpmb/Kconfig b/drivers/rpmb/Kconfig
new file mode 100644
index 000000000000..f2a9ebdc4435
--- /dev/null
+++ b/drivers/rpmb/Kconfig
@@ -0,0 +1,11 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2015-2019, Intel Corporation.
+
+config RPMB
+ tristate "RPMB partition interface"
+ help
+ Unified RPMB partition interface for RPMB capable devices such as
+ eMMC and UFS. Provides interface for in kernel security controllers to
+ access RPMB partition.
+
+ If unsure, select N.
diff --git a/drivers/rpmb/Makefile b/drivers/rpmb/Makefile
new file mode 100644
index 000000000000..24d4752a9a53
--- /dev/null
+++ b/drivers/rpmb/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+# Copyright (c) 2015-2019, Intel Corporation.
+
+obj-$(CONFIG_RPMB) += rpmb.o
+rpmb-objs += core.o
+
+ccflags-y += -D__CHECK_ENDIAN__
diff --git a/drivers/rpmb/core.c b/drivers/rpmb/core.c
new file mode 100644
index 000000000000..50b358a14db6
--- /dev/null
+++ b/drivers/rpmb/core.c
@@ -0,0 +1,434 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright(c) 2015 - 2019 Intel Corporation. All rights reserved.
+ * Copyright(c) 2021 - 2022 Linaro Ltd.
+ */
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/mutex.h>
+#include <linux/list.h>
+#include <linux/device.h>
+#include <linux/slab.h>
+
+#include <linux/rpmb.h>
+
+static DEFINE_IDA(rpmb_ida);
+
+/**
+ * rpmb_dev_get() - increase rpmb device ref counter
+ * @rdev: rpmb device
+ */
+struct rpmb_dev *rpmb_dev_get(struct rpmb_dev *rdev)
+{
+ return get_device(&rdev->dev) ? rdev : NULL;
+}
+EXPORT_SYMBOL_GPL(rpmb_dev_get);
+
+/**
+ * rpmb_dev_put() - decrease rpmb device ref counter
+ * @rdev: rpmb device
+ */
+void rpmb_dev_put(struct rpmb_dev *rdev)
+{
+ put_device(&rdev->dev);
+}
+EXPORT_SYMBOL_GPL(rpmb_dev_put);
+
+/**
+ * rpmb_program_key() - program the RPMB access key
+ * @rdev: rpmb device
+ * @keylen: length of key data
+ * @key: key data
+ *
+ * A successful programming of the key implies it has been set by the
+ * driver and can be used.
+ *
+ * Return:
+ * * 0 on success
+ * * -EINVAL on wrong parameters
+ * * -EPERM key already programmed
+ * * -EOPNOTSUPP if device doesn't support the requested operation
+ * * < 0 if the operation fails
+ */
+int rpmb_program_key(struct rpmb_dev *rdev, int klen, u8 *key, int rlen, u8 *resp)
+{
+ int err;
+
+ if (!rdev || !key)
+ return -EINVAL;
+
+ mutex_lock(&rdev->lock);
+ err = -EOPNOTSUPP;
+ if (rdev->ops && rdev->ops->program_key) {
+ err = rdev->ops->program_key(rdev->dev.parent, rdev->target,
+ klen, key, rlen, resp);
+ }
+ mutex_unlock(&rdev->lock);
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(rpmb_program_key);
+
+/**
+ * rpmb_get_capacity() - returns the capacity of the rpmb device
+ * @rdev: rpmb device
+ *
+ * Return:
+ * * capacity of the device in units of 128K, on success
+ * * -EINVAL on wrong parameters
+ * * -EOPNOTSUPP if device doesn't support the requested operation
+ * * < 0 if the operation fails
+ */
+int rpmb_get_capacity(struct rpmb_dev *rdev)
+{
+ int err;
+
+ if (!rdev)
+ return -EINVAL;
+
+ mutex_lock(&rdev->lock);
+ err = -EOPNOTSUPP;
+ if (rdev->ops && rdev->ops->get_capacity)
+ err = rdev->ops->get_capacity(rdev->dev.parent, rdev->target);
+ mutex_unlock(&rdev->lock);
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(rpmb_get_capacity);
+
+/**
+ * rpmb_get_write_count() - returns the write counter of the rpmb device
+ * @rdev: rpmb device
+ * @len: size of request frame
+ * @request: request frame
+ * @rlen: size of response frame
+ * @resp: response frame
+ *
+ * Return:
+ * * counter
+ * * -EINVAL on wrong parameters
+ * * -EOPNOTSUPP if device doesn't support the requested operation
+ * * < 0 if the operation fails
+ */
+int rpmb_get_write_count(struct rpmb_dev *rdev, int len, u8 *request, int rlen, u8 *resp)
+{
+ int err;
+
+ if (!rdev)
+ return -EINVAL;
+
+ mutex_lock(&rdev->lock);
+ err = -EOPNOTSUPP;
+ if (rdev->ops && rdev->ops->get_write_count)
+ err = rdev->ops->get_write_count(rdev->dev.parent, rdev->target,
+ len, request, rlen, resp);
+ mutex_unlock(&rdev->lock);
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(rpmb_get_write_count);
+
+/**
+ * rpmb_write_blocks() - write data to RPMB device
+ * @rdev: rpmb device
+ * @addr: block address (index of first block - 256B blocks)
+ * @count: number of 256B blosks
+ * @data: pointer to data to program
+ *
+ * Write a series of blocks to the RPMB device.
+ *
+ * Return:
+ * * 0 on success
+ * * -EINVAL on wrong parameters
+ * * -EACCESS no key set
+ * * -EOPNOTSUPP if device doesn't support the requested operation
+ * * < 0 if the operation fails
+ */
+int rpmb_write_blocks(struct rpmb_dev *rdev, int len, u8 *request,
+ int rlen, u8 *response)
+{
+ int err;
+
+ if (!rdev || !len || !request)
+ return -EINVAL;
+
+ mutex_lock(&rdev->lock);
+ err = -EOPNOTSUPP;
+ if (rdev->ops && rdev->ops->write_blocks) {
+ err = rdev->ops->write_blocks(rdev->dev.parent, rdev->target,
+ len, request, rlen, response);
+ }
+ mutex_unlock(&rdev->lock);
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(rpmb_write_blocks);
+
+/**
+ * rpmb_read_blocks() - read data from RPMB device
+ * @rdev: rpmb device
+ * @addr: block address (index of first block - 256B blocks)
+ * @count: number of 256B blocks
+ * @data: pointer to data to read
+ *
+ * Read a series of one or more blocks from the RPMB device.
+ *
+ * Return:
+ * * 0 on success
+ * * -EINVAL on wrong parameters
+ * * -EACCESS no key set
+ * * -EOPNOTSUPP if device doesn't support the requested operation
+ * * < 0 if the operation fails
+ */
+int rpmb_read_blocks(struct rpmb_dev *rdev, int addr, int count, int len, u8 *data)
+{
+ int err;
+
+ if (!rdev || !count || !data)
+ return -EINVAL;
+
+ mutex_lock(&rdev->lock);
+ err = -EOPNOTSUPP;
+ if (rdev->ops && rdev->ops->read_blocks) {
+ err = rdev->ops->read_blocks(rdev->dev.parent, rdev->target,
+ addr, count, len, data);
+ }
+ mutex_unlock(&rdev->lock);
+
+ return err;
+}
+EXPORT_SYMBOL_GPL(rpmb_read_blocks);
+
+
+static void rpmb_dev_release(struct device *dev)
+{
+ struct rpmb_dev *rdev = to_rpmb_dev(dev);
+
+ ida_simple_remove(&rpmb_ida, rdev->id);
+ kfree(rdev);
+}
+
+struct class rpmb_class = {
+ .name = "rpmb",
+ .owner = THIS_MODULE,
+ .dev_release = rpmb_dev_release,
+};
+EXPORT_SYMBOL(rpmb_class);
+
+/**
+ * rpmb_dev_find_device() - return first matching rpmb device
+ * @data: data for the match function
+ * @match: the matching function
+ *
+ * Return: matching rpmb device or NULL on failure
+ */
+static
+struct rpmb_dev *rpmb_dev_find_device(const void *data,
+ int (*match)(struct device *dev,
+ const void *data))
+{
+ struct device *dev;
+
+ dev = class_find_device(&rpmb_class, NULL, data, match);
+
+ return dev ? to_rpmb_dev(dev) : NULL;
+}
+
+struct device_with_target {
+ const struct device *dev;
+ u8 target;
+};
+
+static int match_by_parent(struct device *dev, const void *data)
+{
+ const struct device_with_target *d = data;
+ struct rpmb_dev *rdev = to_rpmb_dev(dev);
+
+ return (d->dev && dev->parent == d->dev && rdev->target == d->target);
+}
+
+/**
+ * rpmb_dev_find_by_device() - retrieve rpmb device from the parent device
+ * @parent: parent device of the rpmb device
+ * @target: RPMB target/region within the physical device
+ *
+ * Return: NULL if there is no rpmb device associated with the parent device
+ */
+struct rpmb_dev *rpmb_dev_find_by_device(struct device *parent, u8 target)
+{
+ struct device_with_target t;
+
+ if (!parent)
+ return NULL;
+
+ t.dev = parent;
+ t.target = target;
+
+ return rpmb_dev_find_device(&t, match_by_parent);
+}
+EXPORT_SYMBOL_GPL(rpmb_dev_find_by_device);
+
+/**
+ * rpmb_dev_unregister() - unregister RPMB partition from the RPMB subsystem
+ * @rdev: the rpmb device to unregister
+ * Return:
+ * * 0 on success
+ * * -EINVAL on wrong parameters
+ */
+int rpmb_dev_unregister(struct rpmb_dev *rdev)
+{
+ if (!rdev)
+ return -EINVAL;
+
+ mutex_lock(&rdev->lock);
+ device_del(&rdev->dev);
+ mutex_unlock(&rdev->lock);
+
+ rpmb_dev_put(rdev);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(rpmb_dev_unregister);
+
+/**
+ * rpmb_dev_unregister_by_device() - unregister RPMB partition
+ * from the RPMB subsystem
+ * @dev: the parent device of the rpmb device
+ * @target: RPMB target/region within the physical device
+ * Return:
+ * * 0 on success
+ * * -EINVAL on wrong parameters
+ * * -ENODEV if a device cannot be find.
+ */
+int rpmb_dev_unregister_by_device(struct device *dev, u8 target)
+{
+ struct rpmb_dev *rdev;
+
+ if (!dev)
+ return -EINVAL;
+
+ rdev = rpmb_dev_find_by_device(dev, target);
+ if (!rdev) {
+ dev_warn(dev, "no disk found %s\n", dev_name(dev->parent));
+ return -ENODEV;
+ }
+
+ rpmb_dev_put(rdev);
+
+ return rpmb_dev_unregister(rdev);
+}
+EXPORT_SYMBOL_GPL(rpmb_dev_unregister_by_device);
+
+/**
+ * rpmb_dev_get_drvdata() - driver data getter
+ * @rdev: rpmb device
+ *
+ * Return: driver private data
+ */
+void *rpmb_dev_get_drvdata(const struct rpmb_dev *rdev)
+{
+ return dev_get_drvdata(&rdev->dev);
+}
+EXPORT_SYMBOL_GPL(rpmb_dev_get_drvdata);
+
+/**
+ * rpmb_dev_set_drvdata() - driver data setter
+ * @rdev: rpmb device
+ * @data: data to store
+ */
+void rpmb_dev_set_drvdata(struct rpmb_dev *rdev, void *data)
+{
+ dev_set_drvdata(&rdev->dev, data);
+}
+EXPORT_SYMBOL_GPL(rpmb_dev_set_drvdata);
+
+/**
+ * rpmb_dev_register - register RPMB partition with the RPMB subsystem
+ * @dev: storage device of the rpmb device
+ * @target: RPMB target/region within the physical device
+ * @ops: device specific operations
+ *
+ * Return: a pointer to rpmb device
+ */
+struct rpmb_dev *rpmb_dev_register(struct device *dev, u8 target,
+ const struct rpmb_ops *ops)
+{
+ struct rpmb_dev *rdev;
+ int id;
+ int ret;
+
+ if (!dev || !ops)
+ return ERR_PTR(-EINVAL);
+
+ if (!ops->program_key)
+ return ERR_PTR(-EINVAL);
+
+ if (!ops->get_capacity)
+ return ERR_PTR(-EINVAL);
+
+ if (!ops->get_write_count)
+ return ERR_PTR(-EINVAL);
+
+ if (!ops->write_blocks)
+ return ERR_PTR(-EINVAL);
+
+ if (!ops->read_blocks)
+ return ERR_PTR(-EINVAL);
+
+ rdev = kzalloc(sizeof(*rdev), GFP_KERNEL);
+ if (!rdev)
+ return ERR_PTR(-ENOMEM);
+
+ id = ida_simple_get(&rpmb_ida, 0, 0, GFP_KERNEL);
+ if (id < 0) {
+ ret = id;
+ goto exit;
+ }
+
+ mutex_init(&rdev->lock);
+ rdev->ops = ops;
+ rdev->id = id;
+ rdev->target = target;
+
+ dev_set_name(&rdev->dev, "rpmb%d", id);
+ rdev->dev.class = &rpmb_class;
+ rdev->dev.parent = dev;
+
+ rpmb_cdev_prepare(rdev);
+
+ ret = device_register(&rdev->dev);
+ if (ret)
+ goto exit;
+
+ dev_dbg(&rdev->dev, "registered device\n");
+
+ return rdev;
+
+exit:
+ if (id >= 0)
+ ida_simple_remove(&rpmb_ida, id);
+ kfree(rdev);
+ return ERR_PTR(ret);
+}
+EXPORT_SYMBOL_GPL(rpmb_dev_register);
+
+static int __init rpmb_init(void)
+{
+ ida_init(&rpmb_ida);
+ class_register(&rpmb_class);
+ return 0;
+}
+
+static void __exit rpmb_exit(void)
+{
+ class_unregister(&rpmb_class);
+ ida_destroy(&rpmb_ida);
+}
+
+subsys_initcall(rpmb_init);
+module_exit(rpmb_exit);
+
+MODULE_AUTHOR("Intel Corporation");
+MODULE_DESCRIPTION("RPMB class");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/rpmb.h b/include/linux/rpmb.h
new file mode 100644
index 000000000000..4ed5e299623e
--- /dev/null
+++ b/include/linux/rpmb.h
@@ -0,0 +1,172 @@
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/*
+ * Copyright (C) 2015-2019 Intel Corp. All rights reserved
+ * Copyright (C) 2021-2022 Linaro Ltd
+ */
+#ifndef __RPMB_H__
+#define __RPMB_H__
+
+#include <linux/types.h>
+#include <linux/device.h>
+#include <linux/kref.h>
+
+/**
+ * struct rpmb_ops - RPMB ops to be implemented by underlying block device
+ *
+ * @program_key : program device key (once only op).
+ * @get_capacity : rpmb size in 128K units in for region/target.
+ * @get_write_count: return the device write counter
+ * @write_blocks : write blocks to RPMB device
+ * @read_blocks : read blocks from RPMB device
+ * @block_size : block size in half sectors (1 == 256B)
+ * @wr_cnt_max : maximal number of blocks that can be
+ * written in one access.
+ * @rd_cnt_max : maximal number of blocks that can be
+ * read in one access.
+ * @dev_id : unique device identifier
+ * @dev_id_len : unique device identifier length
+ */
+struct rpmb_ops {
+ int (*program_key)(struct device *dev, u8 target,
+ int keylen, u8 *key_frame,
+ int rlen, u8 *resp);
+ int (*get_capacity)(struct device *dev, u8 target);
+ int (*get_write_count)(struct device *dev, u8 target,
+ int len, u8 *requests,
+ int rlen, u8 *resp);
+ int (*write_blocks)(struct device *dev, u8 target,
+ int len, u8 *requests,
+ int rlen, u8 *resp);
+ int (*read_blocks)(struct device *dev, u8 target,
+ int addr, int count,
+ int len, u8 *data);
+ u16 block_size;
+ u16 wr_cnt_max;
+ u16 rd_cnt_max;
+ const u8 *dev_id;
+ size_t dev_id_len;
+};
+
+/**
+ * struct rpmb_dev - device which can support RPMB partition
+ *
+ * @lock : the device lock
+ * @dev : device
+ * @id : device id
+ * @target : RPMB target/region within the physical device
+ * @ops : operation exported by rpmb
+ */
+struct rpmb_dev {
+ struct mutex lock; /* device serialization lock */
+ struct device dev;
+ int id;
+ u8 target;
+ const struct rpmb_ops *ops;
+};
+
+#define to_rpmb_dev(x) container_of((x), struct rpmb_dev, dev)
+
+#if IS_ENABLED(CONFIG_RPMB)
+struct rpmb_dev *rpmb_dev_get(struct rpmb_dev *rdev);
+void rpmb_dev_put(struct rpmb_dev *rdev);
+struct rpmb_dev *rpmb_dev_find_by_device(struct device *parent, u8 target);
+struct rpmb_dev *rpmb_dev_get_by_type(u32 type);
+struct rpmb_dev *rpmb_dev_register(struct device *dev, u8 target,
+ const struct rpmb_ops *ops);
+void *rpmb_dev_get_drvdata(const struct rpmb_dev *rdev);
+void rpmb_dev_set_drvdata(struct rpmb_dev *rdev, void *data);
+int rpmb_dev_unregister(struct rpmb_dev *rdev);
+int rpmb_dev_unregister_by_device(struct device *dev, u8 target);
+
+int rpmb_program_key(struct rpmb_dev *rdev,
+ int klen, u8 *key, int rlen, u8 *resp);
+int rpmb_get_capacity(struct rpmb_dev *rdev);
+int rpmb_get_write_count(struct rpmb_dev *rdev,
+ int len, u8 *request, int rlen, u8 *resp);
+int rpmb_write_blocks(struct rpmb_dev *rdev,
+ int len, u8 *request, int rlen, u8 *resp);
+int rpmb_read_blocks(struct rpmb_dev *rdev, int addr, int count, int len, u8 *data);
+
+#else
+static inline struct rpmb_dev *rpmb_dev_get(struct rpmb_dev *rdev)
+{
+ return NULL;
+}
+
+static inline void rpmb_dev_put(struct rpmb_dev *rdev) { }
+
+static inline struct rpmb_dev *rpmb_dev_find_by_device(struct device *parent,
+ u8 target)
+{
+ return NULL;
+}
+
+static inline
+struct rpmb_dev *rpmb_dev_get_by_type(enum rpmb_type type)
+{
+ return NULL;
+}
+
+static inline void *rpmb_dev_get_drvdata(const struct rpmb_dev *rdev)
+{
+ return NULL;
+}
+
+static inline void rpmb_dev_set_drvdata(struct rpmb_dev *rdev, void *data)
+{
+}
+
+static inline struct rpmb_dev *
+rpmb_dev_register(struct device *dev, u8 target, const struct rpmb_ops *ops)
+{
+ return NULL;
+}
+
+static inline int rpmb_dev_unregister(struct rpmb_dev *dev)
+{
+ return 0;
+}
+
+static inline int rpmb_dev_unregister_by_device(struct device *dev, u8 target)
+{
+ return 0;
+}
+
+static inline int rpmb_program_key(struct rpmb_dev *rdev,
+ int klen, u8 *key,
+ int rlen, u8 *resp)
+{
+ return 0;
+}
+
+static inline rpmb_set_key(struct rpmb_dev *rdev, u8 *key, int keylen);
+{
+ return 0;
+}
+
+static inline int rpmb_get_capacity(struct rpmb_dev *rdev)
+{
+ return 0;
+}
+
+static inline int rpmb_get_write_count(struct rpmb_dev *rdev,
+ int len, u8 *request, int rlen, u8 *resp)
+{
+ return 0;
+}
+
+static inline int rpmb_write_blocks(struct rpmb_dev *rdev,
+ int len, u8 *request, int rlen, u8 *resp);
+{
+ return 0;
+}
+
+static inline int rpmb_read_blocks(struct rpmb_dev *rdev, int addr, int count,
+ int len, u8 *data)
+{
+ return 0;
+}
+
+#endif /* CONFIG_RPMB */
+
+#endif /* __RPMB_H__ */
--
2.30.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 1/4] rpmb: add Replay Protected Memory Block (RPMB) subsystem
2022-04-05 9:37 ` [PATCH v2 1/4] rpmb: add Replay Protected Memory Block (RPMB) subsystem Alex Bennée
@ 2022-04-05 21:59 ` Bart Van Assche
2022-04-06 11:21 ` Alex Bennée
0 siblings, 1 reply; 29+ messages in thread
From: Bart Van Assche @ 2022-04-05 21:59 UTC (permalink / raw)
To: Alex Bennée, linux-kernel
Cc: maxim.uvarov, joakim.bech, ulf.hansson, ilias.apalodimas, arnd,
ruchika.gupta, tomas.winkler, yang.huang, bing.zhu, Matti.Moell,
hmo, linux-mmc, linux-scsi, Linus Walleij, Arnd Bergmann,
Eric Biggers
On 4/5/22 02:37, Alex Bennée wrote:
> +int rpmb_get_write_count(struct rpmb_dev *rdev, int len, u8 *request, int rlen, u8 *resp)
> +{
> + int err;
> +
> + if (!rdev)
> + return -EINVAL;
> +
> + mutex_lock(&rdev->lock);
> + err = -EOPNOTSUPP;
> + if (rdev->ops && rdev->ops->get_write_count)
> + err = rdev->ops->get_write_count(rdev->dev.parent, rdev->target,
> + len, request, rlen, resp);
> + mutex_unlock(&rdev->lock);
> +
> + return err;
> +}
The names rpmb_get_write_count() and get_write_count() look confusing to
me since these functions query the write counter. How about adding "er"
at the end of both function names?
Are there any plans to add an implementation of struct rpmb_ops for UFS
devices?
Thanks,
Bart.
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v2 1/4] rpmb: add Replay Protected Memory Block (RPMB) subsystem
2022-04-05 21:59 ` Bart Van Assche
@ 2022-04-06 11:21 ` Alex Bennée
0 siblings, 0 replies; 29+ messages in thread
From: Alex Bennée @ 2022-04-06 11:21 UTC (permalink / raw)
To: Bart Van Assche
Cc: linux-kernel, maxim.uvarov, joakim.bech, ulf.hansson,
ilias.apalodimas, arnd, ruchika.gupta, tomas.winkler, yang.huang,
bing.zhu, Matti.Moell, hmo, linux-mmc, linux-scsi, Linus Walleij,
Arnd Bergmann, Eric Biggers
Bart Van Assche <bvanassche@acm.org> writes:
> On 4/5/22 02:37, Alex Bennée wrote:
>> +int rpmb_get_write_count(struct rpmb_dev *rdev, int len, u8 *request, int rlen, u8 *resp)
>> +{
>> + int err;
>> +
>> + if (!rdev)
>> + return -EINVAL;
>> +
>> + mutex_lock(&rdev->lock);
>> + err = -EOPNOTSUPP;
>> + if (rdev->ops && rdev->ops->get_write_count)
>> + err = rdev->ops->get_write_count(rdev->dev.parent, rdev->target,
>> + len, request, rlen, resp);
>> + mutex_unlock(&rdev->lock);
>> +
>> + return err;
>> +}
>
> The names rpmb_get_write_count() and get_write_count() look confusing
> to me since these functions query the write counter. How about adding
> "er" at the end of both function names?
>
> Are there any plans to add an implementation of struct rpmb_ops for
> UFS devices?
Not by me but I agree it would be a useful exercise to see if a unified
API makes sense.
--
Alex Bennée
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 2/4] char: rpmb: provide a user space interface
2022-04-05 9:37 [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver Alex Bennée
2022-04-05 9:37 ` [PATCH v2 1/4] rpmb: add Replay Protected Memory Block (RPMB) subsystem Alex Bennée
@ 2022-04-05 9:37 ` Alex Bennée
2022-06-16 15:09 ` Harald Mommer
2022-06-16 19:21 ` Arnd Bergmann
2022-04-05 9:37 ` [PATCH v2 3/4] rpmb: create virtio rpmb frontend driver Alex Bennée
` (4 subsequent siblings)
6 siblings, 2 replies; 29+ messages in thread
From: Alex Bennée @ 2022-04-05 9:37 UTC (permalink / raw)
To: linux-kernel
Cc: maxim.uvarov, joakim.bech, ulf.hansson, ilias.apalodimas, arnd,
ruchika.gupta, tomas.winkler, yang.huang, bing.zhu, Matti.Moell,
hmo, linux-mmc, linux-scsi, Alex Bennée, Linus Walleij,
Arnd Bergmann, Alexander Usyskin, Avri Altman
The user space API is achieved via a number of synchronous IOCTLs.
* RPMB_IOC_VER_CMD - simple versioning API
* RPMB_IOC_CAP_CMD - query of underlying capabilities
* RPMB_IOC_PKEY_CMD - one time programming of access key
* RPMB_IOC_COUNTER_CMD - query the write counter
* RPMB_IOC_WBLOCKS_CMD - write blocks to device
* RPMB_IOC_RBLOCKS_CMD - read blocks from device
The operations which require authenticated frames or will respond with
MAC hashes of nonce filled frames that userspace will need to verify
share a common command frame format. The other operations can be
considered generic and allow for common handling.
[AJB: here the are key difference is the avoiding a single ioctl where
all the frame data is put together by user space. User space is still
the only place where certain operations can be verified due to the
need of a secret key]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Ulf Hansson <ulf.hansson@linaro.org>
Cc: Linus Walleij <linus.walleij@linaro.org>
Cc: Arnd Bergmann <arnd.bergmann@linaro.org>
Cc: Ilias Apalodimas <ilias.apalodimas@linaro.org>
Cc: Tomas Winkler <tomas.winkler@intel.com>
Cc: Alexander Usyskin <alexander.usyskin@intel.com>
Cc: Avri Altman <avri.altman@sandisk.com>
---
v2
- drop the key api stuff
---
.../userspace-api/ioctl/ioctl-number.rst | 1 +
MAINTAINERS | 1 +
drivers/rpmb/Kconfig | 7 +
drivers/rpmb/Makefile | 1 +
drivers/rpmb/cdev.c | 309 ++++++++++++++++++
drivers/rpmb/core.c | 7 +-
drivers/rpmb/rpmb-cdev.h | 17 +
include/linux/rpmb.h | 10 +
include/uapi/linux/rpmb.h | 99 ++++++
9 files changed, 451 insertions(+), 1 deletion(-)
create mode 100644 drivers/rpmb/cdev.c
create mode 100644 drivers/rpmb/rpmb-cdev.h
create mode 100644 include/uapi/linux/rpmb.h
diff --git a/Documentation/userspace-api/ioctl/ioctl-number.rst b/Documentation/userspace-api/ioctl/ioctl-number.rst
index e6fce2cbd99e..874d01f11caf 100644
--- a/Documentation/userspace-api/ioctl/ioctl-number.rst
+++ b/Documentation/userspace-api/ioctl/ioctl-number.rst
@@ -355,6 +355,7 @@ Code Seq# Include File Comments
0xB6 all linux/fpga-dfl.h
0xB7 all uapi/linux/remoteproc_cdev.h <mailto:linux-remoteproc@vger.kernel.org>
0xB7 all uapi/linux/nsfs.h <mailto:Andrei Vagin <avagin@openvz.org>>
+0xB8 80-8F uapi/linux/rpmb.h <mailto:linux-mmc@vger.kernel.org>
0xC0 00-0F linux/usb/iowarrior.h
0xCA 00-0F uapi/misc/cxl.h
0xCA 10-2F uapi/misc/ocxl.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 9ab02b589005..0a744da21817 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16749,6 +16749,7 @@ M: ?
L: linux-kernel@vger.kernel.org
S: Supported
F: drivers/rpmb/*
+F: include/uapi/linux/rpmb.h
F: include/linux/rpmb.h
RPMSG TTY DRIVER
diff --git a/drivers/rpmb/Kconfig b/drivers/rpmb/Kconfig
index f2a9ebdc4435..126f336fe9ea 100644
--- a/drivers/rpmb/Kconfig
+++ b/drivers/rpmb/Kconfig
@@ -9,3 +9,10 @@ config RPMB
access RPMB partition.
If unsure, select N.
+
+config RPMB_INTF_DEV
+ bool "RPMB character device interface /dev/rpmbN"
+ depends on RPMB
+ help
+ Say yes here if you want to access RPMB from user space
+ via character device interface /dev/rpmb%d
diff --git a/drivers/rpmb/Makefile b/drivers/rpmb/Makefile
index 24d4752a9a53..f54b3f30514b 100644
--- a/drivers/rpmb/Makefile
+++ b/drivers/rpmb/Makefile
@@ -3,5 +3,6 @@
obj-$(CONFIG_RPMB) += rpmb.o
rpmb-objs += core.o
+rpmb-$(CONFIG_RPMB_INTF_DEV) += cdev.o
ccflags-y += -D__CHECK_ENDIAN__
diff --git a/drivers/rpmb/cdev.c b/drivers/rpmb/cdev.c
new file mode 100644
index 000000000000..d9beeba53432
--- /dev/null
+++ b/drivers/rpmb/cdev.c
@@ -0,0 +1,309 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright(c) 2015 - 2019 Intel Corporation.
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include <linux/fs.h>
+#include <linux/uaccess.h>
+#include <linux/compat.h>
+#include <linux/slab.h>
+#include <linux/capability.h>
+
+#include <linux/rpmb.h>
+
+#include "rpmb-cdev.h"
+
+static dev_t rpmb_devt;
+#define RPMB_MAX_DEVS MINORMASK
+
+#define RPMB_DEV_OPEN 0 /** single open bit (position) */
+
+/**
+ * rpmb_open - the open function
+ *
+ * @inode: pointer to inode structure
+ * @fp: pointer to file structure
+ *
+ * Return: 0 on success, <0 on error
+ */
+static int rpmb_open(struct inode *inode, struct file *fp)
+{
+ struct rpmb_dev *rdev;
+
+ rdev = container_of(inode->i_cdev, struct rpmb_dev, cdev);
+ if (!rdev)
+ return -ENODEV;
+
+ /* the rpmb is single open! */
+ if (test_and_set_bit(RPMB_DEV_OPEN, &rdev->status))
+ return -EBUSY;
+
+ mutex_lock(&rdev->lock);
+
+ fp->private_data = rdev;
+
+ mutex_unlock(&rdev->lock);
+
+ return nonseekable_open(inode, fp);
+}
+
+/**
+ * rpmb_release - the cdev release function
+ *
+ * @inode: pointer to inode structure
+ * @fp: pointer to file structure
+ *
+ * Return: 0 always.
+ */
+static int rpmb_release(struct inode *inode, struct file *fp)
+{
+ struct rpmb_dev *rdev = fp->private_data;
+
+ clear_bit(RPMB_DEV_OPEN, &rdev->status);
+
+ return 0;
+}
+
+static long rpmb_ioctl_ver_cmd(struct rpmb_dev *rdev,
+ struct rpmb_ioc_ver_cmd __user *ptr)
+{
+ struct rpmb_ioc_ver_cmd ver = {
+ .api_version = RPMB_API_VERSION,
+ };
+
+ return copy_to_user(ptr, &ver, sizeof(ver)) ? -EFAULT : 0;
+}
+
+static long rpmb_ioctl_cap_cmd(struct rpmb_dev *rdev,
+ struct rpmb_ioc_cap_cmd __user *ptr)
+{
+ struct rpmb_ioc_cap_cmd cap;
+
+ cap.target = rdev->target;
+ cap.block_size = rdev->ops->block_size;
+ cap.wr_cnt_max = rdev->ops->wr_cnt_max;
+ cap.rd_cnt_max = rdev->ops->rd_cnt_max;
+ cap.capacity = rpmb_get_capacity(rdev);
+ cap.reserved = 0;
+
+ return copy_to_user(ptr, &cap, sizeof(cap)) ? -EFAULT : 0;
+}
+
+static long rpmb_ioctl_pkey_cmd(struct rpmb_dev *rdev, struct rpmb_ioc_reqresp_cmd __user *ptr)
+{
+ struct rpmb_ioc_reqresp_cmd cmd;
+ u8 *request, *resp = NULL;
+ long ret;
+
+ if (copy_from_user(&cmd, ptr, sizeof(struct rpmb_ioc_reqresp_cmd)))
+ return -EFAULT;
+
+ request = kmalloc(cmd.len, GFP_KERNEL);
+
+ if (!request)
+ return -ENOMEM;
+
+ if (cmd.rlen && cmd.response) {
+ resp = kmalloc(cmd.rlen, GFP_KERNEL);
+ if (!resp) {
+ kfree(request);
+ return -ENOMEM;
+ }
+ }
+
+ if (copy_from_user(request, cmd.request, cmd.len))
+ ret = -EFAULT;
+ else
+ ret = rpmb_program_key(rdev, cmd.len, request, cmd.rlen, resp);
+
+ if (!ret)
+ if (copy_to_user(cmd.response, resp, cmd.rlen))
+ ret = -EFAULT;
+
+ kfree(request);
+ kfree(resp);
+
+ return ret;
+}
+
+static long rpmb_ioctl_counter_cmd(struct rpmb_dev *rdev, struct rpmb_ioc_reqresp_cmd __user *ptr)
+{
+ struct rpmb_ioc_reqresp_cmd cmd;
+ u8 *request, *resp = NULL;
+ long count;
+
+ if (copy_from_user(&cmd, ptr, sizeof(struct rpmb_ioc_reqresp_cmd)))
+ return -EFAULT;
+
+ request = kmalloc(cmd.len, GFP_KERNEL);
+
+ if (!request)
+ return -ENOMEM;
+
+ if (cmd.rlen && cmd.response) {
+ resp = kmalloc(cmd.rlen, GFP_KERNEL);
+ if (!resp) {
+ kfree(request);
+ return -ENOMEM;
+ }
+ }
+
+ if (copy_from_user(request, cmd.request, cmd.len)) {
+ count = -EFAULT;
+ } else {
+ count = rpmb_get_write_count(rdev, cmd.len, request, cmd.rlen, resp);
+ if (resp)
+ if (copy_to_user(cmd.response, resp, cmd.rlen))
+ count = -EFAULT;
+ }
+
+ kfree(request);
+ kfree(resp);
+
+ return count;
+}
+
+static long rpmb_ioctl_wblocks_cmd(struct rpmb_dev *rdev,
+ struct rpmb_ioc_reqresp_cmd __user *ptr)
+{
+ struct rpmb_ioc_reqresp_cmd cmd;
+ u8 *data, *resp = NULL;
+
+ long ret;
+
+ if (copy_from_user(&cmd, ptr, sizeof(struct rpmb_ioc_reqresp_cmd)))
+ return -EFAULT;
+
+ data = kmalloc(cmd.len, GFP_KERNEL);
+
+ if (!data)
+ return -ENOMEM;
+
+ if (cmd.rlen && cmd.response) {
+ resp = kmalloc(cmd.rlen, GFP_KERNEL);
+ if (!resp) {
+ kfree(data);
+ return -ENOMEM;
+ }
+ }
+
+ if (copy_from_user(data, cmd.request, cmd.len))
+ ret = -EFAULT;
+ else
+ ret = rpmb_write_blocks(rdev, cmd.len, data, cmd.rlen, resp);
+
+ if (resp)
+ if (copy_to_user(cmd.response, resp, cmd.rlen))
+ ret = -EFAULT;
+
+ kfree(data);
+ kfree(resp);
+
+ return ret;
+}
+
+static long rpmb_ioctl_rblocks_cmd(struct rpmb_dev *rdev,
+ struct rpmb_ioc_rblocks_cmd __user *ptr)
+{
+ struct rpmb_ioc_rblocks_cmd rblocks;
+ long ret;
+ u8 *data;
+
+ if (copy_from_user(&rblocks, ptr, sizeof(struct rpmb_ioc_rblocks_cmd)))
+ return -EFAULT;
+
+ if (rblocks.count > rdev->ops->rd_cnt_max)
+ return -EINVAL;
+
+ if (!rblocks.len || !rblocks.data)
+ return -EINVAL;
+
+ data = kmalloc(rblocks.len, GFP_KERNEL);
+
+ if (!data)
+ return -ENOMEM;
+
+ ret = rpmb_read_blocks(rdev, rblocks.addr, rblocks.count, rblocks.len, data);
+
+ if (ret == 0)
+ ret = copy_to_user(rblocks.data, data, rblocks.len);
+
+ kfree(data);
+ return ret;
+}
+
+/**
+ * rpmb_ioctl - rpmb ioctl dispatcher
+ *
+ * @fp: a file pointer
+ * @cmd: ioctl command RPMB_IOC_SEQ_CMD RPMB_IOC_VER_CMD RPMB_IOC_CAP_CMD
+ * @arg: ioctl data: rpmb_ioc_ver_cmd rpmb_ioc_cap_cmd pmb_ioc_seq_cmd
+ *
+ * Return: 0 on success; < 0 on error
+ */
+static long rpmb_ioctl(struct file *fp, unsigned int cmd, unsigned long arg)
+{
+ struct rpmb_dev *rdev = fp->private_data;
+ void __user *ptr = (void __user *)arg;
+
+ switch (cmd) {
+ case RPMB_IOC_VER_CMD:
+ return rpmb_ioctl_ver_cmd(rdev, ptr);
+ case RPMB_IOC_CAP_CMD:
+ return rpmb_ioctl_cap_cmd(rdev, ptr);
+ case RPMB_IOC_PKEY_CMD:
+ return rpmb_ioctl_pkey_cmd(rdev, ptr);
+ case RPMB_IOC_COUNTER_CMD:
+ return rpmb_ioctl_counter_cmd(rdev, ptr);
+ case RPMB_IOC_WBLOCKS_CMD:
+ return rpmb_ioctl_wblocks_cmd(rdev, ptr);
+ case RPMB_IOC_RBLOCKS_CMD:
+ return rpmb_ioctl_rblocks_cmd(rdev, ptr);
+ default:
+ dev_err(&rdev->dev, "unsupported ioctl 0x%x.\n", cmd);
+ return -ENOIOCTLCMD;
+ }
+}
+
+static const struct file_operations rpmb_fops = {
+ .open = rpmb_open,
+ .release = rpmb_release,
+ .unlocked_ioctl = rpmb_ioctl,
+ .owner = THIS_MODULE,
+ .llseek = noop_llseek,
+};
+
+void rpmb_cdev_prepare(struct rpmb_dev *rdev)
+{
+ rdev->dev.devt = MKDEV(MAJOR(rpmb_devt), rdev->id);
+ rdev->cdev.owner = THIS_MODULE;
+ cdev_init(&rdev->cdev, &rpmb_fops);
+}
+
+void rpmb_cdev_add(struct rpmb_dev *rdev)
+{
+ cdev_add(&rdev->cdev, rdev->dev.devt, 1);
+}
+
+void rpmb_cdev_del(struct rpmb_dev *rdev)
+{
+ if (rdev->dev.devt)
+ cdev_del(&rdev->cdev);
+}
+
+int __init rpmb_cdev_init(void)
+{
+ int ret;
+
+ ret = alloc_chrdev_region(&rpmb_devt, 0, RPMB_MAX_DEVS, "rpmb");
+ if (ret < 0)
+ pr_err("unable to allocate char dev region\n");
+
+ return ret;
+}
+
+void __exit rpmb_cdev_exit(void)
+{
+ unregister_chrdev_region(rpmb_devt, RPMB_MAX_DEVS);
+}
diff --git a/drivers/rpmb/core.c b/drivers/rpmb/core.c
index 50b358a14db6..969536ba53cc 100644
--- a/drivers/rpmb/core.c
+++ b/drivers/rpmb/core.c
@@ -12,6 +12,7 @@
#include <linux/slab.h>
#include <linux/rpmb.h>
+#include "rpmb-cdev.h"
static DEFINE_IDA(rpmb_ida);
@@ -282,6 +283,7 @@ int rpmb_dev_unregister(struct rpmb_dev *rdev)
return -EINVAL;
mutex_lock(&rdev->lock);
+ rpmb_cdev_del(rdev);
device_del(&rdev->dev);
mutex_unlock(&rdev->lock);
@@ -401,6 +403,8 @@ struct rpmb_dev *rpmb_dev_register(struct device *dev, u8 target,
if (ret)
goto exit;
+ rpmb_cdev_add(rdev);
+
dev_dbg(&rdev->dev, "registered device\n");
return rdev;
@@ -417,11 +421,12 @@ static int __init rpmb_init(void)
{
ida_init(&rpmb_ida);
class_register(&rpmb_class);
- return 0;
+ return rpmb_cdev_init();
}
static void __exit rpmb_exit(void)
{
+ rpmb_cdev_exit();
class_unregister(&rpmb_class);
ida_destroy(&rpmb_ida);
}
diff --git a/drivers/rpmb/rpmb-cdev.h b/drivers/rpmb/rpmb-cdev.h
new file mode 100644
index 000000000000..e59ff0c05e9d
--- /dev/null
+++ b/drivers/rpmb/rpmb-cdev.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
+/*
+ * Copyright (C) 2015-2018 Intel Corp. All rights reserved
+ */
+#ifdef CONFIG_RPMB_INTF_DEV
+int __init rpmb_cdev_init(void);
+void __exit rpmb_cdev_exit(void);
+void rpmb_cdev_prepare(struct rpmb_dev *rdev);
+void rpmb_cdev_add(struct rpmb_dev *rdev);
+void rpmb_cdev_del(struct rpmb_dev *rdev);
+#else
+static inline int __init rpmb_cdev_init(void) { return 0; }
+static inline void __exit rpmb_cdev_exit(void) {}
+static inline void rpmb_cdev_prepare(struct rpmb_dev *rdev) {}
+static inline void rpmb_cdev_add(struct rpmb_dev *rdev) {}
+static inline void rpmb_cdev_del(struct rpmb_dev *rdev) {}
+#endif /* CONFIG_RPMB_INTF_DEV */
diff --git a/include/linux/rpmb.h b/include/linux/rpmb.h
index 4ed5e299623e..3b0731c07528 100644
--- a/include/linux/rpmb.h
+++ b/include/linux/rpmb.h
@@ -8,8 +8,12 @@
#include <linux/types.h>
#include <linux/device.h>
+#include <linux/cdev.h>
+#include <uapi/linux/rpmb.h>
#include <linux/kref.h>
+#define RPMB_API_VERSION 0x80000001
+
/**
* struct rpmb_ops - RPMB ops to be implemented by underlying block device
*
@@ -54,6 +58,8 @@ struct rpmb_ops {
* @dev : device
* @id : device id
* @target : RPMB target/region within the physical device
+ * @cdev : character dev
+ * @status : device status
* @ops : operation exported by rpmb
*/
struct rpmb_dev {
@@ -61,6 +67,10 @@ struct rpmb_dev {
struct device dev;
int id;
u8 target;
+#ifdef CONFIG_RPMB_INTF_DEV
+ struct cdev cdev;
+ unsigned long status;
+#endif /* CONFIG_RPMB_INTF_DEV */
const struct rpmb_ops *ops;
};
diff --git a/include/uapi/linux/rpmb.h b/include/uapi/linux/rpmb.h
new file mode 100644
index 000000000000..2f4ee7279b1b
--- /dev/null
+++ b/include/uapi/linux/rpmb.h
@@ -0,0 +1,99 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/*
+ * Copyright (C) 2015-2018 Intel Corp. All rights reserved
+ * Copyright (C) 2021-2022 Linaro Ltd
+ */
+#ifndef _UAPI_LINUX_RPMB_H_
+#define _UAPI_LINUX_RPMB_H_
+
+#include <linux/types.h>
+
+/**
+ * struct rpmb_ioc_ver_cmd - rpmb api version
+ *
+ * @api_version: rpmb API version.
+ */
+struct rpmb_ioc_ver_cmd {
+ __u32 api_version;
+} __packed;
+
+enum rpmb_auth_method {
+ RPMB_HMAC_ALGO_SHA_256 = 0,
+};
+
+/**
+ * struct rpmb_ioc_cap_cmd - rpmb capabilities
+ *
+ * @target: rpmb target/region within RPMB partition.
+ * @capacity: storage capacity (in units of 128K)
+ * @block_size: storage data block size (in units of 256B)
+ * @wr_cnt_max: maximal number of block that can be written in a single request.
+ * @rd_cnt_max: maximal number of block that can be read in a single request.
+ * @auth_method: authentication method: currently always HMAC_SHA_256
+ * @reserved: reserved to align to 4 bytes.
+ */
+struct rpmb_ioc_cap_cmd {
+ __u16 target;
+ __u16 capacity;
+ __u16 block_size;
+ __u16 wr_cnt_max;
+ __u16 rd_cnt_max;
+ __u16 auth_method;
+ __u16 reserved;
+} __packed;
+
+/**
+ * struct rpmb_ioc_reqresp_cmd - general purpose reqresp
+ *
+ * Most RPMB operations consist of a set of request frames and an
+ * optional response frame. If a response is requested the user must
+ * allocate enough space for the response, otherwise the fields should
+ * be set to 0/NULL.
+ *
+ * It is used for programming the key, reading the counter and writing
+ * blocks to the device. If the frames are malformed they may be
+ * rejected by the underlying driver or the device itself.
+ *
+ * Assuming the transaction succeeds it is still up to user space to
+ * validate the response and check MAC values correspond to the
+ * programmed keys.
+ *
+ * @len: length of write counter request
+ * @request: ptr to device specific request frame
+ * @rlen: length of response frame
+ * @resp: ptr to device specific response frame
+ */
+struct rpmb_ioc_reqresp_cmd {
+ __u32 len;
+ __u8 __user *request;
+ __u32 rlen;
+ __u8 __user *response;
+} __packed;
+
+/**
+ * struct rpmb_ioc_rblocks_cmd - read blocks from RPMB
+ *
+ * @addr: index into device (units of 256B blocks)
+ * @count: number of 256B blocks
+ * @len: length of response frame
+ * @data: block data (in device specific framing)
+ *
+ * Reading blocks from an RPMB device doesn't require any specific
+ * authentication. However the result still needs to be validated by
+ * user space.
+ */
+struct rpmb_ioc_rblocks_cmd {
+ __u32 addr;
+ __u32 count;
+ __u32 len;
+ __u8 __user *data;
+} __packed;
+
+#define RPMB_IOC_VER_CMD _IOR(0xB8, 80, struct rpmb_ioc_ver_cmd)
+#define RPMB_IOC_CAP_CMD _IOR(0xB8, 81, struct rpmb_ioc_cap_cmd)
+#define RPMB_IOC_PKEY_CMD _IOWR(0xB8, 82, struct rpmb_ioc_reqresp_cmd)
+#define RPMB_IOC_COUNTER_CMD _IOWR(0xB8, 84, struct rpmb_ioc_reqresp_cmd)
+#define RPMB_IOC_WBLOCKS_CMD _IOWR(0xB8, 85, struct rpmb_ioc_reqresp_cmd)
+#define RPMB_IOC_RBLOCKS_CMD _IOR(0xB8, 86, struct rpmb_ioc_rblocks_cmd)
+
+#endif /* _UAPI_LINUX_RPMB_H_ */
--
2.30.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 2/4] char: rpmb: provide a user space interface
2022-04-05 9:37 ` [PATCH v2 2/4] char: rpmb: provide a user space interface Alex Bennée
@ 2022-06-16 15:09 ` Harald Mommer
2022-06-16 19:21 ` Arnd Bergmann
1 sibling, 0 replies; 29+ messages in thread
From: Harald Mommer @ 2022-06-16 15:09 UTC (permalink / raw)
To: Alex Bennée, linux-kernel
Cc: maxim.uvarov, joakim.bech, ulf.hansson, ilias.apalodimas, arnd,
ruchika.gupta, tomas.winkler, yang.huang, bing.zhu, Matti.Moell,
linux-mmc, linux-scsi, Linus Walleij, Arnd Bergmann,
Alexander Usyskin, Avri Altman
Hello,
On 05.04.22 11:37, Alex Bennée wrote:
> The user space API is achieved via a number of synchronous IOCTLs.
>
> * RPMB_IOC_VER_CMD - simple versioning API
> * RPMB_IOC_CAP_CMD - query of underlying capabilities
> * RPMB_IOC_PKEY_CMD - one time programming of access key
> * RPMB_IOC_COUNTER_CMD - query the write counter
> * RPMB_IOC_WBLOCKS_CMD - write blocks to device
> * RPMB_IOC_RBLOCKS_CMD - read blocks from device
>
> The operations which require authenticated frames or will respond with
> MAC hashes of nonce filled frames that userspace will need to verify
> share a common command frame format. The other operations can be
> considered generic and allow for common handling.
>
> [AJB: here the are key difference is the avoiding a single ioctl where
> all the frame data is put together by user space. User space is still
> the only place where certain operations can be verified due to the
> need of a secret key]
I have less problems to understand this reworked ioctl() interface as I
had with the older one. Nice.
> diff --git a/drivers/rpmb/cdev.c b/drivers/rpmb/cdev.c
> ...
> +static long rpmb_ioctl_cap_cmd(struct rpmb_dev *rdev,
> + struct rpmb_ioc_cap_cmd __user *ptr)
> +{
> + struct rpmb_ioc_cap_cmd cap;
> +
> + cap.target = rdev->target;
> + cap.block_size = rdev->ops->block_size;
> + cap.wr_cnt_max = rdev->ops->wr_cnt_max;
> + cap.rd_cnt_max = rdev->ops->rd_cnt_max;
> + cap.capacity = rpmb_get_capacity(rdev);
> + cap.reserved = 0;
auth_method is still part of the structure but not set. Means arbitrary
data from the stack is copied to user land.
> +
> + return copy_to_user(ptr, &cap, sizeof(cap)) ? -EFAULT : 0;
> +}
> ...
> +/**
> + * struct rpmb_ioc_cap_cmd - rpmb capabilities
> + *
> + * @target: rpmb target/region within RPMB partition.
> + * @capacity: storage capacity (in units of 128K)
> + * @block_size: storage data block size (in units of 256B)
> + * @wr_cnt_max: maximal number of block that can be written in a single request.
> + * @rd_cnt_max: maximal number of block that can be read in a single request.
> + * @auth_method: authentication method: currently always HMAC_SHA_256
> + * @reserved: reserved to align to 4 bytes.
> + */
> +struct rpmb_ioc_cap_cmd {
> + __u16 target;
> + __u16 capacity;
> + __u16 block_size;
> + __u16 wr_cnt_max;
> + __u16 rd_cnt_max;
> + __u16 auth_method;
> + __u16 reserved;
> +} __packed;
> ...+
--
Dipl.-Ing. Harald Mommer
Senior Software Engineer
OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin
Phone: +49 (30) 60 98 540-0 <== Zentrale
Fax: +49 (30) 60 98 540-99
E-Mail: harald.mommer@opensynergy.com
www.opensynergy.com
Handelsregister: Amtsgericht Charlottenburg, HRB 108616B
Geschäftsführer/Managing Director: Regis Adjamah
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v2 2/4] char: rpmb: provide a user space interface
2022-04-05 9:37 ` [PATCH v2 2/4] char: rpmb: provide a user space interface Alex Bennée
2022-06-16 15:09 ` Harald Mommer
@ 2022-06-16 19:21 ` Arnd Bergmann
1 sibling, 0 replies; 29+ messages in thread
From: Arnd Bergmann @ 2022-06-16 19:21 UTC (permalink / raw)
To: Alex Bennée
Cc: Linux Kernel Mailing List, Maxim Uvarov, Joakim Bech,
Ulf Hansson, Ilias Apalodimas, Ruchika Gupta, Winkler, Tomas,
Huang, Yang, Zhu, Bing, Matti.Moell, hmo, linux-mmc, linux-scsi,
Linus Walleij, Arnd Bergmann, Alexander Usyskin, Avri Altman
On Tue, Apr 5, 2022 at 11:37 AM Alex Bennée <alex.bennee@linaro.org> wrote:
> +static int rpmb_open(struct inode *inode, struct file *fp)
> +{
> + struct rpmb_dev *rdev;
> +
> + rdev = container_of(inode->i_cdev, struct rpmb_dev, cdev);
> + if (!rdev)
> + return -ENODEV;
> +
> + /* the rpmb is single open! */
> + if (test_and_set_bit(RPMB_DEV_OPEN, &rdev->status))
> + return -EBUSY;
> +
> + mutex_lock(&rdev->lock);
> +
> + fp->private_data = rdev;
> +
> + mutex_unlock(&rdev->lock);
The mutex here doesn't really do anything, as the file structure is not
reachable from any other context.
I'm not sure the single-open protection gains you anything either,
as it does not prevent the file structure to be shared between
processes.
> +static long rpmb_ioctl_ver_cmd(struct rpmb_dev *rdev,
> + struct rpmb_ioc_ver_cmd __user *ptr)
> +{
> + struct rpmb_ioc_ver_cmd ver = {
> + .api_version = RPMB_API_VERSION,
> + };
> +
> + return copy_to_user(ptr, &ver, sizeof(ver)) ? -EFAULT : 0;
> +}
API version ioctls are generally a bad idea, I think this should be skipped.
The way to handle incompatible API changes is to introduce new ioctl
commands.
> +static const struct file_operations rpmb_fops = {
> + .open = rpmb_open,
> + .release = rpmb_release,
> + .unlocked_ioctl = rpmb_ioctl,
> diff --git a/drivers/rpmb/rpmb-cdev.h b/drivers/rpmb/rpmb-cdev.h
> new file mode 100644
> index 000000000000..e59ff0c05e9d
> --- /dev/null
> +++ b/drivers/rpmb/rpmb-cdev.h
> @@ -0,0 +1,17 @@
> +/* SPDX-License-Identifier: BSD-3-Clause OR GPL-2.0 */
> +/*
> + * Copyright (C) 2015-2018 Intel Corp. All rights reserved
> + */
> +#ifdef CONFIG_RPMB_INTF_DEV
> +int __init rpmb_cdev_init(void);
> +void __exit rpmb_cdev_exit(void);
> +void rpmb_cdev_prepare(struct rpmb_dev *rdev);
> +void rpmb_cdev_add(struct rpmb_dev *rdev);
> +void rpmb_cdev_del(struct rpmb_dev *rdev);
> +#else
> +static inline int __init rpmb_cdev_init(void) { return 0; }
> +static inline void __exit rpmb_cdev_exit(void) {}
> +static inline void rpmb_cdev_prepare(struct rpmb_dev *rdev) {}
> +static inline void rpmb_cdev_add(struct rpmb_dev *rdev) {}
> +static inline void rpmb_cdev_del(struct rpmb_dev *rdev) {}
> +#endif /* CONFIG_RPMB_INTF_DEV */
What is the purpose of the fallback? Would you ever build the code without
the interface?
> +/**
> + * struct rpmb_ioc_ver_cmd - rpmb api version
> + *
> + * @api_version: rpmb API version.
> + */
> +struct rpmb_ioc_ver_cmd {
> + __u32 api_version;
> +} __packed;
Best remove this completely.
> +enum rpmb_auth_method {
> + RPMB_HMAC_ALGO_SHA_256 = 0,
> +};
> +
> +/**
> + * struct rpmb_ioc_cap_cmd - rpmb capabilities
> + *
> + * @target: rpmb target/region within RPMB partition.
> + * @capacity: storage capacity (in units of 128K)
> + * @block_size: storage data block size (in units of 256B)
> + * @wr_cnt_max: maximal number of block that can be written in a single request.
> + * @rd_cnt_max: maximal number of block that can be read in a single request.
> + * @auth_method: authentication method: currently always HMAC_SHA_256
> + * @reserved: reserved to align to 4 bytes.
> + */
> +struct rpmb_ioc_cap_cmd {
> + __u16 target;
> + __u16 capacity;
> + __u16 block_size;
> + __u16 wr_cnt_max;
> + __u16 rd_cnt_max;
> + __u16 auth_method;
> + __u16 reserved;
> +} __packed;
I would remove the __packed attribute here. If you want to keep it, then
it needs to have a __aligned() attribute as well.
It seems strange to have an odd number of 16-bit fields in this, maybe
make it two reserved fields to have a multiple of 32-bit.
> + * struct rpmb_ioc_reqresp_cmd - general purpose reqresp
> + *
> + * Most RPMB operations consist of a set of request frames and an
> + * optional response frame. If a response is requested the user must
> + * allocate enough space for the response, otherwise the fields should
> + * be set to 0/NULL.
> + *
> + * It is used for programming the key, reading the counter and writing
> + * blocks to the device. If the frames are malformed they may be
> + * rejected by the underlying driver or the device itself.
> + *
> + * Assuming the transaction succeeds it is still up to user space to
> + * validate the response and check MAC values correspond to the
> + * programmed keys.
> + *
> + * @len: length of write counter request
> + * @request: ptr to device specific request frame
> + * @rlen: length of response frame
> + * @resp: ptr to device specific response frame
> + */
> +struct rpmb_ioc_reqresp_cmd {
> + __u32 len;
> + __u8 __user *request;
> + __u32 rlen;
> + __u8 __user *response;
> +} __packed;
Try to avoid indirect pointers in ioctl structures, they break
compatibility with
32-bit processes. I think you can do this with a variable-length structure that
has the two length fields followed by the actual data. If there is a reasonable
upper bound for the length, it could also just be a fixed-length buffer.
If you end up having to use a pointer, make it a __u64 value and cast between
that and the actual pointer value.
Whatever you do though, don't use misaligned pointers or implicit padding in the
structure. You can have explicit padding by adding an extra __u32 before
a __u64 pointer value, and then remove the __packed attribute, or reorder the
members so each pointer is naturally aligned.
Arnd
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 3/4] rpmb: create virtio rpmb frontend driver
2022-04-05 9:37 [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver Alex Bennée
2022-04-05 9:37 ` [PATCH v2 1/4] rpmb: add Replay Protected Memory Block (RPMB) subsystem Alex Bennée
2022-04-05 9:37 ` [PATCH v2 2/4] char: rpmb: provide a user space interface Alex Bennée
@ 2022-04-05 9:37 ` Alex Bennée
2022-06-16 15:44 ` Harald Mommer
2022-04-05 9:37 ` [PATCH v2 4/4] tools rpmb: add RPBM access tool Alex Bennée
` (3 subsequent siblings)
6 siblings, 1 reply; 29+ messages in thread
From: Alex Bennée @ 2022-04-05 9:37 UTC (permalink / raw)
To: linux-kernel
Cc: maxim.uvarov, joakim.bech, ulf.hansson, ilias.apalodimas, arnd,
ruchika.gupta, tomas.winkler, yang.huang, bing.zhu, Matti.Moell,
hmo, linux-mmc, linux-scsi, Alex Bennée
This implements a virtio rpmb frontend driver for the RPMB subsystem.
This driver conforms to the rpmb internal API which attempts to stay
common for the majority of cases and only expose the low level frame
details as required. In these cases it is up to something outside of
the driver itself to do the appropriate MAC calculations.
The driver does do some basic verification of the incoming data and
will reject frames which are the wrong size or have the wrong commands
in them.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Tomas Winkler <tomas.winkler@intel.com>
---
ajb:
- rewrite for new API
---
drivers/rpmb/Kconfig | 10 +
drivers/rpmb/Makefile | 1 +
drivers/rpmb/virtio_rpmb.c | 518 +++++++++++++++++++++++++++++++
include/uapi/linux/virtio_rpmb.h | 54 ++++
4 files changed, 583 insertions(+)
create mode 100644 drivers/rpmb/virtio_rpmb.c
create mode 100644 include/uapi/linux/virtio_rpmb.h
diff --git a/drivers/rpmb/Kconfig b/drivers/rpmb/Kconfig
index 126f336fe9ea..cd0d1bb10910 100644
--- a/drivers/rpmb/Kconfig
+++ b/drivers/rpmb/Kconfig
@@ -16,3 +16,13 @@ config RPMB_INTF_DEV
help
Say yes here if you want to access RPMB from user space
via character device interface /dev/rpmb%d
+
+config VIRTIO_RPMB
+ tristate "Virtio RPMB driver"
+ default n
+ depends on VIRTIO
+ select RPMB
+ help
+ Say yes here if you have a Virtio aware RPMB device or want to use
+ RPMB from a Virtual Machine images.
+ This device interface is only for guest/frontend virtio driver.
diff --git a/drivers/rpmb/Makefile b/drivers/rpmb/Makefile
index f54b3f30514b..4b397b50a42c 100644
--- a/drivers/rpmb/Makefile
+++ b/drivers/rpmb/Makefile
@@ -4,5 +4,6 @@
obj-$(CONFIG_RPMB) += rpmb.o
rpmb-objs += core.o
rpmb-$(CONFIG_RPMB_INTF_DEV) += cdev.o
+obj-$(CONFIG_VIRTIO_RPMB) += virtio_rpmb.o
ccflags-y += -D__CHECK_ENDIAN__
diff --git a/drivers/rpmb/virtio_rpmb.c b/drivers/rpmb/virtio_rpmb.c
new file mode 100644
index 000000000000..db309014a157
--- /dev/null
+++ b/drivers/rpmb/virtio_rpmb.c
@@ -0,0 +1,518 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Virtio RPMB Front End Driver
+ *
+ * Copyright (c) 2018-2019 Intel Corporation.
+ * Copyright (c) 2021-2022 Linaro Ltd.
+ */
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+#include <linux/err.h>
+#include <linux/scatterlist.h>
+#include <linux/spinlock.h>
+#include <linux/slab.h>
+#include <linux/virtio.h>
+#include <linux/module.h>
+#include <linux/virtio_ids.h>
+#include <linux/virtio_config.h>
+#include <linux/virtio_rpmb.h>
+#include <linux/uaccess.h>
+#include <linux/byteorder/generic.h>
+#include <linux/rpmb.h>
+
+#define RPMB_MAC_SIZE 32
+#define VIRTIO_RPMB_FRAME_SZ 512
+
+static const char id[] = "RPMB:VIRTIO";
+
+struct virtio_rpmb_info {
+ /* The virtio device we're associated with */
+ struct virtio_device *vdev;
+
+ /* The virtq we use */
+ struct virtqueue *vq;
+
+ struct mutex lock; /* info lock */
+ wait_queue_head_t have_data;
+
+ /* Underlying RPMB device */
+ struct rpmb_dev *rdev;
+
+ /* Config values */
+ u8 max_wr, max_rd, capacity;
+};
+
+/**
+ * virtio_rpmb_recv_done() - vq completion callback
+ */
+static void virtio_rpmb_recv_done(struct virtqueue *vq)
+{
+ struct virtio_rpmb_info *vi;
+ struct virtio_device *vdev = vq->vdev;
+
+ vi = vq->vdev->priv;
+ if (!vi) {
+ dev_err(&vdev->dev, "Error: no found vi data.\n");
+ return;
+ }
+
+ wake_up(&vi->have_data);
+}
+
+/**
+ * do_virtio_transaction() - send sg list and wait for result
+ * @dev: linux device structure
+ * @vi: the device info (where the lock is)
+ * @sgs: array of scatterlists
+ * @out: total outgoing scatter lists
+ * @in: total returning scatter lists
+ *
+ * This is just a simple helper for processing the sg list. It will
+ * block until the response arrives. Returns number of bytes written
+ * back or negative if it failed.
+ */
+static int do_virtio_transaction(struct device *dev,
+ struct virtio_rpmb_info *vi,
+ struct scatterlist *sgs[],
+ int out, int in)
+{
+ int ret, len = 0;
+
+ mutex_lock(&vi->lock);
+ ret = virtqueue_add_sgs(vi->vq, sgs, out, in, vi, GFP_KERNEL);
+ if (ret) {
+ dev_err(dev, "failed to send %d, recv %d sgs (%d) to vq\n",
+ out, in, ret);
+ ret = -1;
+ } else {
+ virtqueue_kick(vi->vq);
+ wait_event(vi->have_data, virtqueue_get_buf(vi->vq, &len));
+ }
+ mutex_unlock(&vi->lock);
+
+ return len;
+}
+
+/**
+ * rpmb_virtio_program_key(): program key into virtio device
+ * @dev: device handle
+ * @target: target region (unused for VirtIO devices)
+ * @klen: length of key programming request
+ * @key_frame: key programming frames
+ * @rlen: length of response buffer
+ * @resp_frame: pointer to optional response frame
+ *
+ * Handle programming of the key (VIRTIO_RPMB_REQ_PROGRAM_KEY)
+ *
+ * The mandatory first frame contains the programming sequence. An
+ * optional second frame may ask for the result of the operation
+ * (VIRTIO_RPMB_REQ_RESULT_READ) which would trigger a response frame.
+ *
+ * Returns success/fail with errno and optional response frame
+ */
+static int rpmb_virtio_program_key(struct device *dev, u8 target,
+ int klen, u8 *key_frame, int rlen, u8 *resp_frame)
+{
+ struct virtio_rpmb_info *vi = dev_get_drvdata(dev);
+ struct virtio_rpmb_frame *pkey = (struct virtio_rpmb_frame *) key_frame;
+ struct virtio_rpmb_frame *resp = NULL;
+ struct scatterlist out_frame;
+ struct scatterlist in_frame;
+ struct scatterlist *sgs[2] = { };
+ int len;
+
+ if (!pkey)
+ return -EINVAL;
+
+ if (be16_to_cpu(pkey->req_resp) != VIRTIO_RPMB_REQ_PROGRAM_KEY)
+ return -EINVAL;
+
+ /* validate incoming frame */
+ switch (klen) {
+ case VIRTIO_RPMB_FRAME_SZ:
+ if (rlen || resp_frame)
+ return -EINVAL;
+ break;
+ case VIRTIO_RPMB_FRAME_SZ * 2:
+ if (!rlen || !resp_frame)
+ return -EINVAL;
+ if (be16_to_cpu(pkey[1].req_resp) != VIRTIO_RPMB_REQ_RESULT_READ)
+ return -EINVAL;
+ if (rlen < VIRTIO_RPMB_FRAME_SZ)
+ return -EINVAL;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ /* setup outgoing frame(s) */
+ sg_init_one(&out_frame, pkey, klen);
+ sgs[0] = &out_frame;
+
+ /* optional incoming frame */
+ if (rlen && resp_frame) {
+ resp = (struct virtio_rpmb_frame *) resp_frame;
+ sg_init_one(&in_frame, resp, sizeof(*resp));
+ sgs[1] = &in_frame;
+ }
+
+ len = do_virtio_transaction(dev, vi, sgs, 1, resp ? 1 : 0);
+
+ if (len > 0 && resp) {
+ if (be16_to_cpu(resp->req_resp) != VIRTIO_RPMB_RESP_PROGRAM_KEY) {
+ dev_err(dev, "Bad response from device (%x/%x)",
+ be16_to_cpu(resp->req_resp), be16_to_cpu(resp->result));
+ return -EPROTO;
+ } else {
+ /* map responses to better errors? */
+ return be16_to_cpu(resp->result) == VIRTIO_RPMB_RES_OK ? 0 : -EIO;
+ }
+ }
+
+ /* Something must have failed at this point. */
+ return len < 0 ? -EIO : 0;
+}
+
+static int rpmb_virtio_get_capacity(struct device *dev, u8 target)
+{
+ struct virtio_rpmb_info *vi = dev_get_drvdata(dev);
+ struct virtio_device *vdev = vi->vdev;
+
+ u8 capacity;
+
+ virtio_cread(vdev, struct virtio_rpmb_config, capacity, &capacity);
+
+ if (capacity > 0x80) {
+ dev_err(&vdev->dev, "Error: invalid capacity reported.\n");
+ capacity = 0x80;
+ }
+
+ return capacity;
+}
+
+static int rpmb_virtio_get_write_count(struct device *dev, u8 target,
+ int len, u8 *req, int rlen, u8 *resp)
+
+{
+ struct virtio_rpmb_info *vi = dev_get_drvdata(dev);
+ struct virtio_rpmb_frame *request = (struct virtio_rpmb_frame *) req;
+ struct virtio_rpmb_frame *response = (struct virtio_rpmb_frame *) resp;
+ struct scatterlist out_frame;
+ struct scatterlist in_frame;
+ struct scatterlist *sgs[2];
+ unsigned int received;
+
+ if (!len || len != VIRTIO_RPMB_FRAME_SZ || !request)
+ return -EINVAL;
+
+ if (!rlen || rlen != VIRTIO_RPMB_FRAME_SZ || !resp)
+ return -EINVAL;
+
+ if (be16_to_cpu(request->req_resp) != VIRTIO_RPMB_REQ_GET_WRITE_COUNTER)
+ return -EINVAL;
+
+ /* Wrap into SG array */
+ sg_init_one(&out_frame, request, VIRTIO_RPMB_FRAME_SZ);
+ sg_init_one(&in_frame, response, VIRTIO_RPMB_FRAME_SZ);
+ sgs[0] = &out_frame;
+ sgs[1] = &in_frame;
+
+ /* Send it, blocks until response */
+ received = do_virtio_transaction(dev, vi, sgs, 1, 1);
+
+ if (received != VIRTIO_RPMB_FRAME_SZ)
+ return -EPROTO;
+
+ if (be16_to_cpu(response->req_resp) != VIRTIO_RPMB_RESP_GET_COUNTER) {
+ dev_err(dev, "failed to get counter (%x/%x)",
+ be16_to_cpu(response->req_resp), be16_to_cpu(response->result));
+ return -EPROTO;
+ }
+
+ return be16_to_cpu(response->result) == VIRTIO_RPMB_RES_OK ?
+ be32_to_cpu(response->write_counter) : -EIO;
+}
+
+static int rpmb_virtio_write_blocks(struct device *dev, u8 target,
+ int len, u8 *req, int rlen, u8 *resp)
+{
+ struct virtio_rpmb_info *vi = dev_get_drvdata(dev);
+ struct virtio_rpmb_frame *request = (struct virtio_rpmb_frame *) req;
+ struct virtio_rpmb_frame *response = (struct virtio_rpmb_frame *) resp;
+ struct scatterlist out_frame;
+ struct scatterlist in_frame;
+ struct scatterlist *sgs[2];
+ int blocks, data_len, received;
+
+ if (!len || (len % VIRTIO_RPMB_FRAME_SZ) != 0 || !request)
+ return -EINVAL;
+
+ /* The first frame will contain the details of the request */
+ if (be16_to_cpu(request->req_resp) != VIRTIO_RPMB_REQ_DATA_WRITE)
+ return -EINVAL;
+
+ blocks = be16_to_cpu(request->block_count);
+ if (blocks > vi->max_wr)
+ return -EINVAL;
+
+ /*
+ * We either have exactly enough frames to write all the data
+ * or we have that plus a frame looking for a response.
+ */
+ data_len = blocks * VIRTIO_RPMB_FRAME_SZ;
+
+ if (len == data_len + VIRTIO_RPMB_FRAME_SZ) {
+ struct virtio_rpmb_frame *reply = &request[blocks];
+
+ if (be16_to_cpu(reply->req_resp) != VIRTIO_RPMB_REQ_RESULT_READ)
+ return -EINVAL;
+
+ if (!rlen || rlen != VIRTIO_RPMB_FRAME_SZ || !resp)
+ return -EINVAL;
+ } else if (len > data_len) {
+ return -E2BIG;
+ } else if (len < data_len) {
+ return -ENOSPC;
+ } else if (rlen || resp) {
+ return -EINVAL;
+ }
+
+ /* time to do the transaction */
+ sg_init_one(&out_frame, request, len);
+ sgs[0] = &out_frame;
+
+ /* optional incoming frame */
+ if (rlen && resp) {
+ sg_init_one(&in_frame, resp, VIRTIO_RPMB_FRAME_SZ);
+ sgs[1] = &in_frame;
+ }
+
+ received = do_virtio_transaction(dev, vi, sgs, 1, resp ? 1 : 0);
+
+ if (response && received != VIRTIO_RPMB_FRAME_SZ)
+ return -EPROTO;
+
+ if (response && be16_to_cpu(response->req_resp) != VIRTIO_RPMB_RESP_DATA_WRITE) {
+ dev_err(dev, "didn't get a response result (%x/%x)",
+ be16_to_cpu(response->req_resp), be16_to_cpu(response->result));
+ return -EPROTO;
+ }
+
+ return be16_to_cpu(response->result) == VIRTIO_RPMB_RES_OK ? 0 : -EIO;
+}
+
+/**
+ * rpmb_virtio_read_blocks(): read blocks of data
+ * @dev: device handle
+ * @target: target region (unused for VirtIO devices)
+ * @addr: block address to start reading from
+ * @count: number of blocks to read
+ * @len: length of receiving buffer
+ * @data: receiving buffer
+ *
+ * Read a number of blocks from RPMB device. As there is no
+ * authentication required to read data we construct the outgoing
+ * frame in this driver.
+ *
+ * Returns success/fail with errno and filling in the buffer pointed
+ * to by @data.
+ */
+static int rpmb_virtio_read_blocks(struct device *dev, u8 target,
+ int addr, int count, int len, u8 *data)
+{
+ struct virtio_rpmb_info *vi = dev_get_drvdata(dev);
+ struct virtio_rpmb_frame *request;
+ struct virtio_rpmb_frame *response = (struct virtio_rpmb_frame *) data;
+ struct scatterlist out_frame;
+ struct scatterlist in_frame;
+ struct scatterlist *sgs[2];
+ int computed_len = count * VIRTIO_RPMB_FRAME_SZ;
+ int received;
+
+ if (!count || !data)
+ return -EINVAL;
+
+ if (addr + count > vi->capacity)
+ return -ESPIPE;
+
+ if (count > vi->max_rd)
+ return -EINVAL;
+
+ /* EMSGSIZE? */
+ if (len < computed_len)
+ return -EFBIG;
+
+ /*
+ * With the basics done we can construct our request.
+ */
+ request = kmalloc(VIRTIO_RPMB_FRAME_SZ, GFP_KERNEL);
+ if (!request)
+ return -ENOMEM;
+
+ request->req_resp = cpu_to_be16(VIRTIO_RPMB_REQ_DATA_READ);
+ request->block_count = cpu_to_be16(count);
+ request->address = cpu_to_be16(addr);
+
+ /* time to do the transaction */
+ sg_init_one(&out_frame, request, sizeof(*request));
+ sgs[0] = &out_frame;
+ sg_init_one(&in_frame, data, len);
+ sgs[1] = &in_frame;
+
+ received = do_virtio_transaction(dev, vi, sgs, 1, 1);
+
+ kfree(request);
+
+ if (received != computed_len)
+ return -EPROTO;
+
+ if (be16_to_cpu(response->req_resp) != VIRTIO_RPMB_RESP_DATA_READ) {
+ dev_err(dev, "didn't get a response result (%x/%x)",
+ be16_to_cpu(response->req_resp), be16_to_cpu(response->result));
+ return -EPROTO;
+ }
+
+ return be16_to_cpu(response->result) == VIRTIO_RPMB_RES_OK ? 0 : -EIO;
+}
+
+static struct rpmb_ops rpmb_virtio_ops = {
+ .program_key = rpmb_virtio_program_key,
+ .get_capacity = rpmb_virtio_get_capacity,
+ .get_write_count = rpmb_virtio_get_write_count,
+ .write_blocks = rpmb_virtio_write_blocks,
+ .read_blocks = rpmb_virtio_read_blocks,
+};
+
+static int rpmb_virtio_dev_init(struct virtio_rpmb_info *vi)
+{
+ struct virtio_device *vdev = vi->vdev;
+ /* XXX this seems very roundabout */
+ struct device *dev = &vi->vq->vdev->dev;
+ int ret = 0;
+
+ virtio_cread(vdev, struct virtio_rpmb_config,
+ max_wr_cnt, &vi->max_wr);
+ virtio_cread(vdev, struct virtio_rpmb_config,
+ max_rd_cnt, &vi->max_rd);
+ virtio_cread(vdev, struct virtio_rpmb_config,
+ capacity, &vi->capacity);
+
+ rpmb_virtio_ops.dev_id_len = strlen(id);
+ rpmb_virtio_ops.dev_id = id;
+ rpmb_virtio_ops.wr_cnt_max = vi->max_wr;
+ rpmb_virtio_ops.rd_cnt_max = vi->max_rd;
+ rpmb_virtio_ops.block_size = 1;
+
+ vi->rdev = rpmb_dev_register(dev, 0, &rpmb_virtio_ops);
+ if (IS_ERR(vi->rdev)) {
+ ret = PTR_ERR(vi->rdev);
+ goto err;
+ }
+
+ dev_set_drvdata(dev, vi);
+err:
+ return ret;
+}
+
+static int virtio_rpmb_init(struct virtio_device *vdev)
+{
+ int ret;
+ struct virtio_rpmb_info *vi;
+
+ vi = kzalloc(sizeof(*vi), GFP_KERNEL);
+ if (!vi)
+ return -ENOMEM;
+
+ init_waitqueue_head(&vi->have_data);
+ mutex_init(&vi->lock);
+
+ /* link virtio_rpmb_info to virtio_device */
+ vdev->priv = vi;
+ vi->vdev = vdev;
+
+ /* We expect a single virtqueue. */
+ vi->vq = virtio_find_single_vq(vdev, virtio_rpmb_recv_done, "request");
+ if (IS_ERR(vi->vq)) {
+ dev_err(&vdev->dev, "get single vq failed!\n");
+ ret = PTR_ERR(vi->vq);
+ goto err;
+ }
+
+ /* create vrpmb device. */
+ ret = rpmb_virtio_dev_init(vi);
+ if (ret) {
+ dev_err(&vdev->dev, "create vrpmb device failed.\n");
+ goto err;
+ }
+
+ dev_info(&vdev->dev, "init done!\n");
+
+ return 0;
+
+err:
+ kfree(vi);
+ return ret;
+}
+
+static void virtio_rpmb_remove(struct virtio_device *vdev)
+{
+ struct virtio_rpmb_info *vi;
+
+ vi = vdev->priv;
+ if (!vi)
+ return;
+
+ if (wq_has_sleeper(&vi->have_data))
+ wake_up(&vi->have_data);
+
+ rpmb_dev_unregister(vi->rdev);
+
+ if (vdev->config->reset)
+ vdev->config->reset(vdev);
+
+ if (vdev->config->del_vqs)
+ vdev->config->del_vqs(vdev);
+
+ kfree(vi);
+}
+
+static int virtio_rpmb_probe(struct virtio_device *vdev)
+{
+ return virtio_rpmb_init(vdev);
+}
+
+#ifdef CONFIG_PM_SLEEP
+static int virtio_rpmb_freeze(struct virtio_device *vdev)
+{
+ virtio_rpmb_remove(vdev);
+ return 0;
+}
+
+static int virtio_rpmb_restore(struct virtio_device *vdev)
+{
+ return virtio_rpmb_init(vdev);
+}
+#endif
+
+static struct virtio_device_id id_table[] = {
+ { VIRTIO_ID_RPMB, VIRTIO_DEV_ANY_ID },
+ { 0 },
+};
+
+static struct virtio_driver virtio_rpmb_driver = {
+ .driver.name = KBUILD_MODNAME,
+ .driver.owner = THIS_MODULE,
+ .id_table = id_table,
+ .probe = virtio_rpmb_probe,
+ .remove = virtio_rpmb_remove,
+#ifdef CONFIG_PM_SLEEP
+ .freeze = virtio_rpmb_freeze,
+ .restore = virtio_rpmb_restore,
+#endif
+};
+
+module_virtio_driver(virtio_rpmb_driver);
+MODULE_DEVICE_TABLE(virtio, id_table);
+
+MODULE_DESCRIPTION("Virtio rpmb frontend driver");
+MODULE_AUTHOR("Intel Corporation");
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/include/uapi/linux/virtio_rpmb.h b/include/uapi/linux/virtio_rpmb.h
new file mode 100644
index 000000000000..f048cd968210
--- /dev/null
+++ b/include/uapi/linux/virtio_rpmb.h
@@ -0,0 +1,54 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+
+#ifndef _UAPI_LINUX_VIRTIO_RPMB_H
+#define _UAPI_LINUX_VIRTIO_RPMB_H
+
+#include <linux/types.h>
+#include <linux/virtio_ids.h>
+#include <linux/virtio_config.h>
+#include <linux/virtio_types.h>
+
+struct virtio_rpmb_config {
+ __u8 capacity;
+ __u8 max_wr_cnt;
+ __u8 max_rd_cnt;
+} __attribute__((packed));
+
+/* RPMB Request Types (in .req_resp) */
+#define VIRTIO_RPMB_REQ_PROGRAM_KEY 0x0001
+#define VIRTIO_RPMB_REQ_GET_WRITE_COUNTER 0x0002
+#define VIRTIO_RPMB_REQ_DATA_WRITE 0x0003
+#define VIRTIO_RPMB_REQ_DATA_READ 0x0004
+#define VIRTIO_RPMB_REQ_RESULT_READ 0x0005
+
+/* RPMB Response Types (in .req_resp) */
+#define VIRTIO_RPMB_RESP_PROGRAM_KEY 0x0100
+#define VIRTIO_RPMB_RESP_GET_COUNTER 0x0200
+#define VIRTIO_RPMB_RESP_DATA_WRITE 0x0300
+#define VIRTIO_RPMB_RESP_DATA_READ 0x0400
+
+struct virtio_rpmb_frame {
+ __u8 stuff[196];
+ __u8 key_mac[32];
+ __u8 data[256];
+ __u8 nonce[16];
+ __be32 write_counter;
+ __be16 address;
+ __be16 block_count;
+ __be16 result;
+ __be16 req_resp;
+} __attribute__((packed));
+
+/* RPMB Operation Results (in .result) */
+#define VIRTIO_RPMB_RES_OK 0x0000
+#define VIRTIO_RPMB_RES_GENERAL_FAILURE 0x0001
+#define VIRTIO_RPMB_RES_AUTH_FAILURE 0x0002
+#define VIRTIO_RPMB_RES_COUNT_FAILURE 0x0003
+#define VIRTIO_RPMB_RES_ADDR_FAILURE 0x0004
+#define VIRTIO_RPMB_RES_WRITE_FAILURE 0x0005
+#define VIRTIO_RPMB_RES_READ_FAILURE 0x0006
+#define VIRTIO_RPMB_RES_NO_AUTH_KEY 0x0007
+#define VIRTIO_RPMB_RES_WRITE_COUNTER_EXPIRED 0x0080
+
+
+#endif /* _UAPI_LINUX_VIRTIO_RPMB_H */
--
2.30.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 3/4] rpmb: create virtio rpmb frontend driver
2022-04-05 9:37 ` [PATCH v2 3/4] rpmb: create virtio rpmb frontend driver Alex Bennée
@ 2022-06-16 15:44 ` Harald Mommer
0 siblings, 0 replies; 29+ messages in thread
From: Harald Mommer @ 2022-06-16 15:44 UTC (permalink / raw)
To: Alex Bennée, linux-kernel
Cc: maxim.uvarov, joakim.bech, ulf.hansson, ilias.apalodimas, arnd,
ruchika.gupta, tomas.winkler, yang.huang, bing.zhu, Matti.Moell,
linux-mmc, linux-scsi
On 05.04.22 11:37, Alex Bennée wrote:
> +++ b/drivers/rpmb/virtio_rpmb.c
> ...
> +static int rpmb_virtio_write_blocks(struct device *dev, u8 target,
> + int len, u8 *req, int rlen, u8 *resp)
> +{
> + struct virtio_rpmb_info *vi = dev_get_drvdata(dev);
> + struct virtio_rpmb_frame *request = (struct virtio_rpmb_frame *) req;
> + struct virtio_rpmb_frame *response = (struct virtio_rpmb_frame *) resp;
> + struct scatterlist out_frame;
> + struct scatterlist in_frame;
> + struct scatterlist *sgs[2];
> + int blocks, data_len, received;
> +
> + if (!len || (len % VIRTIO_RPMB_FRAME_SZ) != 0 || !request)
> + return -EINVAL;
> +
> + /* The first frame will contain the details of the request */
> + if (be16_to_cpu(request->req_resp) != VIRTIO_RPMB_REQ_DATA_WRITE)
> + return -EINVAL;
> +
> + blocks = be16_to_cpu(request->block_count);
> + if (blocks > vi->max_wr)
> + return -EINVAL;
Not exactly. The virtio specification reserves 0 for "unlimited". And I
see no special handling for 0 in your code. Damned trap in the
specification.
What's "unlimited"? As the blocks_count in the rpmb frame is a be16 I
guess it's 65535. But if you consider this theoretically you have a
possible max array allocation of 16 MB - 512B max. instead of the 16 KB
- 512B you have with 255. Getting headache about this "unlimited" in the
virtio specification. I don't like the theoretical possibility having to
allocate 16MB dynamically for a moment due to this "unlimited".
And of course there is the same problem in virtio_rpmb_read_blocks()
with max_rd.
A fix is needed, either 0 is 65535 or 0 is 255. Choosing 255 as
implementation decision would be limiting down but maybe without any
practical impact. For UFS it's a single byte bRPMBReadWriteSize only in
JESD220C-2.2 GEOMETRY DESCRIPTOR and 0 is not defined there as
"unlimited". Unfortunately I've no idea about the other possible
underlying technologies (eMMC, NVMe, ...).
> ...
> +}
> +...
> +static int rpmb_virtio_read_blocks(struct device *dev, u8 target,
> + int addr, int count, int len, u8 *data)
> +{
> ...
> + if (count > vi->max_rd)
> + return -EINVAL;
See above.
> ...
--
Dipl.-Ing. Harald Mommer
Senior Software Engineer
OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin
Phone: +49 (30) 60 98 540-0 <== Zentrale
Fax: +49 (30) 60 98 540-99
E-Mail: harald.mommer@opensynergy.com
www.opensynergy.com
Handelsregister: Amtsgericht Charlottenburg, HRB 108616B
Geschäftsführer/Managing Director: Regis Adjamah
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH v2 4/4] tools rpmb: add RPBM access tool
2022-04-05 9:37 [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver Alex Bennée
` (2 preceding siblings ...)
2022-04-05 9:37 ` [PATCH v2 3/4] rpmb: create virtio rpmb frontend driver Alex Bennée
@ 2022-04-05 9:37 ` Alex Bennée
2022-06-16 13:13 ` Harald Mommer
2022-04-05 14:54 ` [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver Bean Huo
` (2 subsequent siblings)
6 siblings, 1 reply; 29+ messages in thread
From: Alex Bennée @ 2022-04-05 9:37 UTC (permalink / raw)
To: linux-kernel
Cc: maxim.uvarov, joakim.bech, ulf.hansson, ilias.apalodimas, arnd,
ruchika.gupta, tomas.winkler, yang.huang, bing.zhu, Matti.Moell,
hmo, linux-mmc, linux-scsi, Alex Bennée, Alexander Usyskin
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=UTF-8, Size: 31422 bytes --]
Add simple RPMB host testing tool. It can be used to program key,
write and read data block, and retrieve write counter. It's based on
Tomas' original tool but only deals with the virtio-rpmb device. I've
also split the RPMB specific functions (vrpmb_*) from the generic
hooks (op_rpmb_*) to make it easier to add other RPMB implementations.
A simple test.sh script will exercise the interface (assuming a virgin
RPMB device awaiting a programmed key).
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Tomas Winkler <tomas.winkler@intel.com>
Cc: Alexander Usyskin <alexander.usyskin@intel.com>
---
ajb:
- new per op ioctl API
---
MAINTAINERS | 1 +
tools/Makefile | 16 +-
tools/rpmb/.gitignore | 2 +
tools/rpmb/Makefile | 41 ++
tools/rpmb/key | 1 +
tools/rpmb/rpmb.c | 1083 +++++++++++++++++++++++++++++++++++++++++
tools/rpmb/test.sh | 22 +
7 files changed, 1161 insertions(+), 5 deletions(-)
create mode 100644 tools/rpmb/.gitignore
create mode 100644 tools/rpmb/Makefile
create mode 100644 tools/rpmb/key
create mode 100644 tools/rpmb/rpmb.c
create mode 100755 tools/rpmb/test.sh
diff --git a/MAINTAINERS b/MAINTAINERS
index 0a744da21817..70716250aa51 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -16751,6 +16751,7 @@ S: Supported
F: drivers/rpmb/*
F: include/uapi/linux/rpmb.h
F: include/linux/rpmb.h
+F: tools/rpmb/
RPMSG TTY DRIVER
M: Arnaud Pouliquen <arnaud.pouliquen@foss.st.com>
diff --git a/tools/Makefile b/tools/Makefile
index db2f7b8ebed5..0cc62dcae581 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -27,6 +27,7 @@ help:
@echo ' objtool - an ELF object analysis tool'
@echo ' pci - PCI tools'
@echo ' perf - Linux performance measurement and analysis tool'
+ @echo ' rpmb - Replay protected memory block access tool'
@echo ' selftests - various kernel selftests'
@echo ' bootconfig - boot config tool'
@echo ' spi - spi tools'
@@ -65,7 +66,9 @@ acpi: FORCE
cpupower: FORCE
$(call descend,power/$@)
-cgroup counter firewire hv guest bootconfig spi usb virtio vm bpf iio gpio objtool leds wmi pci firmware debugging tracing: FORCE
+cgroup counter firewire hv guest bootconfig spi usb virtio vm bpf iio \
+ gpio objtool leds wmi pci firmware debugging tracing rpmb: FORCE
+
$(call descend,$@)
bpf/%: FORCE
@@ -101,7 +104,7 @@ all: acpi cgroup counter cpupower gpio hv firewire \
perf selftests bootconfig spi turbostat usb \
virtio vm bpf x86_energy_perf_policy \
tmon freefall iio objtool kvm_stat wmi \
- pci debugging tracing
+ pci debugging tracing rpmb
acpi_install:
$(call descend,power/$(@:_install=),install)
@@ -109,7 +112,7 @@ acpi_install:
cpupower_install:
$(call descend,power/$(@:_install=),install)
-cgroup_install counter_install firewire_install gpio_install hv_install iio_install perf_install bootconfig_install spi_install usb_install virtio_install vm_install bpf_install objtool_install wmi_install pci_install debugging_install tracing_install:
+cgroup_install counter_install firewire_install gpio_install hv_install iio_install perf_install bootconfig_install spi_install usb_install virtio_install vm_install bpf_install objtool_install wmi_install pci_install debugging_install tracing_install rpmb_install:
$(call descend,$(@:_install=),install)
selftests_install:
@@ -129,7 +132,7 @@ kvm_stat_install:
install: acpi_install cgroup_install counter_install cpupower_install gpio_install \
hv_install firewire_install iio_install \
- perf_install selftests_install turbostat_install usb_install \
+ perf_install rpmb_install selftests_install turbostat_install usb_install \
virtio_install vm_install bpf_install x86_energy_perf_policy_install \
tmon_install freefall_install objtool_install kvm_stat_install \
wmi_install pci_install debugging_install intel-speed-select_install \
@@ -157,6 +160,9 @@ perf_clean:
$(Q)mkdir -p $(PERF_O) .
$(Q)$(MAKE) --no-print-directory -C perf O=$(PERF_O) subdir= clean
+rpmb_clean:
+ $(call descend,$(@:_clean=),clean)
+
selftests_clean:
$(call descend,testing/$(@:_clean=),clean)
@@ -173,7 +179,7 @@ build_clean:
$(call descend,build,clean)
clean: acpi_clean cgroup_clean counter_clean cpupower_clean hv_clean firewire_clean \
- perf_clean selftests_clean turbostat_clean bootconfig_clean spi_clean usb_clean virtio_clean \
+ perf_clean rpmb_clean selftests_clean turbostat_clean bootconfig_clean spi_clean usb_clean virtio_clean \
vm_clean bpf_clean iio_clean x86_energy_perf_policy_clean tmon_clean \
freefall_clean build_clean libbpf_clean libsubcmd_clean \
gpio_clean objtool_clean leds_clean wmi_clean pci_clean firmware_clean debugging_clean \
diff --git a/tools/rpmb/.gitignore b/tools/rpmb/.gitignore
new file mode 100644
index 000000000000..218f680548e6
--- /dev/null
+++ b/tools/rpmb/.gitignore
@@ -0,0 +1,2 @@
+*.o
+rpmb
diff --git a/tools/rpmb/Makefile b/tools/rpmb/Makefile
new file mode 100644
index 000000000000..e9429be4eaba
--- /dev/null
+++ b/tools/rpmb/Makefile
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: GPL-2.0
+include ../scripts/Makefile.include
+
+CC ?= $(CROSS_COMPILE)gcc
+LD ?= $(CROSS_COMPILE)ld
+PKG_CONFIG = $(CROSS_COMPILE)pkg-config
+
+ifeq ($(srctree),)
+srctree := $(patsubst %/,%,$(dir $(shell pwd)))
+srctree := $(patsubst %/,%,$(dir $(srctree)))
+#$(info Determined 'srctree' to be $(srctree))
+endif
+
+INSTALL = install
+prefix ?= /usr/local
+bindir = $(prefix)/bin
+
+
+CFLAGS += $(HOSTCFLAGS)
+CFLAGS += -D__EXPORTED_HEADERS__
+CFLAGS += -Wall -Wextra -ggdb
+ifdef RPMB_STATIC
+LDFLAGS += -pthread -static -Wl,-u,pthread_mutex_unlock
+CFLAGS += -pthread -static
+PKG_STATIC = --static
+endif
+CFLAGS += -I$(srctree)/include/uapi -I$(srctree)/include
+LDLIBS += $(shell $(PKG_CONFIG) --libs $(PKG_STATIC) libcrypto)
+
+prog := rpmb
+
+all : $(prog)
+
+$(prog): rpmb.o
+
+clean :
+ $(RM) $(prog) *.o
+
+install: $(prog)
+ $(INSTALL) -m755 -d $(DESTDIR)$(bindir)
+ $(INSTALL) $(prog) $(DESTDIR)$(bindir)
diff --git a/tools/rpmb/key b/tools/rpmb/key
new file mode 100644
index 000000000000..2b6bd3bc3fe6
--- /dev/null
+++ b/tools/rpmb/key
@@ -0,0 +1 @@
+˜ƒÆÐh«#×¢ö‹pRTà¿®å\x1fô\x1f\r|OŠ ¯mo«\x10
\ No newline at end of file
diff --git a/tools/rpmb/rpmb.c b/tools/rpmb/rpmb.c
new file mode 100644
index 000000000000..ff7ea198eda1
--- /dev/null
+++ b/tools/rpmb/rpmb.c
@@ -0,0 +1,1083 @@
+// SPDX-License-Identifier: BSD-3-Clause
+/*
+ * Copyright (C) 2016-2019 Intel Corp. All rights reserved
+ * Copyright (C) 2021-2022 Linaro Ltd
+ */
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/ioctl.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <keyutils.h>
+#include <dirent.h>
+#include <sys/stat.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <libgen.h>
+#include <limits.h>
+#include <ctype.h>
+#include <errno.h>
+#include <stdint.h>
+#include <stdbool.h>
+
+#include <openssl/engine.h>
+#include <openssl/hmac.h>
+#include <openssl/rand.h>
+
+/* if uapi header isn't installed, this might not yet exist */
+#ifndef __packed
+#define __packed __attribute__((packed))
+#endif
+
+#include "linux/rpmb.h"
+#include "linux/virtio_rpmb.h"
+
+#define RPMB_KEY_SIZE 32
+#define RPMB_MAC_SIZE 32
+#define RPMB_NONCE_SIZE 16
+#define RPMB_BLOCK_SIZE 256
+
+#define min(a, b) \
+ ({ __typeof__ (a) _a = (a); \
+ __typeof__ (b) _b = (b); \
+ _a < _b ? _a : _b; })
+
+
+static bool verbose;
+#define rpmb_dbg(fmt, ARGS...) do { \
+ if (verbose) \
+ fprintf(stderr, "rpmb: " fmt, ##ARGS); \
+} while (0)
+
+#define rpmb_msg(fmt, ARGS...) \
+ fprintf(stderr, "rpmb: " fmt, ##ARGS)
+
+#define rpmb_err(fmt, ARGS...) \
+ fprintf(stderr, "rpmb:%d error: " fmt, __LINE__, ##ARGS)
+
+
+/*
+ * Utility functions
+ */
+static int open_dev_file(const char *devfile, struct rpmb_ioc_cap_cmd *cap)
+{
+ struct rpmb_ioc_ver_cmd ver;
+ int fd;
+ int ret;
+
+ fd = open(devfile, O_RDWR);
+ if (fd < 0)
+ rpmb_err("Cannot open: %s: %s.\n", devfile, strerror(errno));
+
+ ret = ioctl(fd, RPMB_IOC_VER_CMD, &ver);
+ if (ret < 0) {
+ rpmb_err("ioctl failure %d: %s.\n", ret, strerror(errno));
+ goto err;
+ }
+
+ printf("RPMB API Version %X\n", ver.api_version);
+
+ ret = ioctl(fd, RPMB_IOC_CAP_CMD, cap);
+ if (ret < 0) {
+ rpmb_err("ioctl failure %d: %s.\n", ret, strerror(errno));
+ goto err;
+ }
+
+ rpmb_dbg("RPMB rpmb_target = %d\n", cap->target);
+ rpmb_dbg("RPMB capacity = %d\n", cap->capacity);
+ rpmb_dbg("RPMB block_size = %d\n", cap->block_size);
+ rpmb_dbg("RPMB wr_cnt_max = %d\n", cap->wr_cnt_max);
+ rpmb_dbg("RPMB rd_cnt_max = %d\n", cap->rd_cnt_max);
+ rpmb_dbg("RPMB auth_method = %d\n", cap->auth_method);
+
+ return fd;
+err:
+ close(fd);
+ return -1;
+}
+
+static int open_rd_file(const char *datafile, const char *type)
+{
+ int fd;
+
+ if (!strcmp(datafile, "-"))
+ fd = STDIN_FILENO;
+ else
+ fd = open(datafile, O_RDONLY);
+
+ if (fd < 0)
+ rpmb_err("Cannot open %s: %s: %s.\n",
+ type, datafile, strerror(errno));
+
+ return fd;
+}
+
+static int open_wr_file(const char *datafile, const char *type)
+{
+ int fd;
+
+ if (!strcmp(datafile, "-"))
+ fd = STDOUT_FILENO;
+ else
+ fd = open(datafile, O_WRONLY | O_CREAT | O_APPEND, 0600);
+ if (fd < 0)
+ rpmb_err("Cannot open %s: %s: %s.\n",
+ type, datafile, strerror(errno));
+ return fd;
+}
+
+static void close_fd(int fd)
+{
+ if (fd > 0 && fd != STDIN_FILENO && fd != STDOUT_FILENO)
+ close(fd);
+}
+
+/* need to just cast out 'const' in write(2) */
+typedef ssize_t (*rwfunc_t)(int fd, void *buf, size_t count);
+/* blocking rw wrapper */
+static ssize_t rw(rwfunc_t func, int fd, unsigned char *buf, size_t size)
+{
+ ssize_t ntotal = 0, n;
+ char *_buf = (char *)buf;
+
+ do {
+ n = func(fd, _buf + ntotal, size - ntotal);
+ if (n == -1 && errno != EINTR) {
+ ntotal = -1;
+ break;
+ } else if (n > 0) {
+ ntotal += n;
+ }
+ } while (n != 0 && (size_t)ntotal != size);
+
+ return ntotal;
+}
+
+static ssize_t read_file(int fd, unsigned char *data, size_t size)
+{
+ ssize_t ret;
+
+ ret = rw(read, fd, data, size);
+ if (ret < 0) {
+ rpmb_err("cannot read file: %s\n.", strerror(errno));
+ } else if ((size_t)ret != size) {
+ rpmb_err("read %zd but must be %zu bytes length.\n", ret, size);
+ ret = -EINVAL;
+ }
+
+ return ret;
+}
+
+static ssize_t write_file(int fd, unsigned char *data, size_t size)
+{
+ ssize_t ret;
+
+ ret = rw((rwfunc_t)write, fd, data, size);
+ if (ret < 0) {
+ rpmb_err("cannot read file: %s.\n", strerror(errno));
+ } else if ((size_t)ret != size) {
+ rpmb_err("data is %zd but must be %zu bytes length.\n",
+ ret, size);
+ ret = -EINVAL;
+ }
+ return ret;
+}
+
+static void __dump_buffer(const char *buf)
+{
+ fprintf(stderr, "%s\n", buf);
+}
+
+static void
+dump_hex_buffer(const char *title, const void *buf, size_t len)
+{
+#define PBUF_SZ (16 * 3)
+ const unsigned char *_buf = (const unsigned char *)buf;
+ char pbuf[PBUF_SZ];
+ int j = 0;
+
+ if (title)
+ fprintf(stderr, "%s\n", title);
+ while (len-- > 0) {
+ snprintf(&pbuf[j], PBUF_SZ - j, "%02X ", *_buf++);
+ j += 3;
+ if (j == PBUF_SZ) {
+ __dump_buffer(pbuf);
+ j = 0;
+ }
+ }
+ if (j)
+ __dump_buffer(pbuf);
+}
+
+/*
+ * MAC bits
+ */
+
+/* The hmac calculation is from data to the end of the frame */
+#define vrpmb_hmac_data_len \
+ (sizeof(struct virtio_rpmb_frame) - \
+ offsetof(struct virtio_rpmb_frame, data))
+
+static int vrpmb_calc_hmac_sha256(struct virtio_rpmb_frame *frames,
+ size_t blocks_cnt,
+ const unsigned char key[],
+ unsigned int key_size,
+ unsigned char mac[],
+ unsigned int mac_size)
+{
+ HMAC_CTX *ctx;
+ int ret;
+ unsigned int i;
+
+ /* SSL returns 1 on success 0 on failure */
+
+ ctx = HMAC_CTX_new();
+
+ ret = HMAC_Init_ex(ctx, key, key_size, EVP_sha256(), NULL);
+ if (ret == 0)
+ goto out;
+ for (i = 0; i < blocks_cnt; i++)
+ HMAC_Update(ctx, frames[i].data, vrpmb_hmac_data_len);
+
+ ret = HMAC_Final(ctx, mac, &mac_size);
+ if (ret == 0)
+ goto out;
+ if (mac_size != RPMB_MAC_SIZE)
+ ret = 0;
+
+ ret = 1;
+out:
+ HMAC_CTX_free(ctx);
+ return ret == 1 ? 0 : -1;
+}
+
+
+static int vrpmb_check_mac(const unsigned char *key,
+ struct virtio_rpmb_frame *frames_out,
+ unsigned int cnt_out)
+{
+ unsigned char mac[RPMB_MAC_SIZE];
+
+ if (cnt_out == 0) {
+ rpmb_err("RPMB 0 output frames.\n");
+ return -1;
+ }
+
+ vrpmb_calc_hmac_sha256(frames_out, cnt_out,
+ key, RPMB_KEY_SIZE,
+ mac, RPMB_MAC_SIZE);
+
+ if (memcmp(mac, frames_out[cnt_out - 1].key_mac, RPMB_MAC_SIZE)) {
+ rpmb_err("RPMB hmac mismatch:\n");
+ dump_hex_buffer("Result MAC: ",
+ frames_out[cnt_out - 1].key_mac, RPMB_MAC_SIZE);
+ dump_hex_buffer("Expected MAC: ", mac, RPMB_MAC_SIZE);
+ return -1;
+ }
+
+ return 0;
+}
+
+/* Compute the frames MAC and insert it */
+static void vrpmb_compute_mac(const unsigned char *key,
+ struct virtio_rpmb_frame *frame)
+{
+ vrpmb_calc_hmac_sha256(frame, 1, key, RPMB_KEY_SIZE, &frame->key_mac, RPMB_MAC_SIZE);
+}
+
+/*
+ * VirtIO RPMB Bits
+ */
+
+static const char *vrpmb_op_str(uint16_t op)
+{
+#define RPMB_OP(_op) case VIRTIO_RPMB_REQ_##_op: return #_op
+
+ switch (op) {
+ RPMB_OP(PROGRAM_KEY);
+ RPMB_OP(GET_WRITE_COUNTER);
+ RPMB_OP(DATA_WRITE);
+ RPMB_OP(DATA_READ);
+ RPMB_OP(RESULT_READ);
+ break;
+ default:
+ return "unknown";
+ }
+#undef RPMB_OP
+}
+
+static const char *vrpmb_result_str(uint16_t result)
+{
+#define str(x) #x
+#define RPMB_ERR(_res) case VIRTIO_RPMB_RES_##_res: \
+ { if (result & VIRTIO_RPMB_RES_WRITE_COUNTER_EXPIRED) \
+ return "COUNTER_EXPIRE:" str(_res); \
+ else \
+ return str(_res); \
+ }
+
+ switch (result & 0x000F) {
+ RPMB_ERR(OK);
+ RPMB_ERR(GENERAL_FAILURE);
+ RPMB_ERR(AUTH_FAILURE);
+ RPMB_ERR(COUNT_FAILURE);
+ RPMB_ERR(ADDR_FAILURE);
+ RPMB_ERR(WRITE_FAILURE);
+ RPMB_ERR(READ_FAILURE);
+ RPMB_ERR(NO_AUTH_KEY);
+ break;
+ default:
+ return "unknown";
+ }
+#undef RPMB_ERR
+#undef str
+};
+
+#define RPMB_REQ2RESP(_OP) ((_OP) << 8)
+#define RPMB_RESP2REQ(_OP) ((_OP) >> 8)
+
+static void vrpmb_dump_frame(const char *title, const struct virtio_rpmb_frame *f)
+{
+ uint16_t result, req_resp;
+
+ if (!verbose)
+ return;
+
+ if (!f)
+ return;
+
+ result = be16toh(f->result);
+ req_resp = be16toh(f->req_resp);
+ if (req_resp & 0xf00)
+ req_resp = RPMB_RESP2REQ(req_resp);
+
+ fprintf(stderr, "--------------- %s ---------------\n",
+ title ? title : "start");
+ fprintf(stderr, "ptr: %p\n", f);
+ dump_hex_buffer("key_mac: ", f->key_mac, 32);
+ dump_hex_buffer("data: ", f->data, 256);
+ dump_hex_buffer("nonce: ", f->nonce, 16);
+ fprintf(stderr, "write_counter: %u\n", be32toh(f->write_counter));
+ fprintf(stderr, "address: %0X\n", be16toh(f->address));
+ fprintf(stderr, "block_count: %u\n", be16toh(f->block_count));
+ fprintf(stderr, "result %s:%d\n", vrpmb_result_str(result), result);
+ fprintf(stderr, "req_resp %s\n", vrpmb_op_str(req_resp));
+ fprintf(stderr, "--------------- End ---------------\n");
+}
+
+static bool vrpmb_check_req_resp(uint16_t req, struct virtio_rpmb_frame *f)
+{
+ uint16_t req_resp = be16toh(f->req_resp);
+
+ if (RPMB_REQ2RESP(req) != req_resp) {
+ rpmb_err("RPMB response mismatch %04X != %04X\n.",
+ RPMB_REQ2RESP(req), req_resp);
+ return false;
+ }
+
+ rpmb_msg("validated response: 0x%x\n", req_resp);
+ return true;
+}
+
+
+
+static struct virtio_rpmb_frame * vrpmb_alloc_frames(int n)
+{
+ struct virtio_rpmb_frame *frames;
+
+ frames = calloc(n, sizeof(struct virtio_rpmb_frame));
+ if (frames)
+ memset(frames, 0, n * sizeof(struct virtio_rpmb_frame));
+ return frames;
+}
+
+static int vrpmb_program_key(int dev_fd, void *key)
+{
+ struct rpmb_ioc_reqresp_cmd cmd;
+ struct virtio_rpmb_frame *out, *in;
+ int ret;
+
+ out = vrpmb_alloc_frames(2);
+
+ /* construct outgoing frames */
+ out[0].req_resp = htobe16(VIRTIO_RPMB_REQ_PROGRAM_KEY);
+ out[0].block_count = htobe16(1);
+ memcpy(&out[0].key_mac[0], key, RPMB_MAC_SIZE);
+ RAND_bytes((void *) &out[0].nonce, RPMB_NONCE_SIZE);
+ out[1].req_resp = htobe16(VIRTIO_RPMB_REQ_RESULT_READ);
+ RAND_bytes((void *) &out[1].nonce, RPMB_NONCE_SIZE);
+
+ cmd.len = sizeof(struct virtio_rpmb_frame) * 2;
+ cmd.request = (void *) out;
+
+ vrpmb_dump_frame("pkey", &out[0]);
+ vrpmb_dump_frame("request", &out[1]);
+
+ /* space for response */
+ in = vrpmb_alloc_frames(1);
+ cmd.rlen = sizeof(struct virtio_rpmb_frame);
+ cmd.response = (void *) in;
+
+ /* do it */
+ ret = ioctl(dev_fd, RPMB_IOC_PKEY_CMD, &cmd);
+ if (ret < 0) {
+ rpmb_err("pkey ioctl failure %d: %s.\n", ret, strerror(errno));
+ goto out;
+ }
+
+ vrpmb_dump_frame("response", in);
+
+ /* validate response */
+ if (!vrpmb_check_req_resp(VIRTIO_RPMB_REQ_PROGRAM_KEY, in)) {
+ ret = -1;
+ goto out;
+ }
+
+ ret = vrpmb_check_mac(key, in, 1);
+ if (ret) {
+ rpmb_err("%s: check mac error %d\n", __func__, ret);
+ goto out;
+ }
+
+out:
+ if (ret)
+ rpmb_err("RPMB operation %s failed=%d %s[0x%04x]\n",
+ vrpmb_op_str(out->req_resp), ret,
+ vrpmb_result_str(in->result), in->result);
+
+ free(in);
+ free(out);
+ return ret;
+}
+
+static int vrpmb_get_write_counter(int dev_fd, void *key)
+{
+ struct rpmb_ioc_reqresp_cmd cmd;
+ struct virtio_rpmb_frame *out, *in = NULL;
+ int ret;
+
+ out = vrpmb_alloc_frames(1);
+ in = vrpmb_alloc_frames(1);
+ if (!out || !in) {
+ rpmb_err("couldn't allocate frames");
+ return -ENOMEM;
+ }
+
+ /* Query frame */
+ out[0].req_resp = htobe16(VIRTIO_RPMB_REQ_GET_WRITE_COUNTER);
+ out[0].block_count = htobe16(1);
+ RAND_bytes((void *) &out[0].nonce, RPMB_NONCE_SIZE);
+
+ cmd.request = (void *) out;
+ cmd.len = sizeof(struct virtio_rpmb_frame);
+ cmd.response = (void *) in;
+ cmd.rlen = sizeof(struct virtio_rpmb_frame);
+
+ ret = ioctl(dev_fd, RPMB_IOC_COUNTER_CMD, &cmd);
+ if (ret < 0) {
+ rpmb_err("get wcount ioctl failure %d: %s.\n", ret,
+ strerror(errno));
+ goto out;
+ }
+
+ vrpmb_dump_frame("write_counter", in);
+
+ ret = be32toh(in->write_counter);
+ rpmb_msg("counter 0x%x\n", ret);
+
+ /* validate response */
+ if (!vrpmb_check_req_resp(VIRTIO_RPMB_REQ_GET_WRITE_COUNTER, in)) {
+ ret = -1;
+ goto out;
+ }
+
+ if (memcmp(&in->nonce, &out[0].nonce, RPMB_NONCE_SIZE)) {
+ rpmb_err("RPMB NONCE mismatch\n");
+ dump_hex_buffer("Result NONCE:",
+ &in->nonce, RPMB_NONCE_SIZE);
+ dump_hex_buffer("Expected NONCE: ",
+ &out[0].nonce, RPMB_NONCE_SIZE);
+ ret = -1;
+ goto out;
+ }
+
+ if (key) {
+ ret = vrpmb_check_mac(key, in, 1);
+ if (ret)
+ rpmb_err("%s: check mac error %d\n", __func__, ret);
+ }
+
+ ret = be32toh(in->write_counter);
+ rpmb_msg("counter 0x%x\n", ret);
+
+out:
+ free(out);
+ free(in);
+ return ret;
+
+}
+
+static int vrpmb_write_blocks(int dev_fd, void *key, void *data, int addr, int len)
+{
+ struct rpmb_ioc_reqresp_cmd cmd;
+ struct virtio_rpmb_frame *out, *in = NULL;
+ int frames = (len / 256) + 1;
+ uint8_t *p = (uint8_t *) data;
+ int i, ret;
+ int write_count = vrpmb_get_write_counter(dev_fd, key);
+
+ out = vrpmb_alloc_frames(frames);
+ in = vrpmb_alloc_frames(1);
+ if (!out || !in) {
+ rpmb_err("couldn't allocate frames");
+ return -ENOMEM;
+ }
+
+ /* First frame */
+ out[0].req_resp = htobe16(VIRTIO_RPMB_REQ_DATA_WRITE);
+ out[0].block_count = htobe16(frames - 1);
+ out[0].address = htobe16(addr);
+ out[0].write_counter = htobe32(write_count);
+
+ /* Copy data to write and prepare frames */
+ for (i = 0; i < frames; i++) {
+ struct virtio_rpmb_frame *f = &out[i];
+
+ memcpy(&f->data, &p[i * 256], 256);
+ RAND_bytes((void *) &f->nonce, RPMB_NONCE_SIZE);
+ vrpmb_compute_mac(key, f);
+ }
+
+ vrpmb_dump_frame("write_blocks", &out[0]);
+
+ /* Response request */
+ out[frames - 1].req_resp = htobe16(VIRTIO_RPMB_REQ_RESULT_READ);
+ out[frames - 1].block_count = htobe16(1);
+ RAND_bytes((void *) &out[frames - 1].nonce, RPMB_NONCE_SIZE);
+ vrpmb_compute_mac(key, &out[frames - 1]);
+
+ vrpmb_dump_frame("result_req", &out[frames - 1]);
+
+ cmd.request = (void *) out;
+ cmd.len = frames * sizeof(struct virtio_rpmb_frame);
+ cmd.response = (void *) in;
+ cmd.rlen = sizeof(struct virtio_rpmb_frame);
+
+ ret = ioctl(dev_fd, RPMB_IOC_WBLOCKS_CMD, &cmd);
+ if (ret < 0) {
+ rpmb_err("wblocks ioctl failure %d: %s.\n", ret,
+ strerror(errno));
+ }
+
+ free(out);
+ free(in);
+ return ret;
+}
+
+/*
+ * To read blocks we receive a bunch of frames from the vrpmb device
+ * which we need to validate and extract the actual data into
+ * requested buffer.
+ */
+static int vrpmb_read_blocks(int dev_fd, void *key, int addr, int count, void *data, int len)
+{
+ struct rpmb_ioc_rblocks_cmd cmd;
+ int frame_length = count * sizeof(struct virtio_rpmb_frame);
+ struct virtio_rpmb_frame *frames = malloc(frame_length);
+ int i, ret = -1;
+
+ rpmb_dbg("%s: reading %d blocks into %d bytes (%d bytes of frames)\n",
+ __func__, count, len, frame_length);
+
+ if (!frames) {
+ rpmb_err("unable to allocate memory for frames");
+ return -1;
+ }
+
+ cmd.addr = addr;
+ cmd.count = count;
+ cmd.len = frame_length;
+ cmd.data = (__u8 *) frames;
+
+ ret = ioctl(dev_fd, RPMB_IOC_RBLOCKS_CMD, &cmd);
+ if (ret < 0) {
+ rpmb_err("rblocks ioctl failure %d: %s.\n", ret,
+ strerror(errno));
+ goto out;
+ }
+
+ for (i = 0; i < count; i++) {
+ struct virtio_rpmb_frame *f = &frames[i];
+
+ vrpmb_dump_frame("block data", f);
+
+ if (key) {
+ ret = vrpmb_check_mac(key, f, 1);
+ if (ret) {
+ rpmb_err("%s: check mac error frame %d/%d\n", __func__, i, ret);
+ break;
+ }
+ }
+
+ memcpy(data, &f->data, RPMB_BLOCK_SIZE);
+ data += RPMB_BLOCK_SIZE;
+ }
+ ret = 0;
+
+ out:
+ free(frames);
+ return ret;
+}
+
+/*
+ * Generic RPMB bits
+ */
+static int op_get_info(int nargs, char *argv[])
+{
+ int dev_fd;
+ struct rpmb_ioc_cap_cmd cap;
+
+ if (nargs != 1)
+ return -EINVAL;
+
+ memset(&cap, 0, sizeof(cap));
+ dev_fd = open_dev_file(argv[0], &cap);
+ if (dev_fd < 0)
+ return -errno;
+ argv++;
+
+ printf("RPMB rpmb_target = %d\n", cap.target);
+ printf("RPMB capacity = %d\n", cap.capacity);
+ printf("RPMB block_size = %d\n", cap.block_size);
+ printf("RPMB wr_cnt_max = %d\n", cap.wr_cnt_max);
+ printf("RPMB rd_cnt_max = %d\n", cap.rd_cnt_max);
+ printf("RPMB auth_method = %d\n", cap.auth_method);
+
+ close(dev_fd);
+
+ return 0;
+}
+
+static void *read_key(const char *path)
+{
+ int key_fd = open_rd_file(path, "key file");
+ void *key;
+
+ if (key_fd < 0)
+ return NULL;
+
+ key = malloc(RPMB_KEY_SIZE);
+
+ if (!key) {
+ rpmb_err("out of memory for key\n");
+ return NULL;
+ }
+
+ if (read(key_fd, key, RPMB_KEY_SIZE) != RPMB_KEY_SIZE) {
+ rpmb_err("couldn't read key (%s)\n", strerror(errno));
+ return NULL;
+ }
+
+ close(key_fd);
+ return key;
+}
+
+static int op_rpmb_program_key(int nargs, char *argv[])
+{
+ int ret = -EINVAL, fd = -1;
+ struct rpmb_ioc_cap_cmd cap;
+ void *key;
+
+ if (nargs < 1 || nargs > 2)
+ return ret;
+
+ fd = open_dev_file(argv[0], &cap);
+ if (fd < 0) {
+ perror("opening RPMB device");
+ return ret;
+ }
+ argv++;
+
+ key = read_key(argv[0]);
+
+ if (key)
+ ret = vrpmb_program_key(fd, key);
+
+ close_fd(fd);
+ return ret;
+}
+
+
+static int op_rpmb_get_write_counter(int nargs, char **argv)
+{
+ int ret, fd = -1;
+ struct rpmb_ioc_cap_cmd cap;
+ void *key = NULL;
+
+ ret = -EINVAL;
+ if (nargs < 1 || nargs > 2)
+ return ret;
+
+ fd = open_dev_file(argv[0], &cap);
+ if (fd < 0) {
+ perror("opening RPMB device");
+ return ret;
+ }
+ argv++;
+
+ if (argv[0]) {
+ key = read_key(argv[0]);
+ if (!key)
+ rpmb_err("failed to read key data");
+ }
+
+ ret = vrpmb_get_write_counter(fd, key);
+ if (ret < 0) {
+ rpmb_err("counter ioctl failure %d: %s.\n", ret, strerror(errno));
+ } else {
+ printf("Counter value is: %d\n", ret);
+ ret = 0;
+ }
+
+ close_fd(fd);
+ return ret;
+}
+
+static int op_rpmb_read_blocks(int nargs, char **argv)
+{
+ int ret, data_fd, fd = -1;
+ struct rpmb_ioc_cap_cmd cap;
+ unsigned long numarg;
+ uint16_t addr, blocks_cnt;
+ void *key = NULL;
+
+ ret = -EINVAL;
+ if (nargs < 4 || nargs > 5)
+ return ret;
+
+ fd = open_dev_file(argv[0], &cap);
+ if (fd < 0) {
+ perror("opening RPMB device");
+ return ret;
+ }
+ argv++;
+
+ errno = 0;
+ numarg = strtoul(argv[0], NULL, 0);
+ if (errno || numarg > USHRT_MAX) {
+ rpmb_err("wrong block address\n");
+ goto out;
+ }
+ addr = (uint16_t)numarg;
+ argv++;
+
+ errno = 0;
+ numarg = strtoul(argv[0], NULL, 0);
+ if (errno || numarg > USHRT_MAX) {
+ rpmb_err("wrong blocks count\n");
+ goto out;
+ }
+ blocks_cnt = (uint16_t)numarg;
+ argv++;
+
+ if (blocks_cnt == 0) {
+ rpmb_err("wrong blocks count\n");
+ goto out;
+ }
+
+ data_fd = open_wr_file(argv[0], "output data");
+ if (data_fd < 0)
+ goto out;
+ argv++;
+
+ if (argv[0]) {
+ key = read_key(argv[0]);
+ if (!key) {
+ rpmb_err("failed to read key data");
+ goto out;
+ }
+ }
+
+ while (blocks_cnt > 0) {
+ int to_copy = min(blocks_cnt, cap.rd_cnt_max);
+ int length = to_copy * RPMB_BLOCK_SIZE;
+ void *data = malloc(length);
+
+ if (!data) {
+ ret = ENOMEM;
+ goto out;
+ }
+
+ vrpmb_read_blocks(fd, key, addr, to_copy, data, length);
+
+ ret = write_file(data_fd, data, length);
+ if (ret < 0) {
+ perror("writing data");
+ goto out;
+ } else {
+ rpmb_dbg("wrote %d bytes/%d blocks to file\n", length, to_copy);
+ }
+
+ free(data);
+ addr += to_copy;
+ blocks_cnt -= to_copy;
+ }
+
+ ret = 0;
+out:
+ close_fd(fd);
+ close_fd(data_fd);
+
+ return ret;
+}
+
+static int op_rpmb_write_blocks(int nargs, char **argv)
+{
+ int ret, data_fd, fd = -1;
+ struct rpmb_ioc_cap_cmd cap;
+ unsigned long numarg;
+ uint16_t addr, blocks_cnt;
+ void *key;
+
+ ret = -EINVAL;
+ if (nargs < 4 || nargs > 5)
+ return ret;
+
+ fd = open_dev_file(argv[0], &cap);
+ if (fd < 0) {
+ perror("opening RPMB device");
+ return ret;
+ }
+ argv++;
+
+ errno = 0;
+ numarg = strtoul(argv[0], NULL, 0);
+ if (errno || numarg > USHRT_MAX) {
+ rpmb_err("wrong block address\n");
+ goto out;
+ }
+ addr = (uint16_t)numarg;
+ argv++;
+
+ errno = 0;
+ numarg = strtoul(argv[0], NULL, 0);
+ if (errno || numarg > USHRT_MAX) {
+ rpmb_err("wrong blocks count\n");
+ goto out;
+ }
+ blocks_cnt = (uint16_t)numarg;
+ argv++;
+
+ if (blocks_cnt == 0) {
+ rpmb_err("wrong blocks count\n");
+ goto out;
+ }
+
+ data_fd = open_rd_file(argv[0], "input data");
+ if (data_fd < 0)
+ goto out;
+ argv++;
+
+ key = read_key(nargs == 5 ? argv[0] : NULL);
+ if (!key) {
+ rpmb_err("failed to read key data");
+ goto out;
+ }
+
+ while (blocks_cnt > 0) {
+ int to_copy = min(blocks_cnt, cap.wr_cnt_max);
+ int length = to_copy * 256;
+ void *data = malloc(length);
+
+ if (!data) {
+ ret = ENOMEM;
+ goto out;
+ }
+
+ ret = read_file(data_fd, data, length);
+ if (ret < 0) {
+ perror("reading data");
+ goto out;
+ }
+
+ ret = vrpmb_write_blocks(fd, key, data, addr, length);
+ if (ret < 0) {
+ rpmb_err("wblocks ioctl failure %d: %s.\n", ret,
+ strerror(errno));
+ goto out;
+ }
+
+ free(data);
+ addr += to_copy;
+ blocks_cnt -= to_copy;
+ }
+
+ ret = 0;
+out:
+ close_fd(fd);
+ close_fd(data_fd);
+
+ return ret;
+}
+
+typedef int (*rpmb_op)(int argc, char *argv[]);
+
+struct rpmb_cmd {
+ const char *op_name;
+ rpmb_op op;
+ const char *usage; /* usage title */
+ const char *help; /* help */
+};
+
+static const struct rpmb_cmd cmds[] = {
+ {
+ "get-info",
+ op_get_info,
+ "<RPMB_DEVICE>",
+ " Get RPMB device info\n",
+ },
+ {
+ "program-key",
+ op_rpmb_program_key,
+ "<RPMB_DEVICE> <KEYFILE>",
+ " Program authentication KEYFILE\n"
+ " NOTE: This is a one-time programmable irreversible change.\n",
+ },
+ {
+ "write-counter",
+ op_rpmb_get_write_counter,
+ "<RPMB_DEVICE>",
+ " Rertrive write counter value from the <RPMB_DEVICE> to stdout.\n"
+ },
+ {
+ "write-blocks",
+ op_rpmb_write_blocks,
+ "<RPMB_DEVICE> <address> <block_count> <DATA_FILE> <KEYID>",
+ " <block count> of 256 bytes will be written from the DATA_FILE\n"
+ " to the <RPMB_DEVICE> at block offset <address>.\n"
+ " When DATA_FILE is -, read from standard input.\n",
+ },
+ {
+ "read-blocks",
+ op_rpmb_read_blocks,
+ "<RPMB_DEVICE> <address> <blocks count> <OUTPUT_FILE>",
+ " <block count> of 256 bytes will be read from <RPMB_DEVICE>\n"
+ " to the OUTPUT_FILE\n"
+ " When OUTPUT_FILE is -, write to standard output\n",
+ },
+
+ { NULL, NULL, NULL, NULL }
+};
+
+static void help(const char *prog, const struct rpmb_cmd *cmd)
+{
+ printf("%s %s %s\n", prog, cmd->op_name, cmd->usage);
+ printf("%s\n", cmd->help);
+}
+
+static void usage(const char *prog)
+{
+ int i;
+
+ printf("\n");
+ printf("Usage: %s [-v] <command> <args>\n\n", prog);
+ for (i = 0; cmds[i].op_name; i++)
+ printf(" %s %s %s\n",
+ prog, cmds[i].op_name, cmds[i].usage);
+
+ printf("\n");
+ printf(" %s -v/--verbose: runs in verbose mode\n", prog);
+ printf(" %s help : shows this help\n", prog);
+ printf(" %s help <command>: shows detailed help\n", prog);
+}
+
+static bool call_for_help(const char *arg)
+{
+ return !strcmp(arg, "help") ||
+ !strcmp(arg, "-h") ||
+ !strcmp(arg, "--help");
+}
+
+static bool parse_verbose(const char *arg)
+{
+ return !strcmp(arg, "-v") ||
+ !strcmp(arg, "--verbose");
+}
+
+static const
+struct rpmb_cmd *parse_args(const char *prog, int *_argc, char **_argv[])
+{
+ int i;
+ int argc = *_argc;
+ char **argv = *_argv;
+ const struct rpmb_cmd *cmd = NULL;
+ bool need_help = false;
+
+ argc--; argv++;
+
+ if (argc == 0)
+ goto out;
+
+ if (call_for_help(argv[0])) {
+ argc--; argv++;
+ if (argc == 0)
+ goto out;
+
+ need_help = true;
+ }
+
+ if (parse_verbose(argv[0])) {
+ argc--; argv++;
+ if (argc == 0)
+ goto out;
+
+ verbose = true;
+ }
+
+ for (i = 0; cmds[i].op_name; i++) {
+ if (!strncmp(argv[0], cmds[i].op_name,
+ strlen(cmds[i].op_name))) {
+ cmd = &cmds[i];
+ argc--; argv++;
+ break;
+ }
+ }
+
+ if (!cmd)
+ goto out;
+
+ if (need_help || (argc > 0 && call_for_help(argv[0]))) {
+ help(prog, cmd);
+ argc--; argv++;
+ return NULL;
+ }
+
+out:
+ *_argc = argc;
+ *_argv = argv;
+
+ if (!cmd)
+ usage(prog);
+
+ return cmd;
+}
+
+int main(int argc, char *argv[])
+{
+ const char *prog = basename(argv[0]);
+ const struct rpmb_cmd *cmd;
+ int ret;
+
+ cmd = parse_args(prog, &argc, &argv);
+ if (!cmd)
+ exit(EXIT_SUCCESS);
+
+ ret = cmd->op(argc, argv);
+ if (ret == -EINVAL)
+ help(prog, cmd);
+
+ exit(ret);
+}
diff --git a/tools/rpmb/test.sh b/tools/rpmb/test.sh
new file mode 100755
index 000000000000..2cbba5529618
--- /dev/null
+++ b/tools/rpmb/test.sh
@@ -0,0 +1,22 @@
+#!/bin/sh -e
+# SPDX-License-Identifier: GPL-2.0+
+echo "get info"
+./rpmb -v get-info /dev/rpmb0
+echo "program key"
+./rpmb -v program-key /dev/rpmb0 key
+echo "get write counter"
+./rpmb -v write-counter /dev/rpmb0
+echo "get write counter (and verify)"
+./rpmb -v write-counter /dev/rpmb0 key
+echo "generating data"
+dd if=/dev/urandom of=data.in count=4 bs=256
+echo "write data"
+./rpmb -v write-blocks /dev/rpmb0 0 4 data.in key
+echo "read data back"
+rm -f data.out
+./rpmb -v read-blocks /dev/rpmb0 0 4 data.out
+cmp data.in data.out
+echo "read data back with key check"
+truncate -s 0 data.out
+./rpmb -v read-blocks /dev/rpmb0 0 4 data.out key
+cmp data.in data.out
--
2.30.2
^ permalink raw reply related [flat|nested] 29+ messages in thread* Re: [PATCH v2 4/4] tools rpmb: add RPBM access tool
2022-04-05 9:37 ` [PATCH v2 4/4] tools rpmb: add RPBM access tool Alex Bennée
@ 2022-06-16 13:13 ` Harald Mommer
0 siblings, 0 replies; 29+ messages in thread
From: Harald Mommer @ 2022-06-16 13:13 UTC (permalink / raw)
To: Alex Bennée, linux-kernel
Cc: maxim.uvarov, joakim.bech, ulf.hansson, ilias.apalodimas, arnd,
ruchika.gupta, tomas.winkler, yang.huang, bing.zhu, Matti.Moell,
linux-mmc, linux-scsi, Alexander Usyskin
Hello,
On 05.04.22 11:37, Alex Bennée wrote:
> +/*
> + * Utility functions
> + */
> +static int open_dev_file(const char *devfile, struct rpmb_ioc_cap_cmd *cap)
> +{
> ...
> + rpmb_dbg("RPMB rpmb_target = %d\n", cap->target);
> + rpmb_dbg("RPMB capacity = %d\n", cap->capacity);
> + rpmb_dbg("RPMB block_size = %d\n", cap->block_size);
> + rpmb_dbg("RPMB wr_cnt_max = %d\n", cap->wr_cnt_max);
> + rpmb_dbg("RPMB rd_cnt_max = %d\n", cap->rd_cnt_max);
> + rpmb_dbg("RPMB auth_method = %d\n", cap->auth_method);
The previously present device_type has already gone. Do we loose an
opportunity to inform the user about anything which may not be virtio
RPMB in the future or is this intentional? A new device type VIRTIO_RPMB
instead and keeping the door open for UFS, NVMe, eMMC etc.?
And if the removal was intentional: rpmb_target, block_size,
auth_method: Still needed and if so for what is it good for?
> ...
> +}
> +
> +static struct virtio_rpmb_frame * vrpmb_alloc_frames(int n)
> +{
> + struct virtio_rpmb_frame *frames;
> +
> + frames = calloc(n, sizeof(struct virtio_rpmb_frame));
> + if (frames)
> + memset(frames, 0, n * sizeof(struct virtio_rpmb_frame));
Very minor: calloc() already zeroes.
> + return frames;
> +}
> +
/*
+ * To read blocks we receive a bunch of frames from the vrpmb device
+ * which we need to validate and extract the actual data into
+ * requested buffer.
+ */
+static int vrpmb_read_blocks(int dev_fd, void *key, int addr, int count, void *data, int len)
+{
+...
+ ret = ioctl(dev_fd, RPMB_IOC_RBLOCKS_CMD, &cmd);
+ if (ret < 0) {
+ rpmb_err("rblocks ioctl failure %d: %s.\n", ret,
+ strerror(errno));
+ goto out;
+ }
The (older) rpmb tool always puzzles me with complaining about the mismatch MAC when there was also a result != VIRTIO_RPMB_RES_OK.
I guess the ioctl() returns 0 if the ioctl() as such succeeded but there is an error code indicated in the frame.
Consider adding to print the result if it indicated a problem.
I can remember the old tool used the result of the last frame for this purpose which was probably not a good choice, not sure why this was that way, probably the first frame would be a better choice.
+ for (i = 0; i < count; i++) {
+ struct virtio_rpmb_frame *f = &frames[i];
+
+ vrpmb_dump_frame("block data", f);
+
+ if (key) {
+ ret = vrpmb_check_mac(key, f, 1);
+ if (ret) {
+ rpmb_err("%s: check mac error frame %d/%d\n", __func__, i, ret);
+ break;
+ }
+ }
+
+ memcpy(data, &f->data, RPMB_BLOCK_SIZE);
+ data += RPMB_BLOCK_SIZE;
+ }
+ ret = 0;
+
+ out:
+ free(frames);
+ return ret;
+}
+
+
+static void *read_key(const char *path)
+{
+ int key_fd = open_rd_file(path, "key file");
+ void *key;
+
+ if (key_fd < 0)
+ return NULL;
+
+ key = malloc(RPMB_KEY_SIZE);
Very minor: There's not a single free() for key in the code. Plays no
role here as the program runs only for a fraction of a second, just saw
it. You are making your life harder as necessary by using dynamic memory
when it can be avoided, all those NULL pointer checks and the free(s)
which have to be done...
> +
> + if (!key) {
> + rpmb_err("out of memory for key\n");
> + return NULL;
> + }
> +
> + if (read(key_fd, key, RPMB_KEY_SIZE) != RPMB_KEY_SIZE) {
> + rpmb_err("couldn't read key (%s)\n", strerror(errno));
> + return NULL;
> + }
> +
> + close(key_fd);
> + return key;
> +}
> +
>
> +
> +static const struct rpmb_cmd cmds[] = {
> + {
> + "get-info",
> + op_get_info,
> + "<RPMB_DEVICE>",
> + " Get RPMB device info\n",
> + },
> + {
> + "program-key",
> + op_rpmb_program_key,
> + "<RPMB_DEVICE> <KEYFILE>",
> + " Program authentication KEYFILE\n"
> + " NOTE: This is a one-time programmable irreversible change.\n",
> + },
> + {
> + "write-counter",
> + op_rpmb_get_write_counter,
> + "<RPMB_DEVICE>",
> + " Rertrive write counter value from the <RPMB_DEVICE> to stdout.\n"
Optional parameter explanation [KEYFILE] got lost in write-counter.
> + },
> + {
> + "write-blocks",
> + op_rpmb_write_blocks,
> + "<RPMB_DEVICE> <address> <block_count> <DATA_FILE> <KEYID>",
> + " <block count> of 256 bytes will be written from the DATA_FILE\n"
> + " to the <RPMB_DEVICE> at block offset <address>.\n"
> + " When DATA_FILE is -, read from standard input.\n",
Puzzling naming change. The KEYID is indeed the <KEYFILE>.
> + },
> + {
> + "read-blocks",
> + op_rpmb_read_blocks,
> + "<RPMB_DEVICE> <address> <blocks count> <OUTPUT_FILE>",
> + " <block count> of 256 bytes will be read from <RPMB_DEVICE>\n"
> + " to the OUTPUT_FILE\n"
> + " When OUTPUT_FILE is -, write to standard output\n",
Here also the optional parameter explanation [KEYFILE] got lost in
read-blocks.
For people not knowing the tool trying to get into RPMB a complete and
consistent help is helpful I can still remember when I was in the
situation to understand more playing around with the (older) tool.
> + },
> +
> + { NULL, NULL, NULL, NULL }
> +};
--
Dipl.-Ing. Harald Mommer
Senior Software Engineer
OpenSynergy GmbH
Rotherstr. 20, 10245 Berlin
Phone: +49 (30) 60 98 540-0 <== Zentrale
Fax: +49 (30) 60 98 540-99
E-Mail: harald.mommer@opensynergy.com
www.opensynergy.com
Handelsregister: Amtsgericht Charlottenburg, HRB 108616B
Geschäftsführer/Managing Director: Regis Adjamah
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2022-04-05 9:37 [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver Alex Bennée
` (3 preceding siblings ...)
2022-04-05 9:37 ` [PATCH v2 4/4] tools rpmb: add RPBM access tool Alex Bennée
@ 2022-04-05 14:54 ` Bean Huo
2022-04-05 15:43 ` Alex Bennée
2022-04-22 14:21 ` Alex Bennée
[not found] ` <20230531191007.13460-1-shyamsaini@linux.microsoft.com>
6 siblings, 1 reply; 29+ messages in thread
From: Bean Huo @ 2022-04-05 14:54 UTC (permalink / raw)
To: Alex Bennée, linux-kernel
Cc: maxim.uvarov, joakim.bech, ulf.hansson, ilias.apalodimas, arnd,
ruchika.gupta, tomas.winkler, yang.huang, bing.zhu, Matti.Moell,
hmo, linux-mmc, linux-scsi
Hi Alex,
Thanks for this unified RPMB interface, I wanted to verify this on our
UFS, it seems you didn't add the UFS access interface in this version
from your userspace tools, right?
Kind regards,
Bean
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2022-04-05 14:54 ` [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver Bean Huo
@ 2022-04-05 15:43 ` Alex Bennée
2022-04-05 17:03 ` Bean Huo
0 siblings, 1 reply; 29+ messages in thread
From: Alex Bennée @ 2022-04-05 15:43 UTC (permalink / raw)
To: Bean Huo
Cc: linux-kernel, maxim.uvarov, joakim.bech, ulf.hansson,
ilias.apalodimas, arnd, ruchika.gupta, tomas.winkler, yang.huang,
bing.zhu, Matti.Moell, hmo, linux-mmc, linux-scsi
Bean Huo <huobean@gmail.com> writes:
> Hi Alex,
>
> Thanks for this unified RPMB interface, I wanted to verify this on our
> UFS, it seems you didn't add the UFS access interface in this version
> from your userspace tools, right?
No I didn't but it should be easy enough to add some function pointer
redirection everywhere one of the op_* functions calls a vrpmb_*
function. Do you already have a UFS RPMB device driver?
--
Alex Bennée
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2022-04-05 15:43 ` Alex Bennée
@ 2022-04-05 17:03 ` Bean Huo
2022-04-06 11:22 ` Alex Bennée
0 siblings, 1 reply; 29+ messages in thread
From: Bean Huo @ 2022-04-05 17:03 UTC (permalink / raw)
To: Alex Bennée
Cc: linux-kernel, maxim.uvarov, joakim.bech, ulf.hansson,
ilias.apalodimas, arnd, ruchika.gupta, tomas.winkler, yang.huang,
bing.zhu, Matti.Moell, hmo, linux-mmc, linux-scsi
On Tue, 2022-04-05 at 16:43 +0100, Alex Bennée wrote:
>
> Bean Huo <huobean@gmail.com> writes:
>
> > Hi Alex,
> >
> > Thanks for this unified RPMB interface, I wanted to verify this on
> > our
> > UFS, it seems you didn't add the UFS access interface in this
> > version
> > from your userspace tools, right?
>
> No I didn't but it should be easy enough to add some function pointer
> redirection everywhere one of the op_* functions calls a vrpmb_*
> function. Do you already have a UFS RPMB device driver?
>
Hi Alex,
Thanks for your feedback.
We now access UFS RPMB through the RPMB LUN BSG device, RPMB is a well-
known LU and we have a userspace tool to access it.
I see that if we're going to use your interface, "static struct
rpmb_ops" should be registered from a lower-level driver, for example
in a UFS driver, yes there should be no problem with this registration,
but I don't know with the current way Compared, what are the advantages
to add a driver. maybe the main advantage is that we will have an
unified user space tool for RPMB. right?
Kind regards,
Bean
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2022-04-05 17:03 ` Bean Huo
@ 2022-04-06 11:22 ` Alex Bennée
2022-04-06 17:19 ` Bean Huo
2022-04-06 17:27 ` Bean Huo
0 siblings, 2 replies; 29+ messages in thread
From: Alex Bennée @ 2022-04-06 11:22 UTC (permalink / raw)
To: Bean Huo
Cc: linux-kernel, maxim.uvarov, joakim.bech, ulf.hansson,
ilias.apalodimas, arnd, ruchika.gupta, tomas.winkler, yang.huang,
bing.zhu, Matti.Moell, hmo, linux-mmc, linux-scsi
Bean Huo <huobean@gmail.com> writes:
> On Tue, 2022-04-05 at 16:43 +0100, Alex Bennée wrote:
>>
>> Bean Huo <huobean@gmail.com> writes:
>>
>> > Hi Alex,
>> >
>> > Thanks for this unified RPMB interface, I wanted to verify this on
>> > our
>> > UFS, it seems you didn't add the UFS access interface in this
>> > version
>> > from your userspace tools, right?
>>
>> No I didn't but it should be easy enough to add some function pointer
>> redirection everywhere one of the op_* functions calls a vrpmb_*
>> function. Do you already have a UFS RPMB device driver?
>>
>
> Hi Alex,
> Thanks for your feedback.
>
> We now access UFS RPMB through the RPMB LUN BSG device, RPMB is a well-
> known LU and we have a userspace tool to access it.
>
> I see that if we're going to use your interface, "static struct
> rpmb_ops" should be registered from a lower-level driver, for example
> in a UFS driver, yes there should be no problem with this registration,
> but I don't know with the current way Compared, what are the advantages
> to add a driver. maybe the main advantage is that we will have an
> unified user space tool for RPMB. right?
Pretty much. The main issue for virtio-rpmb is it doesn't really fit
neatly into the block stack because all it does is the RPMB part so a
non-block orientate API makes sense.
Can you point be to where the UFS driver does it's current RPMB stuff?
>
> Kind regards,
> Bean
--
Alex Bennée
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2022-04-06 11:22 ` Alex Bennée
@ 2022-04-06 17:19 ` Bean Huo
2022-04-06 17:32 ` Bart Van Assche
2022-04-06 17:27 ` Bean Huo
1 sibling, 1 reply; 29+ messages in thread
From: Bean Huo @ 2022-04-06 17:19 UTC (permalink / raw)
To: Alex Bennée
Cc: linux-kernel, maxim.uvarov, joakim.bech, ulf.hansson,
ilias.apalodimas, arnd, ruchika.gupta, tomas.winkler, yang.huang,
bing.zhu, Matti.Moell, hmo, linux-mmc, linux-scsi, bvanassche
On Wed, 2022-04-06 at 12:22 +0100, Alex Bennée wrote:
>
> Bean Huo <huobean@gmail.com> writes:
>
> > On Tue, 2022-04-05 at 16:43 +0100, Alex Bennée wrote:
> > >
> > > Bean Huo <huobean@gmail.com> writes:
> > >
> > > > Hi Alex,
> > > >
> > > > Thanks for this unified RPMB interface, I wanted to verify this
> > > > on
> > > > our
> > > > UFS, it seems you didn't add the UFS access interface in this
> > > > version
> > > > from your userspace tools, right?
> > >
> > > No I didn't but it should be easy enough to add some function
> > > pointer
> > > redirection everywhere one of the op_* functions calls a vrpmb_*
> > > function. Do you already have a UFS RPMB device driver?
> > >
> >
> > Hi Alex,
> > Thanks for your feedback.
> >
> > We now access UFS RPMB through the RPMB LUN BSG device, RPMB is a
> > well-
> > known LU and we have a userspace tool to access it.
> >
> > I see that if we're going to use your interface, "static struct
> > rpmb_ops" should be registered from a lower-level driver, for
> > example
> > in a UFS driver, yes there should be no problem with this
> > registration,
> > but I don't know with the current way Compared, what are the
> > advantages
> > to add a driver. maybe the main advantage is that we will have an
> > unified user space tool for RPMB. right?
>
> Pretty much. The main issue for virtio-rpmb is it doesn't really fit
> neatly into the block stack because all it does is the RPMB part so a
> non-block orientate API makes sense.
>
> Can you point be to where the UFS driver does it's current RPMB
> stuff?
>
It's the SCSI BSG driver, in fact, we don't have a dedicated UFS RPMB
driver in the kernel. RPMB is a well known LU, we are using userspace
tools to issue SCSI commands directly to the UFS RPMB LU via ioctl()
from the BSG device node in the /dev/sg/ folder.
Here is the BSG part of the code in the userspace tools:
io_hdr_v4.guard = 'Q';
io_hdr_v4.protocol = BSG_PROTOCOL_SCSI;
io_hdr_v4.subprotocol = BSG_SUB_PROTOCOL_SCSI_CMD;
io_hdr_v4.response = (__u64)sense_buffer;
io_hdr_v4.max_response_len = SENSE_BUFF_LEN;
io_hdr_v4.request_len = cmd_len;
io_hdr_v4.request = (__u64)cdb;
ioctl(fd, SG_IO, &io_hdr_v4))
...
> >
> > Kind regards,
> > Bean
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2022-04-06 17:19 ` Bean Huo
@ 2022-04-06 17:32 ` Bart Van Assche
2022-04-06 18:12 ` Bean Huo
0 siblings, 1 reply; 29+ messages in thread
From: Bart Van Assche @ 2022-04-06 17:32 UTC (permalink / raw)
To: Bean Huo, Alex Bennée
Cc: linux-kernel, maxim.uvarov, joakim.bech, ulf.hansson,
ilias.apalodimas, arnd, ruchika.gupta, tomas.winkler, yang.huang,
bing.zhu, Matti.Moell, hmo, linux-mmc, linux-scsi
On 4/6/22 10:19, Bean Huo wrote:
> On Wed, 2022-04-06 at 12:22 +0100, Alex Bennée wrote:
>>
>> Bean Huo <huobean@gmail.com> writes:
>>
>>> On Tue, 2022-04-05 at 16:43 +0100, Alex Bennée wrote:
>>>>
>>>> Bean Huo <huobean@gmail.com> writes:
>>>>
>>>>> Hi Alex,
>>>>>
>>>>> Thanks for this unified RPMB interface, I wanted to verify this
>>>>> on
>>>>> our
>>>>> UFS, it seems you didn't add the UFS access interface in this
>>>>> version
>>>>> from your userspace tools, right?
>>>>
>>>> No I didn't but it should be easy enough to add some function
>>>> pointer
>>>> redirection everywhere one of the op_* functions calls a vrpmb_*
>>>> function. Do you already have a UFS RPMB device driver?
>>>>
>>>
>>> Hi Alex,
>>> Thanks for your feedback.
>>>
>>> We now access UFS RPMB through the RPMB LUN BSG device, RPMB is a
>>> well-
>>> known LU and we have a userspace tool to access it.
>>>
>>> I see that if we're going to use your interface, "static struct
>>> rpmb_ops" should be registered from a lower-level driver, for
>>> example
>>> in a UFS driver, yes there should be no problem with this
>>> registration,
>>> but I don't know with the current way Compared, what are the
>>> advantages
>>> to add a driver. maybe the main advantage is that we will have an
>>> unified user space tool for RPMB. right?
>>
>> Pretty much. The main issue for virtio-rpmb is it doesn't really fit
>> neatly into the block stack because all it does is the RPMB part so a
>> non-block orientate API makes sense.
>>
>> Can you point be to where the UFS driver does it's current RPMB
>> stuff?
>>
>
> It's the SCSI BSG driver, in fact, we don't have a dedicated UFS RPMB
> driver in the kernel. RPMB is a well known LU, we are using userspace
> tools to issue SCSI commands directly to the UFS RPMB LU via ioctl()
> from the BSG device node in the /dev/sg/ folder.
>
> Here is the BSG part of the code in the userspace tools:
>
> io_hdr_v4.guard = 'Q';
> io_hdr_v4.protocol = BSG_PROTOCOL_SCSI;
> io_hdr_v4.subprotocol = BSG_SUB_PROTOCOL_SCSI_CMD;
> io_hdr_v4.response = (__u64)sense_buffer;
> io_hdr_v4.max_response_len = SENSE_BUFF_LEN;
> io_hdr_v4.request_len = cmd_len;
> io_hdr_v4.request = (__u64)cdb;
>
>
> ioctl(fd, SG_IO, &io_hdr_v4))
Hi Bean,
I'm not sure where the above comes from? The Android RPMB client uses
slightly different code. Additionally, the retry loop around the
submission of SG/IO commands is very important. See also the
check_sg_io_hdr() call in send_ufs_rpmb_req(). See also
https://cs.android.com/android/platform/superproject/+/master:system/core/trusty/storage/proxy/rpmb.c
Thanks,
Bart.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2022-04-06 17:32 ` Bart Van Assche
@ 2022-04-06 18:12 ` Bean Huo
2022-04-06 20:20 ` Bart Van Assche
0 siblings, 1 reply; 29+ messages in thread
From: Bean Huo @ 2022-04-06 18:12 UTC (permalink / raw)
To: Bart Van Assche, Alex Bennée
Cc: linux-kernel, maxim.uvarov, joakim.bech, ulf.hansson,
ilias.apalodimas, arnd, ruchika.gupta, tomas.winkler, yang.huang,
bing.zhu, Matti.Moell, hmo, linux-mmc, linux-scsi
On Wed, 2022-04-06 at 10:32 -0700, Bart Van Assche wrote:
> > It's the SCSI BSG driver, in fact, we don't have a dedicated UFS
> > RPMB
> > driver in the kernel. RPMB is a well known LU, we are using
> > userspace
> > tools to issue SCSI commands directly to the UFS RPMB LU via
> > ioctl()
> > from the BSG device node in the /dev/sg/ folder.
> >
> > Here is the BSG part of the code in the userspace tools:
> >
> > io_hdr_v4.guard = 'Q';
> > io_hdr_v4.protocol = BSG_PROTOCOL_SCSI;
> > io_hdr_v4.subprotocol = BSG_SUB_PROTOCOL_SCSI_CMD;
> > io_hdr_v4.response = (__u64)sense_buffer;
> > io_hdr_v4.max_response_len = SENSE_BUFF_LEN;
> > io_hdr_v4.request_len = cmd_len;
> > io_hdr_v4.request = (__u64)cdb;
> >
> >
> >
> >
> > ioctl(fd, SG_IO, &io_hdr_v4))
>
> Hi Bean,
>
> I'm not sure where the above comes from? The Android RPMB client uses
> slightly different code. Additionally, the retry loop around the
> submission of SG/IO commands is very important. See also the
> check_sg_io_hdr() call in send_ufs_rpmb_req(). See also
> https://cs.android.com/android/platform/superproject/+/master:system/core/trusty/storage/proxy/rpmb.c
>
>
Bart,
It is from the ufs-utils.
So, do you vote to add the UFS RPMB driver based on this new framework
to resolve this conflict?
Kind regards,
Bean
> Thanks,
>
> Bart.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2022-04-06 18:12 ` Bean Huo
@ 2022-04-06 20:20 ` Bart Van Assche
2022-04-07 16:28 ` Bean Huo
0 siblings, 1 reply; 29+ messages in thread
From: Bart Van Assche @ 2022-04-06 20:20 UTC (permalink / raw)
To: Bean Huo, Alex Bennée
Cc: linux-kernel, maxim.uvarov, joakim.bech, ulf.hansson,
ilias.apalodimas, arnd, ruchika.gupta, tomas.winkler, yang.huang,
bing.zhu, Matti.Moell, hmo, linux-mmc, linux-scsi
On 4/6/22 11:12, Bean Huo wrote:
> It is from the ufs-utils.
>
> So, do you vote to add the UFS RPMB driver based on this new framework
> to resolve this conflict?
Are any applications using the RPMB code from ufs-utils? It seems to me
that the ufs-utils code doe not handle SCSI unit attentions correctly.
If a POWER ON unit attention is received as reply to a SECURITY PROTOCOL
OUT transaction, the write counter should be reread instead of retrying
the SECURITY PROTOCOL OUT command with the same write counter.
Regarding adding a UFS RPMB driver: that seems useful to me since
multiple applications make use of the UFS RPMB functionality. My
understanding is that currently storageproxyd multiplexes UFS RPMB
accesses in Android.
Thanks,
Bart.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2022-04-06 20:20 ` Bart Van Assche
@ 2022-04-07 16:28 ` Bean Huo
0 siblings, 0 replies; 29+ messages in thread
From: Bean Huo @ 2022-04-07 16:28 UTC (permalink / raw)
To: Bart Van Assche, Alex Bennée
Cc: linux-kernel, maxim.uvarov, joakim.bech, ulf.hansson,
ilias.apalodimas, arnd, ruchika.gupta, tomas.winkler, yang.huang,
bing.zhu, Matti.Moell, hmo, linux-mmc, linux-scsi
On Wed, 2022-04-06 at 13:20 -0700, Bart Van Assche wrote:
> On 4/6/22 11:12, Bean Huo wrote:
> > It is from the ufs-utils.
> >
> > So, do you vote to add the UFS RPMB driver based on this new
> > framework
> > to resolve this conflict?
>
> Are any applications using the RPMB code from ufs-utils? It seems to
> me
> that the ufs-utils code doe not handle SCSI unit attentions
> correctly.
> If a POWER ON unit attention is received as reply to a SECURITY
> PROTOCOL
> OUT transaction, the write counter should be reread instead of
> retrying
> the SECURITY PROTOCOL OUT command with the same write counter.
>
Not much sure how customers use this tool, based on my little
information from the field, the customer developed their own RPMB code
in the application. Here utils code is good example for them to study
and verify RPMB functinalities.
> Regarding adding a UFS RPMB driver: that seems useful to me since
> multiple applications make use of the UFS RPMB functionality. My
> understanding is that currently storageproxyd multiplexes UFS RPMB
> accesses in Android.
>
I have the same opinion with you, if we have an unified RPMB access
interface, and adding RPMB driver in the kernel, thus is better to
manage RPMB.
Kind regards,
Bean
> Thanks,
>
> Bart.
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2022-04-06 11:22 ` Alex Bennée
2022-04-06 17:19 ` Bean Huo
@ 2022-04-06 17:27 ` Bean Huo
1 sibling, 0 replies; 29+ messages in thread
From: Bean Huo @ 2022-04-06 17:27 UTC (permalink / raw)
To: Alex Bennée
Cc: linux-kernel, maxim.uvarov, joakim.bech, ulf.hansson,
ilias.apalodimas, arnd, ruchika.gupta, tomas.winkler, yang.huang,
bing.zhu, Matti.Moell, hmo, linux-mmc, linux-scsi
>
> Can you point be to where the UFS driver does it's current RPMB
> stuff?
>
If you want to understand the UFS RPMB functionality/command sequence,
you can refer to the ufs-utils tool, it is much like mmc-utils.
https://github.com/westerndigitalcorporation/ufs-utils/blob/dev/ufs_rpmb.c
> >
> > Kind regards,
> > Bean
>
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2022-04-05 9:37 [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver Alex Bennée
` (4 preceding siblings ...)
2022-04-05 14:54 ` [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver Bean Huo
@ 2022-04-22 14:21 ` Alex Bennée
[not found] ` <20230531191007.13460-1-shyamsaini@linux.microsoft.com>
6 siblings, 0 replies; 29+ messages in thread
From: Alex Bennée @ 2022-04-22 14:21 UTC (permalink / raw)
To: linux-kernel
Cc: maxim.uvarov, joakim.bech, ulf.hansson, ilias.apalodimas, arnd,
ruchika.gupta, tomas.winkler, yang.huang, bing.zhu, Matti.Moell,
hmo, linux-mmc, linux-scsi, Alex Bennée
Alex Bennée <alex.bennee@linaro.org> writes:
> Hi,
>
> This is another attempt to come up with an RPMB API for the kernel.
> The last discussion of this was in the thread:
Ping?
Any other comments or reviews? Is there a desire to make other devices
that provide RPMB functionality visible via a common API?
>
> Subject: [RFC PATCH 0/5] RPMB internal and user-space API + WIP virtio-rpmb frontend
> Date: Wed, 3 Mar 2021 13:54:55 +0000
> Message-Id: <20210303135500.24673-1-alex.bennee@linaro.org>
>
> The series provides for the RPMB sub-system, a new chardev API driven
> by ioctls and a full multi-block capable virtio-rpmb driver. You can
> find a working vhost-user backend in my QEMU branch here:
>
> https://github.com/stsquad/qemu/commits/virtio/vhost-user-rpmb-v2
>
> The branch is a little messy but I'll be posting a cleaned up version
> in the following weeks. The only real changes to the backend is the
> multi-block awareness and some tweaks to deal with QEMU internals
> handling VirtIO config space messages which weren't previously
> exercised. The test.sh script in tools/rpmb works through the various
> transactions but isn't comprehensive.
>
> Changes since the last posting:
>
> - frame construction is mostly back in userspace
>
> The previous discussion showed there wasn't any appetite for using
> the kernels keyctl() interface so userspace yet again takes
> responsibility for constructing most* frames. Currently these are
> all pure virtio-rpmb frames but the code is written so we can plug
> in additional frame types. The virtio-rpmb driver does some
> validation and in some cases (* read-blocks) constructs the request
> frame in the driver. It would take someone implementing a driver for
> another RPMB device type to see if this makes sense.
>
> - user-space interface is still split across several ioctls
>
> Although 3 of the ioctls share the common rpmb_ioc_reqresp_cmd
> structure it does mean things like capacity, write_count and
> read_blocks can have their own structure associated with the
> command.
>
> As before I shall follow up with the QEMU based vhost-user backend and
> hopefully a rust-vmm re-implementation. However I've no direct
> interest in implementing the interfaces to real hardware. I leave that
> to people who have access to such things and are willing to take up
> the maintainer burden if this is merged.
>
> Regards,
>
> Alex
>
>
> Alex Bennée (4):
> rpmb: add Replay Protected Memory Block (RPMB) subsystem
> char: rpmb: provide a user space interface
> rpmb: create virtio rpmb frontend driver
> tools rpmb: add RPBM access tool
>
> .../userspace-api/ioctl/ioctl-number.rst | 1 +
> MAINTAINERS | 9 +
> drivers/Kconfig | 2 +
> drivers/Makefile | 1 +
> drivers/rpmb/Kconfig | 28 +
> drivers/rpmb/Makefile | 9 +
> drivers/rpmb/cdev.c | 309 +++++
> drivers/rpmb/core.c | 439 +++++++
> drivers/rpmb/rpmb-cdev.h | 17 +
> drivers/rpmb/virtio_rpmb.c | 518 ++++++++
> include/linux/rpmb.h | 182 +++
> include/uapi/linux/rpmb.h | 99 ++
> include/uapi/linux/virtio_rpmb.h | 54 +
> tools/Makefile | 16 +-
> tools/rpmb/.gitignore | 2 +
> tools/rpmb/Makefile | 41 +
> tools/rpmb/key | 1 +
> tools/rpmb/rpmb.c | 1083 +++++++++++++++++
> tools/rpmb/test.sh | 22 +
> 19 files changed, 2828 insertions(+), 5 deletions(-)
> create mode 100644 drivers/rpmb/Kconfig
> create mode 100644 drivers/rpmb/Makefile
> create mode 100644 drivers/rpmb/cdev.c
> create mode 100644 drivers/rpmb/core.c
> create mode 100644 drivers/rpmb/rpmb-cdev.h
> create mode 100644 drivers/rpmb/virtio_rpmb.c
> create mode 100644 include/linux/rpmb.h
> create mode 100644 include/uapi/linux/rpmb.h
> create mode 100644 include/uapi/linux/virtio_rpmb.h
> create mode 100644 tools/rpmb/.gitignore
> create mode 100644 tools/rpmb/Makefile
> create mode 100644 tools/rpmb/key
> create mode 100644 tools/rpmb/rpmb.c
> create mode 100755 tools/rpmb/test.sh
--
Alex Bennée
^ permalink raw reply [flat|nested] 29+ messages in thread[parent not found: <20230531191007.13460-1-shyamsaini@linux.microsoft.com>]
* RE: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
[not found] ` <20230531191007.13460-1-shyamsaini@linux.microsoft.com>
@ 2023-06-01 1:02 ` Zhu, Bing
2023-06-01 5:31 ` Ilias Apalodimas
2023-06-13 16:47 ` Shyam Saini
0 siblings, 2 replies; 29+ messages in thread
From: Zhu, Bing @ 2023-06-01 1:02 UTC (permalink / raw)
To: Shyam Saini, alex.bennee
Cc: code, Matti.Moell, arnd, hmo, ilias.apalodimas, joakim.bech,
linux-kernel, linux-mmc, linux-scsi, maxim.uvarov, ruchika.gupta,
Winkler, Tomas, ulf.hansson, Huang, Yang, sumit.garg,
jens.wiklander, op-tee
As an alternative, Is it possible to change ftpm design not to depend on RPMB access at the earlier/boot stage? Because to my understanding, typically PCRs don't require persistent/NV storage (for example, before RPMB or tee-supplicant is ready, use TEE memory instead as temporary storage)
Bing
IPAS Security Brown Belt (https://www.credly.com/badges/69ea809f-3a96-4bc7-bb2f-442c1b17af26)
System Software Engineering
Software and Advanced Technology Group
Zizhu Science Park, Shanghai, China
-----Original Message-----
From: Shyam Saini <shyamsaini@linux.microsoft.com>
Sent: Thursday, June 1, 2023 3:10 AM
To: alex.bennee@linaro.org
Cc: code@tyhicks.com; Matti.Moell@opensynergy.com; arnd@linaro.org; Zhu, Bing <bing.zhu@intel.com>; hmo@opensynergy.com; ilias.apalodimas@linaro.org; joakim.bech@linaro.org; linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org; linux-scsi@vger.kernel.org; maxim.uvarov@linaro.org; ruchika.gupta@linaro.org; Winkler, Tomas <tomas.winkler@intel.com>; ulf.hansson@linaro.org; Huang, Yang <yang.huang@intel.com>; sumit.garg@linaro.org; jens.wiklander@linaro.org; op-tee@lists.trustedfirmware.org
Subject: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
Hi Alex,
[ Resending, Sorry for the noise ]
Are you still working on it or planning to resubmit it ?
[1] The current optee tee kernel driver implementation doesn't work when IMA is used with optee implemented ftpm.
The ftpm has dependency on tee-supplicant which comes once the user space is up and running and IMA attestation happens at boot time and it requires to extend ftpm PCRs.
But IMA can't use PCRs if ftpm use secure emmc RPMB partition. As optee can only access RPMB via tee-supplicant(user space). So, there should be a fast path to allow optee os to access the RPMB parititon without waiting for user-space tee supplicant.
To achieve this fast path linux optee driver and mmc driver needs some work and finally it will need RPMB driver which you posted.
Please let me know what's your plan on this.
[1] https://optee.readthedocs.io/en/latest/architecture/secure_storage.html
Best Regards,
Shyam
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2023-06-01 1:02 ` Zhu, Bing
@ 2023-06-01 5:31 ` Ilias Apalodimas
2023-06-01 5:48 ` Sumit Garg
2023-06-13 16:47 ` Shyam Saini
1 sibling, 1 reply; 29+ messages in thread
From: Ilias Apalodimas @ 2023-06-01 5:31 UTC (permalink / raw)
To: Zhu, Bing
Cc: Shyam Saini, alex.bennee, code, Matti.Moell, arnd, hmo,
joakim.bech, linux-kernel, linux-mmc, linux-scsi, maxim.uvarov,
ruchika.gupta, Winkler, Tomas, ulf.hansson, Huang, Yang,
sumit.garg, jens.wiklander, op-tee
Hi Bing
On Thu, 1 Jun 2023 at 04:03, Zhu, Bing <bing.zhu@intel.com> wrote:
>
> As an alternative, Is it possible to change ftpm design not to depend on RPMB access at the earlier/boot stage? Because to my understanding, typically PCRs don't require persistent/NV storage (for example, before RPMB or tee-supplicant is ready, use TEE memory instead as temporary storage)
I am not entirely sure this will solve our problem here. You are
right that we shouldn't depend on the supplicant to extend PCRs. But
what happens if an object is sealed against certain PCR values? We
are back to the same problem
Thanks
/Ilias
>
> Bing
>
> IPAS Security Brown Belt (https://www.credly.com/badges/69ea809f-3a96-4bc7-bb2f-442c1b17af26)
> System Software Engineering
> Software and Advanced Technology Group
> Zizhu Science Park, Shanghai, China
>
> -----Original Message-----
> From: Shyam Saini <shyamsaini@linux.microsoft.com>
> Sent: Thursday, June 1, 2023 3:10 AM
> To: alex.bennee@linaro.org
> Cc: code@tyhicks.com; Matti.Moell@opensynergy.com; arnd@linaro.org; Zhu, Bing <bing.zhu@intel.com>; hmo@opensynergy.com; ilias.apalodimas@linaro.org; joakim.bech@linaro.org; linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org; linux-scsi@vger.kernel.org; maxim.uvarov@linaro.org; ruchika.gupta@linaro.org; Winkler, Tomas <tomas.winkler@intel.com>; ulf.hansson@linaro.org; Huang, Yang <yang.huang@intel.com>; sumit.garg@linaro.org; jens.wiklander@linaro.org; op-tee@lists.trustedfirmware.org
> Subject: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
>
> Hi Alex,
>
> [ Resending, Sorry for the noise ]
>
> Are you still working on it or planning to resubmit it ?
>
> [1] The current optee tee kernel driver implementation doesn't work when IMA is used with optee implemented ftpm.
>
> The ftpm has dependency on tee-supplicant which comes once the user space is up and running and IMA attestation happens at boot time and it requires to extend ftpm PCRs.
>
> But IMA can't use PCRs if ftpm use secure emmc RPMB partition. As optee can only access RPMB via tee-supplicant(user space). So, there should be a fast path to allow optee os to access the RPMB parititon without waiting for user-space tee supplicant.
>
> To achieve this fast path linux optee driver and mmc driver needs some work and finally it will need RPMB driver which you posted.
>
> Please let me know what's your plan on this.
>
> [1] https://optee.readthedocs.io/en/latest/architecture/secure_storage.html
>
> Best Regards,
> Shyam
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2023-06-01 5:31 ` Ilias Apalodimas
@ 2023-06-01 5:48 ` Sumit Garg
2023-06-02 8:25 ` Ilias Apalodimas
0 siblings, 1 reply; 29+ messages in thread
From: Sumit Garg @ 2023-06-01 5:48 UTC (permalink / raw)
To: Ilias Apalodimas
Cc: Zhu, Bing, Shyam Saini, alex.bennee, code, Matti.Moell, arnd,
hmo, joakim.bech, linux-kernel, linux-mmc, linux-scsi,
maxim.uvarov, ruchika.gupta, Winkler, Tomas, ulf.hansson, Huang,
Yang, jens.wiklander, op-tee
On Thu, 1 Jun 2023 at 11:02, Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> Hi Bing
>
> On Thu, 1 Jun 2023 at 04:03, Zhu, Bing <bing.zhu@intel.com> wrote:
> >
> > As an alternative, Is it possible to change ftpm design not to depend on RPMB access at the earlier/boot stage? Because to my understanding, typically PCRs don't require persistent/NV storage (for example, before RPMB or tee-supplicant is ready, use TEE memory instead as temporary storage)
>
> I am not entirely sure this will solve our problem here. You are
> right that we shouldn't depend on the supplicant to extend PCRs. But
> what happens if an object is sealed against certain PCR values? We
> are back to the same problem
+1
Temporary storage may be a stop gap solution for some use-cases but
having a fast path access to RPMB via kernel should be our final goal.
I would suggest we start small with the MMC subsystem to expose RPMB
access APIs for OP-TEE driver rather than a complete RPMB subsystem.
-Sumit
>
> Thanks
> /Ilias
> >
> > Bing
> >
> > IPAS Security Brown Belt (https://www.credly.com/badges/69ea809f-3a96-4bc7-bb2f-442c1b17af26)
> > System Software Engineering
> > Software and Advanced Technology Group
> > Zizhu Science Park, Shanghai, China
> >
> > -----Original Message-----
> > From: Shyam Saini <shyamsaini@linux.microsoft.com>
> > Sent: Thursday, June 1, 2023 3:10 AM
> > To: alex.bennee@linaro.org
> > Cc: code@tyhicks.com; Matti.Moell@opensynergy.com; arnd@linaro.org; Zhu, Bing <bing.zhu@intel.com>; hmo@opensynergy.com; ilias.apalodimas@linaro.org; joakim.bech@linaro.org; linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org; linux-scsi@vger.kernel.org; maxim.uvarov@linaro.org; ruchika.gupta@linaro.org; Winkler, Tomas <tomas.winkler@intel.com>; ulf.hansson@linaro.org; Huang, Yang <yang.huang@intel.com>; sumit.garg@linaro.org; jens.wiklander@linaro.org; op-tee@lists.trustedfirmware.org
> > Subject: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
> >
> > Hi Alex,
> >
> > [ Resending, Sorry for the noise ]
> >
> > Are you still working on it or planning to resubmit it ?
> >
> > [1] The current optee tee kernel driver implementation doesn't work when IMA is used with optee implemented ftpm.
> >
> > The ftpm has dependency on tee-supplicant which comes once the user space is up and running and IMA attestation happens at boot time and it requires to extend ftpm PCRs.
> >
> > But IMA can't use PCRs if ftpm use secure emmc RPMB partition. As optee can only access RPMB via tee-supplicant(user space). So, there should be a fast path to allow optee os to access the RPMB parititon without waiting for user-space tee supplicant.
> >
> > To achieve this fast path linux optee driver and mmc driver needs some work and finally it will need RPMB driver which you posted.
> >
> > Please let me know what's your plan on this.
> >
> > [1] https://optee.readthedocs.io/en/latest/architecture/secure_storage.html
> >
> > Best Regards,
> > Shyam
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2023-06-01 5:48 ` Sumit Garg
@ 2023-06-02 8:25 ` Ilias Apalodimas
2023-06-12 17:06 ` Jens Wiklander
2023-06-13 0:49 ` Shyam Saini
0 siblings, 2 replies; 29+ messages in thread
From: Ilias Apalodimas @ 2023-06-02 8:25 UTC (permalink / raw)
To: Sumit Garg
Cc: Zhu, Bing, Shyam Saini, alex.bennee, code, Matti.Moell, arnd,
hmo, joakim.bech, linux-kernel, linux-mmc, linux-scsi,
maxim.uvarov, ruchika.gupta, Winkler, Tomas, ulf.hansson, Huang,
Yang, jens.wiklander, op-tee
On Thu, 1 Jun 2023 at 08:49, Sumit Garg <sumit.garg@linaro.org> wrote:
>
> On Thu, 1 Jun 2023 at 11:02, Ilias Apalodimas
> <ilias.apalodimas@linaro.org> wrote:
> >
> > Hi Bing
> >
> > On Thu, 1 Jun 2023 at 04:03, Zhu, Bing <bing.zhu@intel.com> wrote:
> > >
> > > As an alternative, Is it possible to change ftpm design not to depend on RPMB access at the earlier/boot stage? Because to my understanding, typically PCRs don't require persistent/NV storage (for example, before RPMB or tee-supplicant is ready, use TEE memory instead as temporary storage)
> >
> > I am not entirely sure this will solve our problem here. You are
> > right that we shouldn't depend on the supplicant to extend PCRs. But
> > what happens if an object is sealed against certain PCR values? We
> > are back to the same problem
>
> +1
>
> Temporary storage may be a stop gap solution for some use-cases but
> having a fast path access to RPMB via kernel should be our final goal.
> I would suggest we start small with the MMC subsystem to expose RPMB
> access APIs for OP-TEE driver rather than a complete RPMB subsystem.
I discussed with the OP-TEE maintainers about adding parts of the
supplicant in the kernel. The supplicant 'just' sends an ioctl to
store/read stuff anyway. So it would make sense to have a closer and
see if that looks reasonable.
Thanks
/Ilias
>
> -Sumit
>
> >
> > Thanks
> > /Ilias
> > >
> > > Bing
> > >
> > > IPAS Security Brown Belt (https://www.credly.com/badges/69ea809f-3a96-4bc7-bb2f-442c1b17af26)
> > > System Software Engineering
> > > Software and Advanced Technology Group
> > > Zizhu Science Park, Shanghai, China
> > >
> > > -----Original Message-----
> > > From: Shyam Saini <shyamsaini@linux.microsoft.com>
> > > Sent: Thursday, June 1, 2023 3:10 AM
> > > To: alex.bennee@linaro.org
> > > Cc: code@tyhicks.com; Matti.Moell@opensynergy.com; arnd@linaro.org; Zhu, Bing <bing.zhu@intel.com>; hmo@opensynergy.com; ilias.apalodimas@linaro.org; joakim.bech@linaro.org; linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org; linux-scsi@vger.kernel.org; maxim.uvarov@linaro.org; ruchika.gupta@linaro.org; Winkler, Tomas <tomas.winkler@intel.com>; ulf.hansson@linaro.org; Huang, Yang <yang.huang@intel.com>; sumit.garg@linaro.org; jens.wiklander@linaro.org; op-tee@lists.trustedfirmware.org
> > > Subject: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
> > >
> > > Hi Alex,
> > >
> > > [ Resending, Sorry for the noise ]
> > >
> > > Are you still working on it or planning to resubmit it ?
> > >
> > > [1] The current optee tee kernel driver implementation doesn't work when IMA is used with optee implemented ftpm.
> > >
> > > The ftpm has dependency on tee-supplicant which comes once the user space is up and running and IMA attestation happens at boot time and it requires to extend ftpm PCRs.
> > >
> > > But IMA can't use PCRs if ftpm use secure emmc RPMB partition. As optee can only access RPMB via tee-supplicant(user space). So, there should be a fast path to allow optee os to access the RPMB parititon without waiting for user-space tee supplicant.
> > >
> > > To achieve this fast path linux optee driver and mmc driver needs some work and finally it will need RPMB driver which you posted.
> > >
> > > Please let me know what's your plan on this.
> > >
> > > [1] https://optee.readthedocs.io/en/latest/architecture/secure_storage.html
> > >
> > > Best Regards,
> > > Shyam
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2023-06-02 8:25 ` Ilias Apalodimas
@ 2023-06-12 17:06 ` Jens Wiklander
2023-06-13 0:49 ` Shyam Saini
1 sibling, 0 replies; 29+ messages in thread
From: Jens Wiklander @ 2023-06-12 17:06 UTC (permalink / raw)
To: alex.bennee
Cc: Ilias Apalodimas, Sumit Garg, Zhu, Bing, Shyam Saini, code,
Matti.Moell, arnd, hmo, joakim.bech, linux-kernel, linux-mmc,
linux-scsi, maxim.uvarov, ruchika.gupta, Winkler, Tomas,
ulf.hansson, Huang, Yang, op-tee
Hi Alex,
On Fri, Jun 2, 2023 at 10:26 AM Ilias Apalodimas
<ilias.apalodimas@linaro.org> wrote:
>
> On Thu, 1 Jun 2023 at 08:49, Sumit Garg <sumit.garg@linaro.org> wrote:
> >
> > On Thu, 1 Jun 2023 at 11:02, Ilias Apalodimas
> > <ilias.apalodimas@linaro.org> wrote:
> > >
> > > Hi Bing
> > >
> > > On Thu, 1 Jun 2023 at 04:03, Zhu, Bing <bing.zhu@intel.com> wrote:
> > > >
> > > > As an alternative, Is it possible to change ftpm design not to depend on RPMB access at the earlier/boot stage? Because to my understanding, typically PCRs don't require persistent/NV storage (for example, before RPMB or tee-supplicant is ready, use TEE memory instead as temporary storage)
> > >
> > > I am not entirely sure this will solve our problem here. You are
> > > right that we shouldn't depend on the supplicant to extend PCRs. But
> > > what happens if an object is sealed against certain PCR values? We
> > > are back to the same problem
> >
> > +1
> >
> > Temporary storage may be a stop gap solution for some use-cases but
> > having a fast path access to RPMB via kernel should be our final goal.
> > I would suggest we start small with the MMC subsystem to expose RPMB
> > access APIs for OP-TEE driver rather than a complete RPMB subsystem.
>
> I discussed with the OP-TEE maintainers about adding parts of the
> supplicant in the kernel. The supplicant 'just' sends an ioctl to
> store/read stuff anyway. So it would make sense to have a closer and
> see if that looks reasonable.
> Thanks
I was trying to create a setup to test this. I've added the kernel
patches on top of https://github.com/linaro-swg/linux/tree/optee. The
QEMU branch is a bit dated and I had to add
3a845a214b42 target/arm: allow setting SCR_EL3.EnTP2 when FEAT_SME is
implemented
d4a7b0ef1a03 hw/arm/boot: set CPTR_EL3.ESM and SCR_EL3.EnTP2 when
booting Linux with EL3
9745a003f878 hw/intc/arm_gicv3: fix prio masking on pmr write
beeec926d24a target/arm: mark SP_EL1 with ARM_CP_EL3_NO_EL2_KEEP
on top of that branch to be able to boot to the Linux kernel.
I have the vhost-user-rpmb process running and connected with QEMU,
but around (guessing really) when the RPMB subsystem is initializing
the process dies with:
================ Vhost user message ================
Request: VHOST_USER_SET_VRING_ADDR (9)
Flags: 0x1
Size: 40
vhost-user-rpmb-INFO: 18:58:08.312: vrpmb_process_msg: msg
VHOST_USER_SET_VRING_ADDR(9)
vhost_vring_addr:
index: 0
flags: 0
desc_user_addr: 0x00007ff15fa91000
used_user_addr: 0x00007ff15fa91080
avail_user_addr: 0x00007ff15fa91040
log_guest_addr: 0x0000000041c91080
Setting virtq addresses:
vring_desc at (nil)
vring_used at (nil)
vring_avail at (nil)
(vhost-user-rpmb:3236474): vhost-user-rpmb-CRITICAL **: 18:58:08.312:
Invalid vring_addr message
Among other options, I'm starting QEMU with -machine
virt,secure=on,mte=off,gic-version=3,virtualization=false to enable
the secure world.
Do you have an idea of what might be wrong? Where should I start looking?
Thanks,
Jens
>
> /Ilias
>
> >
> > -Sumit
> >
> > >
> > > Thanks
> > > /Ilias
> > > >
> > > > Bing
> > > >
> > > > IPAS Security Brown Belt (https://www.credly.com/badges/69ea809f-3a96-4bc7-bb2f-442c1b17af26)
> > > > System Software Engineering
> > > > Software and Advanced Technology Group
> > > > Zizhu Science Park, Shanghai, China
> > > >
> > > > -----Original Message-----
> > > > From: Shyam Saini <shyamsaini@linux.microsoft.com>
> > > > Sent: Thursday, June 1, 2023 3:10 AM
> > > > To: alex.bennee@linaro.org
> > > > Cc: code@tyhicks.com; Matti.Moell@opensynergy.com; arnd@linaro.org; Zhu, Bing <bing.zhu@intel.com>; hmo@opensynergy.com; ilias.apalodimas@linaro.org; joakim.bech@linaro.org; linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org; linux-scsi@vger.kernel.org; maxim.uvarov@linaro.org; ruchika.gupta@linaro.org; Winkler, Tomas <tomas.winkler@intel.com>; ulf.hansson@linaro.org; Huang, Yang <yang.huang@intel.com>; sumit.garg@linaro.org; jens.wiklander@linaro.org; op-tee@lists.trustedfirmware.org
> > > > Subject: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
> > > >
> > > > Hi Alex,
> > > >
> > > > [ Resending, Sorry for the noise ]
> > > >
> > > > Are you still working on it or planning to resubmit it ?
> > > >
> > > > [1] The current optee tee kernel driver implementation doesn't work when IMA is used with optee implemented ftpm.
> > > >
> > > > The ftpm has dependency on tee-supplicant which comes once the user space is up and running and IMA attestation happens at boot time and it requires to extend ftpm PCRs.
> > > >
> > > > But IMA can't use PCRs if ftpm use secure emmc RPMB partition. As optee can only access RPMB via tee-supplicant(user space). So, there should be a fast path to allow optee os to access the RPMB parititon without waiting for user-space tee supplicant.
> > > >
> > > > To achieve this fast path linux optee driver and mmc driver needs some work and finally it will need RPMB driver which you posted.
> > > >
> > > > Please let me know what's your plan on this.
> > > >
> > > > [1] https://optee.readthedocs.io/en/latest/architecture/secure_storage.html
> > > >
> > > > Best Regards,
> > > > Shyam
^ permalink raw reply [flat|nested] 29+ messages in thread* Re: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2023-06-02 8:25 ` Ilias Apalodimas
2023-06-12 17:06 ` Jens Wiklander
@ 2023-06-13 0:49 ` Shyam Saini
1 sibling, 0 replies; 29+ messages in thread
From: Shyam Saini @ 2023-06-13 0:49 UTC (permalink / raw)
To: alex.bennee
Cc: Sumit Garg, Zhu, Bing, alex.bennee, code, Matti.Moell, arnd, hmo,
joakim.bech, linux-kernel, linux-mmc, linux-scsi, maxim.uvarov,
ruchika.gupta, Winkler, Tomas, ulf.hansson, Huang, Yang,
jens.wiklander, op-tee, ilias.apalodimas
Thank you everyone for your valueable feedback.
Alex, are you planning submit this patch series ?
Please let me know.
> On Thu, 1 Jun 2023 at 08:49, Sumit Garg <sumit.garg@linaro.org> wrote:
>>
>> On Thu, 1 Jun 2023 at 11:02, Ilias Apalodimas
>> <ilias.apalodimas@linaro.org> wrote:
>>>
>>> Hi Bing
>>>
>>> On Thu, 1 Jun 2023 at 04:03, Zhu, Bing <bing.zhu@intel.com> wrote:
>>>>
>>>> As an alternative, Is it possible to change ftpm design not to depend on RPMB access at the earlier/boot stage? Because to my understanding, typically PCRs don't require persistent/NV storage (for example, before RPMB or tee-supplicant is ready, use TEE memory instead as temporary storage)
>>>
>>> I am not entirely sure this will solve our problem here. You are
>>> right that we shouldn't depend on the supplicant to extend PCRs. But
>>> what happens if an object is sealed against certain PCR values? We
>>> are back to the same problem
>>
>> +1
>>
>> Temporary storage may be a stop gap solution for some use-cases but
>> having a fast path access to RPMB via kernel should be our final goal.
>> I would suggest we start small with the MMC subsystem to expose RPMB
>> access APIs for OP-TEE driver rather than a complete RPMB subsystem.
>
> I discussed with the OP-TEE maintainers about adding parts of the
> supplicant in the kernel. The supplicant 'just' sends an ioctl to
> store/read stuff anyway. So it would make sense to have a closer and
> see if that looks reasonable.
> Thanks
>
> /Ilias
>
>>
>> -Sumit
>>
>>>
>>> Thanks
>>> /Ilias
>>>>
>>>> Bing
>>>>
>>>> IPAS Security Brown Belt (https://www.credly.com/badges/69ea809f-3a96-4bc7-bb2f-442c1b17af26)
>>>> System Software Engineering
>>>> Software and Advanced Technology Group
>>>> Zizhu Science Park, Shanghai, China
>>>>
>>>> -----Original Message-----
>>>> From: Shyam Saini <shyamsaini@linux.microsoft.com>
>>>> Sent: Thursday, June 1, 2023 3:10 AM
>>>> To: alex.bennee@linaro.org
>>>> Cc: code@tyhicks.com; Matti.Moell@opensynergy.com; arnd@linaro.org; Zhu, Bing <bing.zhu@intel.com>; hmo@opensynergy.com; ilias.apalodimas@linaro.org; joakim.bech@linaro.org; linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org; linux-scsi@vger.kernel.org; maxim.uvarov@linaro.org; ruchika.gupta@linaro.org; Winkler, Tomas <tomas.winkler@intel.com>; ulf.hansson@linaro.org; Huang, Yang <yang.huang@intel.com>; sumit.garg@linaro.org; jens.wiklander@linaro.org; op-tee@lists.trustedfirmware.org
>>>> Subject: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
>>>>
>>>> Hi Alex,
>>>>
>>>> [ Resending, Sorry for the noise ]
>>>>
>>>> Are you still working on it or planning to resubmit it ?
>>>>
>>>> [1] The current optee tee kernel driver implementation doesn't work when IMA is used with optee implemented ftpm.
>>>>
>>>> The ftpm has dependency on tee-supplicant which comes once the user space is up and running and IMA attestation happens at boot time and it requires to extend ftpm PCRs.
>>>>
>>>> But IMA can't use PCRs if ftpm use secure emmc RPMB partition. As optee can only access RPMB via tee-supplicant(user space). So, there should be a fast path to allow optee os to access the RPMB parititon without waiting for user-space tee supplicant.
>>>>
>>>> To achieve this fast path linux optee driver and mmc driver needs some work and finally it will need RPMB driver which you posted.
>>>>
>>>> Please let me know what's your plan on this.
>>>>
>>>> [1] https://optee.readthedocs.io/en/latest/architecture/secure_storage.html
>>>>
>>>> Best Regards,
>>>> Shyam
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* RE: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
2023-06-01 1:02 ` Zhu, Bing
2023-06-01 5:31 ` Ilias Apalodimas
@ 2023-06-13 16:47 ` Shyam Saini
1 sibling, 0 replies; 29+ messages in thread
From: Shyam Saini @ 2023-06-13 16:47 UTC (permalink / raw)
To: Zhu, Bing
Cc: alex.bennee, code, Matti.Moell, arnd, hmo, ilias.apalodimas,
joakim.bech, linux-kernel, linux-mmc, linux-scsi, maxim.uvarov,
ruchika.gupta, Winkler, Tomas, ulf.hansson, Huang, Yang,
sumit.garg, jens.wiklander, op-tee
Hi Bing,
Other than PCRs we also want to store Non volatile ftpm data (NVData),
storing these in volatile DDR shared memory will be a spec violation.
Best Regards,
Shyam
> As an alternative, Is it possible to change ftpm design not to depend on RPMB access at the earlier/boot stage? Because to my understanding, typically PCRs don't require persistent/NV storage (for example, before RPMB or tee-supplicant is ready, use TEE memory instead as temporary storage)
>
> Bing
>
> IPAS Security Brown Belt (https://www.credly.com/badges/69ea809f-3a96-4bc7-bb2f-442c1b17af26)
> System Software Engineering
> Software and Advanced Technology Group
> Zizhu Science Park, Shanghai, China
>
> -----Original Message-----
> From: Shyam Saini <shyamsaini@linux.microsoft.com>
> Sent: Thursday, June 1, 2023 3:10 AM
> To: alex.bennee@linaro.org
> Cc: code@tyhicks.com; Matti.Moell@opensynergy.com; arnd@linaro.org; Zhu, Bing <bing.zhu@intel.com>; hmo@opensynergy.com; ilias.apalodimas@linaro.org; joakim.bech@linaro.org; linux-kernel@vger.kernel.org; linux-mmc@vger.kernel.org; linux-scsi@vger.kernel.org; maxim.uvarov@linaro.org; ruchika.gupta@linaro.org; Winkler, Tomas <tomas.winkler@intel.com>; ulf.hansson@linaro.org; Huang, Yang <yang.huang@intel.com>; sumit.garg@linaro.org; jens.wiklander@linaro.org; op-tee@lists.trustedfirmware.org
> Subject: [PATCH v2 0/4] rpmb subsystem, uapi and virtio-rpmb driver
>
> Hi Alex,
>
> [ Resending, Sorry for the noise ]
>
> Are you still working on it or planning to resubmit it ?
>
> [1] The current optee tee kernel driver implementation doesn't work when IMA is used with optee implemented ftpm.
>
> The ftpm has dependency on tee-supplicant which comes once the user space is up and running and IMA attestation happens at boot time and it requires to extend ftpm PCRs.
>
> But IMA can't use PCRs if ftpm use secure emmc RPMB partition. As optee can only access RPMB via tee-supplicant(user space). So, there should be a fast path to allow optee os to access the RPMB parititon without waiting for user-space tee supplicant.
>
> To achieve this fast path linux optee driver and mmc driver needs some work and finally it will need RPMB driver which you posted.
>
> Please let me know what's your plan on this.
>
> [1] https://optee.readthedocs.io/en/latest/architecture/secure_storage.html
>
> Best Regards,
> Shyam
>
^ permalink raw reply [flat|nested] 29+ messages in thread