From: "Paulo Alcantara (SUSE)" <pc@cjr.nz>
To: smfrench@gmail.com, aaptel@suse.com
Cc: linux-cifs@vger.kernel.org, "Paulo Alcantara (SUSE)" <pc@cjr.nz>
Subject: [PATCH v2 5/7] cifs: Fix potential deadlock when updating vol in cifs_reconnect()
Date: Mon, 25 Nov 2019 13:57:56 -0300 [thread overview]
Message-ID: <20191125165758.3793-6-pc@cjr.nz> (raw)
In-Reply-To: <20191125165758.3793-1-pc@cjr.nz>
We can't hold the vol_lock spinlock while refreshing the DFS cache
because cifs_reconnect() may call dfs_cache_update_vol() while we are
walking through the volume list.
To prevent that, make vol_info refcounted, create a temp list with all
volumes eligible for refreshing, and then use it without any locks
held.
Signed-off-by: Paulo Alcantara (SUSE) <pc@cjr.nz>
---
v1 -> v2:
- mention that vol_info is now refcounted in commit msg
- document put_vol() to say that it must be called with vol_lock held
---
fs/cifs/dfs_cache.c | 46 +++++++++++++++++++++++++++++++++------------
1 file changed, 34 insertions(+), 12 deletions(-)
diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c
index b790f37c0060..263d42d46acc 100644
--- a/fs/cifs/dfs_cache.c
+++ b/fs/cifs/dfs_cache.c
@@ -49,6 +49,8 @@ struct vol_info {
struct smb_vol smb_vol;
char *mntdata;
struct list_head list;
+ struct list_head rlist;
+ int vol_count;
};
static struct kmem_cache *cache_slab __read_mostly;
@@ -516,13 +518,16 @@ static struct cache_entry *lookup_cache_entry(const char *path,
return ce;
}
-static inline void free_vol(struct vol_info *vi)
+/* Must be called with vol_lock held */
+static void put_vol(struct vol_info *vi)
{
- list_del(&vi->list);
- kfree(vi->fullpath);
- kfree(vi->mntdata);
- cifs_cleanup_volume_info_contents(&vi->smb_vol);
- kfree(vi);
+ if (!--vi->vol_count) {
+ list_del_init(&vi->list);
+ kfree(vi->fullpath);
+ kfree(vi->mntdata);
+ cifs_cleanup_volume_info_contents(&vi->smb_vol);
+ kfree(vi);
+ }
}
static inline void free_vol_list(void)
@@ -530,7 +535,7 @@ static inline void free_vol_list(void)
struct vol_info *vi, *nvi;
list_for_each_entry_safe(vi, nvi, &vol_list, list)
- free_vol(vi);
+ put_vol(vi);
}
/**
@@ -1150,6 +1155,7 @@ int dfs_cache_add_vol(char *mntdata, struct smb_vol *vol, const char *fullpath)
goto err_free_fullpath;
vi->mntdata = mntdata;
+ vi->vol_count++;
spin_lock(&vol_lock);
list_add_tail(&vi->list, &vol_list);
@@ -1229,11 +1235,9 @@ void dfs_cache_del_vol(const char *fullpath)
cifs_dbg(FYI, "%s: fullpath: %s\n", __func__, fullpath);
spin_lock(&vol_lock);
-
vi = find_vol(fullpath);
if (!IS_ERR(vi))
- free_vol(vi);
-
+ put_vol(vi);
spin_unlock(&vol_lock);
}
@@ -1456,18 +1460,31 @@ static int refresh_tcon(struct vol_info *vi, struct cifs_tcon *tcon)
*/
static void refresh_cache_worker(struct work_struct *work)
{
- struct vol_info *vi;
+ struct vol_info *vi, *nvi;
struct TCP_Server_Info *server;
+ LIST_HEAD(vols);
LIST_HEAD(tcons);
struct cifs_tcon *tcon, *ntcon;
int rc;
+ /* Get SMB volumes that are eligible (CifsGood) for refreshing */
spin_lock(&vol_lock);
list_for_each_entry(vi, &vol_list, list) {
server = get_tcp_server(&vi->smb_vol);
if (!server)
continue;
+ vi->vol_count++;
+ list_add_tail(&vi->rlist, &vols);
+ put_tcp_server(server);
+ }
+ spin_unlock(&vol_lock);
+
+ list_for_each_entry_safe(vi, nvi, &vols, rlist) {
+ server = get_tcp_server(&vi->smb_vol);
+ if (!server)
+ goto next_vol;
+
get_tcons(server, &tcons);
rc = 0;
@@ -1484,8 +1501,13 @@ static void refresh_cache_worker(struct work_struct *work)
}
put_tcp_server(server);
+
+next_vol:
+ list_del_init(&vi->rlist);
+ spin_lock(&vol_lock);
+ put_vol(vi);
+ spin_unlock(&vol_lock);
}
- spin_unlock(&vol_lock);
queue_delayed_work(dfscache_wq, &refresh_task,
atomic_read(&cache_ttl) * HZ);
--
2.24.0
next prev parent reply other threads:[~2019-11-25 16:58 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-11-25 16:57 [PATCH v2 0/7] DFS fixes Paulo Alcantara (SUSE)
2019-11-25 16:57 ` [PATCH v2 1/7] cifs: Fix use-after-free bug in cifs_reconnect() Paulo Alcantara (SUSE)
2019-11-25 16:57 ` [PATCH v2 2/7] cifs: Fix lookup of root ses in DFS referral cache Paulo Alcantara (SUSE)
2019-11-25 16:57 ` [PATCH v2 3/7] cifs: Fix potential softlockups while refreshing DFS cache Paulo Alcantara (SUSE)
2019-11-25 16:57 ` [PATCH v2 4/7] cifs: Clean up DFS referral cache Paulo Alcantara (SUSE)
2019-11-25 21:06 ` Pavel Shilovsky
2019-11-25 21:18 ` Paulo Alcantara
2019-11-25 16:57 ` Paulo Alcantara (SUSE) [this message]
2019-11-25 16:57 ` [PATCH v2 6/7] cifs: Fix retrieval of DFS referrals in cifs_mount() Paulo Alcantara (SUSE)
2019-11-25 16:57 ` [PATCH v2 7/7] cifs: Always update signing key of first channel Paulo Alcantara (SUSE)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20191125165758.3793-6-pc@cjr.nz \
--to=pc@cjr.nz \
--cc=aaptel@suse.com \
--cc=linux-cifs@vger.kernel.org \
--cc=smfrench@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).