When a storage host unbinds or card is removed, the RPMB provider unregisters the RPMB device via rpmb_dev_unregister(). However, lockless access in rpmb_route_frames() races with provider teardown, risking frame dispatch against an unpowered or torn-down host controller. Furthermore, within a single route_frames() request, provider drivers dispatch multiple discrete security protocol commands to the underlying host (for example, Security Protocol Out followed by Security Protocol In on UFS). Without locking at the RPMB core entry point, concurrent callers can interleave low-level command sequences. Fix these concurrency and teardown issues: - Add a mutex and a dead flag to struct rpmb_dev. - Guard rpmb_route_frames() with the mutex and reject requests with -ENODEV once the device is marked dead. - In rpmb_dev_unregister(), acquire the mutex to drain in-flight requests and mark the device dead before proceeding to device_del(). - Ensure mutex_destroy() is called in rpmb_dev_release() and in the registration error unwind path. - Clarify kerneldoc for rpmb_dev_unregister() to advise calling from the provider's remove/unbind path, not a device release callback. Fixes: 1e9046e3a154 ("rpmb: add Replay Protected Memory Block (RPMB) subsystem") Signed-off-by: Stanley Jhu Cc: stable@vger.kernel.org --- Differences from v2: - Scope lock boundary to discrete protocol frames under route_frames(). - Drop redundant parent device pinning in favor of mutex drain. - Add Context: Might sleep to rpmb_route_frames() kerneldoc. - Fix misleading kerneldoc for rpmb_dev_unregister(). Tested: - Verified clean probe, RPMB I/O, and unbind on QEMU ARM64 without UAF. Note: A companion series fixing the UFS cyclic refcount in ufs-rpmb.c has been submitted to linux-scsi. 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..ca66a80ab888 100644 --- a/include/linux/rpmb.h +++ b/include/linux/rpmb.h @@ -7,6 +7,7 @@ #define __RPMB_H__ #include +#include #include /** @@ -48,15 +49,19 @@ struct rpmb_descr { * struct rpmb_dev - device which can support RPMB partition * * @dev : device + * @lock : protects in-flight operations 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; 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