Commit 8da33fd1 authored by Shyam Prasad N's avatar Shyam Prasad N Committed by Steve French

cifs: avoid deadlocks while updating iface

We use cifs_tcp_ses_lock to protect a lot of things.
Not only does it protect the lists of connections, sessions,
tree connects, open file lists, etc., we also use it to
protect some fields in each of it's entries.

In this case, cifs_mark_ses_for_reconnect takes the
cifs_tcp_ses_lock to traverse the lists, and then calls
cifs_update_iface. However, that can end up calling
cifs_put_tcp_session, which picks up the same lock again.

Avoid this by taking a ref for the session, drop the lock,
and then call update iface.

Also, in cifs_update_iface, avoid nested locking of iface_lock
and chan_lock, as much as possible. When unavoidable, we need
to pick iface_lock first.
Signed-off-by: default avatarShyam Prasad N <sprasad@microsoft.com>
Signed-off-by: default avatarSteve French <stfrench@microsoft.com>
parent 6e1c1c08
...@@ -236,7 +236,7 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server, ...@@ -236,7 +236,7 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
bool mark_smb_session) bool mark_smb_session)
{ {
struct TCP_Server_Info *pserver; struct TCP_Server_Info *pserver;
struct cifs_ses *ses; struct cifs_ses *ses, *nses;
struct cifs_tcon *tcon; struct cifs_tcon *tcon;
/* /*
...@@ -250,10 +250,19 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server, ...@@ -250,10 +250,19 @@ cifs_mark_tcp_ses_conns_for_reconnect(struct TCP_Server_Info *server,
spin_lock(&cifs_tcp_ses_lock); spin_lock(&cifs_tcp_ses_lock);
list_for_each_entry(ses, &pserver->smb_ses_list, smb_ses_list) { list_for_each_entry_safe(ses, nses, &pserver->smb_ses_list, smb_ses_list) {
/* check if iface is still active */ /* check if iface is still active */
if (!cifs_chan_is_iface_active(ses, server)) if (!cifs_chan_is_iface_active(ses, server)) {
/*
* HACK: drop the lock before calling
* cifs_chan_update_iface to avoid deadlock
*/
ses->ses_count++;
spin_unlock(&cifs_tcp_ses_lock);
cifs_chan_update_iface(ses, server); cifs_chan_update_iface(ses, server);
spin_lock(&cifs_tcp_ses_lock);
ses->ses_count--;
}
spin_lock(&ses->chan_lock); spin_lock(&ses->chan_lock);
if (!mark_smb_session && cifs_chan_needs_reconnect(ses, server)) if (!mark_smb_session && cifs_chan_needs_reconnect(ses, server))
......
...@@ -256,29 +256,34 @@ int cifs_try_adding_channels(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses) ...@@ -256,29 +256,34 @@ int cifs_try_adding_channels(struct cifs_sb_info *cifs_sb, struct cifs_ses *ses)
/* /*
* update the iface for the channel if necessary. * update the iface for the channel if necessary.
* will return 0 when iface is updated. 1 otherwise * will return 0 when iface is updated, 1 if removed, 2 otherwise
* Must be called with chan_lock held. * Must be called with chan_lock held.
*/ */
int int
cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server) cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
{ {
unsigned int chan_index = cifs_ses_get_chan_index(ses, server); unsigned int chan_index;
struct cifs_server_iface *iface = NULL; struct cifs_server_iface *iface = NULL;
struct cifs_server_iface *old_iface = NULL; struct cifs_server_iface *old_iface = NULL;
int rc = 0; int rc = 0;
/* primary channel. This can never go away */ spin_lock(&ses->chan_lock);
if (!chan_index) chan_index = cifs_ses_get_chan_index(ses, server);
if (!chan_index) {
spin_unlock(&ses->chan_lock);
return 0; return 0;
}
if (ses->chans[chan_index].iface) { if (ses->chans[chan_index].iface) {
old_iface = ses->chans[chan_index].iface; old_iface = ses->chans[chan_index].iface;
if (old_iface->is_active) if (old_iface->is_active) {
spin_unlock(&ses->chan_lock);
return 1; return 1;
}
} }
spin_unlock(&ses->chan_lock);
spin_lock(&ses->iface_lock); spin_lock(&ses->iface_lock);
/* then look for a new one */ /* then look for a new one */
list_for_each_entry(iface, &ses->iface_list, iface_head) { list_for_each_entry(iface, &ses->iface_list, iface_head) {
if (!iface->is_active || if (!iface->is_active ||
...@@ -295,8 +300,6 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server) ...@@ -295,8 +300,6 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
cifs_dbg(FYI, "unable to find a suitable iface\n"); cifs_dbg(FYI, "unable to find a suitable iface\n");
} }
ses->chans[chan_index].iface = iface;
/* now drop the ref to the current iface */ /* now drop the ref to the current iface */
if (old_iface && iface) { if (old_iface && iface) {
kref_put(&old_iface->refcount, release_iface); kref_put(&old_iface->refcount, release_iface);
...@@ -311,14 +314,20 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server) ...@@ -311,14 +314,20 @@ cifs_chan_update_iface(struct cifs_ses *ses, struct TCP_Server_Info *server)
WARN_ON(!iface); WARN_ON(!iface);
cifs_dbg(FYI, "adding new iface: %pIS\n", &iface->sockaddr); cifs_dbg(FYI, "adding new iface: %pIS\n", &iface->sockaddr);
} }
spin_unlock(&ses->iface_lock); spin_unlock(&ses->iface_lock);
spin_lock(&ses->chan_lock);
chan_index = cifs_ses_get_chan_index(ses, server);
ses->chans[chan_index].iface = iface;
/* No iface is found. if secondary chan, drop connection */ /* No iface is found. if secondary chan, drop connection */
if (!iface && CIFS_SERVER_IS_CHAN(server)) { if (!iface && CIFS_SERVER_IS_CHAN(server))
cifs_put_tcp_session(server, false);
ses->chans[chan_index].server = NULL; ses->chans[chan_index].server = NULL;
}
spin_unlock(&ses->chan_lock);
if (!iface && CIFS_SERVER_IS_CHAN(server))
cifs_put_tcp_session(server, false);
return rc; return rc;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment