[PATCH 0/2] staging: most: prevent module removal if configfs directory is populated

[PATCH 0/2] staging: most: prevent module removal if configfs directory is populated

[Christian Gromm]

Currently the core module does not hold any reference to component modules.
Hence, a component could be unloaded from the system anytime. Even if the
configuration interface is still being populated with configfs
items/groups.
User space then has no chance to fix the usage count by calling rmdir on
such a config item/group, as the respective directory does not exist
anymore. In this situation the core module cannot be unloaded anymore.
This patch set fixes this issue by holding a reference as long as the
configfs items exist.

Christian Gromm (2):
  staging: most: configfs: move configfs subsystems to container struct
  staging: most: block module removal while having active configfs items

 drivers/staging/most/cdev/cdev.c   |   1 +
 drivers/staging/most/configfs.c    | 118 +++++++++++++++++++++++++------------
 drivers/staging/most/core.h        |   1 +
 drivers/staging/most/net/net.c     |   1 +
 drivers/staging/most/sound/sound.c |   1 +
 drivers/staging/most/video/video.c |   1 +
 6 files changed, 85 insertions(+), 38 deletions(-)

-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 1/2] staging: most: configfs: move configfs subsystems to container struct

[Christian Gromm]

This patch moves the declarations of the configfs subsystems to
a superordinate container structure. This is done to get access
to private subsystem data.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/configfs.c | 80 +++++++++++++++++++++++------------------
 1 file changed, 45 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/most/configfs.c b/drivers/staging/most/configfs.c
