[PATCH 0/2] rpmb: add virtio RPMB driver

[PATCH 0/2] rpmb: add virtio RPMB driver

[Haruka Kuze]

This adds a driver on top of the rpmb subsystem for virtio-rpmb devices.

Alex Bennée had an earlier virtio-rpmb driver in his RPMB series [1]
but that stalled due to the user space API.

The one that merged later has no user space API and OP-TEE routes frames
through it so this driver only needs to implement route_frames().

I added patch 1 to fix a use-after-free that I hit while testing
removing the device (OP-TEE can still route frames after driver unregisters
the rpmb device).

I tested this with UML (virtio_uml) and a vhost-user backend I wrote
for this on top of rust-vmm's vhost-device with KASAN, lockdep:

 - key programming
 - write counter
 - single and multi block write and read
 - MAC and counter failures
 - max_wr_cnt
 - unbinding the driver when the rpmb device is held

With OP-TEE 4.11 on QEMU virt arm64 the secure_storage example TA keeps
its objects in RPMB through the driver and an object written to in one
boot is found in the one after.

I have added myself in MAINTAINERS. I would like to maintain the driver.

- haruka

[1] https://lore.kernel.org/all/20220405093759.1126835-1-alex.bennee@linaro.org/

Haruka Kuze (2):
  rpmb: stop the routing after unregister
  rpmb: add a virtio RPMB device driver

 MAINTAINERS                      |   7 +
 drivers/misc/Kconfig             |  10 ++
 drivers/misc/Makefile            |   1 +
 drivers/misc/rpmb-core.c         |  22 ++-
 drivers/misc/virtio_rpmb.c       | 294 +++++++++++++++++++++++++++++++
 include/linux/rpmb.h             |   4 +
 include/uapi/linux/virtio_rpmb.h |  56 ++++++
 7 files changed, 391 insertions(+), 3 deletions(-)
 create mode 100644 drivers/misc/virtio_rpmb.c
 create mode 100644 include/uapi/linux/virtio_rpmb.h


base-commit: c339f4010dea8b0d754f84ace70cd1b089878ef8
-- 
2.55.0

[PATCH 0/2] rpmb: add virtio RPMB driver

[Haruka Kuze via OP-TEE]

This adds a driver on top of the rpmb subsystem for virtio-rpmb devices.

Alex Bennée had an earlier virtio-rpmb driver in his RPMB series [1]
but that stalled due to the user space API.

The one that merged later has no user space API and OP-TEE routes frames
through it so this driver only needs to implement route_frames().

I added patch 1 to fix a use-after-free that I hit while testing
removing the device (OP-TEE can still route frames after driver unregisters
the rpmb device).

I tested this with UML (virtio_uml) and a vhost-user backend I wrote
for this on top of rust-vmm's vhost-device with KASAN, lockdep:

 - key programming
 - write counter
 - single and multi block write and read
 - MAC and counter failures
 - max_wr_cnt
 - unbinding the driver when the rpmb device is held

With OP-TEE 4.11 on QEMU virt arm64 the secure_storage example TA keeps
its objects in RPMB through the driver and an object written to in one
boot is found in the one after.

I have added myself in MAINTAINERS. I would like to maintain the driver.

- haruka

[1] https://lore.kernel.org/all/20220405093759.1126835-1-alex.bennee@linaro.org/

Haruka Kuze (2):
  rpmb: stop the routing after unregister
  rpmb: add a virtio RPMB device driver

 MAINTAINERS                      |   7 +
 drivers/misc/Kconfig             |  10 ++
 drivers/misc/Makefile            |   1 +
 drivers/misc/rpmb-core.c         |  22 ++-
 drivers/misc/virtio_rpmb.c       | 294 +++++++++++++++++++++++++++++++
 include/linux/rpmb.h             |   4 +
 include/uapi/linux/virtio_rpmb.h |  56 ++++++
 7 files changed, 391 insertions(+), 3 deletions(-)
 create mode 100644 drivers/misc/virtio_rpmb.c
 create mode 100644 include/uapi/linux/virtio_rpmb.h


base-commit: c339f4010dea8b0d754f84ace70cd1b089878ef8
-- 
2.55.0

[PATCH 1/2] rpmb: stop the routing after unregister

[Haruka Kuze]

OP-TEE has a reference of RPMB device it used. So, it can route
frames through the device after rpmb_dev_unregister() is called.
Therefore, the parent device passed to route_frames() can be
already freed, for example, mmc_blk_remove_rpmb_part() drops the
last reference after rpmb unregister.

This patch clears the route_frames when the device is going to
be removed by getting new rwsem and makes rpmb_route_frames()
return -ENODEV if the device is gone.

Fixes: 1e9046e3a154 ("rpmb: add Replay Protected Memory Block (RPMB) subsystem")
Assisted-by: LLM
Signed-off-by: Haruka Kuze <harukak@zohomail.eu>
---
 drivers/misc/rpmb-core.c | 22 +++++++++++++++++++---
 include/linux/rpmb.h     |  4 ++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/rpmb-core.c b/drivers/misc/rpmb-core.c
index ecf14acf230a..0538871af584 100644
--- a/drivers/misc/rpmb-core.c
+++ b/drivers/misc/rpmb-core.c
@@ -45,16 +45,23 @@ EXPORT_SYMBOL_GPL(rpmb_dev_put);
  * @rsp:	rpmb response frames
  * @rsp_len:	length of rpmb response frames in bytes
  *
