Skip to content

Commit

Permalink
smb: client: guarantee refcounted children from parent session
Browse files Browse the repository at this point in the history
commit 062a7f0 upstream.

Avoid potential use-after-free bugs when walking DFS referrals,
mounting and performing DFS failover by ensuring that all children
from parent @tcon->ses are also refcounted.  They're all needed across
the entire DFS mount.  Get rid of @tcon->dfs_ses_list while we're at
it, too.

Cc: stable@vger.kernel.org # 6.4+
Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202404021527.ZlRkIxgv-lkp@intel.com/
Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
pcacjr authored and gregkh committed Apr 10, 2024
1 parent 45f2bed commit e1db9ae
Show file tree
Hide file tree
Showing 7 changed files with 76 additions and 72 deletions.
2 changes: 0 additions & 2 deletions fs/smb/client/cifsglob.h
Expand Up @@ -1267,7 +1267,6 @@ struct cifs_tcon {
struct cached_fids *cfids;
/* BB add field for back pointer to sb struct(s)? */
#ifdef CONFIG_CIFS_DFS_UPCALL
struct list_head dfs_ses_list;
struct delayed_work dfs_cache_work;
#endif
struct delayed_work query_interfaces; /* query interfaces workqueue job */
Expand Down Expand Up @@ -1788,7 +1787,6 @@ struct cifs_mount_ctx {
struct TCP_Server_Info *server;
struct cifs_ses *ses;
struct cifs_tcon *tcon;
struct list_head dfs_ses_list;
};

static inline void __free_dfs_info_param(struct dfs_info3_param *param)
Expand Down
20 changes: 10 additions & 10 deletions fs/smb/client/cifsproto.h
Expand Up @@ -723,31 +723,31 @@ struct super_block *cifs_get_tcon_super(struct cifs_tcon *tcon);
void cifs_put_tcon_super(struct super_block *sb);
int cifs_wait_for_server_reconnect(struct TCP_Server_Info *server, bool retry);

/* Put references of @ses and @ses->dfs_root_ses */
/* Put references of @ses and its children */
static inline void cifs_put_smb_ses(struct cifs_ses *ses)
{
struct cifs_ses *rses = ses->dfs_root_ses;
struct cifs_ses *next;

__cifs_put_smb_ses(ses);
if (rses)
__cifs_put_smb_ses(rses);
do {
next = ses->dfs_root_ses;
__cifs_put_smb_ses(ses);
} while ((ses = next));
}

/* Get an active reference of @ses and @ses->dfs_root_ses.
/* Get an active reference of @ses and its children.
*
* NOTE: make sure to call this function when incrementing reference count of
* @ses to ensure that any DFS root session attached to it (@ses->dfs_root_ses)
* will also get its reference count incremented.
*
* cifs_put_smb_ses() will put both references, so call it when you're done.
* cifs_put_smb_ses() will put all references, so call it when you're done.
*/
static inline void cifs_smb_ses_inc_refcount(struct cifs_ses *ses)
{
lockdep_assert_held(&cifs_tcp_ses_lock);

ses->ses_count++;
if (ses->dfs_root_ses)
ses->dfs_root_ses->ses_count++;
for (; ses; ses = ses->dfs_root_ses)
ses->ses_count++;
}

static inline bool dfs_src_pathname_equal(const char *s1, const char *s2)
Expand Down
25 changes: 20 additions & 5 deletions fs/smb/client/connect.c
Expand Up @@ -1869,6 +1869,9 @@ static int match_session(struct cifs_ses *ses, struct smb3_fs_context *ctx)
ctx->sectype != ses->sectype)
return 0;

if (ctx->dfs_root_ses != ses->dfs_root_ses)
return 0;

/*
* If an existing session is limited to less channels than
* requested, it should not be reused
Expand Down Expand Up @@ -2361,9 +2364,9 @@ cifs_get_smb_ses(struct TCP_Server_Info *server, struct smb3_fs_context *ctx)
* need to lock before changing something in the session.
*/
spin_lock(&cifs_tcp_ses_lock);
if (ctx->dfs_root_ses)
cifs_smb_ses_inc_refcount(ctx->dfs_root_ses);
ses->dfs_root_ses = ctx->dfs_root_ses;
if (ses->dfs_root_ses)
ses->dfs_root_ses->ses_count++;
list_add(&ses->smb_ses_list, &server->smb_ses_list);
spin_unlock(&cifs_tcp_ses_lock);

Expand Down Expand Up @@ -3312,6 +3315,9 @@ void cifs_mount_put_conns(struct cifs_mount_ctx *mnt_ctx)
cifs_put_smb_ses(mnt_ctx->ses);
else if (mnt_ctx->server)
cifs_put_tcp_session(mnt_ctx->server, 0);
mnt_ctx->ses = NULL;
mnt_ctx->tcon = NULL;
mnt_ctx->server = NULL;
mnt_ctx->cifs_sb->mnt_cifs_flags &= ~CIFS_MOUNT_POSIX_PATHS;
free_xid(mnt_ctx->xid);
}
Expand Down Expand Up @@ -3590,8 +3596,6 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx)
bool isdfs;
int rc;

INIT_LIST_HEAD(&mnt_ctx.dfs_ses_list);

rc = dfs_mount_share(&mnt_ctx, &isdfs);
if (rc)
goto error;
Expand Down Expand Up @@ -3622,7 +3626,6 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx)
return rc;

error:
dfs_put_root_smb_sessions(&mnt_ctx.dfs_ses_list);
cifs_mount_put_conns(&mnt_ctx);
return rc;
}
Expand All @@ -3637,6 +3640,18 @@ int cifs_mount(struct cifs_sb_info *cifs_sb, struct smb3_fs_context *ctx)
goto error;

rc = cifs_mount_get_tcon(&mnt_ctx);
if (!rc) {
/*
* Prevent superblock from being created with any missing
* connections.
*/
if (WARN_ON(!mnt_ctx.server))
rc = -EHOSTDOWN;
else if (WARN_ON(!mnt_ctx.ses))
rc = -EACCES;
else if (WARN_ON(!mnt_ctx.tcon))
rc = -ENOENT;
}
if (rc)
goto error;

Expand Down
51 changes: 24 additions & 27 deletions fs/smb/client/dfs.c
Expand Up @@ -66,33 +66,20 @@ static int get_session(struct cifs_mount_ctx *mnt_ctx, const char *full_path)
}

/*
* Track individual DFS referral servers used by new DFS mount.
*
* On success, their lifetime will be shared by final tcon (dfs_ses_list).
* Otherwise, they will be put by dfs_put_root_smb_sessions() in cifs_mount().
* Get an active reference of @ses so that next call to cifs_put_tcon() won't
* release it as any new DFS referrals must go through its IPC tcon.
*/
static int add_root_smb_session(struct cifs_mount_ctx *mnt_ctx)
static void add_root_smb_session(struct cifs_mount_ctx *mnt_ctx)
{
struct smb3_fs_context *ctx = mnt_ctx->fs_ctx;
struct dfs_root_ses *root_ses;
struct cifs_ses *ses = mnt_ctx->ses;

if (ses) {
root_ses = kmalloc(sizeof(*root_ses), GFP_KERNEL);
if (!root_ses)
return -ENOMEM;

INIT_LIST_HEAD(&root_ses->list);

spin_lock(&cifs_tcp_ses_lock);
cifs_smb_ses_inc_refcount(ses);
spin_unlock(&cifs_tcp_ses_lock);
root_ses->ses = ses;
list_add_tail(&root_ses->list, &mnt_ctx->dfs_ses_list);
}
/* Select new DFS referral server so that new referrals go through it */
ctx->dfs_root_ses = ses;
return 0;
}