index c06cf84..c292dd3 100644
--- a/drivers/staging/most/configfs.c
+++ b/drivers/staging/most/configfs.c
@@ -395,11 +395,12 @@ static const struct config_item_type mdev_link_type = {
 
 struct most_common {
 	struct config_group group;
+	struct configfs_subsystem subsys;
 };
 
-static struct most_common *to_most_common(struct config_item *item)
+static struct most_common *to_most_common(struct configfs_subsystem *subsys)
 {
-	return container_of(to_config_group(item), struct most_common, group);
+	return container_of(subsys, struct most_common, subsys);
 }
 
 static struct config_item *most_common_make_item(struct config_group *group,
@@ -426,7 +427,9 @@ static struct config_item *most_common_make_item(struct config_group *group,
 
 static void most_common_release(struct config_item *item)
 {
-	kfree(to_most_common(item));
+	struct config_group *group = to_config_group(item);
+
+	kfree(to_most_common(group->cg_subsys));
 }
 
 static struct configfs_item_operations most_common_item_ops = {
@@ -443,29 +446,35 @@ static const struct config_item_type most_common_type = {
 	.ct_owner	= THIS_MODULE,
 };
 
-static struct configfs_subsystem most_cdev_subsys = {
-	.su_group = {
-		.cg_item = {
-			.ci_namebuf = "most_cdev",
-			.ci_type = &most_common_type,
+static struct most_common most_cdev = {
+	.subsys = {
+		.su_group = {
+			.cg_item = {
+				.ci_namebuf = "most_cdev",
+				.ci_type = &most_common_type,
+			},
 		},
 	},
 };
 
-static struct configfs_subsystem most_net_subsys = {
-	.su_group = {
-		.cg_item = {
-			.ci_namebuf = "most_net",
-			.ci_type = &most_common_type,
+static struct most_common most_net = {
+	.subsys = {
+		.su_group = {
+			.cg_item = {
+				.ci_namebuf = "most_net",
+				.ci_type = &most_common_type,
+			},
 		},
 	},
 };
 
-static struct configfs_subsystem most_video_subsys = {
-	.su_group = {
-		.cg_item = {
-			.ci_namebuf = "most_video",
-			.ci_type = &most_common_type,
+static struct most_common most_video = {
+	.subsys = {
+		.su_group = {
+			.cg_item = {
+				.ci_namebuf = "most_video",
+				.ci_type = &most_common_type,
+			},
 		},
 	},
 };
@@ -597,16 +606,17 @@ int most_register_configfs_subsys(struct core_component *c)
 {
 	int ret;
 
-	if (!strcmp(c->name, "cdev"))
-		ret = configfs_register_subsystem(&most_cdev_subsys);
-	else if (!strcmp(c->name, "net"))
-		ret = configfs_register_subsystem(&most_net_subsys);
-	else if (!strcmp(c->name, "video"))
-		ret = configfs_register_subsystem(&most_video_subsys);
-	else if (!strcmp(c->name, "sound"))
+	if (!strcmp(c->name, "cdev")) {
+		ret = configfs_register_subsystem(&most_cdev.subsys);
+	} else if (!strcmp(c->name, "net")) {
+		ret = configfs_register_subsystem(&most_net.subsys);
+	} else if (!strcmp(c->name, "video")) {
+		ret = configfs_register_subsystem(&most_video.subsys);
+	} else if (!strcmp(c->name, "sound")) {
 		ret = configfs_register_subsystem(&most_sound_subsys.subsys);
-	else
+	} else {
 		return -ENODEV;
+	}
 
 	if (ret) {
 		pr_err("Error %d while registering subsystem %s\n",
@@ -635,11 +645,11 @@ void most_interface_register_notify(const char *mdev)
 void most_deregister_configfs_subsys(struct core_component *c)
 {
 	if (!strcmp(c->name, "cdev"))
-		configfs_unregister_subsystem(&most_cdev_subsys);
+		configfs_unregister_subsystem(&most_cdev.subsys);
 	else if (!strcmp(c->name, "net"))
-		configfs_unregister_subsystem(&most_net_subsys);
+		configfs_unregister_subsystem(&most_net.subsys);
 	else if (!strcmp(c->name, "video"))
-		configfs_unregister_subsystem(&most_video_subsys);
+		configfs_unregister_subsystem(&most_video.subsys);
 	else if (!strcmp(c->name, "sound"))
 		configfs_unregister_subsystem(&most_sound_subsys.subsys);
 }
@@ -647,14 +657,14 @@ EXPORT_SYMBOL_GPL(most_deregister_configfs_subsys);
 
 int __init configfs_init(void)
 {
-	config_group_init(&most_cdev_subsys.su_group);
-	mutex_init(&most_cdev_subsys.su_mutex);
+	config_group_init(&most_cdev.subsys.su_group);
+	mutex_init(&most_cdev.subsys.su_mutex);
 
-	config_group_init(&most_net_subsys.su_group);
-	mutex_init(&most_net_subsys.su_mutex);
+	config_group_init(&most_net.subsys.su_group);
+	mutex_init(&most_net.subsys.su_mutex);
 
-	config_group_init(&most_video_subsys.su_group);
-	mutex_init(&most_video_subsys.su_mutex);
+	config_group_init(&most_video.subsys.su_group);
+	mutex_init(&most_video.subsys.su_mutex);
 
 	config_group_init(&most_sound_subsys.subsys.su_group);
 	mutex_init(&most_sound_subsys.subsys.su_mutex);
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH 2/2] staging: most: block module removal while having active configfs items

[Christian Gromm]

This patch avoids that core component modules are being unloaded
while the related configfs interface has active items in its directories.
It is needed to prevent the situation where the core module cannot
be unloaded anymore, because the reference count 'used by' indicates that
the module is still being used and the usage count cannot be decreased by
calling rmdir, as the configfs directory has already been removed.

Signed-off-by: Christian Gromm <christian.gromm@microchip.com>
---
 drivers/staging/most/cdev/cdev.c   |  1 +
 drivers/staging/most/configfs.c    | 38 +++++++++++++++++++++++++++++++++++---
 drivers/staging/most/core.h        |  1 +
 drivers/staging/most/net/net.c     |  1 +
 drivers/staging/most/sound/sound.c |  1 +
 drivers/staging/most/video/video.c |  1 +
 6 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/most/cdev/cdev.c b/drivers/staging/most/cdev/cdev.c
index 724d098..f880147 100644
--- a/drivers/staging/most/cdev/cdev.c
+++ b/drivers/staging/most/cdev/cdev.c
@@ -494,6 +494,7 @@ static int comp_probe(struct most_interface *iface, int channel_id,
 
 static struct cdev_component comp = {
 	.cc = {
+		.mod = THIS_MODULE,
 		.name = "cdev",
 		.probe_channel = comp_probe,
 		.disconnect_channel = comp_disconnect_channel,
diff --git a/drivers/staging/most/configfs.c b/drivers/staging/most/configfs.c
index c292dd3..34a9fb5 100644
--- a/drivers/staging/most/configfs.c
+++ b/drivers/staging/most/configfs.c
@@ -395,6 +395,7 @@ static const struct config_item_type mdev_link_type = {
 
 struct most_common {
 	struct config_group group;
+	struct module *mod;
 	struct configfs_subsystem subsys;
 };
 
@@ -407,11 +408,16 @@ static struct config_item *most_common_make_item(struct config_group *group,
 						 const char *name)
 {
 	struct mdev_link *mdev_link;
+	struct most_common *mc = to_most_common(group->cg_subsys);
 
 	mdev_link = kzalloc(sizeof(*mdev_link), GFP_KERNEL);
 	if (!mdev_link)
 		return ERR_PTR(-ENOMEM);
 
+	if (!try_module_get(mc->mod)) {
+		kfree(mdev_link);
+		return ERR_PTR(-ENOLCK);
+	}
 	config_item_init_type_name(&mdev_link->item, name,
 				   &mdev_link_type);
 
@@ -436,8 +442,17 @@ static struct configfs_item_operations most_common_item_ops = {
 	.release	= most_common_release,
 };
 
+static void most_common_disconnect(struct config_group *group,
+				   struct config_item *item)
+{
+	struct most_common *mc = to_most_common(group->cg_subsys);
+
+	module_put(mc->mod);
+}
+
 static struct configfs_group_operations most_common_group_ops = {
 	.make_item	= most_common_make_item,
+	.disconnect_notify = most_common_disconnect,
 };
 
 static const struct config_item_type most_common_type = {
@@ -558,13 +573,14 @@ static const struct config_item_type most_snd_grp_type = {
 struct most_sound {
 	struct configfs_subsystem subsys;
 	struct list_head soundcard_list;
+	struct module *mod;
 };
 
 static struct config_group *most_sound_make_group(struct config_group *group,
 						  const char *name)
 {
 	struct most_snd_grp *most;
-	struct most_sound *ms = container_of(to_configfs_subsystem(group),
+	struct most_sound *ms = container_of(group->cg_subsys,
 					     struct most_sound, subsys);
 
 	list_for_each_entry(most, &ms->soundcard_list, list) {
@@ -573,17 +589,29 @@ static struct config_group *most_sound_make_group(struct config_group *group,
 			return ERR_PTR(-EPROTO);
 		}
 	}
+	if (!try_module_get(ms->mod))
+		return ERR_PTR(-ENOLCK);
 	most = kzalloc(sizeof(*most), GFP_KERNEL);
-	if (!most)
+	if (!most) {
+		module_put(ms->mod);
 		return ERR_PTR(-ENOMEM);
-
+	}
 	config_group_init_type_name(&most->group, name, &most_snd_grp_type);
 	list_add_tail(&most->list, &ms->soundcard_list);
 	return &most->group;
 }
 
+static void most_sound_disconnect(struct config_group *group,
+				  struct config_item *item)
+{
+	struct most_sound *ms = container_of(group->cg_subsys,
+					     struct most_sound, subsys);
+	module_put(ms->mod);
+}
+
 static struct configfs_group_operations most_sound_group_ops = {
 	.make_group	= most_sound_make_group,
+	.disconnect_notify = most_sound_disconnect,
 };
 
 static const struct config_item_type most_sound_type = {
@@ -607,12 +635,16 @@ int most_register_configfs_subsys(struct core_component *c)
 	int ret;
 
 	if (!strcmp(c->name, "cdev")) {
+		most_cdev.mod = c->mod;
 		ret = configfs_register_subsystem(&most_cdev.subsys);
 	} else if (!strcmp(c->name, "net")) {
+		most_net.mod = c->mod;
 		ret = configfs_register_subsystem(&most_net.subsys);
 	} else if (!strcmp(c->name, "video")) {
+		most_video.mod = c->mod;
 		ret = configfs_register_subsystem(&most_video.subsys);
 	} else if (!strcmp(c->name, "sound")) {
+		most_sound_subsys.mod = c->mod;
 		ret = configfs_register_subsystem(&most_sound_subsys.subsys);
 	} else {
 		return -ENODEV;
diff --git a/drivers/staging/most/core.h b/drivers/staging/most/core.h
index 652aaa7..49859ae 100644
--- a/drivers/staging/most/core.h
+++ b/drivers/staging/most/core.h
@@ -265,6 +265,7 @@ struct most_interface {
 struct core_component {
 	struct list_head list;
 	const char *name;
+	struct module *mod;
 	int (*probe_channel)(struct most_interface *iface, int channel_idx,
 			     struct most_channel_config *cfg, char *name,
 			     char *param);
diff --git a/drivers/staging/most/net/net.c b/drivers/staging/most/net/net.c
index 26a3185..6cab1bb 100644
--- a/drivers/staging/most/net/net.c
+++ b/drivers/staging/most/net/net.c
@@ -498,6 +498,7 @@ static int comp_rx_data(struct mbo *mbo)
 }
 
 static struct core_component comp = {
+	.mod = THIS_MODULE,
 	.name = "net",
 	.probe_channel = comp_probe_channel,
 	.disconnect_channel = comp_disconnect_channel,
diff --git a/drivers/staging/most/sound/sound.c b/drivers/staging/most/sound/sound.c
index 7981706..1359f28 100644
--- a/drivers/staging/most/sound/sound.c
+++ b/drivers/staging/most/sound/sound.c
@@ -782,6 +782,7 @@ static int audio_tx_completion(struct most_interface *iface, int channel_id)
  * Initialization of the struct core_component
  */
 static struct core_component comp = {
+	.mod = THIS_MODULE,
 	.name = DRIVER_NAME,
 	.probe_channel = audio_probe_channel,
 	.disconnect_channel = audio_disconnect_channel,
diff --git a/drivers/staging/most/video/video.c b/drivers/staging/most/video/video.c
index 250af9f..10c1ef7 100644
--- a/drivers/staging/most/video/video.c
+++ b/drivers/staging/most/video/video.c
@@ -528,6 +528,7 @@ static int comp_disconnect_channel(struct most_interface *iface,
 }
 
 static struct core_component comp = {
+	.mod = THIS_MODULE,
 	.name = "video",
 	.probe_channel = comp_probe_channel,
 	.disconnect_channel = comp_disconnect_channel,
-- 
2.7.4

_______________________________________________
devel mailing list
devel@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel