Skip to content

Commit 53e0e11

Browse files
piastrypiastry
committed
CIFS: Fix a possible memory corruption during reconnect
We can not unlock/lock cifs_tcp_ses_lock while walking through ses and tcon lists because it can corrupt list iterator pointers and a tcon structure can be released if we don't hold an extra reference. Fix it by moving a reconnect process to a separate delayed work and acquiring a reference to every tcon that needs to be reconnected. Also do not send an echo request on newly established connections. CC: Stable <stable@vger.kernel.org> Signed-off-by: Pavel Shilovsky <pshilov@microsoft.com>
1 parent e3d240e commit 53e0e11

File tree

5 files changed

+85
-31
lines changed

5 files changed

+85
-31
lines changed

‎fs/cifs/cifsglob.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -647,6 +647,8 @@ struct TCP_Server_Info {
647647
unsigned int max_read;
648648
unsigned int max_write;
649649
__u8 preauth_hash[512];
650+
struct delayed_work reconnect; /* reconnect workqueue job */
651+
struct mutex reconnect_mutex; /* prevent simultaneous reconnects */
650652
#endif /* CONFIG_CIFS_SMB2 */
651653
unsigned long echo_interval;
652654
};
@@ -850,6 +852,7 @@ cap_unix(struct cifs_ses *ses)
850852
struct cifs_tcon {
851853
struct list_head tcon_list;
852854
int tc_count;
855+
struct list_head rlist; /* reconnect list */
853856
struct list_head openFileList;
854857
spinlock_t open_file_lock; /* protects list above */
855858
struct cifs_ses *ses; /* pointer to session associated with */

‎fs/cifs/cifsproto.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,9 @@ extern void cifs_add_pending_open_locked(struct cifs_fid *fid,
206206
struct tcon_link *tlink,
207207
struct cifs_pending_open *open);
208208
extern void cifs_del_pending_open(struct cifs_pending_open *open);
209+
extern void cifs_put_tcp_session(struct TCP_Server_Info *server,
210+
int from_reconnect);
211+
extern void cifs_put_tcon(struct cifs_tcon *tcon);
209212

210213
#if IS_ENABLED(CONFIG_CIFS_DFS_UPCALL)
211214
extern void cifs_dfs_release_automount_timer(void);

‎fs/cifs/connect.c

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,9 @@
5252
#include "nterr.h"
5353
#include "rfc1002pdu.h"
5454
#include "fscache.h"
55+
#ifdef CONFIG_CIFS_SMB2
56+
#include "smb2proto.h"
57+
#endif
5558

5659
#define CIFS_PORT 445
5760
#define RFC1001_PORT 139
@@ -2110,8 +2113,8 @@ cifs_find_tcp_session(struct smb_vol *vol)
21102113
return NULL;
21112114
}
21122115

