[PATCH v5 1/3] rpmb: core: Guard frame requests and teardown with mutex

[Stanley Jhu]

rpmb_route_frames() has no serialisation. That has two independent
consequences.

Concurrent requests on one rpmb_dev corrupt each other. Two kthreads
issuing RPMB_GET_WRITE_COUNTER against the same UFS RPMB region, 20000
iterations each:

  race complete: MISMATCH=17030 ERRORS=0

A mismatch is a response whose echoed nonce belongs to the other thread:
43% of requests returned somebody else's frame, with no transport error.
An authenticated RPMB operation is not a single command. Per JESD220F
12.4.2 a region processes one at a time. Per 12.4.7 any request other
than a result read overwrites the region's result register. The rpmb_dev
is the granularity the device itself assumes.

A request can also be in flight when the provider tears down.
rpmb_dev_unregister() calls device_del(), which drops the reference
device_add() took on the parent. The parent can then be freed while the
rpmb_dev is still alive and routable through rdev->dev.parent. Unbinding
a UFS host under a consumer that keeps issuing requests:

  BUG: KASAN: slab-use-after-free in ufs_rpmb_route_frames+0x328/0x420
  Read of size 8 at addr fff00000c833bce8 by task rpmb_hold/100
  Call trace:
   ufs_rpmb_route_frames+0x328/0x420
   rpmb_route_frames+0x64/0xd0
  Freed by task 1:
   kfree+0x2b8/0x5c4
   ufs_rpmb_device_release+0x3c/0x60
   device_release+0xa0/0x1fc
   device_unregister+0x20/0x38
   ufs_rpmb_remove+0x130/0x230
   ufshcd_remove+0x54/0x22c

The stable tag is for eMMC. mmc_route_rpmb_frames() packs the sequence
into one block request, so eMMC cannot interleave. The teardown window
is still open there. mmc_blk_remove() reaches rpmb_dev_unregister() well
before tearing down the queue that request goes to. That window is from
source reading, not reproduced.

Add a mutex and a dead flag to struct rpmb_dev. One lock held across the
whole request both serialises requests and excludes unregistration. The
kerneldoc is wrong as well: until rpmb_dev_unregister() runs, the child
holds a reference that keeps the parent's release callback from running,
so a provider cannot unregister from there. Both tests then report
MISMATCH=0 and a clean unbind.

Fixes: 1e9046e3a154 ("rpmb: add Replay Protected Memory Block (RPMB) subsystem")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Stanley Jhu <stanleyjhu@xxxxxxxxxx>
Reviewed-by: Bean Huo <beanhuo@xxxxxxxxxx>
---

Notes:
    Both measurements need all three patches applied: on current kernels no
    rpmb_dev registers on UFS, so this patch alone changes nothing there.

 drivers/misc/rpmb-core.c | 34 +++++++++++++++++++++++++++++-----
 include/linux/rpmb.h     |  5 +++++
 2 files changed, 34 insertions(+), 5 deletions(-)

diff --git a/drivers/misc/rpmb-core.c b/drivers/misc/rpmb-core.c
index ecf14acf230a..bbc3c404ad6f 100644
--- a/drivers/misc/rpmb-core.c
+++ b/drivers/misc/rpmb-core.c
@@ -45,16 +45,28 @@ EXPORT_SYMBOL_GPL(rpmb_dev_put);
  * @rsp:	rpmb response frames
  * @rsp_len:	length of rpmb response frames in bytes
  *
