netdev - Re: [net,v2] ipv6: reorder ip6_route_dev_notifier after ipv6_dev

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]

Date:   Mon, 19 Jun 2017 21:54:41 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     jeffy <jeffy.chen@...k-chips.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        David Ahern <dsahern@...il.com>,
        Brian Norris <briannorris@...omium.org>,
        Douglas Anderson <dianders@...omium.org>
Subject: Re: [net,v2] ipv6: reorder ip6_route_dev_notifier after ipv6_dev_notf

Hello,

On Mon, Jun 19, 2017 at 8:15 PM, jeffy <jeffy.chen@...k-chips.com> wrote:
> but actually they are not guaranteed to be paired:
>
> the netdev_run_todo(see the first dump stack above) would call
> netdev_wait_allrefs to rebroadcast unregister notification multiple times,
> unless timed out or all of the "struct net_device"'s refs released:
>
>  * This is called when unregistering network devices.
>  *
>  * Any protocol or device that holds a reference should register
>  * for netdevice notification, and cleanup and put back the
>  * reference if they receive an UNREGISTER event.
>  * We can get stuck here if buggy protocols don't correctly
>  * call dev_put.
>  */
> static void netdev_wait_allrefs(struct net_device *dev)
> {
> ...
>         while (refcnt != 0) {
>                 if (time_after(jiffies, rebroadcast_time + 1 * HZ)) {
>                         rtnl_lock();
>
>                         /* Rebroadcast unregister notification */
>                         call_netdevice_notifiers(NETDEV_UNREGISTER, dev);
>
>                         __rtnl_unlock();
>                         rcu_barrier();
>                         rtnl_lock();
>
>
> call_netdevice_notifiers(NETDEV_UNREGISTER_FINAL, dev);

Interesting, I didn't notice this corner-case, because normally
we would hit the one in rollback_registered_many(). Probably
we need to add a check

if (dev->reg_state == NETREG_UNREGISTERING)

in ip6_route_dev_notify(). Can you give it a try?

I guess we probably need to revise other NETDEV_UNREGISTER
handlers too.

I will send a patch tomorrow.

Thanks!