From 09f124b2871b3cf1e04fcdd3aff7932ecc8c125c Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Mon, 9 Jun 2025 09:35:07 +1000 Subject: smb/server: use lookup_one_unlocked() In process_query_dir_entries(), instead of locking the directory, performing a lookup, then unlocking, we can simply call lookup_one_unlocked(). That takes locks the directory only when needed. This removes the only users of lock_dir() and unlock_dir() so they can be removed. Signed-off-by: NeilBrown Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/smb2pdu.c | 24 ++++-------------------- 1 file changed, 4 insertions(+), 20 deletions(-) diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c index 63d17cea2e95..02ccdcc86af2 100644 --- a/fs/smb/server/smb2pdu.c +++ b/fs/smb/server/smb2pdu.c @@ -4108,20 +4108,6 @@ struct smb2_query_dir_private { int info_level; }; -static void lock_dir(struct ksmbd_file *dir_fp) -{ - struct dentry *dir = dir_fp->filp->f_path.dentry; - - inode_lock_nested(d_inode(dir), I_MUTEX_PARENT); -} - -static void unlock_dir(struct ksmbd_file *dir_fp) -{ - struct dentry *dir = dir_fp->filp->f_path.dentry; - - inode_unlock(d_inode(dir)); -} - static int process_query_dir_entries(struct smb2_query_dir_private *priv) { struct mnt_idmap *idmap = file_mnt_idmap(priv->dir_fp->filp); @@ -4136,12 +4122,10 @@ static int process_query_dir_entries(struct smb2_query_dir_private *priv) if (dentry_name(priv->d_info, priv->info_level)) return -EINVAL; - lock_dir(priv->dir_fp); - dent = lookup_one(idmap, - &QSTR_LEN(priv->d_info->name, - priv->d_info->name_len), - priv->dir_fp->filp->f_path.dentry); - unlock_dir(priv->dir_fp); + dent = lookup_one_unlocked(idmap, + &QSTR_LEN(priv->d_info->name, + priv->d_info->name_len), + priv->dir_fp->filp->f_path.dentry); if (IS_ERR(dent)) { ksmbd_debug(SMB, "Cannot lookup `%s' [%ld]\n", -- cgit v1.2.3 From a5dc90a9c355a30300ea4b4b25f0732270568d83 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Sat, 19 Jul 2025 18:46:42 +0900 Subject: smb/server: simplify ksmbd_vfs_kern_path_locked() ksmbd_vfs_kern_path_locked() first tries to look up the path with the given case. When this fails, if caseless is set, it loops over the components in the path, opening the relevant directory and searching for a name which matches. This name is copied over the original and the the process repeats. Each time a lookup with the newly updated path is repeated from the top (vfs_path_lookup()). When the last component has been case-corrected the simplest next step is to repeat the original lookup with ksmbd_vfs_path_lookup_locked(). If this works it gives exactly what we want, if it fails it gives the correct failure. This observation allows the code to be simplified, in particular removing the ksmbd_vfs_lock_parent() call. This patch also removes the duplication of name and filepath (two names for the one thing) and calls path_put(parent_path) sooner so parent_path can be passed directly to vfs_path_lookup avoiding the need to store it temporarily in path and then copying into parent_path. This patch removes one user of ksmbd_vfs_lock_parent() which will simplify a future patch. Signed-off-by: NeilBrown Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/vfs.c | 106 +++++++++++++++++++++------------------------------- 1 file changed, 42 insertions(+), 64 deletions(-) diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c index d3437f6644e3..b1aba0e8f42b 100644 --- a/fs/smb/server/vfs.c +++ b/fs/smb/server/vfs.c @@ -1200,8 +1200,8 @@ static int ksmbd_vfs_lookup_in_dir(const struct path *dir, char *name, /** * ksmbd_vfs_kern_path_locked() - lookup a file and get path info - * @work: work - * @name: file path that is relative to share + * @work: work + * @filepath: file path that is relative to share * @flags: lookup flags * @parent_path: if lookup succeed, return parent_path info * @path: if lookup succeed, return path info @@ -1209,84 +1209,62 @@ static int ksmbd_vfs_lookup_in_dir(const struct path *dir, char *name, * * Return: 0 on success, otherwise error */ -int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *name, +int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *filepath, unsigned int flags, struct path *parent_path, struct path *path, bool caseless) { struct ksmbd_share_config *share_conf = work->tcon->share_conf; + size_t path_len, remain_len; int err; - err = ksmbd_vfs_path_lookup_locked(share_conf, name, flags, parent_path, +retry: + err = ksmbd_vfs_path_lookup_locked(share_conf, filepath, flags, parent_path, path); - if (!err) - return 0; - - if (caseless) { - char *filepath; - size_t path_len, remain_len; - - filepath = name; - path_len = strlen(filepath); - remain_len = path_len; - - *parent_path = share_conf->vfs_path; - path_get(parent_path); - - while (d_can_lookup(parent_path->dentry)) { - char *filename = filepath + path_len - remain_len; - char *next = strchrnul(filename, '/'); - size_t filename_len = next - filename; - bool is_last = !next[0]; - - if (filename_len == 0) - break; + if (!err || !caseless) + return err; - err = ksmbd_vfs_lookup_in_dir(parent_path, filename, - filename_len, - work->conn->um); - if (err) - goto out2; + path_len = strlen(filepath); + remain_len = path_len; - next[0] = '\0'; + *parent_path = share_conf->vfs_path; + path_get(parent_path); - err = vfs_path_lookup(share_conf->vfs_path.dentry, - share_conf->vfs_path.mnt, - filepath, - flags, - path); - if (!is_last) - next[0] = '/'; - if (err) - goto out2; - else if (is_last) - goto out1; - path_put(parent_path); - *parent_path = *path; + while (d_can_lookup(parent_path->dentry)) { + char *filename = filepath + path_len - remain_len; + char *next = strchrnul(filename, '/'); + size_t filename_len = next - filename; + bool is_last = !next[0]; - remain_len -= filename_len + 1; - } + if (filename_len == 0) + break; - err = -EINVAL; -out2: + err = ksmbd_vfs_lookup_in_dir(parent_path, filename, + filename_len, + work->conn->um); path_put(parent_path); - } - -out1: - if (!err) { - err = mnt_want_write(parent_path->mnt); - if (err) { - path_put(path); - path_put(parent_path); - return err; + if (err) + goto out; + if (is_last) { + caseless = false; + goto retry; } + next[0] = '\0'; + + err = vfs_path_lookup(share_conf->vfs_path.dentry, + share_conf->vfs_path.mnt, + filepath, + flags, + parent_path); + next[0] = '/'; + if (err) + goto out; - err = ksmbd_vfs_lock_parent(parent_path->dentry, path->dentry); - if (err) { - mnt_drop_write(parent_path->mnt); - path_put(path); - path_put(parent_path); - } + remain_len -= filename_len + 1; } + + err = -EINVAL; + path_put(parent_path); +out: return err; } -- cgit v1.2.3 From d5fc1400a34b4ea5e8f2ce296ea12bf8c8421694 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Mon, 9 Jun 2025 09:35:09 +1000 Subject: smb/server: avoid deadlock when linking with ReplaceIfExists If smb2_create_link() is called with ReplaceIfExists set and the name does exist then a deadlock will happen. ksmbd_vfs_kern_path_locked() will return with success and the parent directory will be locked. ksmbd_vfs_remove_file() will then remove the file. ksmbd_vfs_link() will then be called while the parent is still locked. It will try to lock the same parent and will deadlock. This patch moves the ksmbd_vfs_kern_path_unlock() call to *before* ksmbd_vfs_link() and then simplifies the code, removing the file_present flag variable. Signed-off-by: NeilBrown Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/smb2pdu.c | 16 ++++------------ 1 file changed, 4 insertions(+), 12 deletions(-) diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c index 02ccdcc86af2..07f567ed609c 100644 --- a/fs/smb/server/smb2pdu.c +++ b/fs/smb/server/smb2pdu.c @@ -6037,7 +6037,6 @@ static int smb2_create_link(struct ksmbd_work *work, { char *link_name = NULL, *target_name = NULL, *pathname = NULL; struct path path, parent_path; - bool file_present = false; int rc; if (buf_len < (u64)sizeof(struct smb2_file_link_info) + @@ -6070,11 +6069,8 @@ static int smb2_create_link(struct ksmbd_work *work, if (rc) { if (rc != -ENOENT) goto out; - } else - file_present = true; - - if (file_info->ReplaceIfExists) { - if (file_present) { + } else { + if (file_info->ReplaceIfExists) { rc = ksmbd_vfs_remove_file(work, &path); if (rc) { rc = -EINVAL; @@ -6082,21 +6078,17 @@ static int smb2_create_link(struct ksmbd_work *work, link_name); goto out; } - } - } else { - if (file_present) { + } else { rc = -EEXIST; ksmbd_debug(SMB, "link already exists\n"); goto out; } + ksmbd_vfs_kern_path_unlock(&parent_path, &path); } - rc = ksmbd_vfs_link(work, target_name, link_name); if (rc) rc = -EINVAL; out: - if (file_present) - ksmbd_vfs_kern_path_unlock(&parent_path, &path); if (!IS_ERR(link_name)) kfree(link_name); -- cgit v1.2.3 From 4e45cca31d4e70019a5e0fe15208de72f6a55a5e Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Thu, 24 Jul 2025 08:23:38 +0900 Subject: smb/server: add ksmbd_vfs_kern_path() The function ksmbd_vfs_kern_path_locked() seems to serve two functions and as a result has an odd interface. On success it returns with the parent directory locked and with write access on that filesystem requested, but it may have crossed over a mount point to return the path, which makes the lock and the write access irrelevant. This patches separates the functionality into two functions: - ksmbd_vfs_kern_path() does not lock the parent, does not request write access, but does cross mount points - ksmbd_vfs_kern_path_locked() does not cross mount points but does lock the parent and request write access. The parent_path parameter is no longer needed. For the _locked case the final path is sufficient to drop write access and to unlock the parent (using path->dentry->d_parent which is safe while the lock is held). There were 3 caller of ksmbd_vfs_kern_path_locked(). - smb2_create_link() needs to remove the target if it existed and needs the lock and the write-access, so it continues to use ksmbd_vfs_kern_path_locked(). It would not make sense to cross mount points in this case. - smb2_open() is the only user that needs to cross mount points and it has no need for the lock or write access, so it now uses ksmbd_vfs_kern_path() - smb2_creat() does not need to cross mountpoints as it is accessing a file that it has just created on *this* filesystem. But also it does not need the lock or write access because by the time ksmbd_vfs_kern_path_locked() was called it has already created the file. So it could use either interface. It is simplest to use ksmbd_vfs_kern_path(). ksmbd_vfs_kern_path_unlock() is still needed after ksmbd_vfs_kern_path_locked() but it doesn't require the parent_path any more. After a successful call to ksmbd_vfs_kern_path(), only path_put() is needed to release the path. Signed-off-by: NeilBrown Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/smb2pdu.c | 22 +++---- fs/smb/server/vfs.c | 153 ++++++++++++++++++++++++++++++------------------ fs/smb/server/vfs.h | 7 ++- 3 files changed, 111 insertions(+), 71 deletions(-) diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c index 07f567ed609c..929d62768f64 100644 --- a/fs/smb/server/smb2pdu.c +++ b/fs/smb/server/smb2pdu.c @@ -2581,7 +2581,7 @@ static void smb2_update_xattrs(struct ksmbd_tree_connect *tcon, } } -static int smb2_creat(struct ksmbd_work *work, struct path *parent_path, +static int smb2_creat(struct ksmbd_work *work, struct path *path, char *name, int open_flags, umode_t posix_mode, bool is_dir) { @@ -2610,7 +2610,7 @@ static int smb2_creat(struct ksmbd_work *work, struct path *parent_path, return rc; } - rc = ksmbd_vfs_kern_path_locked(work, name, 0, parent_path, path, 0); + rc = ksmbd_vfs_kern_path(work, name, 0, path, 0); if (rc) { pr_err("cannot get linux path (%s), err = %d\n", name, rc); @@ -2860,7 +2860,7 @@ int smb2_open(struct ksmbd_work *work) struct ksmbd_tree_connect *tcon = work->tcon; struct smb2_create_req *req; struct smb2_create_rsp *rsp; - struct path path, parent_path; + struct path path; struct ksmbd_share_config *share = tcon->share_conf; struct ksmbd_file *fp = NULL; struct file *filp = NULL; @@ -3116,8 +3116,8 @@ int smb2_open(struct ksmbd_work *work) goto err_out2; } - rc = ksmbd_vfs_kern_path_locked(work, name, LOOKUP_NO_SYMLINKS, - &parent_path, &path, 1); + rc = ksmbd_vfs_kern_path(work, name, LOOKUP_NO_SYMLINKS, + &path, 1); if (!rc) { file_present = true; @@ -3238,7 +3238,7 @@ int smb2_open(struct ksmbd_work *work) /*create file if not present */ if (!file_present) { - rc = smb2_creat(work, &parent_path, &path, name, open_flags, + rc = smb2_creat(work, &path, name, open_flags, posix_mode, req->CreateOptions & FILE_DIRECTORY_FILE_LE); if (rc) { @@ -3443,7 +3443,7 @@ int smb2_open(struct ksmbd_work *work) } if (file_present || created) - ksmbd_vfs_kern_path_unlock(&parent_path, &path); + path_put(&path); if (!S_ISDIR(file_inode(filp)->i_mode) && open_flags & O_TRUNC && !fp->attrib_only && !stream_name) { @@ -3724,7 +3724,7 @@ reconnected_fp: err_out: if (rc && (file_present || created)) - ksmbd_vfs_kern_path_unlock(&parent_path, &path); + path_put(&path); err_out1: ksmbd_revert_fsids(work); @@ -6036,7 +6036,7 @@ static int smb2_create_link(struct ksmbd_work *work, struct nls_table *local_nls) { char *link_name = NULL, *target_name = NULL, *pathname = NULL; - struct path path, parent_path; + struct path path; int rc; if (buf_len < (u64)sizeof(struct smb2_file_link_info) + @@ -6065,7 +6065,7 @@ static int smb2_create_link(struct ksmbd_work *work, ksmbd_debug(SMB, "target name is %s\n", target_name); rc = ksmbd_vfs_kern_path_locked(work, link_name, LOOKUP_NO_SYMLINKS, - &parent_path, &path, 0); + &path, 0); if (rc) { if (rc != -ENOENT) goto out; @@ -6083,7 +6083,7 @@ static int smb2_create_link(struct ksmbd_work *work, ksmbd_debug(SMB, "link already exists\n"); goto out; } - ksmbd_vfs_kern_path_unlock(&parent_path, &path); + ksmbd_vfs_kern_path_unlock(&path); } rc = ksmbd_vfs_link(work, target_name, link_name); if (rc) diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c index b1aba0e8f42b..05ac2b14a7b1 100644 --- a/fs/smb/server/vfs.c +++ b/fs/smb/server/vfs.c @@ -66,13 +66,12 @@ int ksmbd_vfs_lock_parent(struct dentry *parent, struct dentry *child) return 0; } -static int ksmbd_vfs_path_lookup_locked(struct ksmbd_share_config *share_conf, - char *pathname, unsigned int flags, - struct path *parent_path, - struct path *path) +static int ksmbd_vfs_path_lookup(struct ksmbd_share_config *share_conf, + char *pathname, unsigned int flags, + struct path *path, bool do_lock) { struct qstr last; - struct filename *filename; + struct filename *filename __free(putname) = NULL; struct path *root_share_path = &share_conf->vfs_path; int err, type; struct dentry *d; @@ -89,51 +88,57 @@ static int ksmbd_vfs_path_lookup_locked(struct ksmbd_share_config *share_conf, return PTR_ERR(filename); err = vfs_path_parent_lookup(filename, flags, - parent_path, &last, &type, + path, &last, &type, root_share_path); - if (err) { - putname(filename); + if (err) return err; - } if (unlikely(type != LAST_NORM)) { - path_put(parent_path); - putname(filename); + path_put(path); return -ENOENT; } - err = mnt_want_write(parent_path->mnt); - if (err) { - path_put(parent_path); - putname(filename); + if (do_lock) { + err = mnt_want_write(path->mnt); + if (err) { + path_put(path); + return -ENOENT; + } + + inode_lock_nested(path->dentry->d_inode, I_MUTEX_PARENT); + d = lookup_one_qstr_excl(&last, path->dentry, 0); + + if (!IS_ERR(d)) { + dput(path->dentry); + path->dentry = d; + return 0; + } + inode_unlock(path->dentry->d_inode); + mnt_drop_write(path->mnt); + path_put(path); return -ENOENT; } - inode_lock_nested(parent_path->dentry->d_inode, I_MUTEX_PARENT); - d = lookup_one_qstr_excl(&last, parent_path->dentry, 0); - if (IS_ERR(d)) - goto err_out; - + d = lookup_noperm_unlocked(&last, path->dentry); + if (!IS_ERR(d) && d_is_negative(d)) { + dput(d); + d = ERR_PTR(-ENOENT); + } + if (IS_ERR(d)) { + path_put(path); + return -ENOENT; + } + dput(path->dentry); path->dentry = d; - path->mnt = mntget(parent_path->mnt); if (test_share_config_flag(share_conf, KSMBD_SHARE_FLAG_CROSSMNT)) { err = follow_down(path, 0); if (err < 0) { path_put(path); - goto err_out; + return -ENOENT; } } - - putname(filename); return 0; - -err_out: - inode_unlock(d_inode(parent_path->dentry)); - mnt_drop_write(parent_path->mnt); - path_put(parent_path); - putname(filename); - return -ENOENT; } void ksmbd_vfs_query_maximal_access(struct mnt_idmap *idmap, @@ -1198,38 +1203,28 @@ static int ksmbd_vfs_lookup_in_dir(const struct path *dir, char *name, return ret; } -/** - * ksmbd_vfs_kern_path_locked() - lookup a file and get path info - * @work: work - * @filepath: file path that is relative to share - * @flags: lookup flags - * @parent_path: if lookup succeed, return parent_path info - * @path: if lookup succeed, return path info - * @caseless: caseless filename lookup - * - * Return: 0 on success, otherwise error - */ -int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *filepath, - unsigned int flags, struct path *parent_path, - struct path *path, bool caseless) +static +int __ksmbd_vfs_kern_path(struct ksmbd_work *work, char *filepath, + unsigned int flags, + struct path *path, bool caseless, bool do_lock) { struct ksmbd_share_config *share_conf = work->tcon->share_conf; + struct path parent_path; size_t path_len, remain_len; int err; retry: - err = ksmbd_vfs_path_lookup_locked(share_conf, filepath, flags, parent_path, - path); + err = ksmbd_vfs_path_lookup(share_conf, filepath, flags, path, do_lock); if (!err || !caseless) return err; path_len = strlen(filepath); remain_len = path_len; - *parent_path = share_conf->vfs_path; - path_get(parent_path); + parent_path = share_conf->vfs_path; + path_get(&parent_path); - while (d_can_lookup(parent_path->dentry)) { + while (d_can_lookup(parent_path.dentry)) { char *filename = filepath + path_len - remain_len; char *next = strchrnul(filename, '/'); size_t filename_len = next - filename; @@ -1238,10 +1233,10 @@ retry: if (filename_len == 0) break; - err = ksmbd_vfs_lookup_in_dir(parent_path, filename, + err = ksmbd_vfs_lookup_in_dir(&parent_path, filename, filename_len, work->conn->um); - path_put(parent_path); + path_put(&parent_path); if (err) goto out; if (is_last) { @@ -1254,7 +1249,7 @@ retry: share_conf->vfs_path.mnt, filepath, flags, - parent_path); + &parent_path); next[0] = '/'; if (err) goto out; @@ -1263,17 +1258,59 @@ retry: } err = -EINVAL; - path_put(parent_path); + path_put(&parent_path); out: return err; } -void ksmbd_vfs_kern_path_unlock(struct path *parent_path, struct path *path) +/** + * ksmbd_vfs_kern_path() - lookup a file and get path info + * @work: work + * @filepath: file path that is relative to share + * @flags: lookup flags + * @path: if lookup succeed, return path info + * @caseless: caseless filename lookup + * + * Perform the lookup, possibly crossing over any mount point. + * On return no locks will be held and write-access to filesystem + * won't have been checked. + * Return: 0 if file was found, otherwise error + */ +int ksmbd_vfs_kern_path(struct ksmbd_work *work, char *filepath, + unsigned int flags, + struct path *path, bool caseless) +{ + return __ksmbd_vfs_kern_path(work, filepath, flags, path, + caseless, false); +} + +/** + * ksmbd_vfs_kern_path_locked() - lookup a file and get path info + * @work: work + * @filepath: file path that is relative to share + * @flags: lookup flags + * @path: if lookup succeed, return path info + * @caseless: caseless filename lookup + * + * Perform the lookup, but don't cross over any mount point. + * On return the parent of path->dentry will be locked and write-access to + * filesystem will have been gained. + * Return: 0 on if file was found, otherwise error + */ +int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *filepath, + unsigned int flags, + struct path *path, bool caseless) +{ + return __ksmbd_vfs_kern_path(work, filepath, flags, path, + caseless, true); +} + +void ksmbd_vfs_kern_path_unlock(struct path *path) { - inode_unlock(d_inode(parent_path->dentry)); - mnt_drop_write(parent_path->mnt); + /* While lock is still held, ->d_parent is safe */ + inode_unlock(d_inode(path->dentry->d_parent)); + mnt_drop_write(path->mnt); path_put(path); - path_put(parent_path); } struct dentry *ksmbd_vfs_kern_path_create(struct ksmbd_work *work, diff --git a/fs/smb/server/vfs.h b/fs/smb/server/vfs.h index 2893f59803a6..d47472f3e30b 100644 --- a/fs/smb/server/vfs.h +++ b/fs/smb/server/vfs.h @@ -117,10 +117,13 @@ int ksmbd_vfs_xattr_stream_name(char *stream_name, char **xattr_stream_name, int ksmbd_vfs_remove_xattr(struct mnt_idmap *idmap, const struct path *path, char *attr_name, bool get_write); +int ksmbd_vfs_kern_path(struct ksmbd_work *work, char *name, + unsigned int flags, + struct path *path, bool caseless); int ksmbd_vfs_kern_path_locked(struct ksmbd_work *work, char *name, - unsigned int flags, struct path *parent_path, + unsigned int flags, struct path *path, bool caseless); -void ksmbd_vfs_kern_path_unlock(struct path *parent_path, struct path *path); +void ksmbd_vfs_kern_path_unlock(struct path *path); struct dentry *ksmbd_vfs_kern_path_create(struct ksmbd_work *work, const char *name, unsigned int flags, -- cgit v1.2.3 From 9b493ab6f35178afd8d619800df9071992f715de Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Mon, 21 Jul 2025 14:28:55 +0900 Subject: ksmbd: fix null pointer dereference error in generate_encryptionkey If client send two session setups with krb5 authenticate to ksmbd, null pointer dereference error in generate_encryptionkey could happen. sess->Preauth_HashValue is set to NULL if session is valid. So this patch skip generate encryption key if session is valid. Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-27654 Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/smb2pdu.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c index 929d62768f64..0caf30fb1df1 100644 --- a/fs/smb/server/smb2pdu.c +++ b/fs/smb/server/smb2pdu.c @@ -1621,11 +1621,24 @@ static int krb5_authenticate(struct ksmbd_work *work, rsp->SecurityBufferLength = cpu_to_le16(out_len); - if ((conn->sign || server_conf.enforced_signing) || + /* + * If session state is SMB2_SESSION_VALID, We can assume + * that it is reauthentication. And the user/password + * has been verified, so return it here. + */ + if (sess->state == SMB2_SESSION_VALID) { + if (conn->binding) + goto binding_session; + return 0; + } + + if ((rsp->SessionFlags != SMB2_SESSION_FLAG_IS_GUEST_LE && + (conn->sign || server_conf.enforced_signing)) || (req->SecurityMode & SMB2_NEGOTIATE_SIGNING_REQUIRED)) sess->sign = true; - if (smb3_encryption_negotiated(conn)) { + if (smb3_encryption_negotiated(conn) && + !(req->Flags & SMB2_SESSION_REQ_FLAG_BINDING)) { retval = conn->ops->generate_encryptionkey(conn, sess); if (retval) { ksmbd_debug(SMB, @@ -1638,6 +1651,7 @@ static int krb5_authenticate(struct ksmbd_work *work, sess->sign = false; } +binding_session: if (conn->dialect >= SMB30_PROT_ID) { chann = lookup_chann_list(sess, conn); if (!chann) { -- cgit v1.2.3 From ecd9d6bf88ddd64e3dc7beb9a065fd5fa4714f72 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Mon, 21 Jul 2025 14:29:43 +0900 Subject: ksmbd: check return value of xa_store() in krb5_authenticate xa_store() may fail so check its return value and return error code if error occurred. Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/smb2pdu.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c index 0caf30fb1df1..b46fbc8087e0 100644 --- a/fs/smb/server/smb2pdu.c +++ b/fs/smb/server/smb2pdu.c @@ -1594,7 +1594,7 @@ static int krb5_authenticate(struct ksmbd_work *work, struct ksmbd_conn *conn = work->conn; struct ksmbd_session *sess = work->sess; char *in_blob, *out_blob; - struct channel *chann = NULL; + struct channel *chann = NULL, *old; u64 prev_sess_id; int in_len, out_len; int retval; @@ -1660,7 +1660,12 @@ binding_session: return -ENOMEM; chann->conn = conn; - xa_store(&sess->ksmbd_chann_list, (long)conn, chann, KSMBD_DEFAULT_GFP); + old = xa_store(&sess->ksmbd_chann_list, (long)conn, + chann, KSMBD_DEFAULT_GFP); + if (xa_is_err(old)) { + kfree(chann); + return xa_err(old); + } } } -- cgit v1.2.3 From 44a3059c4c8cc635a1fb2afd692d0730ca1ba4b6 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Fri, 25 Jul 2025 08:13:31 +0900 Subject: ksmbd: fix Preauh_HashValue race condition If client send multiple session setup requests to ksmbd, Preauh_HashValue race condition could happen. There is no need to free sess->Preauh_HashValue at session setup phase. It can be freed together with session at connection termination phase. Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-27661 Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/smb2pdu.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c index b46fbc8087e0..6fc6ad63d004 100644 --- a/fs/smb/server/smb2pdu.c +++ b/fs/smb/server/smb2pdu.c @@ -1852,8 +1852,6 @@ int smb2_sess_setup(struct ksmbd_work *work) ksmbd_conn_set_good(conn); sess->state = SMB2_SESSION_VALID; } - kfree(sess->Preauth_HashValue); - sess->Preauth_HashValue = NULL; } else if (conn->preferred_auth_mech == KSMBD_AUTH_NTLMSSP) { if (negblob->MessageType == NtLmNegotiate) { rc = ntlm_negotiate(work, negblob, negblob_len, rsp); @@ -1880,8 +1878,6 @@ int smb2_sess_setup(struct ksmbd_work *work) kfree(preauth_sess); } } - kfree(sess->Preauth_HashValue); - sess->Preauth_HashValue = NULL; } else { pr_info_ratelimited("Unknown NTLMSSP message type : 0x%x\n", le32_to_cpu(negblob->MessageType)); -- cgit v1.2.3 From 4f8ff9486fd94b9d6a4932f2aefb9f2fc3bd0cf6 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Fri, 25 Jul 2025 10:33:28 +0900 Subject: ksmbd: fix corrupted mtime and ctime in smb2_open If STATX_BASIC_STATS flags are not given as an argument to vfs_getattr, It can not get ctime and mtime in kstat. This causes a problem showing mtime and ctime outdated from cifs.ko. File: /xfstest.test/foo Size: 4096 Blocks: 8 IO Block: 1048576 regular file Device: 0,65 Inode: 2033391 Links: 1 Access: (0755/-rwxr-xr-x) Uid: ( 0/ root) Gid: ( 0/ root) Context: system_u:object_r:cifs_t:s0 Access: 2025-07-23 22:15:30.136051900 +0100 Modify: 1970-01-01 01:00:00.000000000 +0100 Change: 1970-01-01 01:00:00.000000000 +0100 Birth: 2025-07-23 22:15:30.136051900 +0100 Cc: stable@vger.kernel.org Reported-by: David Howells Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/vfs.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/smb/server/vfs.c b/fs/smb/server/vfs.c index 05ac2b14a7b1..2bd8cc20215a 100644 --- a/fs/smb/server/vfs.c +++ b/fs/smb/server/vfs.c @@ -553,7 +553,8 @@ int ksmbd_vfs_getattr(const struct path *path, struct kstat *stat) { int err; - err = vfs_getattr(path, stat, STATX_BTIME, AT_STATX_SYNC_AS_STAT); + err = vfs_getattr(path, stat, STATX_BASIC_STATS | STATX_BTIME, + AT_STATX_SYNC_AS_STAT); if (err) pr_err("getattr failed, err %d\n", err); return err; -- cgit v1.2.3