linux-kernel - [RFC] component: Fix: Unassign components' masters if bringing up master fails

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <1455183351-22823-1-git-send-email-architt@codeaurora.org>
Date:	Thu, 11 Feb 2016 15:05:51 +0530
From:	Archit Taneja <architt@...eaurora.org>
To:	linux@....linux.org.uk
Cc:	linux-arm-msm@...r.kernel.org, linux-kernel@...r.kernel.org,
	Archit Taneja <architt@...eaurora.org>
Subject: [RFC] component: Fix: Unassign components' masters if bringing up master fails

component_master_add_with_match can fail if the master's bind op doesn't
go through successfully. In such a scenario, all the components in the
master's match array have their 'master' pointer set to the given master.
These pointers need to be set to NULL again. If they aren't, successive
calls to component_master_add_with_match will fail because the driver
thinks these components already have a master.

This issue can be seen when a driver defers probe because of missing
resources. It is seen after the introduction of commit:

"component: track components via array rather than list"

Add 'master_remove_components' which sets the all the components's masters
in the match array to NULL. This function is also re-used in
component_master_del and replaces code that did the same thing.

Signed-off-by: Archit Taneja <architt@...eaurora.org>
---
 drivers/base/component.c | 45 +++++++++++++++++++++++++++++----------------
 1 file changed, 29 insertions(+), 16 deletions(-)

diff --git a/drivers/base/component.c b/drivers/base/component.c
index 89f5cf68..a2ecc35 100644
--- a/drivers/base/component.c
+++ b/drivers/base/component.c
@@ -130,6 +130,23 @@ static void remove_component(struct master *master, struct component *c)
 			master->match->compare[i].component = NULL;
 }
 
+/* Detach all components from associated master */
+static void master_remove_components(struct master *master)
+{
+	struct component_match *match = master->match;
+	size_t i;
+
+	if (!match)
+		return;
+
+	for (i = 0; i < match->num; i++) {
+		struct component *c = match->compare[i].component;
+
+		if (c)
+			c->master = NULL;
+	}
+}
+
 /*
  * Try to bring up a master.  If component is NULL, we're interested in
  * this master, otherwise it's a component which must be present to try
@@ -140,34 +157,39 @@ static void remove_component(struct master *master, struct component *c)
 static int try_to_bring_up_master(struct master *master,
 	struct component *component)
 {
-	int ret;
+	int ret = 0;
 
 	dev_dbg(master->dev, "trying to bring up master\n");
 
 	if (find_components(master)) {
 		dev_dbg(master->dev, "master has incomplete components\n");
-		return 0;
+		goto err;
 	}
 
 	if (component && component->master != master) {
 		dev_dbg(master->dev, "master is not for this component (%s)\n",
 			dev_name(component->dev));
-		return 0;
+		goto err;
 	}
 
-	if (!devres_open_group(master->dev, NULL, GFP_KERNEL))
-		return -ENOMEM;
+	if (!devres_open_group(master->dev, NULL, GFP_KERNEL)) {
+		ret = -ENOMEM;
+		goto err;
+	}
 
 	/* Found all components */
 	ret = master->ops->bind(master->dev);
 	if (ret < 0) {
 		devres_release_group(master->dev, NULL);
 		dev_info(master->dev, "master bind failed: %d\n", ret);
-		return ret;
+		goto err;
 	}
 
 	master->bound = true;
 	return 1;
+err:
+	master_remove_components(master);
+	return ret;
 }
 
 static int try_to_bring_up_masters(struct component *component)
@@ -324,24 +346,15 @@ void component_master_del(struct device *dev,
 	const struct component_master_ops *ops)
 {
 	struct master *master;
-	int i;
 
 	mutex_lock(&component_mutex);
 	master = __master_find(dev, ops);
 	if (master) {
-		struct component_match *match = master->match;
-
 		take_down_master(master);
 
 		list_del(&master->node);
 
-		if (match) {
-			for (i = 0; i < match->num; i++) {
-				struct component *c = match->compare[i].component;
-				if (c)
-					c->master = NULL;
-			}
-		}
+		master_remove_components(master);
 		kfree(master);
 	}
 	mutex_unlock(&component_mutex);
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation