From: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com> mainline inclusion from mainline-v7.3-rc1 commit 74ee3b2f5767447c57959994341e5b95f1079977 category: bugfix bugzilla: https://atomgit.com/openeuler/kernel/issues/9905 Reference: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?i... -------------------------------- There's no synchronization between rproc_crash_handler_work() and rproc_del(), as such it's possible for a driver to be removed while crash-handler work is scheduled, or even executing - resulting in use-after-free issues. To avoid this the scheduled work need to be cancelled and synchronized against before the removal proceeds. In order to ensure that this doesn't race with the reporting, and thereby scheduling new work, a "deleting" flag is introduced. This is similar to the RPROC_DELETE state that was introduced to ensure that "start" didn't race with rproc_del(), but the existing mechanism can not be used as it's valid to call rproc_report_crash() in atomic context - and the "state" is protected by a mutex. In the event that work is cancelled the pm_stay_awake() is left unbalanced and need to be unrolled. The blocking and cancelling of crash-handler work prior to the actual rproc_shutdown() call does have the explicit side-effect that crashes resulting from the shutdown process will not enter the crash-handling path, and as such will not generate devcoredumps etc. Due to the existing mutual exclusion between these code paths there's no concrete reduction in functionality, but further work would be needed to handle this case. Assisted-by: OpenCode:GPT-5.5 Fixes: 8afd519c3470 ("remoteproc: add rproc_report_crash function to notify rproc crashes") Signed-off-by: Bjorn Andersson <bjorn.andersson@oss.qualcomm.com> Reviewed-by: Pradnya Dahiwale <pradnya.dahiwale@oss.qualcomm.com> Link: https://lore.kernel.org/r/20260723-rproc-rmmod-not-crashing-v1-2-546dfd5de0e... Signed-off-by: Bjorn Andersson <andersson@kernel.org> Conflicts: drivers/remoteproc/remoteproc_core.c drivers/remoteproc/remoteproc_sysfs.c include/linux/remoteproc.h [Target tree lacks the attach feature and rproc_recovery_wq; retained rproc_notifier_head_init() and schedule_work() semantics, renumbered rproc_state enum by removing RPROC_DELETED in favor of RPROC_ATTACHED to match the upstream enum reordering.] Signed-off-by: Chen Jinghuang <chenjinghuang2@huawei.com> --- drivers/remoteproc/remoteproc_core.c | 39 ++++++++++++++++++++++----- drivers/remoteproc/remoteproc_sysfs.c | 1 - include/linux/remoteproc.h | 10 +++++-- 3 files changed, 40 insertions(+), 10 deletions(-) diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c index cc55ff0128cf..d5206e4a2c48 100644 --- a/drivers/remoteproc/remoteproc_core.c +++ b/drivers/remoteproc/remoteproc_core.c @@ -1695,6 +1695,11 @@ int rproc_trigger_recovery(struct rproc *rproc) if (ret) return ret; + if (READ_ONCE(rproc->deleting)) { + ret = -ENODEV; + goto unlock_mutex; + } + /* State could have changed before we got the mutex */ if (rproc->state != RPROC_CRASHED) goto unlock_mutex; @@ -1741,6 +1746,11 @@ static void rproc_crash_handler_work(struct work_struct *work) mutex_lock(&rproc->lock); + if (READ_ONCE(rproc->deleting)) { + mutex_unlock(&rproc->lock); + goto out; + } + if (rproc->state == RPROC_CRASHED) { /* handle only the first crash detected */ mutex_unlock(&rproc->lock); @@ -1796,9 +1806,9 @@ int rproc_boot(struct rproc *rproc) return ret; } - if (rproc->state == RPROC_DELETED) { + if (READ_ONCE(rproc->deleting)) { ret = -ENODEV; - dev_err(dev, "can't boot deleted rproc %s\n", rproc->name); + dev_err(dev, "can't boot deleting rproc %s\n", rproc->name); goto unlock_mutex; } @@ -2227,6 +2237,7 @@ struct rproc *rproc_alloc(struct device *dev, const char *name, INIT_LIST_HEAD(&rproc->dump_segments); INIT_WORK(&rproc->crash_handler, rproc_crash_handler_work); + spin_lock_init(&rproc->crash_handler_lock); rproc->state = RPROC_OFFLINE; @@ -2286,18 +2297,23 @@ EXPORT_SYMBOL(rproc_put); */ int rproc_del(struct rproc *rproc) { + unsigned long flags; + if (!rproc) return -EINVAL; + spin_lock_irqsave(&rproc->crash_handler_lock, flags); + WRITE_ONCE(rproc->deleting, true); + spin_unlock_irqrestore(&rproc->crash_handler_lock, flags); + + if (cancel_work_sync(&rproc->crash_handler)) + pm_relax(rproc->dev.parent); + /* if rproc is marked always-on, rproc_add() booted it */ /* TODO: make sure this works with rproc->power > 1 */ if (rproc->auto_boot) rproc_shutdown(rproc); - mutex_lock(&rproc->lock); - rproc->state = RPROC_DELETED; - mutex_unlock(&rproc->lock); - rproc_delete_debug_dir(rproc); /* the rproc is downref'ed as soon as it's removed from the klist */ @@ -2409,19 +2425,28 @@ EXPORT_SYMBOL(rproc_get_by_child); */ void rproc_report_crash(struct rproc *rproc, enum rproc_crash_type type) { + unsigned long flags; + if (!rproc) { pr_err("NULL rproc pointer\n"); return; } + spin_lock_irqsave(&rproc->crash_handler_lock, flags); + if (READ_ONCE(rproc->deleting)) { + spin_unlock_irqrestore(&rproc->crash_handler_lock, flags); + return; + } + /* Prevent suspend while the remoteproc is being recovered */ pm_stay_awake(rproc->dev.parent); + schedule_work(&rproc->crash_handler); + spin_unlock_irqrestore(&rproc->crash_handler_lock, flags); dev_err(&rproc->dev, "crash detected in %s: type %s\n", rproc->name, rproc_crash_to_string(type)); /* create a new task to handle the error */ - schedule_work(&rproc->crash_handler); } EXPORT_SYMBOL(rproc_report_crash); diff --git a/drivers/remoteproc/remoteproc_sysfs.c b/drivers/remoteproc/remoteproc_sysfs.c index d1cf7bf277c4..171d240e5ae9 100644 --- a/drivers/remoteproc/remoteproc_sysfs.c +++ b/drivers/remoteproc/remoteproc_sysfs.c @@ -200,7 +200,6 @@ static const char * const rproc_state_string[] = { [RPROC_SUSPENDED] = "suspended", [RPROC_RUNNING] = "running", [RPROC_CRASHED] = "crashed", - [RPROC_DELETED] = "deleted", [RPROC_DETACHED] = "detached", [RPROC_LAST] = "invalid", }; diff --git a/include/linux/remoteproc.h b/include/linux/remoteproc.h index 3fa3ba6498e8..729e4d98a35c 100644 --- a/include/linux/remoteproc.h +++ b/include/linux/remoteproc.h @@ -37,6 +37,7 @@ #include <linux/types.h> #include <linux/mutex.h> +#include <linux/spinlock.h> #include <linux/virtio.h> #include <linux/cdev.h> #include <linux/completion.h> @@ -402,7 +403,8 @@ struct rproc_ops { * a message. * @RPROC_RUNNING: device is up and running * @RPROC_CRASHED: device has crashed; need to start recovery - * @RPROC_DELETED: device is deleted + * @RPROC_ATTACHED: device has been booted by another entity and the core + * has attached to it * @RPROC_DETACHED: device has been booted by another entity and waiting * for the core to attach to it * @RPROC_LAST: just keep this one at the end @@ -418,7 +420,7 @@ enum rproc_state { RPROC_SUSPENDED = 1, RPROC_RUNNING = 2, RPROC_CRASHED = 3, - RPROC_DELETED = 4, + RPROC_ATTACHED = 4, RPROC_DETACHED = 5, RPROC_LAST = 6, }; @@ -499,6 +501,8 @@ struct rproc_dump_segment { * @notifyids: idr for dynamically assigning rproc-wide unique notify ids * @index: index of this rproc device * @crash_handler: workqueue for handling a crash + * @crash_handler_lock: serializes crash handler queueing and deletion + * @deleting: remoteproc deletion has begun * @crash_cnt: crash counter * @recovery_disabled: flag that state if recovery was disabled * @max_notifyid: largest allocated notify id. @@ -536,6 +540,8 @@ struct rproc { struct idr notifyids; int index; struct work_struct crash_handler; + spinlock_t crash_handler_lock; + bool deleting; unsigned int crash_cnt; bool recovery_disabled; int max_notifyid; -- 2.34.1