static inline int parse_dfs_target(struct smb3_fs_context *ctx,
Expand Down Expand Up @@ -185,11 +172,8 @@ static int __dfs_referral_walk(struct cifs_mount_ctx *mnt_ctx,
continue;
}

if (is_refsrv) {
rc = add_root_smb_session(mnt_ctx);
if (rc)
goto out;
}
if (is_refsrv)
add_root_smb_session(mnt_ctx);

rc = ref_walk_advance(rw);
if (!rc) {
Expand Down Expand Up @@ -232,13 +216,26 @@ static int __dfs_mount_share(struct cifs_mount_ctx *mnt_ctx)
struct smb3_fs_context *ctx = mnt_ctx->fs_ctx;
struct cifs_tcon *tcon;
char *origin_fullpath;
bool new_tcon = true;
int rc;

origin_fullpath = dfs_get_path(cifs_sb, ctx->source);
if (IS_ERR(origin_fullpath))
return PTR_ERR(origin_fullpath);

rc = dfs_referral_walk(mnt_ctx);
if (!rc) {
/*
* Prevent superblock from being created with any missing
* connections.
*/
if (WARN_ON(!mnt_ctx->server))
rc = -EHOSTDOWN;
else if (WARN_ON(!mnt_ctx->ses))
rc = -EACCES;
else if (WARN_ON(!mnt_ctx->tcon))
rc = -ENOENT;
}
if (rc)
goto out;

Expand All @@ -247,15 +244,14 @@ static int __dfs_mount_share(struct cifs_mount_ctx *mnt_ctx)
if (!tcon->origin_fullpath) {
tcon->origin_fullpath = origin_fullpath;
origin_fullpath = NULL;
} else {
new_tcon = false;
}
spin_unlock(&tcon->tc_lock);

if (list_empty(&tcon->dfs_ses_list)) {
list_replace_init(&mnt_ctx->dfs_ses_list, &tcon->dfs_ses_list);
if (new_tcon) {
queue_delayed_work(dfscache_wq, &tcon->dfs_cache_work,
dfs_cache_get_ttl() * HZ);
} else {
dfs_put_root_smb_sessions(&mnt_ctx->dfs_ses_list);
}

out:
Expand Down Expand Up @@ -298,7 +294,6 @@ int dfs_mount_share(struct cifs_mount_ctx *mnt_ctx, bool *isdfs)
if (rc)
return rc;

ctx->dfs_root_ses = mnt_ctx->ses;
/*
* If called with 'nodfs' mount option, then skip DFS resolving. Otherwise unconditionally
* try to get an DFS referral (even cached) to determine whether it is an DFS mount.
Expand All @@ -324,7 +319,9 @@ int dfs_mount_share(struct cifs_mount_ctx *mnt_ctx, bool *isdfs)

*isdfs = true;
add_root_smb_session(mnt_ctx);
return __dfs_mount_share(mnt_ctx);
rc = __dfs_mount_share(mnt_ctx);
dfs_put_root_smb_sessions(mnt_ctx);
return rc;
}

/* Update dfs referral path of superblock */
Expand Down
33 changes: 21 additions & 12 deletions fs/smb/client/dfs.h
Expand Up @@ -7,7 +7,9 @@
#define _CIFS_DFS_H

#include "cifsglob.h"
#include "cifsproto.h"
#include "fs_context.h"
#include "dfs_cache.h"
#include "cifs_unicode.h"
#include <linux/namei.h>

Expand Down Expand Up @@ -114,11 +116,6 @@ static inline void ref_walk_set_tgt_hint(struct dfs_ref_walk *rw)
ref_walk_tit(rw));
}

struct dfs_root_ses {
struct list_head list;
struct cifs_ses *ses;
};

int dfs_parse_target_referral(const char *full_path, const struct dfs_info3_param *ref,
struct smb3_fs_context *ctx);
int dfs_mount_share(struct cifs_mount_ctx *mnt_ctx, bool *isdfs);
Expand All @@ -133,20 +130,32 @@ static inline int dfs_get_referral(struct cifs_mount_ctx *mnt_ctx, const char *p
{
struct smb3_fs_context *ctx = mnt_ctx->fs_ctx;
struct cifs_sb_info *cifs_sb = mnt_ctx->cifs_sb;
struct cifs_ses *rses = ctx->dfs_root_ses ?: mnt_ctx->ses;

return dfs_cache_find(mnt_ctx->xid, ctx->dfs_root_ses, cifs_sb->local_nls,
return dfs_cache_find(mnt_ctx->xid, rses, cifs_sb->local_nls,
cifs_remap(cifs_sb), path, ref, tl);
}

static inline void dfs_put_root_smb_sessions(struct list_head *head)
/*
* cifs_get_smb_ses() already guarantees an active reference of
* @ses->dfs_root_ses when a new session is created, so we need to put extra
* references of all DFS root sessions that were used across the mount process
* in dfs_mount_share().
*/
static inline void dfs_put_root_smb_sessions(struct cifs_mount_ctx *mnt_ctx)
{
struct dfs_root_ses *root, *tmp;
const struct smb3_fs_context *ctx = mnt_ctx->fs_ctx;
struct cifs_ses *ses = ctx->dfs_root_ses;
struct cifs_ses *cur;

if (!ses)
return;

list_for_each_entry_safe(root, tmp, head, list) {
list_del_init(&root->list);
cifs_put_smb_ses(root->ses);
kfree(root);
for (cur = ses; cur; cur = cur->dfs_root_ses) {
if (cur->dfs_root_ses)
cifs_put_smb_ses(cur->dfs_root_ses);
}
cifs_put_smb_ses(ses);
}

#endif /* _CIFS_DFS_H */
11 changes: 1 addition & 10 deletions fs/smb/client/dfs_cache.c
Expand Up @@ -1278,21 +1278,12 @@ int dfs_cache_remount_fs(struct cifs_sb_info *cifs_sb)
void dfs_cache_refresh(struct work_struct *work)
{
struct TCP_Server_Info *server;
struct dfs_root_ses *rses;
struct cifs_tcon *tcon;
struct cifs_ses *ses;

tcon = container_of(work, struct cifs_tcon, dfs_cache_work.work);
ses = tcon->ses;
server = ses->server;

mutex_lock(&server->refpath_lock);
if (server->leaf_fullpath)
__refresh_tcon(server->leaf_fullpath + 1, ses, false);
mutex_unlock(&server->refpath_lock);

list_for_each_entry(rses, &tcon->dfs_ses_list, list) {
ses = rses->ses;
for (ses = tcon->ses; ses; ses = ses->dfs_root_ses) {
server = ses->server;
mutex_lock(&server->refpath_lock);
if (server->leaf_fullpath)
Expand Down
6 changes: 0 additions & 6 deletions fs/smb/client/misc.c
Expand Up @@ -141,9 +141,6 @@ tcon_info_alloc(bool dir_leases_enabled)
atomic_set(&ret_buf->num_local_opens, 0);
atomic_set(&ret_buf->num_remote_opens, 0);
ret_buf->stats_from_time = ktime_get_real_seconds();
#ifdef CONFIG_CIFS_DFS_UPCALL
INIT_LIST_HEAD(&ret_buf->dfs_ses_list);
#endif

return ret_buf;
}
Expand All @@ -159,9 +156,6 @@ tconInfoFree(struct cifs_tcon *tcon)
atomic_dec(&tconInfoAllocCount);
kfree(tcon->nativeFileSystem);
kfree_sensitive(tcon->password);
#ifdef CONFIG_CIFS_DFS_UPCALL
dfs_put_root_smb_sessions(&tcon->dfs_ses_list);
#endif
kfree(tcon->origin_fullpath);
kfree(tcon);
}
Expand Down

0 comments on commit e1db9ae

Please sign in to comment.