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

[Stanley Jhu]

rpmb_route_frames() invokes the provider callback without serialization,
which causes two races:

1. Concurrent callers on the same rpmb_dev interleave multi-frame
   authenticated sequences on the region's single result register and
   receive each other's responses without error.
2. rpmb_dev_unregister() calls device_del() and allows the parent
   provider device to be freed while an in-flight rpmb_route_frames()
   call still dereferences rdev->dev.parent, causing a use-after-free.

Serialize rpmb_route_frames() and rpmb_dev_unregister() with a mutex in
struct rpmb_dev and clear descr.route_frames on unregistration so new
requests fail with -ENODEV. Also update the stale kerneldoc note
advising providers to unregister from their parent release callback,
which cannot run while the child rpmb_dev still holds a parent
reference.

Fixes: 1e9046e3a154 ("rpmb: add Replay Protected Memory Block (RPMB) subsystem")
Cc: stable@xxxxxxxxxxxxxxx
Signed-off-by: Stanley Jhu <stanleyjhu@xxxxxxxxxx>
---
Jens, if this looks good to you, an Acked-by would let Martin take
the whole series through the SCSI tree. Patches 2/3 and 3/3 depend
on this one.

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

diff --git a/drivers/misc/rpmb-core.c b/drivers/misc/rpmb-core.c
index ecf14acf230a..0fda893dd74d 100644
--- a/drivers/misc/rpmb-core.c
+++ b/drivers/misc/rpmb-core.c
@@ -45,6 +45,8 @@ 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,
@@ -53,6 +55,10 @@ int rpmb_route_frames(struct rpmb_dev *rdev, u8 *req,
 	if (!req || !req_len || !rsp || !rsp_len)
 		return -EINVAL;
 
+	guard(mutex)(&rdev->lock);
+	if (!rdev->descr.route_frames)
+		return -ENODEV;
+
 	return rdev->descr.route_frames(rdev->dev.parent, req, req_len,
 					rsp, rsp_len);
 }
@@ -62,6 +68,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 +130,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 +141,12 @@ int rpmb_dev_unregister(struct rpmb_dev *rdev)
 	if (!rdev)
 		return -EINVAL;
 
+	scoped_guard(mutex, &rdev->lock) {
+		if (!rdev->descr.route_frames)
+			return 0;
+		rdev->descr.route_frames = NULL;
+	}
+
 	device_del(&rdev->dev);
 
 	rpmb_dev_put(rdev);
@@ -164,6 +178,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 +209,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..11a5d2646e38 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,12 +49,14 @@ 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
  */
 struct rpmb_dev {
 	struct device dev;
+	struct mutex lock;
 	int id;
 	struct list_head list_node;
 	struct rpmb_descr descr;
-- 
2.56.0.567.gdb80ae8cd5-goog