cifs: protect access of TCP_Server_Info::{origin,leaf}_fullpath

Protect access of TCP_Server_Info::{origin,leaf}_fullpath when
matching DFS connections, and get rid of
TCP_Server_Info::current_fullpath while we're at it.

Cc: stable@vger.kernel.org # v6.2+
Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
Signed-off-by: Steve French <stfrench@microsoft.com>

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index 08a73dcb778641bc4d313d34632bf06d9d3ed621..a624474048519dffc76e0edcd93af3290ce0757e 100644 (file)
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -736,17 +736,23 @@ struct TCP_Server_Info {
 #endif
        struct mutex refpath_lock; /* protects leaf_fullpath */
        /*
-        * Canonical DFS full paths that were used to chase referrals in mount and reconnect.
+        * origin_fullpath: Canonical copy of smb3_fs_context::source.
+        *                  It is used for matching existing DFS tcons.
         *
-        * origin_fullpath: first or original referral path
-        * leaf_fullpath: last referral path (might be changed due to nested links in reconnect)
+        * leaf_fullpath: Canonical DFS referral path related to this
+        *                connection.
+        *                It is used in DFS cache refresher, reconnect and may
+        *                change due to nested DFS links.
         *
-        * current_fullpath: pointer to either origin_fullpath or leaf_fullpath
-        * NOTE: cannot be accessed outside cifs_reconnect() and smb2_reconnect()
+        * Both protected by @refpath_lock and @srv_lock.  The @refpath_lock is
+        * mosly used for not requiring a copy of @leaf_fullpath when getting
+        * cached or new DFS referrals (which might also sleep during I/O).
+        * While @srv_lock is held for making string and NULL comparions against
+        * both fields as in mount(2) and cache refresh.
         *
-        * format: \\HOST\SHARE\[OPTIONAL PATH]
+        * format: \\HOST\SHARE[\OPTIONAL PATH]
         */
-       char *origin_fullpath, *leaf_fullpath, *current_fullpath;
+       char *origin_fullpath, *leaf_fullpath;
 };
 
 static inline bool is_smb1(struct TCP_Server_Info *server)

diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index c71505a29482c885e12af1cc54ab5191c0e3bd97..9451b1023af8be13bcc78dbc2316c5e0f7cf44d2 100644 (file)
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -454,7 +454,6 @@ static int reconnect_target_unlocked(struct TCP_Server_Info *server, struct dfs_
 static int reconnect_dfs_server(struct TCP_Server_Info *server)
 {
        int rc = 0;
-       const char *refpath = server->current_fullpath + 1;
        struct dfs_cache_tgt_list tl = DFS_CACHE_TGT_LIST_INIT(tl);
        struct dfs_cache_tgt_iterator *target_hint = NULL;
        int num_targets = 0;
@@ -467,8 +466,10 @@ static int reconnect_dfs_server(struct TCP_Server_Info *server)
         * through /proc/fs/cifs/dfscache or the target list is empty due to server settings after
         * refreshing the referral, so, in this case, default it to 1.
         */
-       if (!dfs_cache_noreq_find(refpath, NULL, &tl))
+       mutex_lock(&server->refpath_lock);
+       if (!dfs_cache_noreq_find(server->leaf_fullpath + 1, NULL, &tl))
                num_targets = dfs_cache_get_nr_tgts(&tl);
+       mutex_unlock(&server->refpath_lock);
        if (!num_targets)
                num_targets = 1;
 
@@ -512,7 +513,9 @@ static int reconnect_dfs_server(struct TCP_Server_Info *server)
                mod_delayed_work(cifsiod_wq, &server->reconnect, 0);
        } while (server->tcpStatus == CifsNeedReconnect);
 
-       dfs_cache_noreq_update_tgthint(refpath, target_hint);
+       mutex_lock(&server->refpath_lock);
+       dfs_cache_noreq_update_tgthint(server->leaf_fullpath + 1, target_hint);
+       mutex_unlock(&server->refpath_lock);
        dfs_cache_free_tgts(&tl);
 
        /* Need to set up echo worker again once connection has been established */
@@ -1582,7 +1585,6 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx,
                        rc = -ENOMEM;
                        goto out_err;
                }
-               tcp_ses->current_fullpath = tcp_ses->leaf_fullpath;
        }
 
        if (ctx->nosharesock)

diff --git a/fs/cifs/dfs.c b/fs/cifs/dfs.c
index 37f7da4f5c8b0f2f10e31a5ad3246ee9489e5cd2..c4ec5c67087b41567a20251b419d9467a9d51d85 100644 (file)
--- a/fs/cifs/dfs.c
+++ b/fs/cifs/dfs.c
@@ -248,11 +248,12 @@ static int __dfs_mount_share(struct cifs_mount_ctx *mnt_ctx)
                tcon = mnt_ctx->tcon;
 
                mutex_lock(&server->refpath_lock);
