netdev - Re: net: Fix inconsistent teardown and release of private netdev state.

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:   Fri, 7 Jul 2017 15:39:09 -0700
From:   Cong Wang <xiyou.wangcong@...il.com>
To:     "Jason A. Donenfeld" <Jason@...c4.com>
Cc:     Netdev <netdev@...r.kernel.org>
Subject: Re: net: Fix inconsistent teardown and release of private netdev state.

On Thu, Jul 6, 2017 at 7:24 AM, Jason A. Donenfeld <Jason@...c4.com> wrote:
>         list_add(&priv->list, &list_of_things);
>
>         ret = register_netdevice(); // if ret is < 0, then destruct above is automatically called
>
>         // RACE WITH LIST_ADD/LIST_DEL!! It's impossible to call list_add only after
>         // things are brought up successfully. This is problematic.
>
>         if (!ret)
>                 pr_info("Yay it worked!\n");

I fail to understand what you mean by RACE here.

Here you should already have RTNL lock, so it can't race with any other
newlink() calls. In fact you can't acquire RTNL lock in your destructor
since register_netdevice() already gets it. Perhaps you mean
netdev_run_todo() calls it without RTNL lock?

I don't know why you reorder the above list_add(), you can order it
as it was before, aka, call it after register_netdevice(), but you have to
init the priv->list now for the list_del() on error path.