Patchwork net: remove the unnecessary list_del

login
register
mail settings
Submitter Gao feng
Date Feb. 25, 2013, 4:41 a.m.
Message ID <1361767316-28583-1-git-send-email-gaofeng@cn.fujitsu.com>
Download mbox | patch
Permalink /patch/222844/
State Rejected
Delegated to: David Miller
Headers show

Comments

Gao feng - Feb. 25, 2013, 4:41 a.m.
These lists are used by unregister_netdevice_many,
they are local variables,will not be seen by others.
there is no need to delete them.

Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
---
 drivers/net/macvlan.c | 1 -
 net/core/dev.c        | 1 -
 net/core/rtnetlink.c  | 1 -
 net/mac80211/iface.c  | 1 -
 4 files changed, 4 deletions(-)
David Miller - Feb. 25, 2013, 4:54 a.m.
From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Mon, 25 Feb 2013 12:41:56 +0800

> These lists are used by unregister_netdevice_many,
> they are local variables,will not be seen by others.
> there is no need to delete them.
> 
> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>

What about the devices on the list?  The ones at the front and the
rear of the list will have list pointers that point into no longer
valid stack frame locations.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gao feng - Feb. 25, 2013, 5:03 a.m.
On 2013/02/25 12:54, David Miller wrote:
> From: Gao feng <gaofeng@cn.fujitsu.com>
> Date: Mon, 25 Feb 2013 12:41:56 +0800
> 
>> These lists are used by unregister_netdevice_many,
>> they are local variables,will not be seen by others.
>> there is no need to delete them.
>>
>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
> 
> What about the devices on the list?  The ones at the front and the
> rear of the list will have list pointers that point into no longer
> valid stack frame locations.

These lists are only used by unregister_netdevice_many,
we don't access these lists again after unregister_netdevice_many,
so I think it doesn't have invalid stack frame locations problems.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Feb. 25, 2013, 5:39 a.m.
From: Gao feng <gaofeng@cn.fujitsu.com>
Date: Mon, 25 Feb 2013 13:03:04 +0800

> On 2013/02/25 12:54, David Miller wrote:
>> From: Gao feng <gaofeng@cn.fujitsu.com>
>> Date: Mon, 25 Feb 2013 12:41:56 +0800
>> 
>>> These lists are used by unregister_netdevice_many,
>>> they are local variables,will not be seen by others.
>>> there is no need to delete them.
>>>
>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>> 
>> What about the devices on the list?  The ones at the front and the
>> rear of the list will have list pointers that point into no longer
>> valid stack frame locations.
> 
> These lists are only used by unregister_netdevice_many,
> we don't access these lists again after unregister_netdevice_many,
> so I think it doesn't have invalid stack frame locations problems.

I do not see unregister_netdevice_many() list_del()'ing the devices
on the list, therefore those netdevice objects have pointers into
the stale stack frame.

You cannot make this change.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Gao feng - Feb. 25, 2013, 8:01 a.m.
On 2013/02/25 13:39, David Miller wrote:
> From: Gao feng <gaofeng@cn.fujitsu.com>
> Date: Mon, 25 Feb 2013 13:03:04 +0800
> 
>> On 2013/02/25 12:54, David Miller wrote:
>>> From: Gao feng <gaofeng@cn.fujitsu.com>
>>> Date: Mon, 25 Feb 2013 12:41:56 +0800
>>>
>>>> These lists are used by unregister_netdevice_many,
>>>> they are local variables,will not be seen by others.
>>>> there is no need to delete them.
>>>>
>>>> Signed-off-by: Gao feng <gaofeng@cn.fujitsu.com>
>>>
>>> What about the devices on the list?  The ones at the front and the
>>> rear of the list will have list pointers that point into no longer
>>> valid stack frame locations.
>>
>> These lists are only used by unregister_netdevice_many,
>> we don't access these lists again after unregister_netdevice_many,
>> so I think it doesn't have invalid stack frame locations problems.
> 
> I do not see unregister_netdevice_many() list_del()'ing the devices
> on the list, therefore those netdevice objects have pointers into
> the stale stack frame.
> 

Yes, I agree that this will make the netdevice objects have pointers
into the stale stack frame.

I check the codes,and found there are tons of codes that don't call
list_del after unregister_netdevice_many(). such as mroute_clean_tables,
ip6_tnl_destroy_tunnels...

I think it's better to make sure netdevice objects have valid pointers
by calling list_del in unregister_netdevice_many.

> You cannot make this change.
> 

Will send another patch.

Thanks
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/drivers/net/macvlan.c b/drivers/net/macvlan.c
index defcd8a..9b728f1 100644
--- a/drivers/net/macvlan.c
+++ b/drivers/net/macvlan.c
@@ -945,7 +945,6 @@  static int macvlan_device_event(struct notifier_block *unused,
 		list_for_each_entry_safe(vlan, next, &port->vlans, list)
 			vlan->dev->rtnl_link_ops->dellink(vlan->dev, &list_kill);
 		unregister_netdevice_many(&list_kill);
-		list_del(&list_kill);
 		break;
 	case NETDEV_PRE_TYPE_CHANGE:
 		/* Forbid underlaying device to change its type. */
diff --git a/net/core/dev.c b/net/core/dev.c
index 18d8b5a..7c5e9d2 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -6172,7 +6172,6 @@  static void __net_exit default_device_exit_batch(struct list_head *net_list)
 		}
 	}
 	unregister_netdevice_many(&dev_kill_list);
-	list_del(&dev_kill_list);
 	rtnl_unlock();
 }
 
diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index d8aa20f..8e35210 100644
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -1613,7 +1613,6 @@  static int rtnl_dellink(struct sk_buff *skb, struct nlmsghdr *nlh, void *arg)
 
 	ops->dellink(dev, &list_kill);
 	unregister_netdevice_many(&list_kill);
-	list_del(&list_kill);
 	return 0;
 }
 
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 2c059e5..a799629 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1639,7 +1639,6 @@  void ieee80211_remove_interfaces(struct ieee80211_local *local)
 	}
 	mutex_unlock(&local->iflist_mtx);
 	unregister_netdevice_many(&unreg_list);
-	list_del(&unreg_list);
 
 	list_for_each_entry_safe(sdata, tmp, &wdev_list, list) {
 		list_del(&sdata->list);