netdev - Re: [PATCH net] ppp: fix race in ppp device destruction

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] [day] [month] [year] [list]

Message-Id: <20171006.101708.2248081297399135224.davem@davemloft.net>
Date:   Fri, 06 Oct 2017 10:17:08 -0700 (PDT)
From:   David Miller <davem@...emloft.net>
To:     g.nault@...halink.fr
Cc:     netdev@...r.kernel.org, bgalvani@...hat.com,
        linux-ppp@...r.kernel.org, paulus@...ba.org, dsahern@...il.com,
        gfree.wind@....163.com
Subject: Re: [PATCH net] ppp: fix race in ppp device destruction

From: Guillaume Nault <g.nault@...halink.fr>
Date: Fri, 6 Oct 2017 17:05:49 +0200

> ppp_release() tries to ensure that netdevices are unregistered before
> decrementing the unit refcount and running ppp_destroy_interface().
> 
> This is all fine as long as the the device is unregistered by
> ppp_release(): the unregister_netdevice() call, followed by
> rtnl_unlock(), guarantee that the unregistration process completes
> before rtnl_unlock() returns.
> 
> However, the device may be unregistered by other means (like
> ppp_nl_dellink()). If this happens right before ppp_release() calling
> rtnl_lock(), then ppp_release() has to wait for the concurrent
> unregistration code to release the lock.
> But rtnl_unlock() releases the lock before completing the device
> unregistration process. This allows ppp_release() to proceed and
> eventually call ppp_destroy_interface() before the unregistration
> process completes. Calling free_netdev() on this partially unregistered
> device will BUG():
 ...
> We could set the ->needs_free_netdev flag on PPP devices and move the
> ppp_destroy_interface() logic in the ->priv_destructor() callback. But
> that'd be quite intrusive as we'd first need to unlink from the other
> channels and units that depend on the device (the ones that used the
> PPPIOCCONNECT and PPPIOCATTACH ioctls).
> 
> Instead, we can just let the netdevice hold a reference on its
> ppp_file. This reference is dropped in ->priv_destructor(), at the very
> end of the unregistration process, so that neither ppp_release() nor
> ppp_disconnect_channel() can call ppp_destroy_interface() in the interim.
> 
> Reported-by: Beniamino Galvani <bgalvani@...hat.com>
> Fixes: 8cb775bc0a34 ("ppp: fix device unregistration upon netns deletion")
> Signed-off-by: Guillaume Nault <g.nault@...halink.fr>

Applied and queued up for -stable, thanks.