• Guillaume Nault's avatar
    ppp: fix race in ppp device destruction · 6151b8b3
    Guillaume Nault authored
    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():
    
     ------------[ cut here ]------------
     kernel BUG at net/core/dev.c:8141!
     invalid opcode: 0000 [#1] SMP
    
     CPU: 1 PID: 1557 Comm: pppd Not tainted 4.14.0-rc2+ #4
     Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.10.2-1.fc26 04/01/2014
    
     Call Trace:
      ppp_destroy_interface+0xd8/0xe0 [ppp_generic]
      ppp_disconnect_channel+0xda/0x110 [ppp_generic]
      ppp_unregister_channel+0x5e/0x110 [ppp_generic]
      pppox_unbind_sock+0x23/0x30 [pppox]
      pppoe_connect+0x130/0x440 [pppoe]
      SYSC_connect+0x98/0x110
      ? do_fcntl+0x2c0/0x5d0
      SyS_connect+0xe/0x10
      entry_SYSCALL_64_fastpath+0x1a/0xa5
    
     RIP: free_netdev+0x107/0x110 RSP: ffffc28a40573d88
     ---[ end trace ed294ff0cc40eeff ]---
    
    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: default avatarBeniamino Galvani <bgalvani@redhat.com>
    Fixes: 8cb775bc ("ppp: fix device unregistration upon netns deletion")
    Signed-off-by: default avatarGuillaume Nault <g.nault@alphalink.fr>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    6151b8b3
ppp_generic.c 76.2 KB