Subject: Re: [PATCH 6/14][RTNL]: Introduce the rtnl_kill_links call.

From: Pavel Emelyanov <[email-masked]>
Date: Fri, 11 Apr 2008 12:01:31 +0400

>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
>> index da99ac0..bc39e41 100644
>> --- a/net/core/rtnetlink.c
>> +++ b/net/core/rtnetlink.c
>> @@ -269,6 +269,26 @@ int rtnl_link_register(struct rtnl_link_ops *ops)
>>
>>  EXPORT_SYMBOL_GPL(rtnl_link_register);
>>
>> +static void __rtnl_kill_links(struct net *net, struct rtnl_link_ops *ops)
>> +{
>> +	struct net_device *dev;
>> +restart:
>> +	for_each_netdev(net, dev) {
>> +		if (dev->rtnl_link_ops == ops) {
>> +			ops->dellink(dev);
>> +			goto restart;
>> +		}
>> +	}
>> +}
>> +
>> +void rtnl_kill_links(struct net *net, struct rtnl_link_ops *ops)
>> +{
>> +	rtnl_lock();
>> +	__rtnl_kill_links(net, ops);
>> +	rtnl_unlock();
>> +}
>> +EXPORT_SYMBOL_GPL(rtnl_kill_links);
>> +
>>  /**
>>   * __rtnl_link_unregister - Unregister rtnl_link_ops from rtnetlink.
>>   * @ops: struct rtnl_link_ops * to unregister
>> @@ -277,17 +297,10 @@ EXPORT_SYMBOL_GPL(rtnl_link_register);
>>   */
>>  void __rtnl_link_unregister(struct rtnl_link_ops *ops)
>>  {
>> -	struct net_device *dev, *n;
>>  	struct net *net;
>>
>>  	for_each_net(net) {
>> -restart:
>> -		for_each_netdev_safe(net, dev, n) {
>> -			if (dev->rtnl_link_ops == ops) {
>> -				ops->dellink(dev);
>> -				goto restart;
>> -			}
>> -		}
>> +		__rtnl_kill_links(net, ops);
> 
> This was _safe, and now it's not. Is that intentional?

Yup - we goto restart in case we del some link, so there's no need
in _safe iteration. 

This goto was added by Partick (commit 68365458 [NET]: rtnl_link: 
fix use-after-free) and I suspect he simply forgot to remove the 
_safe iterator (I put him in Cc to correct me if I'm wrong).

>>  	}
>>  	list_del(&ops->list);
>>  }
>> --
>> 1.5.3.4