+ * Context: Might sleep.
+ *
  * Returns: < 0 on failure
  */
 int rpmb_route_frames(struct rpmb_dev *rdev, u8 *req,
 		      unsigned int req_len, u8 *rsp, unsigned int rsp_len)
 {
-	if (!req || !req_len || !rsp || !rsp_len)
+	int ret;
+
+	if (!rdev || !req || !req_len || !rsp || !rsp_len)
 		return -EINVAL;
 
-	return rdev->descr.route_frames(rdev->dev.parent, req, req_len,
-					rsp, rsp_len);
+	mutex_lock(&rdev->lock);
+	if (rdev->dead) {
+		mutex_unlock(&rdev->lock);
+		return -ENODEV;
+	}
+
+	ret = rdev->descr.route_frames(rdev->dev.parent, req, req_len,
+				       rsp, rsp_len);
+	mutex_unlock(&rdev->lock);
+	return ret;
 }
 EXPORT_SYMBOL_GPL(rpmb_route_frames);
 
@@ -62,6 +74,7 @@ static void rpmb_dev_release(struct device *dev)
 {
 	struct rpmb_dev *rdev = to_rpmb_dev(dev);
 
+	mutex_destroy(&rdev->lock);
 	ida_free(&rpmb_ida, rdev->id);
 	kfree(rdev->descr.dev_id);
 	kfree(rdev);
@@ -123,8 +136,9 @@ EXPORT_SYMBOL_GPL(rpmb_interface_unregister);
  * rpmb_dev_unregister() - unregister RPMB partition from the RPMB subsystem
  * @rdev: the rpmb device to unregister
  *
- * This function should be called from the release function of the
- * underlying device used when the RPMB device was registered.
+ * This function should be called from the remove or unbind callback of the
+ * underlying device used when the RPMB device was registered, never from
+ * a device release callback.
  *
  * Returns: < 0 on failure
  */
@@ -133,6 +147,14 @@ int rpmb_dev_unregister(struct rpmb_dev *rdev)
 	if (!rdev)
 		return -EINVAL;
 
+	mutex_lock(&rdev->lock);
+	if (rdev->dead) {
+		mutex_unlock(&rdev->lock);
+		return 0;
+	}
+	rdev->dead = true;
+	mutex_unlock(&rdev->lock);
+
 	device_del(&rdev->dev);
 
 	rpmb_dev_put(rdev);
@@ -164,6 +186,7 @@ struct rpmb_dev *rpmb_dev_register(struct device *dev,
 	rdev = kzalloc_obj(*rdev);
 	if (!rdev)
 		return ERR_PTR(-ENOMEM);
+	mutex_init(&rdev->lock);
 	rdev->descr = *descr;
 	rdev->descr.dev_id = kmemdup(descr->dev_id, descr->dev_id_len,
 				     GFP_KERNEL);
@@ -194,6 +217,7 @@ struct rpmb_dev *rpmb_dev_register(struct device *dev,
 err_free_dev_id:
 	kfree(rdev->descr.dev_id);
 err_free_rdev:
+	mutex_destroy(&rdev->lock);
 	kfree(rdev);
 	return ERR_PTR(ret);
 }
diff --git a/include/linux/rpmb.h b/include/linux/rpmb.h
index ed3f8e431eff..65807f597735 100644
--- a/include/linux/rpmb.h
+++ b/include/linux/rpmb.h
@@ -7,6 +7,7 @@
 #define __RPMB_H__
 
 #include <linux/device.h>
+#include <linux/mutex.h>
 #include <linux/types.h>
 
 /**
@@ -48,15 +49,19 @@ struct rpmb_descr {
  * struct rpmb_dev - device which can support RPMB partition
  *
  * @dev              : device
+ * @lock             : serialises requests and protects them against teardown
  * @id               : device_id
  * @list_node        : linked list node
  * @descr            : RPMB description
+ * @dead             : set to true when device is unregistered
  */
 struct rpmb_dev {
 	struct device dev;
+	struct mutex lock;	/* Serialises route_frames(), guards @dead */
 	int id;
 	struct list_head list_node;
 	struct rpmb_descr descr;
+	bool dead;
 };
 
 #define to_rpmb_dev(x)		container_of((x), struct rpmb_dev, dev)
-- 
2.55.0.1007.g17ff1f9808-goog