All of lore.kernel.org
 help / color / mirror / Atom feed
From: Suman Anna <s-anna@ti.com>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Ohad Ben-Cohen <ohad@wizery.com>,
	Loic Pallardy <loic.pallardy@st.com>,
	Arnaud Pouliquen <arnaud.pouliquen@st.com>,
	linux-remoteproc@vger.kernel.org, linux-kernel@vger.kernel.org,
	Suman Anna <s-anna@ti.com>
Subject: [PATCH 1/5] remoteproc: Fix unbalanced boot with sysfs for no auto-boot rprocs
Date: Fri, 14 Sep 2018 19:37:21 -0500	[thread overview]
Message-ID: <20180915003725.17549-2-s-anna@ti.com> (raw)
In-Reply-To: <20180915003725.17549-1-s-anna@ti.com>

The remoteproc core performs automatic boot and shutdown of a remote
processor during rproc_add() and rproc_del() for remote processors
supporting 'auto-boot'. The remoteproc devices not using 'auto-boot'
require either a remoteproc client driver or a userspace client to
use the sysfs 'state' variable to perform the boot and shutdown. The
in-kernel client drivers hold the corresponding remoteproc driver
module's reference count when they acquire a rproc handle through
the rproc_get_by_phandle() API, but there is no such support for
userspace applications performing the boot through sysfs interface.

The shutdown of a remoteproc upon removing a remoteproc platform
driver is automatic only with 'auto-boot' and this can cause a
remoteproc with no auto-boot to stay powered on and never freed
up if booted using the sysfs interface without a matching stop,
and when the remoteproc driver module is removed or unbound from
the device. This will result in a memory leak as well as the
corresponding remoteproc ida being never deallocated. Fix this
by holding a module reference count for the remoteproc's driver
during a sysfs 'start' and releasing it during the sysfs 'stop'
operation.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/remoteproc/remoteproc_sysfs.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_sysfs.c b/drivers/remoteproc/remoteproc_sysfs.c
index 47be411400e5..2142b3ea726e 100644
--- a/drivers/remoteproc/remoteproc_sysfs.c
+++ b/drivers/remoteproc/remoteproc_sysfs.c
@@ -11,6 +11,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/module.h>
 #include <linux/remoteproc.h>
 
 #include "remoteproc_internal.h"
