remoteproc: core: Move state checking to remoteproc_core
authorShengjiu Wang <shengjiu.wang@nxp.com>
Mon, 28 Mar 2022 02:20:12 +0000 (10:20 +0800)
committerMathieu Poirier <mathieu.poirier@linaro.org>
Thu, 14 Apr 2022 17:13:33 +0000 (11:13 -0600)
There is no mutex protection of these state checking for 'stop'
and 'detach' which can't guarantee there is no another instance
is trying to do same operation.

Consider two instances case:
Instance1: echo stop > /sys/class/remoteproc/remoteproc0/state
Instance2: echo stop > /sys/class/remoteproc/remoteproc0/state

The issue is that the instance2 case may success, Or it
may fail with -EINVAL, which is uncertain.

So move this state checking in rproc_cdev_write() and
state_store() for 'stop', 'detach' operation to
'rproc_shutdown' , 'rproc_detach' function under the mutex
protection.

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
Link: https://lore.kernel.org/r/1648434012-16655-3-git-send-email-shengjiu.wang@nxp.com
Signed-off-by: Mathieu Poirier <mathieu.poirier@linaro.org>
drivers/remoteproc/remoteproc_cdev.c
drivers/remoteproc/remoteproc_core.c
drivers/remoteproc/remoteproc_sysfs.c

index 62001eda780c080078fce5072df46416e480d166..687f205fd70a675e321de358e0c38f168ff82466 100644 (file)
@@ -34,15 +34,8 @@ static ssize_t rproc_cdev_write(struct file *filp, const char __user *buf, size_
        if (!strncmp(cmd, "start", len)) {
                ret = rproc_boot(rproc);
        } else if (!strncmp(cmd, "stop", len)) {
-               if (rproc->state != RPROC_RUNNING &&
-                   rproc->state != RPROC_ATTACHED)
-                       return -EINVAL;
-
                ret = rproc_shutdown(rproc);
        } else if (!strncmp(cmd, "detach", len)) {
-               if (rproc->state != RPROC_ATTACHED)
-                       return -EINVAL;
-
                ret = rproc_detach(rproc);
        } else {
                dev_err(&rproc->dev, "Unrecognized option\n");
index 3c8382f66ce2ef1fe0f49a6334f2e779af393961..02a04ab34a23086260ffb406a7923718d35ace9a 100644 (file)
@@ -2071,6 +2071,12 @@ int rproc_shutdown(struct rproc *rproc)
                return ret;
        }
 
+       if (rproc->state != RPROC_RUNNING &&
+           rproc->state != RPROC_ATTACHED) {
+               ret = -EINVAL;
+               goto out;
+       }
+
        /* if the remote proc is still needed, bail out */
        if (!atomic_dec_and_test(&rproc->power))
                goto out;
@@ -2130,6 +2136,11 @@ int rproc_detach(struct rproc *rproc)
                return ret;
        }
 
+       if (rproc->state != RPROC_ATTACHED) {
+               ret = -EINVAL;
+               goto out;
+       }
+
        /* if the remote proc is still needed, bail out */
        if (!atomic_dec_and_test(&rproc->power)) {
                ret = 0;
index ac64d69085abfc7f21fefd3df3f083aa9d781df8..8c7ea8922638885abb9b9ef939f827a0688e2f54 100644 (file)
@@ -198,15 +198,8 @@ static ssize_t state_store(struct device *dev,
                if (ret)
                        dev_err(&rproc->dev, "Boot failed: %d\n", ret);
        } else if (sysfs_streq(buf, "stop")) {
-               if (rproc->state != RPROC_RUNNING &&
-                   rproc->state != RPROC_ATTACHED)
-                       return -EINVAL;
-
                ret = rproc_shutdown(rproc);
        } else if (sysfs_streq(buf, "detach")) {
-               if (rproc->state != RPROC_ATTACHED)
-                       return -EINVAL;
-
                ret = rproc_detach(rproc);
        } else {
                dev_err(&rproc->dev, "Unrecognised option: %s\n", buf);