[PATCH 2/2] most: core: serialize comp_list and pin the matched component

[Yu Zhang <yuz08559@gmail.com>]

comp_list is walked by match_component() and components_show() and
mutated by most_register_component() (list_add_tail) and
most_deregister_component() (list_del), all with no lock - the only locks
in the core are per-channel. A components_show() read or a
match_component() lookup can therefore run concurrently with a component
module unregistering and unloading, walking into the list node that
most_deregister_component() removed and the module freed, i.e. a
list-traversal use-after-free.

match_component() also returned a bare pointer that its callers
(most_add_link(), most_remove_link(), most_cfg_complete()) dereference
after the lookup, so the component could be torn down between the lookup
and the use.

Add a mutex around every comp_list access, and have match_component()
take a reference to the component's module before returning so it stays
alive while the caller uses it; the callers drop it with module_put().

Signed-off-by: Yu Zhang <yuz08559@gmail.com>
---
 drivers/most/core.c | 42 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 35 insertions(+), 7 deletions(-)

diff --git a/drivers/most/core.c b/drivers/most/core.c
index c2616da..d17f538 100644
--- a/drivers/most/core.c
+++ b/drivers/most/core.c
@@ -28,6 +28,7 @@
 static struct ida mdev_id;
 static int dummy_num_buffers;
 static struct list_head comp_list;
+static DEFINE_MUTEX(comp_list_mutex); /* protects comp_list */
 
 struct pipe {
 	struct most_component *comp;
@@ -445,14 +446,21 @@ static const struct attribute_group *interface_attr_groups[] = {
 	NULL,
 };
 
+/* On success takes a module ref; caller must module_put(comp->mod). */
 static struct most_component *match_component(char *name)
 {
 	struct most_component *comp;
 
+	mutex_lock(&comp_list_mutex);
 	list_for_each_entry(comp, &comp_list, list) {
-		if (!strcmp(comp->name, name))
-			return comp;
+		if (strcmp(comp->name, name))
+			continue;
+		if (!try_module_get(comp->mod))
+			comp = NULL;
+		mutex_unlock(&comp_list_mutex);
+		return comp;
 	}
+	mutex_unlock(&comp_list_mutex);
 	return NULL;
 }
 
@@ -517,10 +525,12 @@ static ssize_t components_show(struct device_driver *drv, char *buf)
 	struct most_component *comp;
 	int offs = 0;
 
+	mutex_lock(&comp_list_mutex);
 	list_for_each_entry(comp, &comp_list, list) {
 		offs += scnprintf(buf + offs, PAGE_SIZE - offs, "%s\n",
 				 comp->name);
 	}
+	mutex_unlock(&comp_list_mutex);
 	return offs;
 }
 
@@ -666,12 +676,15 @@ int most_set_cfg_packets_xact(char *mdev, char *mdev_ch, u16 val)
 int most_cfg_complete(char *comp_name)
 {
 	struct most_component *comp;
+	int ret;
 
 	comp = match_component(comp_name);
 	if (!comp)
 		return -ENODEV;
 
-	return comp->cfg_complete();
+	ret = comp->cfg_complete();
+	module_put(comp->mod);
+	return ret;
 }
 
 int most_add_link(char *mdev, char *mdev_ch, char *comp_name, char *link_name,
@@ -679,11 +692,17 @@ int most_add_link(char *mdev, char *mdev_ch, char *comp_name, char *link_name,
 {
 	struct most_channel *c = get_channel(mdev, mdev_ch);
 	struct most_component *comp = match_component(comp_name);
+	int ret;
 
-	if (!c || !comp)
+	if (!c || !comp) {
+		if (comp)
+			module_put(comp->mod);
 		return -ENODEV;
+	}
 
-	return link_channel_to_component(c, comp, link_name, comp_param);
+	ret = link_channel_to_component(c, comp, link_name, comp_param);
+	module_put(comp->mod);
+	return ret;
 }
 
 int most_remove_link(char *mdev, char *mdev_ch, char *comp_name)
@@ -695,15 +714,20 @@ int most_remove_link(char *mdev, char *mdev_ch, char *comp_name)
 	if (!comp)
 		return -ENODEV;
 	c = get_channel(mdev, mdev_ch);
-	if (!c)
+	if (!c) {
+		module_put(comp->mod);
 		return -ENODEV;
+	}
 
-	if (comp->disconnect_channel(c->iface, c->channel_id))
+	if (comp->disconnect_channel(c->iface, c->channel_id)) {
+		module_put(comp->mod);
 		return -EIO;
+	}
 	if (c->pipe0.comp == comp)
 		c->pipe0.comp = NULL;
 	if (c->pipe1.comp == comp)
 		c->pipe1.comp = NULL;
+	module_put(comp->mod);
 	return 0;
 }
 
@@ -1222,7 +1246,9 @@ int most_register_component(struct most_component *comp)
 		pr_err("Bad component\n");
 		return -EINVAL;
 	}
+	mutex_lock(&comp_list_mutex);
 	list_add_tail(&comp->list, &comp_list);
+	mutex_unlock(&comp_list_mutex);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(most_register_component);
@@ -1257,7 +1283,9 @@ int most_deregister_component(struct most_component *comp)
 	}
 
 	bus_for_each_dev(&mostbus, NULL, comp, disconnect_channels);
+	mutex_lock(&comp_list_mutex);
 	list_del(&comp->list);
+	mutex_unlock(&comp_list_mutex);
 	return 0;
 }
 EXPORT_SYMBOL_GPL(most_deregister_component);
-- 
2.43.0