diff mbox series

[v4,4/4] remoteproc: core: Cleanup device in case of failure

Message ID 1623783824-13395-5-git-send-email-sidgup@codeaurora.org
State Accepted
Commit 7dbdb8bd7c028c83ac75e5c97536559a7274c797
Headers show
Series [v4,1/4] remoteproc: core: Move cdev add before device add | expand

Commit Message

Siddharth Gupta June 15, 2021, 7:03 p.m. UTC
When a failure occurs in rproc_add() it returns an error, but does
not cleanup after itself. This change adds the failure path in such
cases.

Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>
Cc: stable@vger.kernel.org
---
 drivers/remoteproc/remoteproc_core.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

Comments

Greg KH June 16, 2021, 5:56 a.m. UTC | #1
On Tue, Jun 15, 2021 at 01:21:11PM -0700, Siddharth Gupta wrote:
> 

> On 6/15/2021 12:06 PM, Greg KH wrote:

> > On Tue, Jun 15, 2021 at 12:03:44PM -0700, Siddharth Gupta wrote:

> > > When a failure occurs in rproc_add() it returns an error, but does

> > > not cleanup after itself. This change adds the failure path in such

> > > cases.

> > > 

> > > Signed-off-by: Siddharth Gupta <sidgup@codeaurora.org>

> > > Cc: stable@vger.kernel.org

> > > ---

> > >   drivers/remoteproc/remoteproc_core.c | 15 ++++++++++++---

> > >   1 file changed, 12 insertions(+), 3 deletions(-)

> > Why is this needed for stable kernels?  And again, a Fixes: tag?

> Patch 2 and patch 3 are leading up to fix rproc_add()

> in case of a failure. This means we'll have errors with

> use after free unless we call device_del() or cdev_del(),

> also the sysfs and devtempfs nodes will also not be

> removed.


Then please explain that better in the changelogs.  At it is, no one
knows this.

greg k-h
diff mbox series

Patch

diff --git a/drivers/remoteproc/remoteproc_core.c b/drivers/remoteproc/remoteproc_core.c
index b874280..d823f70 100644
--- a/drivers/remoteproc/remoteproc_core.c
+++ b/drivers/remoteproc/remoteproc_core.c
@@ -2343,8 +2343,10 @@  int rproc_add(struct rproc *rproc)
 		return ret;
 
 	ret = device_add(dev);
-	if (ret < 0)
-		return ret;
+	if (ret < 0) {
+		put_device(dev);
+		goto rproc_remove_cdev;
+	}
 
 	dev_info(dev, "%s is available\n", rproc->name);
 
@@ -2355,7 +2357,7 @@  int rproc_add(struct rproc *rproc)
 	if (rproc->auto_boot) {
 		ret = rproc_trigger_auto_boot(rproc);
 		if (ret < 0)
-			return ret;
+			goto rproc_remove_dev;
 	}
 
 	/* expose to rproc_get_by_phandle users */
@@ -2364,6 +2366,13 @@  int rproc_add(struct rproc *rproc)
 	mutex_unlock(&rproc_list_mutex);
 
 	return 0;
+
+rproc_remove_dev:
+	rproc_delete_debug_dir(rproc);
+	device_del(dev);
+rproc_remove_cdev:
+	rproc_char_device_remove(rproc);
+	return ret;
 }
 EXPORT_SYMBOL(rproc_add);