+               spin_lock(&server->srv_lock);
                if (!server->origin_fullpath) {
                        server->origin_fullpath = origin_fullpath;
-                       server->current_fullpath = server->leaf_fullpath;
                        origin_fullpath = NULL;
                }
+               spin_unlock(&server->srv_lock);
                mutex_unlock(&server->refpath_lock);
 
                if (list_empty(&tcon->dfs_ses_list)) {
@@ -342,10 +343,11 @@ static int update_server_fullpath(struct TCP_Server_Info *server, struct cifs_sb
                rc = PTR_ERR(npath);
        } else {
                mutex_lock(&server->refpath_lock);
+               spin_lock(&server->srv_lock);
                kfree(server->leaf_fullpath);
                server->leaf_fullpath = npath;
+               spin_unlock(&server->srv_lock);
                mutex_unlock(&server->refpath_lock);
-               server->current_fullpath = server->leaf_fullpath;
        }
        return rc;
 }
@@ -450,7 +452,7 @@ static int __tree_connect_dfs_target(const unsigned int xid, struct cifs_tcon *t
                share = prefix = NULL;
 
                /* Check if share matches with tcp ses */
-               rc = dfs_cache_get_tgt_share(server->current_fullpath + 1, tit, &share, &prefix);
+               rc = dfs_cache_get_tgt_share(server->leaf_fullpath + 1, tit, &share, &prefix);
                if (rc) {
                        cifs_dbg(VFS, "%s: failed to parse target share: %d\n", __func__, rc);
                        break;
@@ -464,7 +466,7 @@ static int __tree_connect_dfs_target(const unsigned int xid, struct cifs_tcon *t
                        continue;
                }
 
-               dfs_cache_noreq_update_tgthint(server->current_fullpath + 1, tit);
+               dfs_cache_noreq_update_tgthint(server->leaf_fullpath + 1, tit);
                tree_connect_ipc(xid, tree, cifs_sb, tcon);
 
                scnprintf(tree, MAX_TREE_SIZE, "\\%s", share);
@@ -582,8 +584,8 @@ int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon, const stru
        cifs_sb = CIFS_SB(sb);
 
        /* If it is not dfs or there was no cached dfs referral, then reconnect to same share */
-       if (!server->current_fullpath ||
-           dfs_cache_noreq_find(server->current_fullpath + 1, &ref, &tl)) {
+       if (!server->leaf_fullpath ||
+           dfs_cache_noreq_find(server->leaf_fullpath + 1, &ref, &tl)) {
                rc = ops->tree_connect(xid, tcon->ses, tcon->tree_name, tcon, cifs_sb->local_nls);
                goto out;
        }

diff --git a/fs/cifs/dfs.h b/fs/cifs/dfs.h
index 0b8cbf721fff6ed6e9aac7c4f01239f039acefdf..1c90df5ecfbdae089b2964276a6cf6d518768479 100644 (file)
--- a/fs/cifs/dfs.h
+++ b/fs/cifs/dfs.h
@@ -43,8 +43,12 @@ static inline char *dfs_get_automount_devname(struct dentry *dentry, void *page)
        size_t len;
        char *s;
 
-       if (unlikely(!server->origin_fullpath))
+       spin_lock(&server->srv_lock);
+       if (unlikely(!server->origin_fullpath)) {
+               spin_unlock(&server->srv_lock);
                return ERR_PTR(-EREMOTE);
+       }
+       spin_unlock(&server->srv_lock);
 
        s = dentry_path_raw(dentry, page, PATH_MAX);
        if (IS_ERR(s))
@@ -53,13 +57,18 @@ static inline char *dfs_get_automount_devname(struct dentry *dentry, void *page)
        if (!s[1])
                s++;
 
+       spin_lock(&server->srv_lock);
        len = strlen(server->origin_fullpath);
-       if (s < (char *)page + len)
+       if (s < (char *)page + len) {
+               spin_unlock(&server->srv_lock);
                return ERR_PTR(-ENAMETOOLONG);
+       }
 
        s -= len;
        memcpy(s, server->origin_fullpath, len);
+       spin_unlock(&server->srv_lock);
        convert_delimiter(s, '/');
+
        return s;
 }
 

diff --git a/fs/cifs/dfs_cache.c b/fs/cifs/dfs_cache.c
index 30cbdf8514a5969c8a2b862277b0544405d2a151..6557d7b2798a034397e2dfb5f794e7ae14a52dd2 100644 (file)
--- a/fs/cifs/dfs_cache.c
+++ b/fs/cifs/dfs_cache.c
@@ -1278,8 +1278,12 @@ static void refresh_cache_worker(struct work_struct *work)
 
        spin_lock(&cifs_tcp_ses_lock);
        list_for_each_entry(server, &cifs_tcp_ses_list, tcp_ses_list) {
-               if (!server->leaf_fullpath)
+               spin_lock(&server->srv_lock);
+               if (!server->leaf_fullpath) {
+                       spin_unlock(&server->srv_lock);
                        continue;
+               }
+               spin_unlock(&server->srv_lock);
 
                list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) {
                        if (ses->tcon_ipc) {