2113-
static void
2114-
cifs_put_tcp_session(struct TCP_Server_Info *server)
2116+
void
2117+
cifs_put_tcp_session(struct TCP_Server_Info *server, int from_reconnect)
21152118
{
21162119
struct task_struct *task;
21172120

@@ -2128,6 +2131,19 @@ cifs_put_tcp_session(struct TCP_Server_Info *server)
21282131

21292132
cancel_delayed_work_sync(&server->echo);
21302133

2134+
#ifdef CONFIG_CIFS_SMB2
2135+
if (from_reconnect)
2136+
/*
2137+
* Avoid deadlock here: reconnect work calls
2138+
* cifs_put_tcp_session() at its end. Need to be sure
2139+
* that reconnect work does nothing with server pointer after
2140+
* that step.
2141+
*/
2142+
cancel_delayed_work(&server->reconnect);
2143+
else
2144+
cancel_delayed_work_sync(&server->reconnect);
2145+
#endif
2146+
21312147
spin_lock(&GlobalMid_Lock);
21322148
server->tcpStatus = CifsExiting;
21332149
spin_unlock(&GlobalMid_Lock);
@@ -2192,6 +2208,10 @@ cifs_get_tcp_session(struct smb_vol *volume_info)
21922208
INIT_LIST_HEAD(&tcp_ses->tcp_ses_list);
21932209
INIT_LIST_HEAD(&tcp_ses->smb_ses_list);
21942210
INIT_DELAYED_WORK(&tcp_ses->echo, cifs_echo_request);
2211+
#ifdef CONFIG_CIFS_SMB2
2212+
INIT_DELAYED_WORK(&tcp_ses->reconnect, smb2_reconnect_server);
2213+
mutex_init(&tcp_ses->reconnect_mutex);
2214+
#endif
21952215
memcpy(&tcp_ses->srcaddr, &volume_info->srcaddr,
21962216
sizeof(tcp_ses->srcaddr));
21972217
memcpy(&tcp_ses->dstaddr, &volume_info->dstaddr,
@@ -2350,7 +2370,7 @@ cifs_put_smb_ses(struct cifs_ses *ses)
23502370
spin_unlock(&cifs_tcp_ses_lock);
23512371

23522372
sesInfoFree(ses);
2353-
cifs_put_tcp_session(server);
2373+
cifs_put_tcp_session(server, 0);
23542374
}
23552375

23562376
#ifdef CONFIG_KEYS
@@ -2524,7 +2544,7 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb_vol *volume_info)
25242544
mutex_unlock(&ses->session_mutex);
25252545

25262546
/* existing SMB ses has a server reference already */
2527-
cifs_put_tcp_session(server);
2547+
cifs_put_tcp_session(server, 0);
25282548
free_xid(xid);
25292549
return ses;
25302550
}
@@ -2620,7 +2640,7 @@ cifs_find_tcon(struct cifs_ses *ses, struct smb_vol *volume_info)
26202640
return NULL;
26212641
}
26222642

2623-
static void
2643+
void
26242644
cifs_put_tcon(struct cifs_tcon *tcon)
26252645
{
26262646
unsigned int xid;
@@ -3824,7 +3844,7 @@ cifs_mount(struct cifs_sb_info *cifs_sb, struct smb_vol *volume_info)
38243844
else if (ses)
38253845
cifs_put_smb_ses(ses);
38263846
else
3827-
cifs_put_tcp_session(server);
3847+
cifs_put_tcp_session(server, 0);
38283848
bdi_destroy(&cifs_sb->bdi);
38293849
}
38303850

@@ -4135,7 +4155,7 @@ cifs_construct_tcon(struct cifs_sb_info *cifs_sb, kuid_t fsuid)
41354155
ses = cifs_get_smb_ses(master_tcon->ses->server, vol_info);
41364156
if (IS_ERR(ses)) {
41374157
tcon = (struct cifs_tcon *)ses;
4138-
cifs_put_tcp_session(master_tcon->ses->server);
4158+
cifs_put_tcp_session(master_tcon->ses->server, 0);
41394159
goto out;
41404160
}
41414161

‎fs/cifs/smb2pdu.c

Lines changed: 51 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1972,6 +1972,54 @@ smb2_echo_callback(struct mid_q_entry *mid)
19721972
add_credits(server, credits_received, CIFS_ECHO_OP);
19731973
}
19741974

1975+
void smb2_reconnect_server(struct work_struct *work)
1976+
{
1977+
struct TCP_Server_Info *server = container_of(work,
1978+
struct TCP_Server_Info, reconnect.work);
1979+
struct cifs_ses *ses;
1980+
struct cifs_tcon *tcon, *tcon2;
1981+
struct list_head tmp_list;
1982+
int tcon_exist = false;
1983+
1984+
/* Prevent simultaneous reconnects that can corrupt tcon->rlist list */
1985+
mutex_lock(&server->reconnect_mutex);
1986+
1987+
INIT_LIST_HEAD(&tmp_list);
1988+
cifs_dbg(FYI, "Need negotiate, reconnecting tcons\n");
1989+
1990+
spin_lock(&cifs_tcp_ses_lock);
1991+
list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
1992+
list_for_each_entry(tcon, &ses->tcon_list, tcon_list) {
1993+
if (tcon->need_reconnect) {
1994+
tcon->tc_count++;
1995+
list_add_tail(&tcon->rlist, &tmp_list);
1996+
tcon_exist = true;
1997+
}
1998+
}
1999+
}
2000+
/*
2001+
* Get the reference to server struct to be sure that the last call of
2002+
* cifs_put_tcon() in the loop below won't release the server pointer.
2003+
*/
2004+
if (tcon_exist)
2005+
server->srv_count++;
2006+
2007+
spin_unlock(&cifs_tcp_ses_lock);
2008+
2009+
list_for_each_entry_safe(tcon, tcon2, &tmp_list, rlist) {
2010+
smb2_reconnect(SMB2_ECHO, tcon);
2011+
list_del_init(&tcon->rlist);
2012+
cifs_put_tcon(tcon);
2013+
}
2014+
2015+
cifs_dbg(FYI, "Reconnecting tcons finished\n");
2016+
mutex_unlock(&server->reconnect_mutex);
2017+
2018+
/* now we can safely release srv struct */
2019+
if (tcon_exist)
2020+
cifs_put_tcp_session(server, 1);
2021+
}
2022+
19752023
int
19762024
SMB2_echo(struct TCP_Server_Info *server)
19772025
{
@@ -1984,32 +2032,11 @@ SMB2_echo(struct TCP_Server_Info *server)
19842032
cifs_dbg(FYI, "In echo request\n");
19852033

19862034
if (server->tcpStatus == CifsNeedNegotiate) {
1987-
struct list_head *tmp, *tmp2;
1988-
struct cifs_ses *ses;
1989-
struct cifs_tcon *tcon;
1990-
1991-
cifs_dbg(FYI, "Need negotiate, reconnecting tcons\n");
1992-
spin_lock(&cifs_tcp_ses_lock);
1993-
list_for_each(tmp, &server->smb_ses_list) {
1994-
ses = list_entry(tmp, struct cifs_ses, smb_ses_list);
1995-
list_for_each(tmp2, &ses->tcon_list) {
1996-
tcon = list_entry(tmp2, struct cifs_tcon,
1997-
tcon_list);
1998-
/* add check for persistent handle reconnect */
1999-
if (tcon && tcon->need_reconnect) {
2000-
spin_unlock(&cifs_tcp_ses_lock);
2001-
rc = smb2_reconnect(SMB2_ECHO, tcon);
2002-
spin_lock(&cifs_tcp_ses_lock);
2003-
}
2004-
}
2005-
}
2006-
spin_unlock(&cifs_tcp_ses_lock);
2035+
/* No need to send echo on newly established connections */
2036+
queue_delayed_work(cifsiod_wq, &server->reconnect, 0);
2037+
return rc;
20072038
}
20082039

2009-
/* if no session, renegotiate failed above */
2010-
if (server->tcpStatus == CifsNeedNegotiate)
2011-
return -EIO;
2012-
20132040
rc = small_smb2_init(SMB2_ECHO, NULL, (void **)&req);
20142041
if (rc)
20152042
return rc;

‎fs/cifs/smb2proto.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ extern int smb2_open_file(const unsigned int xid,
9696
extern int smb2_unlock_range(struct cifsFileInfo *cfile,
9797
struct file_lock *flock, const unsigned int xid);
9898
extern int smb2_push_mandatory_locks(struct cifsFileInfo *cfile);
99+
extern void smb2_reconnect_server(struct work_struct *work);
99100

100101
/*
101102
* SMB2 Worker functions - most of protocol specific implementation details

0 commit comments

Comments
 (0)