Skip to content

Commit bc96215

Browse files
sprasad-microsoftsmfrench
sprasad-microsoft
authored and
smfrench
committed
cifs: avoid race conditions with parallel reconnects
When multiple processes/channels do reconnects in parallel we used to return success immediately negotiate/session-setup/tree-connect, causing race conditions between processes that enter the function in parallel. This caused several errors related to session not found to show up during parallel reconnects. Signed-off-by: Shyam Prasad N <[email protected]> Reviewed-by: Paulo Alcantara (SUSE) <[email protected]> Cc: [email protected] Signed-off-by: Steve French <[email protected]>
1 parent fddc6cc commit bc96215

3 files changed

Lines changed: 76 additions & 33 deletions

File tree

‎fs/cifs/connect.c‎

Lines changed: 36 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -212,31 +212,42 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
212212
cifs_chan_update_iface(ses, server);
213213

214214
spin_lock(&ses->chan_lock);
215-
if (!mark_smb_session && cifs_chan_needs_reconnect(ses, server))
216-
goto next_session;
215+
if (!mark_smb_session && cifs_chan_needs_reconnect(ses, server)) {
216+
spin_unlock(&ses->chan_lock);
217+
continue;
218+
}
217219

218220
if (mark_smb_session)
219221
CIFS_SET_ALL_CHANS_NEED_RECONNECT(ses);
220222
else
221223
cifs_chan_set_need_reconnect(ses, server);
222224

225+
cifs_dbg(FYI, "%s: channel connect bitmap: 0x%lx\n",
226+
__func__, ses->chans_need_reconnect);
227+
223228
/* If all channels need reconnect, then tcon needs reconnect */
224-
if (!mark_smb_session && !CIFS_ALL_CHANS_NEED_RECONNECT(ses))
225-
goto next_session;
229+
if (!mark_smb_session && !CIFS_ALL_CHANS_NEED_RECONNECT(ses)) {
230+
spin_unlock(&ses->chan_lock);
231+
continue;
232+
}
233+
spin_unlock(&ses->chan_lock);
226234

235+
spin_lock(&ses->ses_lock);
227236
ses->ses_status = SES_NEED_RECON;
237+
spin_unlock(&ses->ses_lock);
228238

