From 0e60dafe97eca61721f3db456f97d97a80c6c8ae Mon Sep 17 00:00:00 2001 From: Ali Ganiyev Date: Mon, 25 May 2026 10:23:47 +0900 Subject: [PATCH 1/3] ksmbd: OOB read regression in smb_check_perm_dacl() ACE-walk loops Commit d07b26f39246 ("ksmbd: require minimum ACE size in smb_check_perm_dacl()") introduced a transposed bounds check: if (offsetof(struct smb_ace, sid) + aces_size < CIFS_SID_BASE_SIZE) Since offsetof(..sid) is 8 and CIFS_SID_BASE_SIZE is 8, this evaluates to `aces_size < 0`. Because `aces_size` is always non-negative, this check becomes dead code and never breaks the loop. Worse, that commit removed the old 4-byte guard, meaning the loop now reads `ace->size` (offset 2) even when `aces_size` is 0-3 bytes. This re-opens a 2-byte heap out-of-bounds (OOB) read past the pntsd allocation during subsequent SMB2_CREATE operations. Fix this by properly transposing the comparison to require at least 16 bytes (8-byte offset + 8-byte SID base), matching the correct form used in smb_inherit_dacl(). Fixes: d07b26f39246 ("ksmbd: require minimum ACE size in smb_check_perm_dacl()") Cc: stable@vger.kernel.org Signed-off-by: Ali Ganiyev Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/smbacl.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fs/smb/server/smbacl.c b/fs/smb/server/smbacl.c index c2d9be52a311..664b1b4a3233 100644 --- a/fs/smb/server/smbacl.c +++ b/fs/smb/server/smbacl.c @@ -1446,8 +1446,8 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path, ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl)); aces_size = acl_size - sizeof(struct smb_acl); for (i = 0; i < le16_to_cpu(pdacl->num_aces); i++) { - if (offsetof(struct smb_ace, sid) + - aces_size < CIFS_SID_BASE_SIZE) + if (aces_size < offsetof(struct smb_ace, sid) + + CIFS_SID_BASE_SIZE) break; ace_size = le16_to_cpu(ace->size); if (ace_size > aces_size || @@ -1467,8 +1467,8 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, const struct path *path, ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl)); aces_size = acl_size - sizeof(struct smb_acl); for (i = 0; i < le16_to_cpu(pdacl->num_aces); i++) { - if (offsetof(struct smb_ace, sid) + - aces_size < CIFS_SID_BASE_SIZE) + if (aces_size < offsetof(struct smb_ace, sid) + + CIFS_SID_BASE_SIZE) break; ace_size = le16_to_cpu(ace->size); if (ace_size > aces_size || From 2f15dcd0d4b502c704a52f5c7de128b163677978 Mon Sep 17 00:00:00 2001 From: Aleksandr Golovnya Date: Tue, 26 May 2026 01:50:18 +0700 Subject: [PATCH 2/3] ksmbd: release ksmbd_inode ref via ksmbd_inode_put on lookup paths ksmbd_query_inode_status() and ksmbd_lookup_fd_inode() both take a reference on a ksmbd_inode via __ksmbd_inode_lookup() (which performs atomic_inc_not_zero()) and later release it using a bare atomic_dec(&ci->m_count). Unlike ksmbd_inode_put(), a bare atomic_dec() does not check whether the reference count has reached zero, so if the caller happens to drop the last reference, the ksmbd_inode is leaked: it stays in the global inode hash table with m_count == 0, future __ksmbd_inode_lookup() calls reject it via atomic_inc_not_zero(), and ksmbd_inode_free() is never invoked. The race is: T1: __ksmbd_inode_lookup() -> atomic_inc_not_zero(): m_count = 2 T2: ksmbd_inode_put() -> atomic_dec_and_test(): m_count = 1 (not freed) T1: atomic_dec(&ci->m_count) -> m_count = 0 return (LEAK) In ksmbd_lookup_fd_inode() the matched-fp path (which now also uses ksmbd_inode_put()) cannot currently reach m_count == 0 because the matched ksmbd_file holds its own reference on ci, but converting it to the proper API keeps the three call sites consistent and avoids future regressions if the locking changes. Because ksmbd_inode_put() may free the ksmbd_inode if this drops the last reference, the call must happen after up_read(&ci->m_lock) on the two affected paths in ksmbd_lookup_fd_inode(). On the no-match path this is a pure reordering; on the matched path ksmbd_fp_get() is moved above the unlock so that the returned ksmbd_file is pinned before the inode reference is released. Signed-off-by: Aleksandr Golovnya Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/vfs_cache.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/fs/smb/server/vfs_cache.c b/fs/smb/server/vfs_cache.c index 5a232d94f567..4d2d33df6231 100644 --- a/fs/smb/server/vfs_cache.c +++ b/fs/smb/server/vfs_cache.c @@ -217,7 +217,7 @@ int ksmbd_query_inode_status(struct dentry *dentry) ret = KSMBD_INODE_STATUS_OK; up_read(&ci->m_lock); - atomic_dec(&ci->m_count); + ksmbd_inode_put(ci); return ret; } @@ -719,14 +719,14 @@ struct ksmbd_file *ksmbd_lookup_fd_inode(struct dentry *dentry) down_read(&ci->m_lock); list_for_each_entry(lfp, &ci->m_fp_list, node) { if (inode == file_inode(lfp->filp)) { - atomic_dec(&ci->m_count); lfp = ksmbd_fp_get(lfp); up_read(&ci->m_lock); + ksmbd_inode_put(ci); return lfp; } } - atomic_dec(&ci->m_count); up_read(&ci->m_lock); + ksmbd_inode_put(ci); return NULL; } From cc57232cae23c0df91b4a59d0f519141ce9b5b02 Mon Sep 17 00:00:00 2001 From: Sean Shen Date: Tue, 26 May 2026 22:07:16 +0900 Subject: [PATCH 3/3] ksmbd: fix FSCTL permission bypass by adding a permission check for FSCTL_SET_SPARSE FSCTL_SET_SPARSE in fsctl_set_sparse() modifies the file's sparse attribute and saves it through xattr without any permission checks. This exposes two issues: 1) A client on a read-only share can change the sparse attribute on files it opened, even though the share is read-only. Other FSCTL write operations already check test_tree_conn_flag(work->tcon, KSMBD_TREE_CONN_FLAG_WRITABLE), but FSCTL_SET_SPARSE does not. 2) Even on writable shares, clients without FILE_WRITE_DATA or FILE_WRITE_ATTRIBUTES access should not modify the sparse attribute. Similar handle-level checks exist in other functions but are missing here. Add both share-level writable check and per-handle access check. Use goto out on error to avoid leaking file references. Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") Cc: Namjae Jeon Cc: Sergey Senozhatsky Cc: Steve French Signed-off-by: Sean Shen Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/smb/server/smb2pdu.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c index 5128a693aca6..620bcfbbfd92 100644 --- a/fs/smb/server/smb2pdu.c +++ b/fs/smb/server/smb2pdu.c @@ -8202,9 +8202,20 @@ static inline int fsctl_set_sparse(struct ksmbd_work *work, u64 id, int ret = 0; __le32 old_fattr; + if (!test_tree_conn_flag(work->tcon, KSMBD_TREE_CONN_FLAG_WRITABLE)) { + ksmbd_debug(SMB, "User does not have write permission\n"); + return -EACCES; + } + fp = ksmbd_lookup_fd_fast(work, id); if (!fp) return -ENOENT; + + if (!(fp->daccess & (FILE_WRITE_DATA_LE | FILE_WRITE_ATTRIBUTES_LE))) { + ret = -EACCES; + goto out; + } + idmap = file_mnt_idmap(fp->filp); old_fattr = fp->f_ci->m_fattr;