• Vladimir Oltean's avatar
    net: dsa: avoid call to __dev_set_promiscuity() while rtnl_mutex isn't held · 8940e6b6
    Vladimir Oltean authored
    If the DSA master doesn't support IFF_UNICAST_FLT, then the following
    call path is possible:
    
    dsa_slave_switchdev_event_work
    -> dsa_port_host_fdb_add
       -> dev_uc_add
          -> __dev_set_rx_mode
             -> __dev_set_promiscuity
    
    Since the blamed commit, dsa_slave_switchdev_event_work() no longer
    holds rtnl_lock(), which triggers the ASSERT_RTNL() from
    __dev_set_promiscuity().
    
    Taking rtnl_lock() around dev_uc_add() is impossible, because all the
    code paths that call dsa_flush_workqueue() do so from contexts where the
    rtnl_mutex is already held - so this would lead to an instant deadlock.
    
    dev_uc_add() in itself doesn't require the rtnl_mutex for protection.
    There is this comment in __dev_set_rx_mode() which assumes so:
    
    		/* Unicast addresses changes may only happen under the rtnl,
    		 * therefore calling __dev_set_promiscuity here is safe.
    		 */
    
    but it is from commit 4417da66 ("[NET]: dev: secondary unicast
    address support") dated June 2007, and in the meantime, commit
    f1f28aa3 ("netdev: Add addr_list_lock to struct net_device."), dated
    July 2008, has added &dev->addr_list_lock to protect this instead of the
    global rtnl_mutex.
    
    Nonetheless, __dev_set_promiscuity() does assume rtnl_mutex protection,
    but it is the uncommon path of what we typically expect dev_uc_add()
    to do. So since only the uncommon path requires rtnl_lock(), just check
    ahead of time whether dev_uc_add() would result into a call to
    __dev_set_promiscuity(), and handle that condition separately.
    
    DSA already configures the master interface to be promiscuous if the
    tagger requires this. We can extend this to also cover the case where
    the master doesn't handle dev_uc_add() (doesn't support IFF_UNICAST_FLT),
    and on the premise that we'd end up making it promiscuous during
    operation anyway, either if a DSA slave has a non-inherited MAC address,
    or if the bridge notifies local FDB entries for its own MAC address, the
    address of a station learned on a foreign port, etc.
    
    Fixes: 0faf890f ("net: dsa: drop rtnl_lock from dsa_slave_switchdev_event_work")
    Reported-by: default avatarOleksij Rempel <o.rempel@pengutronix.de>
    Signed-off-by: default avatarVladimir Oltean <vladimir.oltean@nxp.com>
    Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
    8940e6b6
port.c 32.8 KB