When processing the unregister notify for a hard interface, removing
the sysfs files may lead to a circular deadlock (rtnl mutex -
s_active).
To overcome this problem, postpone the sysfs removal in a worker.
Reported-by: Sasha Levin sasha.le...@oracle.com
Reported-by: Sven Eckelmann s...@narfation.org
Signed-off-by: Simon Wunderlich s...@hrz.tu-chemnitz.de
---
Changes from PATCHv2:
* kerneldoc for functions
Changes from PATCHv1:
* INIT_WORK() in respective struct initialization functions
* kerneldoc
Changes from RFCv1:
* use work_struct properly, instead of delayed_work
* postpone for softifs as well as for hardifs
Postponing the sysfs removal for the hardif unregister is easier than
other alternatives involving deferring. This should bring us to the
same level to the bridge code, which also messes with sysfs in the
notifier processing function, and uses rtnl_trylock when called
from within sysfs.
As far as I could understand the net/core code, only the unregister
case is the critical one, so the original bug should hopefully be
fixed.
---
hard-interface.c | 24 ++--
soft-interface.c | 32 +---
types.h |6 ++
3 files changed, 57 insertions(+), 5 deletions(-)
diff --git a/hard-interface.c b/hard-interface.c
index 78cf350..368219e 100644
--- a/hard-interface.c
+++ b/hard-interface.c
@@ -457,6 +457,24 @@ out:
batadv_hardif_free_ref(primary_if);
}
+/**
+ * batadv_hardif_remove_interface_finish - cleans up the remains of a hardif
+ * @work: work queue item
+ *
+ * Free the parts of the hard interface which can not be removed under
+ * rtnl lock (to prevent deadlock situations).
+ */
+static void batadv_hardif_remove_interface_finish(struct work_struct *work)
+{
+ struct batadv_hard_iface *hard_iface;
+
+ hard_iface = container_of(work, struct batadv_hard_iface,
+ cleanup_work);
+
+ batadv_sysfs_del_hardif(hard_iface-hardif_obj);
+ batadv_hardif_free_ref(hard_iface);
+}
+
static struct batadv_hard_iface *
batadv_hardif_add_interface(struct net_device *net_dev)
{
@@ -484,6 +502,9 @@ batadv_hardif_add_interface(struct net_device *net_dev)
hard_iface-soft_iface = NULL;
hard_iface-if_status = BATADV_IF_NOT_IN_USE;
INIT_LIST_HEAD(hard_iface-list);
+ INIT_WORK(hard_iface-cleanup_work,
+ batadv_hardif_remove_interface_finish);
+
/* extra reference for return */
atomic_set(hard_iface-refcount, 2);
@@ -518,8 +539,7 @@ static void batadv_hardif_remove_interface(struct
batadv_hard_iface *hard_iface)
return;
hard_iface-if_status = BATADV_IF_TO_BE_REMOVED;
- batadv_sysfs_del_hardif(hard_iface-hardif_obj);
- batadv_hardif_free_ref(hard_iface);
+ queue_work(batadv_event_workqueue, hard_iface-cleanup_work);
}
void batadv_hardif_remove_interfaces(void)
diff --git a/soft-interface.c b/soft-interface.c
index e67e013..2711e87 100644
--- a/soft-interface.c
+++ b/soft-interface.c
@@ -449,6 +449,30 @@ static void batadv_interface_setup(struct net_device *dev)
memset(priv, 0, sizeof(*priv));
}
+/**
+ * batadv_softif_destroy_finish - cleans up the remains of a softif
+ * @work: work queue item
+ *
+ * Free the parts of the soft interface which can not be removed under
+ * rtnl lock (to prevent deadlock situations).
+ */
+static void batadv_softif_destroy_finish(struct work_struct *work)
+{
+ struct batadv_priv *bat_priv;
+ struct net_device *soft_iface;
+
+ bat_priv = container_of(work, struct batadv_priv,
+ cleanup_work);
+ soft_iface = bat_priv-soft_iface;
+
+ batadv_debugfs_del_meshif(soft_iface);
+ batadv_sysfs_del_meshif(soft_iface);
+
+ rtnl_lock();
+ unregister_netdevice(soft_iface);
+ rtnl_unlock();
+}
+
struct net_device *batadv_softif_create(const char *name)
{
struct net_device *soft_iface;
@@ -463,6 +487,8 @@ struct net_device *batadv_softif_create(const char *name)
goto out;
bat_priv = netdev_priv(soft_iface);
+ bat_priv-soft_iface = soft_iface;
+ INIT_WORK(bat_priv-cleanup_work, batadv_softif_destroy_finish);
/* batadv_interface_stats() needs to be available as soon as
* register_netdevice() has been called
@@ -551,10 +577,10 @@ out:
void batadv_softif_destroy(struct net_device *soft_iface)
{
- batadv_debugfs_del_meshif(soft_iface);
- batadv_sysfs_del_meshif(soft_iface);
+ struct batadv_priv *bat_priv = netdev_priv(soft_iface);
+
batadv_mesh_free(soft_iface);
- unregister_netdevice(soft_iface);
+ queue_work(batadv_event_workqueue, bat_priv-cleanup_work);
}
int batadv_softif_is_valid(const struct net_device *net_dev)
diff --git a/types.h b/types.h
index 89ac1fb..4cd87a0 100644
--- a/types.h
+++ b/types.h
@@ -68,6 +68,7 @@ struct
