diff options
author | Ying Xue <ying.xue@windriver.com> | 2018-08-07 03:52:32 -0400 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2018-08-07 16:15:35 -0400 |
commit | 37436d9c0e8f62c3eebe204ff5776ff31fd64658 (patch) | |
tree | 432587995f086cb753ff509f8c5a1f57f7d9b12b /net/tipc | |
parent | 455f05ecd2b219e9a216050796d30c830d9bc393 (diff) |
tipc: fix an interrupt unsafe locking scenario
Commit 9faa89d4ed9d ("tipc: make function tipc_net_finalize() thread
safe") tries to make it thread safe to set node address, so it uses
node_list_lock lock to serialize the whole process of setting node
address in tipc_net_finalize(). But it causes the following interrupt
unsafe locking scenario:
CPU0 CPU1
---- ----
rht_deferred_worker()
rhashtable_rehash_table()
lock(&(&ht->lock)->rlock)
tipc_nl_compat_doit()
tipc_net_finalize()
local_irq_disable();
lock(&(&tn->node_list_lock)->rlock);
tipc_sk_reinit()
rhashtable_walk_enter()
lock(&(&ht->lock)->rlock);
<Interrupt>
tipc_disc_rcv()
tipc_node_check_dest()
tipc_node_create()
lock(&(&tn->node_list_lock)->rlock);
*** DEADLOCK ***
When rhashtable_rehash_table() holds ht->lock on CPU0, it doesn't
disable BH. So if an interrupt happens after the lock, it can create
an inverse lock ordering between ht->lock and tn->node_list_lock. As
a consequence, deadlock might happen.
The reason causing the inverse lock ordering scenario above is because
the initial purpose of node_list_lock is not designed to do the
serialization of node address setting.
As cmpxchg() can guarantee CAS (compare-and-swap) process is atomic,
we use it to replace node_list_lock to ensure setting node address can
be atomically finished. It turns out the potential deadlock can be
avoided as well.
Fixes: 9faa89d4ed9d ("tipc: make function tipc_net_finalize() thread safe")
Signed-off-by: Ying Xue <ying.xue@windriver.com>
Acked-by: Jon Maloy <maloy@donjonn.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/tipc')
-rw-r--r-- | net/tipc/net.c | 4 |
1 files changed, 1 insertions, 3 deletions
diff --git a/net/tipc/net.c b/net/tipc/net.c index a7f6964c3a4b..62199cf5a56c 100644 --- a/net/tipc/net.c +++ b/net/tipc/net.c | |||
@@ -123,15 +123,13 @@ void tipc_net_finalize(struct net *net, u32 addr) | |||
123 | { | 123 | { |
124 | struct tipc_net *tn = tipc_net(net); | 124 | struct tipc_net *tn = tipc_net(net); |
125 | 125 | ||
126 | spin_lock_bh(&tn->node_list_lock); | 126 | if (!cmpxchg(&tn->node_addr, 0, addr)) { |
127 | if (!tipc_own_addr(net)) { | ||
128 | tipc_set_node_addr(net, addr); | 127 | tipc_set_node_addr(net, addr); |
129 | tipc_named_reinit(net); | 128 | tipc_named_reinit(net); |
130 | tipc_sk_reinit(net); | 129 | tipc_sk_reinit(net); |
131 | tipc_nametbl_publish(net, TIPC_CFG_SRV, addr, addr, | 130 | tipc_nametbl_publish(net, TIPC_CFG_SRV, addr, addr, |
132 | TIPC_CLUSTER_SCOPE, 0, addr); | 131 | TIPC_CLUSTER_SCOPE, 0, addr); |
133 | } | 132 | } |
134 | spin_unlock_bh(&tn->node_list_lock); | ||
135 | } | 133 | } |
136 | 134 | ||
137 | void tipc_net_stop(struct net *net) | 135 | void tipc_net_stop(struct net *net) |