From e2839b887e267410586e27e7a6d4464cd421112e Mon Sep 17 00:00:00 2001 From: Satya Durga Srinivasu Prabhala Date: Sun, 29 Jan 2023 14:36:56 -0800 Subject: [PATCH] ANDROID: remoteproc: sysfs: fix race while updating recovery flag We have debug features [1] to allow recovery of subsystem or crash entire system to collect dumps for further analysis based on recovery flag and when multiple clients (user space or kernel via android_vh_rproc_recovery_set() vendor hook) try to update the recovery flag, it is possible that, race condition would lead to undesired results as updates to recovery flag isn't protected by any mechanism today. To avoid such issues, take remoteproc mutex lock before updating recovery flag and release the lock once done. Here is the scenario: ==================== 1. We have downstream API which can be used by Kernel clients to update the recovery_disabled flag. 2. Kernel client calls API to set recovery_disabled to false to recover the subsystem instead of taking down entire system. 3. At around same time user space (via sysfs) tries to set the recovery_disabled to true to take down entire system. CPUX CPUY (update via sysfs) (update via Kernel client) recovery_store() | save_restore_recovery() recovery_disabled = true; | | recovery_disabled = false; android_vh_rproc_recovery_set(); At this point, vendor_cb() running on CPUX invoked by vendor hook which has save/restore functionality for recovery_disabled will see undesired results. [1] https://lore.kernel.org/lkml/20221228162040.m3ucsyau3s55rkfn@builder.lan/T/ https://lore.kernel.org/lkml/20230201054609.14575-1-quic_satyap@quicinc.com/T/ Bug: 266790242 Change-Id: If880122be7f637b4215629117595f6ed7e833cd4 Signed-off-by: Satya Durga Srinivasu Prabhala --- drivers/remoteproc/remoteproc_sysfs.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/remoteproc/remoteproc_sysfs.c b/drivers/remoteproc/remoteproc_sysfs.c index 22c316d52a36..4517bfb64ef2 100644 --- a/drivers/remoteproc/remoteproc_sysfs.c +++ b/drivers/remoteproc/remoteproc_sysfs.c @@ -51,12 +51,16 @@ static ssize_t recovery_store(struct device *dev, if (sysfs_streq(buf, "enabled")) { /* change the flag and begin the recovery process if needed */ + mutex_lock(&rproc->lock); rproc->recovery_disabled = false; trace_android_vh_rproc_recovery_set(rproc); + mutex_unlock(&rproc->lock); rproc_trigger_recovery(rproc); } else if (sysfs_streq(buf, "disabled")) { + mutex_lock(&rproc->lock); rproc->recovery_disabled = true; trace_android_vh_rproc_recovery_set(rproc); + mutex_unlock(&rproc->lock); } else if (sysfs_streq(buf, "recover")) { /* begin the recovery process without changing the flag */ rproc_trigger_recovery(rproc);