diff options
author | Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com> | 2017-01-24 07:00:46 -0500 |
---|---|---|
committer | David S. Miller <davem@davemloft.net> | 2017-01-24 16:14:58 -0500 |
commit | 9dc3abdd1f7ea524e8552e0a3ef01219892ed1f4 (patch) | |
tree | 5e8606b5a3c31d69a77e798c59d7f13bba6452fa /net/tipc | |
parent | fc0adfc8fd18b61b6f7a3f28b429e134d6f3a008 (diff) |
tipc: fix nametbl_lock soft lockup at module exit
Commit 333f796235a527 ("tipc: fix a race condition leading to
subscriber refcnt bug") reveals a soft lockup while acquiring
nametbl_lock.
Before commit 333f796235a527, we call tipc_conn_shutdown() from
tipc_close_conn() in the context of tipc_topsrv_stop(). In that
context, we are allowed to grab the nametbl_lock.
Commit 333f796235a527, moved tipc_conn_release (renamed from
tipc_conn_shutdown) to the connection refcount cleanup. This allows
either tipc_nametbl_withdraw() or tipc_topsrv_stop() to the cleanup.
Since tipc_exit_net() first calls tipc_topsrv_stop() and then
tipc_nametble_withdraw() increases the chances for the later to
perform the connection cleanup.
The soft lockup occurs in the call chain of tipc_nametbl_withdraw(),
when it performs the tipc_conn_kref_release() as it tries to grab
nametbl_lock again while holding it already.
tipc_nametbl_withdraw() grabs nametbl_lock
tipc_nametbl_remove_publ()
tipc_subscrp_report_overlap()
tipc_subscrp_send_event()
tipc_conn_sendmsg()
<< if (con->flags != CF_CONNECTED) we do conn_put(),
triggering the cleanup as refcount=0. >>
tipc_conn_kref_release
tipc_sock_release
tipc_conn_release
tipc_subscrb_delete
tipc_subscrp_delete
tipc_nametbl_unsubscribe << Soft Lockup >>
The previous changes in this series fixes the race conditions fixed
by commit 333f796235a527. Hence we can now revert the commit.
Fixes: 333f796235a52727 ("tipc: fix a race condition leading to subscriber refcnt bug")
Reported-and-Tested-by: John Thompson <thompa.atl@gmail.com>
Acked-by: Ying Xue <ying.xue@windriver.com>
Acked-by: Jon Maloy <jon.maloy@ericsson.com>
Signed-off-by: Parthasarathy Bhuvaragan <parthasarathy.bhuvaragan@ericsson.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/tipc')
-rw-r--r-- | net/tipc/server.c | 16 |
1 files changed, 5 insertions, 11 deletions
diff --git a/net/tipc/server.c b/net/tipc/server.c index 2e803601aa99..826cde2c401e 100644 --- a/net/tipc/server.c +++ b/net/tipc/server.c | |||
@@ -86,7 +86,6 @@ struct outqueue_entry { | |||
86 | static void tipc_recv_work(struct work_struct *work); | 86 | static void tipc_recv_work(struct work_struct *work); |
87 | static void tipc_send_work(struct work_struct *work); | 87 | static void tipc_send_work(struct work_struct *work); |
88 | static void tipc_clean_outqueues(struct tipc_conn *con); | 88 | static void tipc_clean_outqueues(struct tipc_conn *con); |
89 | static void tipc_sock_release(struct tipc_conn *con); | ||
90 | 89 | ||
91 | static void tipc_conn_kref_release(struct kref *kref) | 90 | static void tipc_conn_kref_release(struct kref *kref) |
92 | { | 91 | { |
@@ -104,7 +103,6 @@ static void tipc_conn_kref_release(struct kref *kref) | |||
104 | } | 103 | } |
105 | saddr->scope = -TIPC_NODE_SCOPE; | 104 | saddr->scope = -TIPC_NODE_SCOPE; |
106 | kernel_bind(sock, (struct sockaddr *)saddr, sizeof(*saddr)); | 105 | kernel_bind(sock, (struct sockaddr *)saddr, sizeof(*saddr)); |
107 | tipc_sock_release(con); | ||
108 | sock_release(sock); | 106 | sock_release(sock); |
109 | con->sock = NULL; | 107 | con->sock = NULL; |
110 | 108 | ||
@@ -194,19 +192,15 @@ static void tipc_unregister_callbacks(struct tipc_conn *con) | |||
194 | write_unlock_bh(&sk->sk_callback_lock); | 192 | write_unlock_bh(&sk->sk_callback_lock); |
195 | } | 193 | } |
196 | 194 | ||
197 | static void tipc_sock_release(struct tipc_conn *con) | 195 | static void tipc_close_conn(struct tipc_conn *con) |
198 | { | 196 | { |
199 | struct tipc_server *s = con->server; | 197 | struct tipc_server *s = con->server; |
200 | 198 | ||
201 | if (con->conid) | ||
202 | s->tipc_conn_release(con->conid, con->usr_data); | ||
203 | |||
204 | tipc_unregister_callbacks(con); | ||
205 | } | ||
206 | |||
207 | static void tipc_close_conn(struct tipc_conn *con) | ||
208 | { | ||
209 | if (test_and_clear_bit(CF_CONNECTED, &con->flags)) { | 199 | if (test_and_clear_bit(CF_CONNECTED, &con->flags)) { |
200 | tipc_unregister_callbacks(con); | ||
201 | |||
202 | if (con->conid) | ||
203 | s->tipc_conn_release(con->conid, con->usr_data); | ||
210 | 204 | ||
211 | /* We shouldn't flush pending works as we may be in the | 205 | /* We shouldn't flush pending works as we may be in the |
212 | * thread. In fact the races with pending rx/tx work structs | 206 | * thread. In fact the races with pending rx/tx work structs |