[v5,4/4] batman-adv: Avoid sysfs name collision for netns moves

Message ID 1465796492-5039-4-git-send-email-sven@narfation.org (mailing list archive)
State Accepted, archived
Commit bebf17830eff9ddc667e97ced2acf71fa324c533
Delegated to: Marek Lindner
Headers

Commit Message

Sven Eckelmann June 13, 2016, 5:41 a.m. UTC
  The kobject_put is only removing the sysfs entry and corresponding entries
when its reference counter becomes zero. This tends to lead to collisions
when a device is moved between two different network namespaces because
some of the sysfs files have to be removed first and then added again to
the already moved sysfs entry.

    WARNING: CPU: 0 PID: 290 at lib/kobject.c:240 kobject_add_internal+0x5ec/0x8a0
    kobject_add_internal failed for batman_adv with -EEXIST, don't try to register things with the same name in the same directory.

But the caller of kobject_put can already remove the sysfs entry before it
does the kobject_put. This removal is done even when the reference counter
is not yet zero and thus avoids the problem.

Signed-off-by: Sven Eckelmann <sven@narfation.org>
---
v5:
 - rebase on top of current master
v4:
 - rebase on top of current master
 - avoid deletes of vlan->kobj when it is a pointer to bat_priv->mesh_obj
   (happens for untagged vlan and can cause some kernfs warnings)
v3:
 - rebased on top of current master to fix conflicts with newest patches
v2:
 - rebased on top of current master to fix conflicts with newest patches
---
 net/batman-adv/sysfs.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
  

Comments

Marek Lindner June 20, 2016, 8:37 a.m. UTC | #1
On Monday, June 13, 2016 07:41:32 Sven Eckelmann wrote:
> The kobject_put is only removing the sysfs entry and corresponding entries
> when its reference counter becomes zero. This tends to lead to collisions
> when a device is moved between two different network namespaces because
> some of the sysfs files have to be removed first and then added again to
> the already moved sysfs entry.
> 
>     WARNING: CPU: 0 PID: 290 at lib/kobject.c:240
> kobject_add_internal+0x5ec/0x8a0 kobject_add_internal failed for batman_adv
> with -EEXIST, don't try to register things with the same name in the same
> directory.
> 
> But the caller of kobject_put can already remove the sysfs entry before it
> does the kobject_put. This removal is done even when the reference counter
> is not yet zero and thus avoids the problem.
> 
> Signed-off-by: Sven Eckelmann <sven@narfation.org>
> ---
> v5:
>  - rebase on top of current master
> v4:
>  - rebase on top of current master
>  - avoid deletes of vlan->kobj when it is a pointer to bat_priv->mesh_obj
>    (happens for untagged vlan and can cause some kernfs warnings)
> v3:
>  - rebased on top of current master to fix conflicts with newest patches
> v2:
>  - rebased on top of current master to fix conflicts with newest patches
> ---
>  net/batman-adv/sysfs.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)

Applied in revision bebf178.

Thanks,
Marek
  

Patch

diff --git a/net/batman-adv/sysfs.c b/net/batman-adv/sysfs.c
index 8528959..4e06cb7 100644
--- a/net/batman-adv/sysfs.c
+++ b/net/batman-adv/sysfs.c
@@ -713,6 +713,8 @@  rem_attr:
 	for (bat_attr = batadv_mesh_attrs; *bat_attr; ++bat_attr)
 		sysfs_remove_file(bat_priv->mesh_obj, &((*bat_attr)->attr));
 
+	kobject_uevent(bat_priv->mesh_obj, KOBJ_REMOVE);
+	kobject_del(bat_priv->mesh_obj);
 	kobject_put(bat_priv->mesh_obj);
 	bat_priv->mesh_obj = NULL;
 out:
@@ -727,6 +729,8 @@  void batadv_sysfs_del_meshif(struct net_device *dev)
 	for (bat_attr = batadv_mesh_attrs; *bat_attr; ++bat_attr)
 		sysfs_remove_file(bat_priv->mesh_obj, &((*bat_attr)->attr));
 
+	kobject_uevent(bat_priv->mesh_obj, KOBJ_REMOVE);
+	kobject_del(bat_priv->mesh_obj);
 	kobject_put(bat_priv->mesh_obj);
 	bat_priv->mesh_obj = NULL;
 }
@@ -782,6 +786,10 @@  rem_attr:
 	for (bat_attr = batadv_vlan_attrs; *bat_attr; ++bat_attr)
 		sysfs_remove_file(vlan->kobj, &((*bat_attr)->attr));
 
+	if (vlan->kobj != bat_priv->mesh_obj) {
+		kobject_uevent(vlan->kobj, KOBJ_REMOVE);
+		kobject_del(vlan->kobj);
+	}
 	kobject_put(vlan->kobj);
 	vlan->kobj = NULL;
 out:
@@ -801,6 +809,10 @@  void batadv_sysfs_del_vlan(struct batadv_priv *bat_priv,
 	for (bat_attr = batadv_vlan_attrs; *bat_attr; ++bat_attr)
 		sysfs_remove_file(vlan->kobj, &((*bat_attr)->attr));
 
+	if (vlan->kobj != bat_priv->mesh_obj) {
+		kobject_uevent(vlan->kobj, KOBJ_REMOVE);
+		kobject_del(vlan->kobj);
+	}
 	kobject_put(vlan->kobj);
 	vlan->kobj = NULL;
 }
@@ -1103,6 +1115,8 @@  out:
 
 void batadv_sysfs_del_hardif(struct kobject **hardif_obj)
 {
+	kobject_uevent(*hardif_obj, KOBJ_REMOVE);
+	kobject_del(*hardif_obj);
 	kobject_put(*hardif_obj);
 	*hardif_obj = NULL;
 }