229239
list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
230240
tcon->need_reconnect = true;
241+
spin_lock(&tcon->tc_lock);
231242
tcon->status = TID_NEED_RECON;
243+
spin_unlock(&tcon->tc_lock);
232244
}
233245
if (ses->tcon_ipc) {
234246
ses->tcon_ipc->need_reconnect = true;
247+
spin_lock(&ses->tcon_ipc->tc_lock);
235248
ses->tcon_ipc->status = TID_NEED_RECON;
249+
spin_unlock(&ses->tcon_ipc->tc_lock);
236250
}
237-
238-
next_session:
239-
spin_unlock(&ses->chan_lock);
240251
}
241252
spin_unlock(&cifs_tcp_ses_lock);
242253
}
@@ -3653,11 +3664,19 @@ cifs_negotiate_protocol(const unsigned int xid, struct cifs_ses *ses,
36533664

36543665
/* only send once per connect */
36553666
spin_lock(&server->srv_lock);
3656-
if (!server->ops->need_neg(server) ||
3667+
if (server->tcpStatus != CifsGood &&
3668+
server->tcpStatus != CifsNew &&
36573669
server->tcpStatus != CifsNeedNegotiate) {
3670+
spin_unlock(&server->srv_lock);
3671+
return -EHOSTDOWN;
3672+
}
3673+
3674+
if (!server->ops->need_neg(server) &&
3675+
server->tcpStatus == CifsGood) {
36583676
spin_unlock(&server->srv_lock);
36593677
return 0;
36603678
}
3679+
36613680
server->tcpStatus = CifsInNegotiate;
36623681
spin_unlock(&server->srv_lock);
36633682

@@ -3691,23 +3710,28 @@ cifs_setup_session(const unsigned int xid, struct cifs_ses *ses,
36913710
bool is_binding = false;
36923711

36933712
spin_lock(&ses->ses_lock);
3713+
cifs_dbg(FYI, "%s: channel connect bitmap: 0x%lx\n",
3714+
__func__, ses->chans_need_reconnect);
3715+
36943716
if (ses->ses_status != SES_GOOD &&
36953717
ses->ses_status != SES_NEW &&
36963718
ses->ses_status != SES_NEED_RECON) {
36973719
spin_unlock(&ses->ses_lock);
3698-
return 0;
3720+
return -EHOSTDOWN;
36993721
}
37003722

37013723
/* only send once per connect */
37023724
spin_lock(&ses->chan_lock);
3703-
if (CIFS_ALL_CHANS_GOOD(ses) ||
3704-
cifs_chan_in_reconnect(ses, server)) {
3725+
if (CIFS_ALL_CHANS_GOOD(ses)) {
3726+
if (ses->ses_status == SES_NEED_RECON)
3727+
ses->ses_status = SES_GOOD;
37053728
spin_unlock(&ses->chan_lock);
37063729
spin_unlock(&ses->ses_lock);
37073730
return 0;
37083731
}
3709-
is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
3732+
37103733
cifs_chan_set_in_reconnect(ses, server);
3734+
is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
37113735
spin_unlock(&ses->chan_lock);
37123736

37133737
if (!is_binding)

‎fs/cifs/smb2pdu.c‎

Lines changed: 26 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
203203
}
204204
spin_unlock(&server->srv_lock);
205205

206+
again:
206207
rc = cifs_wait_for_server_reconnect(server, tcon->retry);
207208
if (rc)
208209
return rc;
@@ -221,6 +222,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
221222

222223
nls_codepage = load_nls_default();
223224

225+
mutex_lock(&ses->session_mutex);
224226
/*
225227
* Recheck after acquire mutex. If another thread is negotiating
226228
* and the server never sends an answer the socket will be closed
@@ -229,6 +231,11 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
229231
spin_lock(&server->srv_lock);
230232
if (server->tcpStatus == CifsNeedReconnect) {
231233
spin_unlock(&server->srv_lock);
234+
mutex_unlock(&ses->session_mutex);
235+
236+
if (tcon->retry)
237+
goto again;
238+
232239
rc = -EHOSTDOWN;
233240
goto out;
234241
}
@@ -238,19 +245,22 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
238245
* need to prevent multiple threads trying to simultaneously
239246
* reconnect the same SMB session
240247
*/
248+
spin_lock(&ses->ses_lock);
241249
spin_lock(&ses->chan_lock);
242-
if (!cifs_chan_needs_reconnect(ses, server)) {
250+
if (!cifs_chan_needs_reconnect(ses, server) &&
251+
ses->ses_status == SES_GOOD) {
243252
spin_unlock(&ses->chan_lock);
244-
253+
spin_unlock(&ses->ses_lock);
245254
/* this means that we only need to tree connect */
246255
if (tcon->need_reconnect)
247256
goto skip_sess_setup;
248257

258+
mutex_unlock(&ses->session_mutex);
249259
goto out;
250260
}
251261
spin_unlock(&ses->chan_lock);
262+
spin_unlock(&ses->ses_lock);
252263

253-
mutex_lock(&ses->session_mutex);
254264
rc = cifs_negotiate_protocol(0, ses, server);
255265
if (!rc) {
256266
rc = cifs_setup_session(0, ses, server, nls_codepage);
@@ -266,10 +276,8 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
266276
mutex_unlock(&ses->session_mutex);
267277
goto out;
268278
}
269-
mutex_unlock(&ses->session_mutex);
270279

271280
skip_sess_setup:
272-
mutex_lock(&ses->session_mutex);
273281
if (!tcon->need_reconnect) {
274282
mutex_unlock(&ses->session_mutex);
275283
goto out;
@@ -284,7 +292,7 @@ smb2_reconnect(__le16 smb2_command, struct cifs_tcon *tcon,
284292
cifs_dbg(FYI, "reconnect tcon rc = %d\n", rc);
285293
if (rc) {
286294
/* If sess reconnected but tcon didn't, something strange ... */
287-
pr_warn_once("reconnect tcon failed rc = %d\n", rc);
295+
cifs_dbg(VFS, "reconnect tcon failed rc = %d\n", rc);
288296
goto out;
289297
}
290298

@@ -1256,9 +1264,9 @@ SMB2_sess_alloc_buffer(struct SMB2_sess_data *sess_data)
12561264
if (rc)
12571265
return rc;
12581266

1259-
spin_lock(&ses->chan_lock);
1260-
is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
1261-
spin_unlock(&ses->chan_lock);
1267+
spin_lock(&ses->ses_lock);
1268+
is_binding = (ses->ses_status == SES_GOOD);
1269+
spin_unlock(&ses->ses_lock);
12621270

12631271
if (is_binding) {
12641272
req->hdr.SessionId = cpu_to_le64(ses->Suid);
@@ -1416,9 +1424,9 @@ SMB2_auth_kerberos(struct SMB2_sess_data *sess_data)
14161424
goto out_put_spnego_key;
14171425
}
14181426

1419-
spin_lock(&ses->chan_lock);
1420-
is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
1421-
spin_unlock(&ses->chan_lock);
1427+
spin_lock(&ses->ses_lock);
1428+
is_binding = (ses->ses_status == SES_GOOD);
1429+
spin_unlock(&ses->ses_lock);
14221430

14231431
/* keep session key if binding */
14241432
if (!is_binding) {
@@ -1542,9 +1550,9 @@ SMB2_sess_auth_rawntlmssp_negotiate(struct SMB2_sess_data *sess_data)
15421550

15431551
cifs_dbg(FYI, "rawntlmssp session setup challenge phase\n");
15441552

1545-
spin_lock(&ses->chan_lock);
1546-
is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
1547-
spin_unlock(&ses->chan_lock);
1553+
spin_lock(&ses->ses_lock);
1554+
is_binding = (ses->ses_status == SES_GOOD);
1555+
spin_unlock(&ses->ses_lock);
15481556

15491557
/* keep existing ses id and flags if binding */
15501558
if (!is_binding) {
@@ -1610,9 +1618,9 @@ SMB2_sess_auth_rawntlmssp_authenticate(struct SMB2_sess_data *sess_data)
16101618

16111619
rsp = (struct smb2_sess_setup_rsp *)sess_data->iov[0].iov_base;
16121620

1613-
spin_lock(&ses->chan_lock);
1614-
is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
1615-
spin_unlock(&ses->chan_lock);
1621+
spin_lock(&ses->ses_lock);
1622+
is_binding = (ses->ses_status == SES_GOOD);
1623+
spin_unlock(&ses->ses_lock);
16161624

16171625
/* keep existing ses id and flags if binding */
16181626
if (!is_binding) {

‎fs/cifs/smb2transport.c‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ int smb2_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key)
8181
struct cifs_ses *ses = NULL;
8282
int i;
8383
int rc = 0;
84+
bool is_binding = false;
8485

8586
spin_lock(&cifs_tcp_ses_lock);
8687

@@ -97,16 +98,20 @@ int smb2_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key)
9798
goto out;
9899

99100
found:
101+
spin_lock(&ses->ses_lock);
100102
spin_lock(&ses->chan_lock);
101-
if (cifs_chan_needs_reconnect(ses, server) &&
102-
!CIFS_ALL_CHANS_NEED_RECONNECT(ses)) {
103+
104+
is_binding = (cifs_chan_needs_reconnect(ses, server) &&
105+
ses->ses_status == SES_GOOD);
106+
if (is_binding) {
103107
/*
104108
* If we are in the process of binding a new channel
105109
* to an existing session, use the master connection
106110
* session key
107111
*/
108112
memcpy(key, ses->smb3signingkey, SMB3_SIGN_KEY_SIZE);
109113
spin_unlock(&ses->chan_lock);
114+
spin_unlock(&ses->ses_lock);
110115
goto out;
111116
}
112117

@@ -119,10 +124,12 @@ int smb2_get_sign_key(__u64 ses_id, struct TCP_Server_Info *server, u8 *key)
119124
if (chan->server == server) {
120125
memcpy(key, chan->signkey, SMB3_SIGN_KEY_SIZE);
121126
spin_unlock(&ses->chan_lock);
127+
spin_unlock(&ses->ses_lock);
122128
goto out;
123129
}
124130
}
125131
spin_unlock(&ses->chan_lock);
132+
spin_unlock(&ses->ses_lock);
126133

127134
cifs_dbg(VFS,
128135
"%s: Could not find channel signing key for session 0x%llx\n",
@@ -392,11 +399,15 @@ generate_smb3signingkey(struct cifs_ses *ses,
392399
bool is_binding = false;
393400
int chan_index = 0;
394401

402+
spin_lock(&ses->ses_lock);
395403
spin_lock(&ses->chan_lock);
396-
is_binding = !CIFS_ALL_CHANS_NEED_RECONNECT(ses);
404+
is_binding = (cifs_chan_needs_reconnect(ses, server) &&
405+
ses->ses_status == SES_GOOD);
406+
397407
chan_index = cifs_ses_get_chan_index(ses, server);
398408
/* TODO: introduce ref counting for channels when the can be freed */
399409
spin_unlock(&ses->chan_lock);
410+
spin_unlock(&ses->ses_lock);
400411

401412
/*
402413
* All channels use the same encryption/decryption keys but

0 commit comments

Comments
 (0)