Re: [PATCH v2] remoteproc: sysfs/debugfs: fix race while updating recovery flag

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2/1/23 4:05 AM, Mukesh Ojha wrote:


On 2/1/2023 11:16 AM, Satya Durga Srinivasu Prabhala wrote:
When multiple clients 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.

Signed-off-by: Satya Durga Srinivasu Prabhala <quic_satyap@xxxxxxxxxxx>

LGTM.

Reviewed-by: Mukesh Ojha <quic_mojha@xxxxxxxxxxx>

Thanks Mukesh.

Hi Bjorn,

Can you help cross check?

Best,
Satya


-Mukesh
---
v1 -> v2:
- addressed comments from Mukesh Ojha
   1. take & release lock only while updating recovery flag
   2. update debugfs

  drivers/remoteproc/remoteproc_debugfs.c | 4 ++++
  drivers/remoteproc/remoteproc_sysfs.c   | 4 ++++
  2 files changed, 8 insertions(+)

diff --git a/drivers/remoteproc/remoteproc_debugfs.c b/drivers/remoteproc/remoteproc_debugfs.c
index b86c1d09c70c..2c44d375024e 100644
--- a/drivers/remoteproc/remoteproc_debugfs.c
+++ b/drivers/remoteproc/remoteproc_debugfs.c
@@ -226,10 +226,14 @@ rproc_recovery_write(struct file *filp, const char __user *user_buf,

      if (!strncmp(buf, "enabled", count)) {
          /* change the flag and begin the recovery process if needed */
+        mutex_lock(&rproc->lock);
          rproc->recovery_disabled = false;
+        mutex_unlock(&rproc->lock);
          rproc_trigger_recovery(rproc);
      } else if (!strncmp(buf, "disabled", count)) {
+        mutex_lock(&rproc->lock);
          rproc->recovery_disabled = true;
+        mutex_unlock(&rproc->lock);
      } else if (!strncmp(buf, "recover", count)) {
          /* begin the recovery process without changing the flag */
          rproc_trigger_recovery(rproc);
diff --git a/drivers/remoteproc/remoteproc_sysfs.c b/drivers/remoteproc/remoteproc_sysfs.c
index 8c7ea8922638..628e0de9a132 100644
--- a/drivers/remoteproc/remoteproc_sysfs.c
+++ b/drivers/remoteproc/remoteproc_sysfs.c
@@ -50,10 +50,14 @@ 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;
+        mutex_unlock(&rproc->lock);
          rproc_trigger_recovery(rproc);
      } else if (sysfs_streq(buf, "disabled")) {
+        mutex_lock(&rproc->lock);
          rproc->recovery_disabled = true;
+        mutex_unlock(&rproc->lock);
      } else if (sysfs_streq(buf, "recover")) {
          /* begin the recovery process without changing the flag */
          rproc_trigger_recovery(rproc);




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [Linux for Sparc]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux