Skip to content

Commit b41f527

Browse files
sprasad-microsoftpaniakin-aws
authored andcommitted
cifs: track individual channel status using chans_need_reconnect
commit d1a931c upstream. We needed a way to identify the channels under the smb session which are in reconnect, so that the traffic to other channels can continue. So I replaced the bool need_reconnect with a bitmask identifying all the channels that need reconnection (named chans_need_reconnect). When a channel needs reconnection, the bit corresponding to the index of the server in ses->chans is used to set this bitmask. Checking if no channels or all the channels need reconnect then becomes very easy. Also wrote some helper macros for checking and setting the bits. Signed-off-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com> Stable-dep-of: 24a9799 ("smb: client: fix UAF in smb2_reconnect_server()") [v5.10: resolved contextual conflicts in cifs_get_smb_ses and moved changes from cifs_mark_tcp_ses_conns_for_reconnect to cifs_reconnect] Signed-off-by: Shaoying Xu <shaoyi@amazon.com>
1 parent a8e2d0d commit b41f527

File tree

6 files changed

+208
-21
lines changed

6 files changed

+208
-21
lines changed

fs/cifs/cifsglob.h

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1000,7 +1000,6 @@ struct cifs_ses {
10001000
struct ntlmssp_auth *ntlmssp; /* ciphertext, flags, server challenge */
10011001
enum securityEnum sectype; /* what security flavor was specified? */
10021002
bool sign; /* is signing required? */
1003-
bool need_reconnect:1; /* connection reset, uid now invalid */
10041003
bool domainAuto:1;
10051004
bool binding:1; /* are we binding the session? */
10061005
__u16 session_flags;
@@ -1030,11 +1029,25 @@ struct cifs_ses {
10301029
spinlock_t chan_lock;
10311030
/* ========= begin: protected by chan_lock ======== */
10321031
#define CIFS_MAX_CHANNELS 16
1032+
#define CIFS_ALL_CHANNELS_SET(ses) \
1033+
((1UL << (ses)->chan_count) - 1)
1034+
#define CIFS_ALL_CHANS_NEED_RECONNECT(ses) \
1035+
((ses)->chans_need_reconnect == CIFS_ALL_CHANNELS_SET(ses))
1036+
#define CIFS_CHAN_NEEDS_RECONNECT(ses, index) \
1037+
test_bit((index), &(ses)->chans_need_reconnect)
1038+
10331039
struct cifs_chan chans[CIFS_MAX_CHANNELS];
10341040
struct cifs_chan *binding_chan;
10351041
size_t chan_count;
10361042
size_t chan_max;
10371043
atomic_t chan_seq; /* round robin state */
1044+
1045+
/*
1046+
* chans_need_reconnect is a bitmap indicating which of the channels
1047+
* under this smb session needs to be reconnected.
1048+
* If not multichannel session, only one bit will be used.
1049+
*/
1050+
unsigned long chans_need_reconnect;
10381051
/* ========= end: protected by chan_lock ======== */
10391052
};
10401053

fs/cifs/cifsproto.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,19 @@ bool is_server_using_iface(struct TCP_Server_Info *server,
613613
struct cifs_server_iface *iface);
614614
bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface);
615615

616+
unsigned int
617+
cifs_ses_get_chan_index(struct cifs_ses *ses,
618+
struct TCP_Server_Info *server);
619+
void
620+
cifs_chan_set_need_reconnect(struct cifs_ses *ses,
621+
struct TCP_Server_Info *server);
622+
void
623+
cifs_chan_clear_need_reconnect(struct cifs_ses *ses,
624+
struct TCP_Server_Info *server);
625+
bool
626+
cifs_chan_needs_reconnect(struct cifs_ses *ses,
627+
struct TCP_Server_Info *server);
628+
616629
void extract_unc_hostname(const char *unc, const char **h, size_t *len);
617630
int copy_path_name(char *dst, const char *src);
618631
int smb2_parse_query_directory(struct cifs_tcon *tcon, struct kvec *rsp_iov,

fs/cifs/cifssmb.c

Lines changed: 42 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -195,8 +195,12 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
195195
retries = server->nr_targets;
196196
}
197197

198-
if (!ses->need_reconnect && !tcon->need_reconnect)
198+
spin_lock(&ses->chan_lock);
199+
if (!cifs_chan_needs_reconnect(ses, server) && !tcon->need_reconnect) {
200+
spin_unlock(&ses->chan_lock);
199201
return 0;
202+
}
203+
spin_unlock(&ses->chan_lock);
200204

201205
nls_codepage = load_nls_default();
202206

@@ -217,8 +221,25 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
217221
goto out;
218222
}
219223

224+
/*
225+
* need to prevent multiple threads trying to simultaneously
226+
* reconnect the same SMB session
227+
*/
228+
spin_lock(&ses->chan_lock);
229+
if (!cifs_chan_needs_reconnect(ses, server)) {
230+
spin_unlock(&ses->chan_lock);
231+
/* this just means that we only need to tcon */
232+
if (tcon->need_reconnect)
233+
goto skip_sess_setup;
234+
235+
rc = -EHOSTDOWN;
236+
mutex_unlock(&ses->session_mutex);
237+
goto out;
238+
}
239+
spin_unlock(&ses->chan_lock);
240+
220241
rc = cifs_negotiate_protocol(0, ses);
221-
if (rc == 0 && ses->need_reconnect)
242+
if (!rc)
222243
rc = cifs_setup_session(0, ses, nls_codepage);
223244

224245
/* do we need to reconnect tcon? */
@@ -227,6 +248,7 @@ cifs_reconnect_tcon(struct cifs_tcon *tcon, int smb_command)
227248
goto out;
228249
}
229250

251+
skip_sess_setup:
230252
cifs_mark_open_files_invalid(tcon);
231253
rc = cifs_tree_connect(0, tcon, nls_codepage);
232254
mutex_unlock(&ses->session_mutex);
@@ -366,8 +388,13 @@ static int
366388
smb_init_no_reconnect(int smb_command, int wct, struct cifs_tcon *tcon,
367389
void **request_buf, void **response_buf)
368390
{
369-
if (tcon->ses->need_reconnect || tcon->need_reconnect)
391+
spin_lock(&tcon->ses->chan_lock);
392+
if (cifs_chan_needs_reconnect(tcon->ses, tcon->ses->server) ||
393+
tcon->need_reconnect) {
394+
spin_unlock(&tcon->ses->chan_lock);
370395
return -EHOSTDOWN;
396+
}
397+
spin_unlock(&tcon->ses->chan_lock);
371398

372399
return __smb_init(smb_command, wct, tcon, request_buf, response_buf);
373400
}
@@ -717,8 +744,12 @@ CIFSSMBTDis(const unsigned int xid, struct cifs_tcon *tcon)
717744
* the tcon is no longer on the list, so no need to take lock before
718745
* checking this.
719746
*/
720-
if ((tcon->need_reconnect) || (tcon->ses->need_reconnect))
721-
return 0;
747+
spin_lock(&tcon->ses->chan_lock);
748+
if ((tcon->need_reconnect) || CIFS_ALL_CHANS_NEED_RECONNECT(tcon->ses)) {
749+
spin_unlock(&tcon->ses->chan_lock);
750+
return -EIO;
751+
}
752+
spin_unlock(&tcon->ses->chan_lock);
722753

723754
rc = small_smb_init(SMB_COM_TREE_DISCONNECT, 0, tcon,
724755
(void **)&smb_buffer);
@@ -813,9 +844,14 @@ CIFSSMBLogoff(const unsigned int xid, struct cifs_ses *ses)
813844
return -EIO;
814845

815846
mutex_lock(&ses->session_mutex);
816-
if (ses->need_reconnect)
847+
spin_lock(&ses->chan_lock);
848+
if (CIFS_ALL_CHANS_NEED_RECONNECT(ses)) {
849+
spin_unlock(&ses->chan_lock);
817850
goto session_already_dead; /* no need to send SMBlogoff if uid
818851
already closed due to reconnect */
852+
}
853+
spin_unlock(&ses->chan_lock);
854+
819855
rc = small_smb_init(SMB_COM_LOGOFF_ANDX, 2, NULL, (void **)&pSMB);
820856
if (rc) {
821857
mutex_unlock(&ses->session_mutex);

fs/cifs/connect.c

Lines changed: 24 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -455,13 +455,25 @@ cifs_reconnect(struct TCP_Server_Info *server)
455455
spin_lock(&cifs_tcp_ses_lock);
456456
list_for_each(tmp, &server->smb_ses_list) {
457457
ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
458-
ses->need_reconnect = true;
458+
spin_lock(&ses->chan_lock);
459+
if (cifs_chan_needs_reconnect(ses, server))
460+
goto next_session;
461+
462+
cifs_chan_set_need_reconnect(ses, server);
463+
464+
/* If all channels need reconnect, then tcon needs reconnect */
465+
if (!CIFS_ALL_CHANS_NEED_RECONNECT(ses))
466+
goto next_session;
467+
459468
list_for_each(tmp2, &ses->tcon_list) {
460469
tcon = list_entry(tmp2, struct cifs_tcon, tcon_list);
461470
tcon->need_reconnect = true;
462471
}
463472
if (ses->tcon_ipc)
464473
ses->tcon_ipc->need_reconnect = true;
474+
475+
next_session:
476+
spin_unlock(&ses->chan_lock);
465477
}
466478
spin_unlock(&cifs_tcp_ses_lock);
467479

@@ -3076,7 +3088,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
30763088
free_xid(xid);
30773089
return ERR_PTR(rc);
30783090
}
3079-
if (ses->need_reconnect) {
3091+
spin_lock(&ses->chan_lock);
3092+
if (cifs_chan_needs_reconnect(ses, server)) {
3093+
spin_unlock(&ses->chan_lock);
30803094
cifs_dbg(FYI, "Session needs reconnect\n");
30813095
rc = cifs_setup_session(xid, ses,
30823096
volume_info->local_nls);
@@ -3087,7 +3101,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
30873101
free_xid(xid);
30883102
return ERR_PTR(rc);
30893103
}
3104+
spin_lock(&ses->chan_lock);
30903105
}
3106+
spin_unlock(&ses->chan_lock);
30913107
mutex_unlock(&ses->session_mutex);
30923108

30933109
/* existing SMB ses has a server reference already */
@@ -3141,6 +3157,7 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
31413157
ses->chans[0].server = server;
31423158
ses->chan_count = 1;
31433159
ses->chan_max = volume_info->multichannel ? volume_info->max_channels:1;
3160+
ses->chans_need_reconnect = 1;
31443161
spin_unlock(&ses->chan_lock);
31453162

31463163
rc = cifs_negotiate_protocol(xid, ses);
@@ -3155,7 +3172,11 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
31553172
if (rc)
31563173
goto get_ses_fail;
31573174

3158-
/* success, put it on the list and add it as first channel */
3175+
/*
3176+
* success, put it on the list and add it as first channel
3177+
* note: the session becomes active soon after this. So you'll
3178+
* need to lock before changing something in the session.
3179+
*/
31593180
spin_lock(&cifs_tcp_ses_lock);
31603181
list_add(&ses->smb_ses_list, &server->smb_ses_list);
31613182
spin_unlock(&cifs_tcp_ses_lock);

fs/cifs/sess.c

Lines changed: 65 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,53 @@ bool is_ses_using_iface(struct cifs_ses *ses, struct cifs_server_iface *iface)
7373
return false;
7474
}
7575

76+
unsigned int
77+
cifs_ses_get_chan_index(struct cifs_ses *ses,
78+
struct TCP_Server_Info *server)
79+
{
80+
unsigned int i;
81+
82+
for (i = 0; i < ses->chan_count; i++) {
83+
if (ses->chans[i].server == server)
84+
return i;
85+
}
86+
87+
/* If we didn't find the channel, it is likely a bug */
88+
WARN_ON(1);
89+
return 0;
90+
}
91+
92+
void
93+
cifs_chan_set_need_reconnect(struct cifs_ses *ses,
94+
struct TCP_Server_Info *server)
95+
{
96+
unsigned int chan_index = cifs_ses_get_chan_index(ses, server);
97+
98+
set_bit(chan_index, &ses->chans_need_reconnect);
99+
cifs_dbg(FYI, "Set reconnect bitmask for chan %u; now 0x%lx\n",
100+
chan_index, ses->chans_need_reconnect);
101+
}
102+
103+
void
104+
cifs_chan_clear_need_reconnect(struct cifs_ses *ses,
105+
struct TCP_Server_Info *server)
106+
{
107+
unsigned int chan_index = cifs_ses_get_chan_index(ses, server);
108+
109+
clear_bit(chan_index, &ses->chans_need_reconnect);
110+
cifs_dbg(FYI, "Cleared reconnect bitmask for chan %u; now 0x%lx\n",
111+
chan_index, ses->chans_need_reconnect);
112+
}
113+
114+
bool
115+
cifs_chan_needs_reconnect(struct cifs_ses *ses,
116+
struct TCP_Server_Info *server)
117+
{
118+
unsigned int chan_index = cifs_ses_get_chan_index(ses, server);
119+
120+
return CIFS_CHAN_NEEDS_RECONNECT(ses, chan_index);
121+
}
122+
76123
/* returns number of channels added */
77124
int cifs_try_adding_channels(struct cifs_ses *ses)
78125
{
@@ -321,11 +368,21 @@ cifs_ses_add_channel(struct cifs_ses *ses, struct cifs_server_iface *iface)
321368
spin_lock(&ses->chan_lock);
322369
ses->chan_count++;
323370
atomic_set(&ses->chan_seq, 0);
371+
372+
/* Mark this channel as needing connect/setup */
373+
cifs_chan_set_need_reconnect(ses, chan->server);
324374
spin_unlock(&ses->chan_lock);
325375

326376
out:
327377
ses->binding = false;
328378
ses->binding_chan = NULL;
379+
380+
if (rc && chan->server) {
381+
/* we rely on all bits beyond chan_count to be clear */
382+
cifs_chan_clear_need_reconnect(ses, chan->server);
383+
ses->chan_count--;
384+
}
385+
329386
mutex_unlock(&ses->session_mutex);
330387

331388
if (rc && chan->server)
@@ -955,9 +1012,15 @@ sess_establish_session(struct sess_data *sess_data)
9551012
mutex_unlock(&ses->server->srv_mutex);
9561013

9571014
cifs_dbg(FYI, "CIFS session established successfully\n");
1015+
if (ses->binding)
1016+
cifs_chan_clear_need_reconnect(ses, ses->binding_chan->server);
1017+
else
1018+
cifs_chan_clear_need_reconnect(ses, ses->server);
1019+
1020+
/* keep existing ses state if binding */
9581021
spin_lock(&GlobalMid_Lock);
959-
ses->status = CifsGood;
960-
ses->need_reconnect = false;
1022+
if (!ses->binding)
1023+
ses->status = CifsGood;
9611024
spin_unlock(&GlobalMid_Lock);
9621025

9631026
return 0;

0 commit comments

Comments
 (0)