net: Write lock dev_base_lock without disabling bottom halves.

The writer acquires dev_base_lock with disabled bottom halves.
The reader can acquire dev_base_lock without disabling bottom halves
because there is no writer in softirq context.

On PREEMPT_RT the softirqs are preemptible and local_bh_disable() acts
as a lock to ensure that resources, that are protected by disabling
bottom halves, remain protected.
This leads to a circular locking dependency if the lock acquired with
disabled bottom halves (as in write_lock_bh()) and somewhere else with
enabled bottom halves (as by read_lock() in netstat_show()) followed by
disabling bottom halves (cxgb_get_stats() -> t4_wr_mbox_meat_timeout()
-> spin_lock_bh()). This is the reverse locking order.

All read_lock() invocation are from sysfs callback which are not invoked
from softirq context. Therefore there is no need to disable bottom
halves while acquiring a write lock.

Acquire the write lock of dev_base_lock without disabling bottom halves.

Reported-by: Pei Zhang <pezhang@redhat.com>
Reported-by: Luis Claudio R. Goncalves <lgoncalv@redhat.com>
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/core/dev.c b/net/core/dev.c
index 823917de0d2b0d1c4b3e124e30569cc694d57e7d..d30adecc2bb2b744b283dbda89d9488b8eb6d47e 100644 (file)
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -365,12 +365,12 @@ static void list_netdevice(struct net_device *dev)
 
        ASSERT_RTNL();
 
-       write_lock_bh(&dev_base_lock);
+       write_lock(&dev_base_lock);
        list_add_tail_rcu(&dev->dev_list, &net->dev_base_head);
        netdev_name_node_add(net, dev->name_node);
        hlist_add_head_rcu(&dev->index_hlist,
                           dev_index_hash(net, dev->ifindex));
-       write_unlock_bh(&dev_base_lock);
+       write_unlock(&dev_base_lock);
 
        dev_base_seq_inc(net);
 }
@@ -383,11 +383,11 @@ static void unlist_netdevice(struct net_device *dev)
        ASSERT_RTNL();
 
        /* Unlink dev from the device chain */
-       write_lock_bh(&dev_base_lock);
+       write_lock(&dev_base_lock);
        list_del_rcu(&dev->dev_list);
        netdev_name_node_del(dev->name_node);
        hlist_del_rcu(&dev->index_hlist);
-       write_unlock_bh(&dev_base_lock);
+       write_unlock(&dev_base_lock);
 
        dev_base_seq_inc(dev_net(dev));
 }
@@ -1188,15 +1188,15 @@ rollback:
 
        netdev_adjacent_rename_links(dev, oldname);
 
-       write_lock_bh(&dev_base_lock);
+       write_lock(&dev_base_lock);
        netdev_name_node_del(dev->name_node);
-       write_unlock_bh(&dev_base_lock);
+       write_unlock(&dev_base_lock);
 
        synchronize_rcu();
 
-       write_lock_bh(&dev_base_lock);
+       write_lock(&dev_base_lock);
        netdev_name_node_add(net, dev->name_node);
-       write_unlock_bh(&dev_base_lock);
+       write_unlock(&dev_base_lock);
 
        ret = call_netdevice_notifiers(NETDEV_CHANGENAME, dev);
        ret = notifier_to_errno(ret);

diff --git a/net/core/link_watch.c b/net/core/link_watch.c
index 1a455847da54fc06802598a4631c277e6a2eeeed..9599afd0862dabfca4560de6ec36606bf3742fd6 100644 (file)
--- a/net/core/link_watch.c
+++ b/net/core/link_watch.c
@@ -55,7 +55,7 @@ static void rfc2863_policy(struct net_device *dev)
        if (operstate == dev->operstate)
                return;
 
-       write_lock_bh(&dev_base_lock);
+       write_lock(&dev_base_lock);
 
        switch(dev->link_mode) {
        case IF_LINK_MODE_TESTING:
@@ -74,7 +74,7 @@ static void rfc2863_policy(struct net_device *dev)
 
        dev->operstate = operstate;
 
-       write_unlock_bh(&dev_base_lock);
+       write_unlock(&dev_base_lock);
 }
 
 

diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c
index 6f25c0a8aebe17e036ef624f23573d0b194f3982..d6eba554b13781ce25769539b91c0ef6282a42f2 100644 (file)
--- a/net/core/rtnetlink.c
+++ b/net/core/rtnetlink.c
@@ -842,9 +842,9 @@ static void set_operstate(struct net_device *dev, unsigned char transition)
        }
 
        if (dev->operstate != operstate) {
-               write_lock_bh(&dev_base_lock);
+               write_lock(&dev_base_lock);
                dev->operstate = operstate;
-               write_unlock_bh(&dev_base_lock);
+               write_unlock(&dev_base_lock);
                netdev_state_change(dev);
        }
 }
@@ -2778,11 +2778,11 @@ static int do_setlink(const struct sk_buff *skb,
        if (tb[IFLA_LINKMODE]) {
                unsigned char value = nla_get_u8(tb[IFLA_LINKMODE]);
 
-               write_lock_bh(&dev_base_lock);
+               write_lock(&dev_base_lock);
                if (dev->link_mode ^ value)
                        status |= DO_SETLINK_NOTIFY;
                dev->link_mode = value;
-               write_unlock_bh(&dev_base_lock);
+               write_unlock(&dev_base_lock);
        }
 
        if (tb[IFLA_VFINFO_LIST]) {

diff --git a/net/hsr/hsr_device.c b/net/hsr/hsr_device.c
index 737e4f17e1c6ddb241ce9df499b38141eeb3b477..e57fdad9ef942f89d4a289e4d84a6127f3523052 100644 (file)
--- a/net/hsr/hsr_device.c
+++ b/net/hsr/hsr_device.c
@@ -30,13 +30,13 @@ static bool is_slave_up(struct net_device *dev)
 
 static void __hsr_set_operstate(struct net_device *dev, int transition)
 {
-       write_lock_bh(&dev_base_lock);
+       write_lock(&dev_base_lock);
        if (dev->operstate != transition) {
                dev->operstate = transition;
-               write_unlock_bh(&dev_base_lock);
+               write_unlock(&dev_base_lock);
                netdev_state_change(dev);
        } else {
-               write_unlock_bh(&dev_base_lock);
+               write_unlock(&dev_base_lock);
        }
 }