netdev - Re: [RFC PATCH net-next 17/22] nexthop: Replay nexthops when registering a notifier

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <74187436-6a30-2d44-de7f-0a52268b7f11@gmail.com>
Date:   Fri, 11 Sep 2020 10:47:13 -0600
From:   David Ahern <dsahern@...il.com>
To:     Ido Schimmel <idosch@...sch.org>
Cc:     netdev@...r.kernel.org, davem@...emloft.net, kuba@...nel.org,
        roopa@...dia.com, mlxsw@...dia.com,
        Ido Schimmel <idosch@...dia.com>
Subject: Re: [RFC PATCH net-next 17/22] nexthop: Replay nexthops when
 registering a notifier

On 9/11/20 10:40 AM, Ido Schimmel wrote:
>>> @@ -2116,11 +2137,40 @@ static struct notifier_block nh_netdev_notifier = {
>>>  	.notifier_call = nh_netdev_event,
>>>  };
>>>  
>>> +static int nexthops_dump(struct net *net, struct notifier_block *nb,
>>> +			 struct netlink_ext_ack *extack)
>>> +{
>>> +	struct rb_root *root = &net->nexthop.rb_root;
>>> +	struct rb_node *node;
>>> +	int err = 0;
>>> +
>>> +	for (node = rb_first(root); node; node = rb_next(node)) {
>>> +		struct nexthop *nh;
>>> +
>>> +		nh = rb_entry(node, struct nexthop, rb_node);
>>> +		err = call_nexthop_notifier(nb, net, NEXTHOP_EVENT_REPLACE, nh,
>>> +					    extack);
>>> +		if (err)
>>> +			break;
>>> +	}
>>> +
>>> +	return err;
>>> +}
>>> +
>>>  int register_nexthop_notifier(struct net *net, struct notifier_block *nb,
>>>  			      struct netlink_ext_ack *extack)
>>>  {
>>> -	return blocking_notifier_chain_register(&net->nexthop.notifier_chain,
>>> -						nb);
>>> +	int err;
>>> +
>>> +	rtnl_lock();
>>> +	err = nexthops_dump(net, nb, extack);
>>
>> can the unlock be moved here? register function below should not need it.
> 
> It can result in this unlikely race:
> 
> <t0> - rtnl_lock(); nexthops_dump(); rtnl_unlock()
> <t1> - Nexthop is added / deleted
> <t2> - blocking_notifier_chain_register()
> 

ok. Let's keep the order you have which I believe is consistent with FIB
notifiers.