@@ -100,14 +101,27 @@ static ssize_t state_store(struct device *dev,
 		if (rproc->state == RPROC_RUNNING)
 			return -EBUSY;
 
+		/*
+		 * prevent underlying implementation from being removed
+		 * when remoteproc does not support auto-boot
+		 */
+		if (!rproc->auto_boot &&
+		    !try_module_get(dev->parent->driver->owner))
+			return -EINVAL;
+
 		ret = rproc_boot(rproc);
-		if (ret)
+		if (ret) {
 			dev_err(&rproc->dev, "Boot failed: %d\n", ret);
+			if (!rproc->auto_boot)
+				module_put(dev->parent->driver->owner);
+		}
 	} else if (sysfs_streq(buf, "stop")) {
 		if (rproc->state != RPROC_RUNNING)
 			return -EINVAL;
 
 		rproc_shutdown(rproc);
+		if (!rproc->auto_boot)
+			module_put(dev->parent->driver->owner);
 	} else {
 		dev_err(&rproc->dev, "Unrecognised option: %s\n", buf);
 		ret = -EINVAL;
-- 
2.18.0

WARNING: multiple messages have this Message-ID (diff)
From: Suman Anna <s-anna@ti.com>
To: Bjorn Andersson <bjorn.andersson@linaro.org>
Cc: Ohad Ben-Cohen <ohad@wizery.com>,
	Loic Pallardy <loic.pallardy@st.com>,
	Arnaud Pouliquen <arnaud.pouliquen@st.com>,
	<linux-remoteproc@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, Suman Anna <s-anna@ti.com>
Subject: [PATCH 1/5] remoteproc: Fix unbalanced boot with sysfs for no auto-boot rprocs
Date: Fri, 14 Sep 2018 19:37:21 -0500	[thread overview]
Message-ID: <20180915003725.17549-2-s-anna@ti.com> (raw)
In-Reply-To: <20180915003725.17549-1-s-anna@ti.com>

The remoteproc core performs automatic boot and shutdown of a remote
processor during rproc_add() and rproc_del() for remote processors
supporting 'auto-boot'. The remoteproc devices not using 'auto-boot'
require either a remoteproc client driver or a userspace client to
use the sysfs 'state' variable to perform the boot and shutdown. The
in-kernel client drivers hold the corresponding remoteproc driver
module's reference count when they acquire a rproc handle through
the rproc_get_by_phandle() API, but there is no such support for
userspace applications performing the boot through sysfs interface.

The shutdown of a remoteproc upon removing a remoteproc platform
driver is automatic only with 'auto-boot' and this can cause a
remoteproc with no auto-boot to stay powered on and never freed
up if booted using the sysfs interface without a matching stop,
and when the remoteproc driver module is removed or unbound from
the device. This will result in a memory leak as well as the
corresponding remoteproc ida being never deallocated. Fix this
by holding a module reference count for the remoteproc's driver
during a sysfs 'start' and releasing it during the sysfs 'stop'
operation.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
 drivers/remoteproc/remoteproc_sysfs.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/remoteproc/remoteproc_sysfs.c b/drivers/remoteproc/remoteproc_sysfs.c
index 47be411400e5..2142b3ea726e 100644
--- a/drivers/remoteproc/remoteproc_sysfs.c
+++ b/drivers/remoteproc/remoteproc_sysfs.c
@@ -11,6 +11,7 @@
  * GNU General Public License for more details.
  */
 
+#include <linux/module.h>
 #include <linux/remoteproc.h>
 
 #include "remoteproc_internal.h"
@@ -100,14 +101,27 @@ static ssize_t state_store(struct device *dev,
 		if (rproc->state == RPROC_RUNNING)
 			return -EBUSY;
 
+		/*
+		 * prevent underlying implementation from being removed
+		 * when remoteproc does not support auto-boot
+		 */
+		if (!rproc->auto_boot &&
+		    !try_module_get(dev->parent->driver->owner))
+			return -EINVAL;
+
 		ret = rproc_boot(rproc);
-		if (ret)
+		if (ret) {
 			dev_err(&rproc->dev, "Boot failed: %d\n", ret);
+			if (!rproc->auto_boot)
+				module_put(dev->parent->driver->owner);
+		}
 	} else if (sysfs_streq(buf, "stop")) {
 		if (rproc->state != RPROC_RUNNING)
 			return -EINVAL;
 
 		rproc_shutdown(rproc);
+		if (!rproc->auto_boot)
+			module_put(dev->parent->driver->owner);
 	} else {
 		dev_err(&rproc->dev, "Unrecognised option: %s\n", buf);
 		ret = -EINVAL;
-- 
2.18.0


  reply	other threads:[~2018-09-15  0:37 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-15  0:37 [PATCH 0/5] remoteproc sysfs fixes/improvements Suman Anna
2018-09-15  0:37 ` Suman Anna
2018-09-15  0:37 ` Suman Anna [this message]
2018-09-15  0:37   ` [PATCH 1/5] remoteproc: Fix unbalanced boot with sysfs for no auto-boot rprocs Suman Anna
2018-10-02  9:27   ` Arnaud Pouliquen
2018-10-02  9:27     ` Arnaud Pouliquen
2018-10-06  6:13   ` Bjorn Andersson
2018-10-08 16:42     ` Suman Anna
2018-10-08 16:42       ` Suman Anna
2018-09-15  0:37 ` [PATCH 2/5] remoteproc: Check for NULL firmwares in sysfs interface Suman Anna
2018-09-15  0:37   ` Suman Anna
2018-10-02  9:43   ` Arnaud Pouliquen
2018-10-02  9:43     ` Arnaud Pouliquen
2018-10-02 15:05     ` Suman Anna
2018-10-02 15:05       ` Suman Anna
2018-10-06  6:14   ` Bjorn Andersson
2018-09-15  0:37 ` [PATCH 3/5] remoteproc: Add missing kernel-doc comment for auto-boot Suman Anna
2018-09-15  0:37   ` Suman Anna
2018-10-06  6:14   ` Bjorn Andersson
2018-09-15  0:37 ` [PATCH 4/5] remoteproc: Introduce deny_sysfs_ops flag Suman Anna
2018-09-15  0:37   ` Suman Anna
2018-10-02  9:47   ` Arnaud Pouliquen
2018-10-02  9:47     ` Arnaud Pouliquen
2018-10-02 15:14     ` Suman Anna
2018-10-02 15:14       ` Suman Anna
2018-09-15  0:37 ` [PATCH 5/5] remoteproc/wkup_m3: Set " Suman Anna
2018-09-15  0:37   ` Suman Anna
2018-10-01 21:11 ` [PATCH 0/5] remoteproc sysfs fixes/improvements Suman Anna
2018-10-01 21:11   ` Suman Anna

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180915003725.17549-2-s-anna@ti.com \
    --to=s-anna@ti.com \
    --cc=arnaud.pouliquen@st.com \
    --cc=bjorn.andersson@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-remoteproc@vger.kernel.org \
    --cc=loic.pallardy@st.com \
    --cc=ohad@wizery.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.