[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250813013208.1564486-1-wangzhaolong@huaweicloud.com>
Date: Wed, 13 Aug 2025 09:32:08 +0800
From: Wang Zhaolong <wangzhaolong@...weicloud.com>
To: sfrench@...ba.org,
pc@...guebit.org
Cc: linux-cifs@...r.kernel.org,
samba-technical@...ts.samba.org,
linux-kernel@...r.kernel.org,
chengzhihao1@...wei.com,
yi.zhang@...wei.com,
yangerkun@...wei.com
Subject: [PATCH V3] smb: client: Fix mount deadlock by avoiding super block iteration in DFS reconnect
An AA deadlock occurs when network interruption during mount triggers
DFS reconnection logic that calls iterate_supers_type().
The detailed call process is as follows:
mount.cifs
-------------------------
path_mount
do_new_mount
vfs_get_tree
smb3_get_tree
cifs_smb3_do_mount
sget
alloc_super
down_write_nested(&s->s_umount, ..); // Hold lock
cifs_root_iget
cifs_get_inode_info
smb2_query_path_info
smb2_compound_op
SMB2_open_init
smb2_plain_req_init
smb2_reconnect // Trigger reconnection
cifs_tree_connect
cifs_get_dfs_tcon_super
__cifs_get_super
iterate_supers_type
super_lock_shared
super_lock
__super_lock
down_read(&sb->s_umount); // Deadlock
do_new_mount_fc
up_write(&sb->s_umount); // Release lock
During mount phase, if reconnection is triggered, the foreground mount
process may enter smb2_reconnect prior to the reconnect worker being
scheduled, leading to a deadlock when subsequent DFS tree connect
attempts reacquire the s_umount lock.
The essential condition for triggering the issue is that the API
iterate_supers_type() reacquires the s_umount lock. Therefore, one
possible solution is to avoid using iterate_supers_type() and instead
directly access the superblock through internal data structures.
This patch fixes the problem by:
- Add vfs_sb back-pointer to cifs_sb_info for direct access
- Protect list traversal with existing tcon->sb_list_lock
- Use atomic operations to safely manage super block references
- Remove complex callback-based iteration in favor of simple loop
- Rename cifs_put_tcp_super() to cifs_put_super() to avoid confusion
Fixes: 3ae872de4107 ("smb: client: fix shared DFS root mounts with different prefixes")
Signed-off-by: Wang Zhaolong <wangzhaolong@...weicloud.com>
---
V3:
- Adjust the trace diagram for the super_lock_shared() section to align with
the latest mainline call flow.
V2:
- Adjust the trace diagram in the commit message to indicate when the lock
is released
fs/smb/client/cifs_fs_sb.h | 1 +
fs/smb/client/cifsfs.c | 1 +
fs/smb/client/cifsproto.h | 2 +-
fs/smb/client/dfs.c | 2 +-
fs/smb/client/misc.c | 84 ++++++++++++++------------------------
5 files changed, 34 insertions(+), 56 deletions(-)
diff --git a/fs/smb/client/cifs_fs_sb.h b/fs/smb/client/cifs_fs_sb.h
index 5e8d163cb5f8..8c513e4c0efe 100644
--- a/fs/smb/client/cifs_fs_sb.h
+++ b/fs/smb/client/cifs_fs_sb.h
@@ -49,10 +49,11 @@
struct cifs_sb_info {
struct rb_root tlink_tree;
struct list_head tcon_sb_link;
spinlock_t tlink_tree_lock;
+ struct super_block *vfs_sb;
struct tcon_link *master_tlink;
struct nls_table *local_nls;
struct smb3_fs_context *ctx;
atomic_t active;
unsigned int mnt_cifs_flags;
diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c
index 3bd85ab2deb1..383f651eb43f 100644
--- a/fs/smb/client/cifsfs.c
+++ b/fs/smb/client/cifsfs.c
@@ -939,10 +939,11 @@ cifs_get_root(struct smb3_fs_context *ctx, struct super_block *sb)
static int cifs_set_super(struct super_block *sb, void *data)
{
struct cifs_mnt_data *mnt_data = data;
sb->s_fs_info = mnt_data->cifs_sb;
+ mnt_data->cifs_sb->vfs_sb = sb;
return set_anon_super(sb, NULL);
}
struct dentry *
cifs_smb3_do_mount(struct file_system_type *fs_type,
diff --git a/fs/smb/client/cifsproto.h b/fs/smb/client/cifsproto.h
index c34c533b2efa..6415bb961c1e 100644
--- a/fs/smb/client/cifsproto.h
+++ b/fs/smb/client/cifsproto.h
@@ -678,11 +678,11 @@ int copy_path_name(char *dst, const char *src);
int smb2_parse_query_directory(struct cifs_tcon *tcon, struct kvec *rsp_iov,
int resp_buftype,
struct cifs_search_info *srch_inf);
struct super_block *cifs_get_dfs_tcon_super(struct cifs_tcon *tcon);
-void cifs_put_tcp_super(struct super_block *sb);
+void cifs_put_super(struct super_block *sb);
int cifs_update_super_prepath(struct cifs_sb_info *cifs_sb, char *prefix);
char *extract_hostname(const char *unc);
char *extract_sharename(const char *unc);
int parse_reparse_point(struct reparse_data_buffer *buf,
u32 plen, struct cifs_sb_info *cifs_sb,
diff --git a/fs/smb/client/dfs.c b/fs/smb/client/dfs.c
index f65a8a90ba27..55bcdde4fe26 100644
--- a/fs/smb/client/dfs.c
+++ b/fs/smb/client/dfs.c
@@ -446,11 +446,11 @@ int cifs_tree_connect(const unsigned int xid, struct cifs_tcon *tcon)
&tl);
free_dfs_info_param(&ref);
out:
kfree(tree);
- cifs_put_tcp_super(sb);
+ cifs_put_super(sb);
if (rc) {
spin_lock(&tcon->tc_lock);
if (tcon->status == TID_IN_TCON)
tcon->status = TID_NEED_TCON;
diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c
index da23cc12a52c..3b6920a52daa 100644
--- a/fs/smb/client/misc.c
+++ b/fs/smb/client/misc.c
@@ -1108,84 +1108,60 @@ int copy_path_name(char *dst, const char *src)
/* we count the trailing nul */
name_len++;
return name_len;
}
-struct super_cb_data {
- void *data;
- struct super_block *sb;
-};
-
-static void tcon_super_cb(struct super_block *sb, void *arg)
+static struct super_block *cifs_get_tcon_super(struct cifs_tcon *tcon)
{
- struct super_cb_data *sd = arg;
+ struct super_block *sb;
struct cifs_sb_info *cifs_sb;
- struct cifs_tcon *t1 = sd->data, *t2;
- if (sd->sb)
- return;
+ if (!tcon)
+ return NULL;
- cifs_sb = CIFS_SB(sb);
- t2 = cifs_sb_master_tcon(cifs_sb);
-
- spin_lock(&t2->tc_lock);
- if ((t1->ses == t2->ses ||
- t1->ses->dfs_root_ses == t2->ses->dfs_root_ses) &&
- t1->ses->server == t2->ses->server &&
- t2->origin_fullpath &&
- dfs_src_pathname_equal(t2->origin_fullpath, t1->origin_fullpath))
- sd->sb = sb;
- spin_unlock(&t2->tc_lock);
-}
+ spin_lock(&tcon->sb_list_lock);
+ list_for_each_entry(cifs_sb, &tcon->cifs_sb_list, tcon_sb_link) {
-static struct super_block *__cifs_get_super(void (*f)(struct super_block *, void *),
- void *data)
-{
- struct super_cb_data sd = {
- .data = data,
- .sb = NULL,
- };
- struct file_system_type **fs_type = (struct file_system_type *[]) {
- &cifs_fs_type, &smb3_fs_type, NULL,
- };
-
- for (; *fs_type; fs_type++) {
- iterate_supers_type(*fs_type, f, &sd);
- if (sd.sb) {
- /*
- * Grab an active reference in order to prevent automounts (DFS links)
- * of expiring and then freeing up our cifs superblock pointer while
- * we're doing failover.
- */
- cifs_sb_active(sd.sb);
- return sd.sb;
- }
+ if (!cifs_sb->vfs_sb)
+ continue;
+
+ sb = cifs_sb->vfs_sb;
+
+ /* Safely increment s_active only if it's not zero.
+ *
+ * When s_active == 0, the super block is being deactivated
+ * and should not be used. This prevents UAF scenarios
+ * where we might grab a reference to a super block that's
+ * in the middle of destruction.
+ */
+ if (!atomic_add_unless(&sb->s_active, 1, 0))
+ continue;
+
+ spin_unlock(&tcon->sb_list_lock);
+ return sb;
}
- pr_warn_once("%s: could not find dfs superblock\n", __func__);
- return ERR_PTR(-EINVAL);
-}
+ spin_unlock(&tcon->sb_list_lock);
-static void __cifs_put_super(struct super_block *sb)
-{
- if (!IS_ERR_OR_NULL(sb))
- cifs_sb_deactive(sb);
+ return NULL;
}
struct super_block *cifs_get_dfs_tcon_super(struct cifs_tcon *tcon)
{
spin_lock(&tcon->tc_lock);
if (!tcon->origin_fullpath) {
spin_unlock(&tcon->tc_lock);
return ERR_PTR(-ENOENT);
}
spin_unlock(&tcon->tc_lock);
- return __cifs_get_super(tcon_super_cb, tcon);
+
+ return cifs_get_tcon_super(tcon);
}
-void cifs_put_tcp_super(struct super_block *sb)
+void cifs_put_super(struct super_block *sb)
{
- __cifs_put_super(sb);
+ if (!IS_ERR_OR_NULL(sb))
+ deactivate_super(sb);
}
#ifdef CONFIG_CIFS_DFS_UPCALL
int match_target_ip(struct TCP_Server_Info *server,
const char *host, size_t hostlen,
--
2.39.2
Powered by blists - more mailing lists