- * Returns: < 0 on failure
+ * Returns: < 0 on failure, -ENODEV if the device has been unregistered
  */
 int rpmb_route_frames(struct rpmb_dev *rdev, u8 *req,
 		      unsigned int req_len, u8 *rsp, unsigned int rsp_len)
 {
+	int ret = -ENODEV;
+
 	if (!req || !req_len || !rsp || !rsp_len)
 		return -EINVAL;
 
-	return rdev->descr.route_frames(rdev->dev.parent, req, req_len,
-					rsp, rsp_len);
+	down_read(&rdev->lock);
+	if (rdev->descr.route_frames)
+		ret = rdev->descr.route_frames(rdev->dev.parent, req, req_len,
+					       rsp, rsp_len);
+	up_read(&rdev->lock);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(rpmb_route_frames);
 
@@ -126,6 +133,10 @@ EXPORT_SYMBOL_GPL(rpmb_interface_unregister);
  * This function should be called from the release function of the
  * underlying device used when the RPMB device was registered.
  *
+ * Once this returns, rpmb_route_frames() no longer calls into
+ * the driver, even though the users might still hold a reference
+ * to the RPMB dev.
+ *
  * Returns: < 0 on failure
  */
 int rpmb_dev_unregister(struct rpmb_dev *rdev)
@@ -133,6 +144,10 @@ int rpmb_dev_unregister(struct rpmb_dev *rdev)
 	if (!rdev)
 		return -EINVAL;
 
+	down_write(&rdev->lock);
+	rdev->descr.route_frames = NULL;
+	up_write(&rdev->lock);
+
 	device_del(&rdev->dev);
 
 	rpmb_dev_put(rdev);
@@ -165,6 +180,7 @@ struct rpmb_dev *rpmb_dev_register(struct device *dev,
 	if (!rdev)
 		return ERR_PTR(-ENOMEM);
 	rdev->descr = *descr;
+	init_rwsem(&rdev->lock);
 	rdev->descr.dev_id = kmemdup(descr->dev_id, descr->dev_id_len,
 				     GFP_KERNEL);
 	if (!rdev->descr.dev_id) {
diff --git a/include/linux/rpmb.h b/include/linux/rpmb.h
index ed3f8e431eff..0a976572d1cd 100644
--- a/include/linux/rpmb.h
+++ b/include/linux/rpmb.h
@@ -7,6 +7,7 @@
 #define __RPMB_H__
 
 #include <linux/device.h>
+#include <linux/rwsem.h>
 #include <linux/types.h>
 
 /**
@@ -51,12 +52,15 @@ struct rpmb_descr {
  * @id               : device_id
  * @list_node        : linked list node
  * @descr            : RPMB description
+ * @lock             : protect @descr.route_frames which is cleared when this
+ *                     device is unregistered
  */
 struct rpmb_dev {
 	struct device dev;
 	int id;
 	struct list_head list_node;
 	struct rpmb_descr descr;
+	struct rw_semaphore lock;
 };
 
 #define to_rpmb_dev(x)		container_of((x), struct rpmb_dev, dev)
-- 
2.55.0

[PATCH 1/2] rpmb: stop the routing after unregister

[Haruka Kuze via OP-TEE]

OP-TEE has a reference of RPMB device it used. So, it can route
frames through the device after rpmb_dev_unregister() is called.
Therefore, the parent device passed to route_frames() can be
already freed, for example, mmc_blk_remove_rpmb_part() drops the
last reference after rpmb unregister.

This patch clears the route_frames when the device is going to
be removed by getting new rwsem and makes rpmb_route_frames()
return -ENODEV if the device is gone.

Fixes: 1e9046e3a154 ("rpmb: add Replay Protected Memory Block (RPMB) subsystem")
Assisted-by: LLM
Signed-off-by: Haruka Kuze <harukak@zohomail.eu>
---
 drivers/misc/rpmb-core.c | 22 +++++++++++++++++++---
 include/linux/rpmb.h     |  4 ++++
 2 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/drivers/misc/rpmb-core.c b/drivers/misc/rpmb-core.c
index ecf14acf230a..0538871af584 100644
--- a/drivers/misc/rpmb-core.c
+++ b/drivers/misc/rpmb-core.c
@@ -45,16 +45,23 @@ EXPORT_SYMBOL_GPL(rpmb_dev_put);
  * @rsp:	rpmb response frames
  * @rsp_len:	length of rpmb response frames in bytes
  *
- * Returns: < 0 on failure
+ * Returns: < 0 on failure, -ENODEV if the device has been unregistered
  */
 int rpmb_route_frames(struct rpmb_dev *rdev, u8 *req,
 		      unsigned int req_len, u8 *rsp, unsigned int rsp_len)
 {
+	int ret = -ENODEV;
+
 	if (!req || !req_len || !rsp || !rsp_len)
 		return -EINVAL;
 
-	return rdev->descr.route_frames(rdev->dev.parent, req, req_len,
-					rsp, rsp_len);
+	down_read(&rdev->lock);
+	if (rdev->descr.route_frames)
+		ret = rdev->descr.route_frames(rdev->dev.parent, req, req_len,
+					       rsp, rsp_len);
+	up_read(&rdev->lock);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(rpmb_route_frames);
 
@@ -126,6 +133,10 @@ EXPORT_SYMBOL_GPL(rpmb_interface_unregister);
  * This function should be called from the release function of the
  * underlying device used when the RPMB device was registered.
  *
+ * Once this returns, rpmb_route_frames() no longer calls into
+ * the driver, even though the users might still hold a reference
+ * to the RPMB dev.
+ *
  * Returns: < 0 on failure
  */
 int rpmb_dev_unregister(struct rpmb_dev *rdev)
@@ -133,6 +144,10 @@ int rpmb_dev_unregister(struct rpmb_dev *rdev)
 	if (!rdev)
 		return -EINVAL;
 
+	down_write(&rdev->lock);
+	rdev->descr.route_frames = NULL;
+	up_write(&rdev->lock);
+
 	device_del(&rdev->dev);
 
 	rpmb_dev_put(rdev);
@@ -165,6 +180,7 @@ struct rpmb_dev *rpmb_dev_register(struct device *dev,
 	if (!rdev)
 		return ERR_PTR(-ENOMEM);
 	rdev->descr = *descr;
+	init_rwsem(&rdev->lock);
 	rdev->descr.dev_id = kmemdup(descr->dev_id, descr->dev_id_len,
 				     GFP_KERNEL);
 	if (!rdev->descr.dev_id) {
diff --git a/include/linux/rpmb.h b/include/linux/rpmb.h
index ed3f8e431eff..0a976572d1cd 100644
--- a/include/linux/rpmb.h
+++ b/include/linux/rpmb.h
@@ -7,6 +7,7 @@
 #define __RPMB_H__
 
 #include <linux/device.h>
+#include <linux/rwsem.h>
 #include <linux/types.h>
 
 /**
@@ -51,12 +52,15 @@ struct rpmb_descr {
  * @id               : device_id
  * @list_node        : linked list node
  * @descr            : RPMB description
+ * @lock             : protect @descr.route_frames which is cleared when this
+ *                     device is unregistered
  */
 struct rpmb_dev {
 	struct device dev;
 	int id;
 	struct list_head list_node;
 	struct rpmb_descr descr;
+	struct rw_semaphore lock;
 };
 
 #define to_rpmb_dev(x)		container_of((x), struct rpmb_dev, dev)
-- 
2.55.0

Re: [PATCH 1/2] rpmb: stop the routing after unregister

[sashiko-bot]

> OP-TEE has a reference of RPMB device it used. So, it can route
> frames through the device after rpmb_dev_unregister() is called.
> Therefore, the parent device passed to route_frames() can be
> already freed, for example, mmc_blk_remove_rpmb_part() drops the
> last reference after rpmb unregister.
> 
> This patch clears the route_frames when the device is going to
> be removed by getting new rwsem and makes rpmb_route_frames()
> return -ENODEV if the device is gone.
> 
> Fixes: 1e9046e3a154 ("rpmb: add Replay Protected Memory Block (RPMB) subsystem")
> Assisted-by: LLM
> Signed-off-by: Haruka Kuze <harukak@zohomail.eu>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029081317.2264887-1-harukak@zohomail.eu?part=1

[PATCH 2/2] rpmb: add a virtio RPMB device driver

[Haruka Kuze]

No driver exists for virtio-rpmb devices (virtio ID 28), so OP-TEE
running in a guest can not use them.

This registers the virtio-rpmb devices found with the RPMB sub-system.
Each request is done in one transfer on request queue, and for key
programming and data writing a result read frame is appended so that
the device can return the result.

These frames are copied since the caller's buffers are not always
usable for DMA.

These frames follow the emmc definition, so that the device is
registered as RPMB_TYPE_EMMC. The device has no identifier of own, so
that a fixed dev_id is used for the key derivation for the device.

Based on the virtio-rpmb driver in Alex Bennée's RPMB series.

Link: https://lore.kernel.org/all/20220405093759.1126835-4-alex.bennee@linaro.org/
Signed-off-by: Haruka Kuze <harukak@zohomail.eu>
---
 MAINTAINERS                      |   7 +
 drivers/misc/Kconfig             |  10 ++
 drivers/misc/Makefile            |   1 +
 drivers/misc/virtio_rpmb.c       | 294 +++++++++++++++++++++++++++++++
 include/uapi/linux/virtio_rpmb.h |  56 ++++++
 5 files changed, 368 insertions(+)
 create mode 100644 drivers/misc/virtio_rpmb.c
 create mode 100644 include/uapi/linux/virtio_rpmb.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 748ce4d1c6a2..488003a28534 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -29707,6 +29707,13 @@ S:	Maintained
 F:	drivers/char/hw_random/virtio-rng.c
 F:	include/uapi/linux/virtio_rng.h
 
+VIRTIO RPMB DRIVER
+M:	Haruka Kuze <harukak@zohomail.eu>
+L:	virtualization@lists.linux.dev
+S:	Maintained
+F:	drivers/misc/virtio_rpmb.c
+F:	include/uapi/linux/virtio_rpmb.h
+
 VIRTIO RTC DRIVER
 M:	Peter Hilber <peter.hilber@oss.qualcomm.com>
 L:	virtualization@lists.linux.dev
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index c40e89cc5724..eb3518ecc8fe 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -91,6 +91,16 @@ config RPMB
 
 	  If unsure, select N.
 
+config VIRTIO_RPMB
+	tristate "Virtio RPMB driver"
+	depends on VIRTIO && RPMB
+	help
+	  Driver for virtio RPMB devices. The device is registered with the
+	  RPMB subsystem, where in-kernel users such as OP-TEE can use it.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called virtio_rpmb.
+
 config TI_FPC202
 	tristate "TI FPC202 Dual Port Controller"
 	depends on I2C
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 7cc3d615d37a..e4390bb8bdc0 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_TIFM_CORE)       	+= tifm_core.o
 obj-$(CONFIG_TIFM_7XX1)       	+= tifm_7xx1.o
 obj-$(CONFIG_PHANTOM)		+= phantom.o
 obj-$(CONFIG_RPMB)		+= rpmb-core.o
+obj-$(CONFIG_VIRTIO_RPMB)	+= virtio_rpmb.o
 obj-$(CONFIG_QCOM_COINCELL)	+= qcom-coincell.o
 obj-$(CONFIG_QCOM_FASTRPC)	+= fastrpc.o
 obj-$(CONFIG_SENSORS_BH1770)	+= bh1770glc.o
diff --git a/drivers/misc/virtio_rpmb.c b/drivers/misc/virtio_rpmb.c
new file mode 100644
index 000000000000..f02bdf13aa43
--- /dev/null
+++ b/drivers/misc/virtio_rpmb.c
@@ -0,0 +1,294 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Virtio RPMB driver
+ *
+ * Copyright (c) 2018-2019 Intel Corporation.
+ * Copyright (c) 2021-2022 Linaro Ltd.
+ *
+ * Based on the virtio-rpmb driver in Alex Bennée's RPMB series.
+ */
+
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/rpmb.h>
+#include <linux/scatterlist.h>
+#include <linux/slab.h>
+#include <linux/virtio.h>
+#include <linux/virtio_config.h>
+#include <linux/virtio_ids.h>
+#include <linux/virtio_rpmb.h>
+
+#define VIRTIO_RPMB_FRAME_SIZE	sizeof(struct virtio_rpmb_frame)
+static_assert(VIRTIO_RPMB_FRAME_SIZE == 512);
+
+/*
+ * This device has no identifier on its own.
+ * This value is used as input when OP-TEE is deriving the RPMB
+ * key, so it shall stay stable.
+ */
+static const u8 virtio_rpmb_dev_id[16] = "virtio-rpmb";
+
+/**
+ * struct virtio_rpmb - virtio RPMB device state
+ * @vdev: virtio device
+ * @vq: request queue, NULL while the device is gone or frozen
+ * @lock: serializes requests and protects @vq
+ * @rdev: RPMB device
+ * @max_wr: maximum number of blocks in one write request, 0 if unlimited
+ * @max_rd: maximum number of blocks in one read request, 0 if unlimited
+ */
+struct virtio_rpmb {
+	struct virtio_device *vdev;
+	struct virtqueue *vq;
+	struct mutex lock;
+	struct rpmb_dev *rdev;
+	u8 max_wr;
+	u8 max_rd;
+};
+
+struct virtio_rpmb_req {
+	struct completion done;
+	unsigned int len;
+};
+
+static void virtio_rpmb_done(struct virtqueue *vq)
+{
+	struct virtio_rpmb_req *req;
+	unsigned int len;
+
+	while ((req = virtqueue_get_buf(vq, &len))) {
+		req->len = len;
+		complete(&req->done);
+	}
+}
+
+static int virtio_rpmb_xfer(struct virtio_rpmb *vr, void *out,
+			    unsigned int out_len, void *in, unsigned int in_len)
+{
+	struct scatterlist out_sg, in_sg, *sgs[2] = { &out_sg, &in_sg };
+	struct virtio_rpmb_req req;
+	int ret;
+
+	sg_init_one(&out_sg, out, out_len);
+	sg_init_one(&in_sg, in, in_len);
+	init_completion(&req.done);
+
+	mutex_lock(&vr->lock);
+	if (!vr->vq) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	ret = virtqueue_add_sgs(vr->vq, sgs, 1, 1, &req, GFP_KERNEL);
+	if (ret)
+		goto out;
+
+	virtqueue_kick(vr->vq);
+	wait_for_completion(&req.done);
+
+	if (req.len != in_len)
+		ret = -EIO;
+out:
+	mutex_unlock(&vr->lock);
+	return ret;
+}
+
+static int virtio_rpmb_route_frames(struct device *dev, u8 *req,
+				    unsigned int req_len, u8 *resp,
+				    unsigned int resp_len)
+{
+	struct virtio_rpmb *vr = dev_to_virtio(dev)->priv;
+	struct virtio_rpmb_frame *frm = (struct virtio_rpmb_frame *)req;
+	struct virtio_rpmb_frame *result;
+	unsigned int out_len = req_len;
+	bool result_read = false;
+	void *out, *in;
+	int ret;
+
+	if (req_len < VIRTIO_RPMB_FRAME_SIZE)
+		return -EINVAL;
+
+	switch (be16_to_cpu(frm->req_resp)) {
+	case VIRTIO_RPMB_REQ_PROGRAM_KEY:
+		if (req_len != VIRTIO_RPMB_FRAME_SIZE ||
+		    resp_len != VIRTIO_RPMB_FRAME_SIZE)
+			return -EINVAL;
+		result_read = true;
+		break;
+	case VIRTIO_RPMB_REQ_GET_WRITE_COUNTER:
+		if (req_len != VIRTIO_RPMB_FRAME_SIZE ||
+		    resp_len != VIRTIO_RPMB_FRAME_SIZE)
+			return -EINVAL;
+		break;
+	case VIRTIO_RPMB_REQ_DATA_WRITE:
+		if (req_len % VIRTIO_RPMB_FRAME_SIZE ||
+		    resp_len != VIRTIO_RPMB_FRAME_SIZE)
+			return -EINVAL;
+		if (vr->max_wr && req_len / VIRTIO_RPMB_FRAME_SIZE > vr->max_wr)
+			return -EINVAL;
+		result_read = true;
+		break;
+	case VIRTIO_RPMB_REQ_DATA_READ:
+		if (req_len != VIRTIO_RPMB_FRAME_SIZE ||
+		    !resp_len || resp_len % VIRTIO_RPMB_FRAME_SIZE)
+			return -EINVAL;
+		if (vr->max_rd && resp_len / VIRTIO_RPMB_FRAME_SIZE > vr->max_rd)
+			return -EINVAL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/*
+	 * The caller's buffers may be unsuitable for DMA, so
+	 * copy the frames.  Writes are followed by result read
+	 * request in the same rpmb xfer, which makes the
+	 * device return the result.
+	 */
+	if (result_read)
+		out_len += VIRTIO_RPMB_FRAME_SIZE;
+
+	out = kmalloc(out_len, GFP_KERNEL);
+	if (!out)
+		return -ENOMEM;
+	in = kzalloc(resp_len, GFP_KERNEL);
+	if (!in) {
+		kfree(out);
+		return -ENOMEM;
+	}
+
+	memcpy(out, req, req_len);
+	if (result_read) {
+		result = out + req_len;
+		memset(result, 0, sizeof(*result));
+		result->block_count = cpu_to_be16(1);
+		result->req_resp = cpu_to_be16(VIRTIO_RPMB_REQ_RESULT_READ);
+	}
+
+	ret = virtio_rpmb_xfer(vr, out, out_len, in, resp_len);
+	if (!ret)
+		memcpy(resp, in, resp_len);
+
+	kfree(in);
+	kfree(out);
+	return ret;
+}
+
+static int virtio_rpmb_init_vq(struct virtio_rpmb *vr)
+{
+	struct virtqueue *vq;
+
+	vq = virtio_find_single_vq(vr->vdev, virtio_rpmb_done, "request");
+	if (IS_ERR(vq))
+		return PTR_ERR(vq);
+
+	mutex_lock(&vr->lock);
+	vr->vq = vq;
+	mutex_unlock(&vr->lock);
+	return 0;
+}
+
+static void virtio_rpmb_del_vq(struct virtio_rpmb *vr)
+{
+	struct virtio_device *vdev = vr->vdev;
+
+	mutex_lock(&vr->lock);
+	vr->vq = NULL;
+	mutex_unlock(&vr->lock);
+
+	virtio_reset_device(vdev);
+	vdev->config->del_vqs(vdev);
+}
+
+static int virtio_rpmb_probe(struct virtio_device *vdev)
+{
+	struct rpmb_descr descr = {
+		/* Frames and their semantics are that of eMMC */
+		.type = RPMB_TYPE_EMMC,
+		.route_frames = virtio_rpmb_route_frames,
+		.dev_id = (u8 *)virtio_rpmb_dev_id,
+		.dev_id_len = sizeof(virtio_rpmb_dev_id),
+	};
+	struct virtio_rpmb *vr;
+	u8 capacity;
+	int ret;
+
+	capacity = virtio_cread8(vdev, offsetof(struct virtio_rpmb_config,
+						capacity));
+	if (!capacity || capacity > 0x80)
+		return dev_err_probe(&vdev->dev, -EINVAL,
+				     "invalid capacity %u\n", capacity);
+
+	vr = devm_kzalloc(&vdev->dev, sizeof(*vr), GFP_KERNEL);
+	if (!vr)
+		return -ENOMEM;
+
+	vdev->priv = vr;
+	vr->vdev = vdev;
+	mutex_init(&vr->lock);
+	vr->max_wr = virtio_cread8(vdev, offsetof(struct virtio_rpmb_config,
+						  max_wr_cnt));
+	vr->max_rd = virtio_cread8(vdev, offsetof(struct virtio_rpmb_config,
+						  max_rd_cnt));
+
+	ret = virtio_rpmb_init_vq(vr);
+	if (ret)
+		return ret;
+
+	virtio_device_ready(vdev);
+
+	/*
+	 * eMMC reports the reliable wr count in 512 byte sectors, but the device
+	 * limit is in 256 byte blocks.
+	 */
+	descr.capacity = capacity;
+	descr.reliable_wr_count = (vr->max_wr ?: U8_MAX) / 2;
+
+	vr->rdev = rpmb_dev_register(&vdev->dev, &descr);
+	if (IS_ERR(vr->rdev)) {
+		virtio_rpmb_del_vq(vr);
+		return PTR_ERR(vr->rdev);
+	}
+
+	return 0;
+}
+
+static void virtio_rpmb_remove(struct virtio_device *vdev)
+{
+	struct virtio_rpmb *vr = vdev->priv;
+
+	rpmb_dev_unregister(vr->rdev);
+	virtio_rpmb_del_vq(vr);
+}
+
+static int virtio_rpmb_freeze(struct virtio_device *vdev)
+{
+	virtio_rpmb_del_vq(vdev->priv);
+	return 0;
+}
+
+static int virtio_rpmb_restore(struct virtio_device *vdev)
+{
+	return virtio_rpmb_init_vq(vdev->priv);
+}
+
+static const struct virtio_device_id id_table[] = {
+	{ VIRTIO_ID_RPMB, VIRTIO_DEV_ANY_ID },
+	{}
+};
+MODULE_DEVICE_TABLE(virtio, id_table);
+
+static struct virtio_driver virtio_rpmb_driver = {
+	.driver.name	= KBUILD_MODNAME,
+	.id_table	= id_table,
+	.probe		= virtio_rpmb_probe,
+	.remove		= virtio_rpmb_remove,
+	.freeze		= pm_sleep_ptr(virtio_rpmb_freeze),
+	.restore	= pm_sleep_ptr(virtio_rpmb_restore),
+};
+module_virtio_driver(virtio_rpmb_driver);
+
+MODULE_DESCRIPTION("Virtio RPMB driver");
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/linux/virtio_rpmb.h b/include/uapi/linux/virtio_rpmb.h
new file mode 100644
index 000000000000..66f45202857a
--- /dev/null
+++ b/include/uapi/linux/virtio_rpmb.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/*
+ * Virtio RPMB Device
+ *
+ * The Virtio RPMB Specification:
+ * https://github.com/oasis-tcs/virtio-spec/tree/master/device-types/rpmb
+ */
+
+#ifndef _UAPI_LINUX_VIRTIO_RPMB_H
+#define _UAPI_LINUX_VIRTIO_RPMB_H
+
+#include <linux/types.h>
+
+struct virtio_rpmb_config {
+	__u8 capacity;
+	__u8 max_wr_cnt;
+	__u8 max_rd_cnt;
+};
+
+/* Request types (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
+
+/* Response types (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
+
+/* Operation results (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
+
+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;
+};
+
+#endif /* _UAPI_LINUX_VIRTIO_RPMB_H */
-- 
2.55.0

[PATCH 2/2] rpmb: add a virtio RPMB device driver

[Haruka Kuze via OP-TEE]

No driver exists for virtio-rpmb devices (virtio ID 28), so OP-TEE
running in a guest can not use them.

This registers the virtio-rpmb devices found with the RPMB sub-system.
Each request is done in one transfer on request queue, and for key
programming and data writing a result read frame is appended so that
the device can return the result.

These frames are copied since the caller's buffers are not always
usable for DMA.

These frames follow the emmc definition, so that the device is
registered as RPMB_TYPE_EMMC. The device has no identifier of own, so
that a fixed dev_id is used for the key derivation for the device.

Based on the virtio-rpmb driver in Alex Bennée's RPMB series.

Link: https://lore.kernel.org/all/20220405093759.1126835-4-alex.bennee@linaro.org/
Signed-off-by: Haruka Kuze <harukak@zohomail.eu>
---
 MAINTAINERS                      |   7 +
 drivers/misc/Kconfig             |  10 ++
 drivers/misc/Makefile            |   1 +
 drivers/misc/virtio_rpmb.c       | 294 +++++++++++++++++++++++++++++++
 include/uapi/linux/virtio_rpmb.h |  56 ++++++
 5 files changed, 368 insertions(+)
 create mode 100644 drivers/misc/virtio_rpmb.c
 create mode 100644 include/uapi/linux/virtio_rpmb.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 748ce4d1c6a2..488003a28534 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -29707,6 +29707,13 @@ S:	Maintained
 F:	drivers/char/hw_random/virtio-rng.c
 F:	include/uapi/linux/virtio_rng.h
 
+VIRTIO RPMB DRIVER
+M:	Haruka Kuze <harukak@zohomail.eu>
+L:	virtualization@lists.linux.dev
+S:	Maintained
+F:	drivers/misc/virtio_rpmb.c
+F:	include/uapi/linux/virtio_rpmb.h
+
 VIRTIO RTC DRIVER
 M:	Peter Hilber <peter.hilber@oss.qualcomm.com>
 L:	virtualization@lists.linux.dev
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index c40e89cc5724..eb3518ecc8fe 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -91,6 +91,16 @@ config RPMB
 
 	  If unsure, select N.
 
+config VIRTIO_RPMB
+	tristate "Virtio RPMB driver"
+	depends on VIRTIO && RPMB
+	help
+	  Driver for virtio RPMB devices. The device is registered with the
+	  RPMB subsystem, where in-kernel users such as OP-TEE can use it.
+
+	  To compile this driver as a module, choose M here: the module will
+	  be called virtio_rpmb.
+
 config TI_FPC202
 	tristate "TI FPC202 Dual Port Controller"
 	depends on I2C
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 7cc3d615d37a..e4390bb8bdc0 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -16,6 +16,7 @@ obj-$(CONFIG_TIFM_CORE)       	+= tifm_core.o
 obj-$(CONFIG_TIFM_7XX1)       	+= tifm_7xx1.o
 obj-$(CONFIG_PHANTOM)		+= phantom.o
 obj-$(CONFIG_RPMB)		+= rpmb-core.o
+obj-$(CONFIG_VIRTIO_RPMB)	+= virtio_rpmb.o
 obj-$(CONFIG_QCOM_COINCELL)	+= qcom-coincell.o
 obj-$(CONFIG_QCOM_FASTRPC)	+= fastrpc.o
 obj-$(CONFIG_SENSORS_BH1770)	+= bh1770glc.o
diff --git a/drivers/misc/virtio_rpmb.c b/drivers/misc/virtio_rpmb.c
new file mode 100644
index 000000000000..f02bdf13aa43
--- /dev/null
+++ b/drivers/misc/virtio_rpmb.c
@@ -0,0 +1,294 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Virtio RPMB driver
+ *
+ * Copyright (c) 2018-2019 Intel Corporation.
+ * Copyright (c) 2021-2022 Linaro Ltd.
+ *
+ * Based on the virtio-rpmb driver in Alex Bennée's RPMB series.
+ */
+
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/rpmb.h>
+#include <linux/scatterlist.h>
+#include <linux/slab.h>
+#include <linux/virtio.h>
+#include <linux/virtio_config.h>
+#include <linux/virtio_ids.h>
+#include <linux/virtio_rpmb.h>
+
+#define VIRTIO_RPMB_FRAME_SIZE	sizeof(struct virtio_rpmb_frame)
+static_assert(VIRTIO_RPMB_FRAME_SIZE == 512);
+
+/*
+ * This device has no identifier on its own.
+ * This value is used as input when OP-TEE is deriving the RPMB
+ * key, so it shall stay stable.
+ */
+static const u8 virtio_rpmb_dev_id[16] = "virtio-rpmb";
+
+/**
+ * struct virtio_rpmb - virtio RPMB device state
+ * @vdev: virtio device
+ * @vq: request queue, NULL while the device is gone or frozen
+ * @lock: serializes requests and protects @vq
+ * @rdev: RPMB device
+ * @max_wr: maximum number of blocks in one write request, 0 if unlimited
+ * @max_rd: maximum number of blocks in one read request, 0 if unlimited
+ */
+struct virtio_rpmb {
+	struct virtio_device *vdev;
+	struct virtqueue *vq;
+	struct mutex lock;
+	struct rpmb_dev *rdev;
+	u8 max_wr;
+	u8 max_rd;
+};
+
+struct virtio_rpmb_req {
+	struct completion done;
+	unsigned int len;
+};
+
+static void virtio_rpmb_done(struct virtqueue *vq)
+{
+	struct virtio_rpmb_req *req;
+	unsigned int len;
+
+	while ((req = virtqueue_get_buf(vq, &len))) {
+		req->len = len;
+		complete(&req->done);
+	}
+}
+
+static int virtio_rpmb_xfer(struct virtio_rpmb *vr, void *out,
+			    unsigned int out_len, void *in, unsigned int in_len)
+{
+	struct scatterlist out_sg, in_sg, *sgs[2] = { &out_sg, &in_sg };
+	struct virtio_rpmb_req req;
+	int ret;
+
+	sg_init_one(&out_sg, out, out_len);
+	sg_init_one(&in_sg, in, in_len);
+	init_completion(&req.done);
+
+	mutex_lock(&vr->lock);
+	if (!vr->vq) {
+		ret = -ENODEV;
+		goto out;
+	}
+
+	ret = virtqueue_add_sgs(vr->vq, sgs, 1, 1, &req, GFP_KERNEL);
+	if (ret)
+		goto out;
+
+	virtqueue_kick(vr->vq);
+	wait_for_completion(&req.done);
+
+	if (req.len != in_len)
+		ret = -EIO;
+out:
+	mutex_unlock(&vr->lock);
+	return ret;
+}
+
+static int virtio_rpmb_route_frames(struct device *dev, u8 *req,
+				    unsigned int req_len, u8 *resp,
+				    unsigned int resp_len)
+{
+	struct virtio_rpmb *vr = dev_to_virtio(dev)->priv;
+	struct virtio_rpmb_frame *frm = (struct virtio_rpmb_frame *)req;
+	struct virtio_rpmb_frame *result;
+	unsigned int out_len = req_len;
+	bool result_read = false;
+	void *out, *in;
+	int ret;
+
+	if (req_len < VIRTIO_RPMB_FRAME_SIZE)
+		return -EINVAL;
+
+	switch (be16_to_cpu(frm->req_resp)) {
+	case VIRTIO_RPMB_REQ_PROGRAM_KEY:
+		if (req_len != VIRTIO_RPMB_FRAME_SIZE ||
+		    resp_len != VIRTIO_RPMB_FRAME_SIZE)
+			return -EINVAL;
+		result_read = true;
+		break;
+	case VIRTIO_RPMB_REQ_GET_WRITE_COUNTER:
+		if (req_len != VIRTIO_RPMB_FRAME_SIZE ||
+		    resp_len != VIRTIO_RPMB_FRAME_SIZE)
+			return -EINVAL;
+		break;
+	case VIRTIO_RPMB_REQ_DATA_WRITE:
+		if (req_len % VIRTIO_RPMB_FRAME_SIZE ||
+		    resp_len != VIRTIO_RPMB_FRAME_SIZE)
+			return -EINVAL;
+		if (vr->max_wr && req_len / VIRTIO_RPMB_FRAME_SIZE > vr->max_wr)
+			return -EINVAL;
+		result_read = true;
+		break;
+	case VIRTIO_RPMB_REQ_DATA_READ:
+		if (req_len != VIRTIO_RPMB_FRAME_SIZE ||
+		    !resp_len || resp_len % VIRTIO_RPMB_FRAME_SIZE)
+			return -EINVAL;
+		if (vr->max_rd && resp_len / VIRTIO_RPMB_FRAME_SIZE > vr->max_rd)
+			return -EINVAL;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	/*
+	 * The caller's buffers may be unsuitable for DMA, so
+	 * copy the frames.  Writes are followed by result read
+	 * request in the same rpmb xfer, which makes the
+	 * device return the result.
+	 */
+	if (result_read)
+		out_len += VIRTIO_RPMB_FRAME_SIZE;
+
+	out = kmalloc(out_len, GFP_KERNEL);
+	if (!out)
+		return -ENOMEM;
+	in = kzalloc(resp_len, GFP_KERNEL);
+	if (!in) {
+		kfree(out);
+		return -ENOMEM;
+	}
+
+	memcpy(out, req, req_len);
+	if (result_read) {
+		result = out + req_len;
+		memset(result, 0, sizeof(*result));
+		result->block_count = cpu_to_be16(1);
+		result->req_resp = cpu_to_be16(VIRTIO_RPMB_REQ_RESULT_READ);
+	}
+
+	ret = virtio_rpmb_xfer(vr, out, out_len, in, resp_len);
+	if (!ret)
+		memcpy(resp, in, resp_len);
+
+	kfree(in);
+	kfree(out);
+	return ret;
+}
+
+static int virtio_rpmb_init_vq(struct virtio_rpmb *vr)
+{
+	struct virtqueue *vq;
+
+	vq = virtio_find_single_vq(vr->vdev, virtio_rpmb_done, "request");
+	if (IS_ERR(vq))
+		return PTR_ERR(vq);
+
+	mutex_lock(&vr->lock);
+	vr->vq = vq;
+	mutex_unlock(&vr->lock);
+	return 0;
+}
+
+static void virtio_rpmb_del_vq(struct virtio_rpmb *vr)
+{
+	struct virtio_device *vdev = vr->vdev;
+
+	mutex_lock(&vr->lock);
+	vr->vq = NULL;
+	mutex_unlock(&vr->lock);
+
+	virtio_reset_device(vdev);
+	vdev->config->del_vqs(vdev);
+}
+
+static int virtio_rpmb_probe(struct virtio_device *vdev)
+{
+	struct rpmb_descr descr = {
+		/* Frames and their semantics are that of eMMC */
+		.type = RPMB_TYPE_EMMC,
+		.route_frames = virtio_rpmb_route_frames,
+		.dev_id = (u8 *)virtio_rpmb_dev_id,
+		.dev_id_len = sizeof(virtio_rpmb_dev_id),
+	};
+	struct virtio_rpmb *vr;
+	u8 capacity;
+	int ret;
+
+	capacity = virtio_cread8(vdev, offsetof(struct virtio_rpmb_config,
+						capacity));
+	if (!capacity || capacity > 0x80)
+		return dev_err_probe(&vdev->dev, -EINVAL,
+				     "invalid capacity %u\n", capacity);
+
+	vr = devm_kzalloc(&vdev->dev, sizeof(*vr), GFP_KERNEL);
+	if (!vr)
+		return -ENOMEM;
+
+	vdev->priv = vr;
+	vr->vdev = vdev;
+	mutex_init(&vr->lock);
+	vr->max_wr = virtio_cread8(vdev, offsetof(struct virtio_rpmb_config,
+						  max_wr_cnt));
+	vr->max_rd = virtio_cread8(vdev, offsetof(struct virtio_rpmb_config,
+						  max_rd_cnt));
+
+	ret = virtio_rpmb_init_vq(vr);
+	if (ret)
+		return ret;
+
+	virtio_device_ready(vdev);
+
+	/*
+	 * eMMC reports the reliable wr count in 512 byte sectors, but the device
+	 * limit is in 256 byte blocks.
+	 */
+	descr.capacity = capacity;
+	descr.reliable_wr_count = (vr->max_wr ?: U8_MAX) / 2;
+
+	vr->rdev = rpmb_dev_register(&vdev->dev, &descr);
+	if (IS_ERR(vr->rdev)) {
+		virtio_rpmb_del_vq(vr);
+		return PTR_ERR(vr->rdev);
+	}
+
+	return 0;
+}
+
+static void virtio_rpmb_remove(struct virtio_device *vdev)
+{
+	struct virtio_rpmb *vr = vdev->priv;
+
+	rpmb_dev_unregister(vr->rdev);
+	virtio_rpmb_del_vq(vr);
+}
+
+static int virtio_rpmb_freeze(struct virtio_device *vdev)
+{
+	virtio_rpmb_del_vq(vdev->priv);
+	return 0;
+}
+
+static int virtio_rpmb_restore(struct virtio_device *vdev)
+{
+	return virtio_rpmb_init_vq(vdev->priv);
+}
+
+static const struct virtio_device_id id_table[] = {
+	{ VIRTIO_ID_RPMB, VIRTIO_DEV_ANY_ID },
+	{}
+};
+MODULE_DEVICE_TABLE(virtio, id_table);
+
+static struct virtio_driver virtio_rpmb_driver = {
+	.driver.name	= KBUILD_MODNAME,
+	.id_table	= id_table,
+	.probe		= virtio_rpmb_probe,
+	.remove		= virtio_rpmb_remove,
+	.freeze		= pm_sleep_ptr(virtio_rpmb_freeze),
+	.restore	= pm_sleep_ptr(virtio_rpmb_restore),
+};
+module_virtio_driver(virtio_rpmb_driver);
+
+MODULE_DESCRIPTION("Virtio RPMB driver");
+MODULE_LICENSE("GPL");
diff --git a/include/uapi/linux/virtio_rpmb.h b/include/uapi/linux/virtio_rpmb.h
new file mode 100644
index 000000000000..66f45202857a
--- /dev/null
+++ b/include/uapi/linux/virtio_rpmb.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: ((GPL-2.0 WITH Linux-syscall-note) OR BSD-3-Clause) */
+/*
+ * Virtio RPMB Device
+ *
+ * The Virtio RPMB Specification:
+ * https://github.com/oasis-tcs/virtio-spec/tree/master/device-types/rpmb
+ */
+
+#ifndef _UAPI_LINUX_VIRTIO_RPMB_H
+#define _UAPI_LINUX_VIRTIO_RPMB_H
+
+#include <linux/types.h>
+
+struct virtio_rpmb_config {
+	__u8 capacity;
+	__u8 max_wr_cnt;
+	__u8 max_rd_cnt;
+};
+
+/* Request types (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
+
+/* Response types (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
+
+/* Operation results (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
+
+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;
+};
+
+#endif /* _UAPI_LINUX_VIRTIO_RPMB_H */
-- 
2.55.0

Re: [PATCH 2/2] rpmb: add a virtio RPMB device driver

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] rpmb: virtio_rpmb_probe() erroneously divides max_wr by 2
- [Medium] virtio_rpmb: vr->vq published in virtio_rpmb_init_vq() before device ready
--

commit 0fd82617fed90856fa2c50ebdd578db85ac1bddd
Author: Haruka Kuze <harukak@zohomail.eu>

rpmb: add a virtio RPMB device driver

This commit adds a driver for virtio-rpmb devices (virtio ID 28) and registers
them with the RPMB subsystem as RPMB_TYPE_EMMC so in-kernel consumers such as
OP-TEE can use them.

Link: https://lore.kernel.org/all/20220405093759.1126835-4-alex.bennee@linaro.org/

> diff --git a/drivers/misc/virtio_rpmb.c b/drivers/misc/virtio_rpmb.c
> new file mode 100644
> index 0000000000000..f02bdf13aa43c
> --- /dev/null
> +++ b/drivers/misc/virtio_rpmb.c

[ ... ]

> +static int virtio_rpmb_init_vq(struct virtio_rpmb *vr)
> +{
> +	struct virtqueue *vq;
> +
> +	vq = virtio_find_single_vq(vr->vdev, virtio_rpmb_done, "request");
> +	if (IS_ERR(vq))
> +		return PTR_ERR(vq);
> +
> +	mutex_lock(&vr->lock);
> +	vr->vq = vq;
> +	mutex_unlock(&vr->lock);
> +	return 0;
> +}

[ ... ]

> +static int virtio_rpmb_probe(struct virtio_device *vdev)
> +{
> +	struct rpmb_descr descr = {
> +		/* Frames and their semantics are that of eMMC */
> +		.type = RPMB_TYPE_EMMC,
> +		.route_frames = virtio_rpmb_route_frames,
> +		.dev_id = (u8 *)virtio_rpmb_dev_id,
> +		.dev_id_len = sizeof(virtio_rpmb_dev_id),
> +	};
> +	struct virtio_rpmb *vr;
> +	u8 capacity;
> +	int ret;
> +
> +	capacity = virtio_cread8(vdev, offsetof(struct virtio_rpmb_config,
> +						capacity));
> +	if (!capacity || capacity > 0x80)
> +		return dev_err_probe(&vdev->dev, -EINVAL,
> +				     "invalid capacity %u\n", capacity);
> +
> +	vr = devm_kzalloc(&vdev->dev, sizeof(*vr), GFP_KERNEL);
> +	if (!vr)
> +		return -ENOMEM;
> +
> +	vdev->priv = vr;
> +	vr->vdev = vdev;
> +	mutex_init(&vr->lock);
> +	vr->max_wr = virtio_cread8(vdev, offsetof(struct virtio_rpmb_config,
> +						  max_wr_cnt));
> +	vr->max_rd = virtio_cread8(vdev, offsetof(struct virtio_rpmb_config,
> +						  max_rd_cnt));
> +
> +	ret = virtio_rpmb_init_vq(vr);
> +	if (ret)
> +		return ret;
> +
> +	virtio_device_ready(vdev);
> +
> +	/*
> +	 * eMMC reports the reliable wr count in 512 byte sectors, but the device
> +	 * limit is in 256 byte blocks.
> +	 */
> +	descr.capacity = capacity;
> +	descr.reliable_wr_count = (vr->max_wr ?: U8_MAX) / 2;

[Severity: High]
Should descr.reliable_wr_count be set without dividing vr->max_wr by 2?

Each 256-byte RPMB data block is carried inside a single 512-byte
struct virtio_rpmb_frame, so one 256-byte RPMB block corresponds 1-to-1 with
one 512-byte RPMB frame/sector. This matches how virtio_rpmb_route_frames()
validates write requests against vr->max_wr:

	case VIRTIO_RPMB_REQ_DATA_WRITE:
		if (req_len % VIRTIO_RPMB_FRAME_SIZE ||
		    resp_len != VIRTIO_RPMB_FRAME_SIZE)
			return -EINVAL;
		if (vr->max_wr && req_len / VIRTIO_RPMB_FRAME_SIZE > vr->max_wr)
			return -EINVAL;

When a virtio-rpmb device reports max_wr_cnt == 1, dividing vr->max_wr by 2
truncates descr.reliable_wr_count to 0. In-kernel consumers such as OP-TEE
receive rdev->descr.reliable_wr_count directly in
handle_rpc_func_rpmb_probe_next(), so reporting 0 makes the device unusable
for writes when max_wr_cnt is 1, and halves the maximum write count for larger
values.

> +
> +	vr->rdev = rpmb_dev_register(&vdev->dev, &descr);
> +	if (IS_ERR(vr->rdev)) {
> +		virtio_rpmb_del_vq(vr);
> +		return PTR_ERR(vr->rdev);
> +	}
> +
> +	return 0;
> +}
> +
> +static void virtio_rpmb_remove(struct virtio_device *vdev)
> +{
> +	struct virtio_rpmb *vr = vdev->priv;
> +
> +	rpmb_dev_unregister(vr->rdev);
> +	virtio_rpmb_del_vq(vr);
> +}
> +
> +static int virtio_rpmb_freeze(struct virtio_device *vdev)
> +{
> +	virtio_rpmb_del_vq(vdev->priv);
> +	return 0;
> +}
> +
> +static int virtio_rpmb_restore(struct virtio_device *vdev)
> +{
> +	return virtio_rpmb_init_vq(vdev->priv);
> +}

[Severity: Medium]
Can virtio_rpmb_restore() allow virtio_rpmb_xfer() to kick the virtqueue
before the device is marked VIRTIO_CONFIG_S_DRIVER_OK?

During freeze and restore, vr->rdev remains registered with the RPMB core, and
virtio_rpmb_xfer() uses vr->vq under vr->lock to check whether the device is
available:

	mutex_lock(&vr->lock);
	if (!vr->vq) {
		ret = -ENODEV;
		goto out;
	}

	ret = virtqueue_add_sgs(vr->vq, sgs, 1, 1, &req, GFP_KERNEL);
	if (ret)
		goto out;

	virtqueue_kick(vr->vq);
	wait_for_completion(&req.done);

In drivers/virtio/virtio.c, virtio_device_restore_priv() only calls
virtio_device_ready(dev) after drv->restore(dev) returns:

	if (restore) {
		if (drv->restore) {
			ret = drv->restore(dev);
			if (ret)
				goto err;
		}
	...
	/* If restore didn't do it, mark device DRIVER_OK ourselves. */
	if (!(dev->config->get_status(dev) & VIRTIO_CONFIG_S_DRIVER_OK))
		virtio_device_ready(dev);

Because virtio_rpmb_init_vq() sets vr->vq = vq and releases vr->lock before
virtio_device_ready(vdev) is called, a concurrent rpmb_route_frames() ->
virtio_rpmb_route_frames() -> virtio_rpmb_xfer() call can observe a non-NULL
vr->vq and call virtqueue_add_sgs() and virtqueue_kick() before
VIRTIO_CONFIG_S_DRIVER_OK is set (or while vq->broken is still true under
CONFIG_VIRTIO_HARDEN_NOTIFICATION).

Should virtio_device_ready(vr->vdev) be called inside virtio_rpmb_init_vq()
before publishing vr->vq under vr->lock?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261029081317.2264887-1-harukak@zohomail.eu?part=2

Re: [PATCH 2/2] rpmb: add a virtio RPMB device driver

[Greg Kroah-Hartman]

On Thu, Oct 29, 2026 at 05:13:17PM +0900, Haruka Kuze wrote:
> No driver exists for virtio-rpmb devices (virtio ID 28), so OP-TEE
> running in a guest can not use them.
> 
> This registers the virtio-rpmb devices found with the RPMB sub-system.
> Each request is done in one transfer on request queue, and for key
> programming and data writing a result read frame is appended so that
> the device can return the result.
> 
> These frames are copied since the caller's buffers are not always
> usable for DMA.
> 
> These frames follow the emmc definition, so that the device is
> registered as RPMB_TYPE_EMMC. The device has no identifier of own, so
> that a fixed dev_id is used for the key derivation for the device.
> 
> Based on the virtio-rpmb driver in Alex Bennée's RPMB series.
> 
> Link: https://lore.kernel.org/all/20220405093759.1126835-4-alex.bennee@linaro.org/
> Signed-off-by: Haruka Kuze <harukak@zohomail.eu>
> ---
>  MAINTAINERS                      |   7 +
>  drivers/misc/Kconfig             |  10 ++
>  drivers/misc/Makefile            |   1 +
>  drivers/misc/virtio_rpmb.c       | 294 +++++++++++++++++++++++++++++++
>  include/uapi/linux/virtio_rpmb.h |  56 ++++++
>  5 files changed, 368 insertions(+)
>  create mode 100644 drivers/misc/virtio_rpmb.c
>  create mode 100644 include/uapi/linux/virtio_rpmb.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 748ce4d1c6a2..488003a28534 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -29707,6 +29707,13 @@ S:	Maintained
>  F:	drivers/char/hw_random/virtio-rng.c
>  F:	include/uapi/linux/virtio_rng.h
>  
> +VIRTIO RPMB DRIVER
> +M:	Haruka Kuze <harukak@zohomail.eu>
> +L:	virtualization@lists.linux.dev
> +S:	Maintained
> +F:	drivers/misc/virtio_rpmb.c
> +F:	include/uapi/linux/virtio_rpmb.h
> +
>  VIRTIO RTC DRIVER
>  M:	Peter Hilber <peter.hilber@oss.qualcomm.com>
>  L:	virtualization@lists.linux.dev
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index c40e89cc5724..eb3518ecc8fe 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -91,6 +91,16 @@ config RPMB
>  
>  	  If unsure, select N.
>  
> +config VIRTIO_RPMB
> +	tristate "Virtio RPMB driver"
> +	depends on VIRTIO && RPMB
> +	help
> +	  Driver for virtio RPMB devices. The device is registered with the
> +	  RPMB subsystem, where in-kernel users such as OP-TEE can use it.
> +
> +	  To compile this driver as a module, choose M here: the module will
> +	  be called virtio_rpmb.
> +
>  config TI_FPC202
>  	tristate "TI FPC202 Dual Port Controller"
>  	depends on I2C
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 7cc3d615d37a..e4390bb8bdc0 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -16,6 +16,7 @@ obj-$(CONFIG_TIFM_CORE)       	+= tifm_core.o
>  obj-$(CONFIG_TIFM_7XX1)       	+= tifm_7xx1.o
>  obj-$(CONFIG_PHANTOM)		+= phantom.o
>  obj-$(CONFIG_RPMB)		+= rpmb-core.o
> +obj-$(CONFIG_VIRTIO_RPMB)	+= virtio_rpmb.o
>  obj-$(CONFIG_QCOM_COINCELL)	+= qcom-coincell.o
>  obj-$(CONFIG_QCOM_FASTRPC)	+= fastrpc.o
>  obj-$(CONFIG_SENSORS_BH1770)	+= bh1770glc.o
> diff --git a/drivers/misc/virtio_rpmb.c b/drivers/misc/virtio_rpmb.c
> new file mode 100644
> index 000000000000..f02bdf13aa43
> --- /dev/null
> +++ b/drivers/misc/virtio_rpmb.c
> @@ -0,0 +1,294 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Virtio RPMB driver
> + *
> + * Copyright (c) 2018-2019 Intel Corporation.
> + * Copyright (c) 2021-2022 Linaro Ltd.
> + *
> + * Based on the virtio-rpmb driver in Alex Bennée's RPMB series.
> + */
> +
> +#include <linux/completion.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/rpmb.h>
> +#include <linux/scatterlist.h>
> +#include <linux/slab.h>
> +#include <linux/virtio.h>
> +#include <linux/virtio_config.h>
> +#include <linux/virtio_ids.h>
> +#include <linux/virtio_rpmb.h>
> +
> +#define VIRTIO_RPMB_FRAME_SIZE	sizeof(struct virtio_rpmb_frame)
> +static_assert(VIRTIO_RPMB_FRAME_SIZE == 512);
> +
> +/*
> + * This device has no identifier on its own.
> + * This value is used as input when OP-TEE is deriving the RPMB
> + * key, so it shall stay stable.
> + */
> +static const u8 virtio_rpmb_dev_id[16] = "virtio-rpmb";
> +
> +/**
> + * struct virtio_rpmb - virtio RPMB device state
> + * @vdev: virtio device
> + * @vq: request queue, NULL while the device is gone or frozen
> + * @lock: serializes requests and protects @vq
> + * @rdev: RPMB device
> + * @max_wr: maximum number of blocks in one write request, 0 if unlimited
> + * @max_rd: maximum number of blocks in one read request, 0 if unlimited
> + */
> +struct virtio_rpmb {
> +	struct virtio_device *vdev;
> +	struct virtqueue *vq;
> +	struct mutex lock;
> +	struct rpmb_dev *rdev;
> +	u8 max_wr;
> +	u8 max_rd;
> +};
> +
> +struct virtio_rpmb_req {
> +	struct completion done;
> +	unsigned int len;
> +};
> +
> +static void virtio_rpmb_done(struct virtqueue *vq)
> +{
> +	struct virtio_rpmb_req *req;
> +	unsigned int len;
> +
> +	while ((req = virtqueue_get_buf(vq, &len))) {
> +		req->len = len;
> +		complete(&req->done);
> +	}
> +}
> +
> +static int virtio_rpmb_xfer(struct virtio_rpmb *vr, void *out,
> +			    unsigned int out_len, void *in, unsigned int in_len)
> +{
> +	struct scatterlist out_sg, in_sg, *sgs[2] = { &out_sg, &in_sg };
> +	struct virtio_rpmb_req req;
> +	int ret;
> +
> +	sg_init_one(&out_sg, out, out_len);
> +	sg_init_one(&in_sg, in, in_len);
> +	init_completion(&req.done);
> +
> +	mutex_lock(&vr->lock);

guard()?

thanks,

greg k-h