From e6142a8bfc230c7263eb8b0475249c958ce49367 Mon Sep 17 00:00:00 2001 From: Diego Oliva Date: Wed, 2 Sep 2026 11:42:06 +0100 Subject: [PATCH 01/18] smb: client: reject short READ responses in CIFSSMBRead() CIFSSMBRead() reads DataLengthHigh, DataLength and DataOffset out of the READ_RSP returned by the server without first checking that a whole READ_RSP was actually received. The length of the response is recorded in rsp_iov.iov_len, but nothing constrains it to be at least read_rsp_size before those fields are dereferenced. A malicious or compromised SMB1 server can return a response shorter than the READ_RSP header, so that parsing the header itself reads past the end of the receive buffer. SMB1 is not negotiated by default; reaching this code requires an explicit vers=1.0 mount. Reject the response unless it is at least read_rsp_size bytes long. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Suggested-by: Paulo Alcantara Cc: stable@vger.kernel.org # 6.19.x Assisted-by: Bynario AI Signed-off-by: Diego Oliva Reviewed-by: David Howells Signed-off-by: Paulo Alcantara --- fs/smb/client/cifssmb.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c index f8aa9e7b4bc6..be13ab37039d 100644 --- a/fs/smb/client/cifssmb.c +++ b/fs/smb/client/cifssmb.c @@ -1719,6 +1719,14 @@ CIFSSMBRead(const unsigned int xid, struct cifs_io_parms *io_parms, pSMBr = (READ_RSP *)rsp_iov.iov_base; if (rc) { cifs_dbg(VFS, "Send error in read = %d\n", rc); + } else if (rsp_iov.iov_len < tcon->ses->server->vals->read_rsp_size) { + /* check that the received response can hold a whole READ_RSP */ + cifs_dbg(FYI, "%s: server returned short header. got=%zu expected=%zu\n", + __func__, rsp_iov.iov_len, + tcon->ses->server->vals->read_rsp_size); + rc = smb_EIO2(smb_eio_trace_read_rsp_short, + rsp_iov.iov_len, tcon->ses->server->vals->read_rsp_size); + *nbytes = 0; } else { int data_length = le16_to_cpu(pSMBr->DataLengthHigh); data_length = data_length << 16; From 5be5bdda5863eacc964b609ba927764f253431b3 Mon Sep 17 00:00:00 2001 From: Diego Oliva Date: Wed, 2 Sep 2026 11:42:07 +0100 Subject: [PATCH 02/18] smb: client: reject out-of-bounds DataOffset in CIFSSMBRead() The SMB1 synchronous read helper CIFSSMBRead() validates the server's DataLength against CIFSMaxBufSize and the caller's count, but never validates DataOffset. The copy source is formed as &pSMBr->hdr.Protocol + le16_to_cpu(pSMBr->DataOffset) and memcpy()'d for DataLength bytes with no check that the [DataOffset, DataOffset + DataLength) range lies within the response actually received from the server. A malicious or compromised SMB1 server can return a response carrying an in-range DataLength and a large DataOffset, driving the source pointer past the end of the response buffer. The memcpy() then copies adjacent kernel heap into the caller's read buffer (information disclosure), or reads unmapped memory and oopses (denial of service). SMB1 is not negotiated by default; reaching this code requires an explicit vers=1.0 mount. Both DataOffset and the received response length recorded in rsp_iov.iov_len are relative to the start of the SMB header, so reject the response unless DataOffset + DataLength fits within that length, using overflow-safe arithmetic, before forming the source pointer. The response length has been validated by the previous patch, so the DataOffset and DataLength fields can be read safely here. While here, make data_length unsigned. It holds a length derived from unsigned on-the-wire fields and is only ever compared against unsigned quantities; print it with %u accordingly, and add __func__ to the cifs_dbg() calls in this function. Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Cc: stable@vger.kernel.org # 6.19.x Assisted-by: Bynario AI Signed-off-by: Diego Oliva Reviewed-by: David Howells Signed-off-by: Paulo Alcantara --- fs/smb/client/cifssmb.c | 18 +++++++++++++----- fs/smb/client/trace.h | 1 + 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c index be13ab37039d..f9aff0712794 100644 --- a/fs/smb/client/cifssmb.c +++ b/fs/smb/client/cifssmb.c @@ -1728,7 +1728,8 @@ CIFSSMBRead(const unsigned int xid, struct cifs_io_parms *io_parms, rsp_iov.iov_len, tcon->ses->server->vals->read_rsp_size); *nbytes = 0; } else { - int data_length = le16_to_cpu(pSMBr->DataLengthHigh); + unsigned int data_length = le16_to_cpu(pSMBr->DataLengthHigh); + __u16 data_offset = le16_to_cpu(pSMBr->DataOffset); data_length = data_length << 16; data_length += le16_to_cpu(pSMBr->DataLength); *nbytes = data_length; @@ -1736,14 +1737,21 @@ CIFSSMBRead(const unsigned int xid, struct cifs_io_parms *io_parms, /*check that DataLength would not go beyond end of SMB */ if ((data_length > CIFSMaxBufSize) || (data_length > count)) { - cifs_dbg(FYI, "bad length %d for count %d\n", - data_length, count); + cifs_dbg(FYI, "%s: bad length %u for count %u\n", + __func__, data_length, count); rc = smb_EIO2(smb_eio_trace_read_overlarge, data_length, count); *nbytes = 0; + } else if (data_offset < sizeof(*pSMBr) || + (size_t)data_offset + data_length > rsp_iov.iov_len) { + /* check that the data lies within the received response */ + cifs_dbg(FYI, "%s: bad data offset %u length %u for response of %zu\n", + __func__, data_offset, data_length, rsp_iov.iov_len); + rc = smb_EIO2(smb_eio_trace_read_bad_offset, + data_offset, data_length); + *nbytes = 0; } else { - pReadData = (char *) (&pSMBr->hdr.Protocol) + - le16_to_cpu(pSMBr->DataOffset); + pReadData = (char *) (&pSMBr->hdr.Protocol) + data_offset; /* if (rc = copy_to_user(buf, pReadData, data_length)) { cifs_dbg(VFS, "Faulting on read rc = %d\n",rc); rc = -EFAULT; diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h index 12241abb8e2e..b442cccd1530 100644 --- a/fs/smb/client/trace.h +++ b/fs/smb/client/trace.h @@ -79,6 +79,7 @@ EM(smb_eio_trace_qreparse_setup_count, "qreparse_setup_count") \ EM(smb_eio_trace_qreparse_sizes_wrong, "qreparse_sizes_wrong") \ EM(smb_eio_trace_qsym_bcc_too_small, "qsym_bcc_too_small") \ + EM(smb_eio_trace_read_bad_offset, "read_bad_offset") \ EM(smb_eio_trace_read_mid_state_unknown, "read_mid_state_unknown") \ EM(smb_eio_trace_read_overlarge, "read_overlarge") \ EM(smb_eio_trace_read_rsp_malformed, "read_rsp_malformed") \ From d9d7eeb0cea5b55b82888f443622fd8d4ee064f3 Mon Sep 17 00:00:00 2001 From: Aohan Mei Date: Wed, 2 Sep 2026 20:52:13 +0800 Subject: [PATCH 03/18] smb: client: reject userspace cifs.idmap descriptions cifs.idmap key descriptions carry authority-bearing fields (owner and group SIDs and uid/gid values in "os:"/"gs:"/"oi:"/"gi:" form) that the cifs.idmap upcall helper treats as kernel-originating inputs. Unlike its sibling cifs.spnego, the cifs.idmap key type has no vet_description hook, so userspace can create keys of this type through request_key(2)/add_key(2) and supply those fields without CIFS origin. A request_key(2) call with a non-NULL callout then drives a root usermodehelper upcall (/sbin/request-key -> cifs.idmap) that consumes the unvetted description in root context. Only accept cifs.idmap descriptions while CIFS is using its private root_cred to request the key. id_to_sid()/sid_to_id() already run under override_creds(root_cred), so the kernel-originated path is unaffected. This mirrors commit 3da1fdf4efbc ("smb: client: reject userspace cifs.spnego descriptions"), which applied the same restriction to cifs.spnego. Fixes: 4d79dba0e007 ("cifs: Add idmap key and related data structures and functions (try #17 repost)") Reported-by: TencentOS Corvus AI Cc: stable@vger.kernel.org Assisted-by: CodeBuddy:Kimi-K3 Signed-off-by: Aohan Mei Acked-by: David Howells Signed-off-by: Paulo Alcantara --- fs/smb/client/cifsacl.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c index 12005f46307d..213a421bf8e9 100644 --- a/fs/smb/client/cifsacl.c +++ b/fs/smb/client/cifsacl.c @@ -100,8 +100,23 @@ cifs_idmap_key_destroy(struct key *key) kfree(key->payload.data[0]); } +static int +cifs_idmap_key_vet_description(const char *description) +{ + /* + * cifs.idmap descriptions are authority-bearing inputs to the + * cifs.idmap upcall helper. Only allow the kernel to create this + * type of key using the private root_cred installed in + * init_cifs_idmap; reject userspace request_key(2)/add_key(2). + */ + if (current_cred() != root_cred) + return -EPERM; + return 0; +} + static struct key_type cifs_idmap_key_type = { .name = "cifs.idmap", + .vet_description = cifs_idmap_key_vet_description, .instantiate = cifs_idmap_key_instantiate, .destroy = cifs_idmap_key_destroy, .describe = user_describe, From d806d5a85dcbe2a0f181b2f0f9f61ddfbefa1818 Mon Sep 17 00:00:00 2001 From: Karl Mehltretter Date: Wed, 2 Sep 2026 20:28:14 +0200 Subject: [PATCH 04/18] smb: client: pin DFS superblock in iterator callback tcon_super_cb() stores a raw superblock pointer, but __cifs_get_super() takes its active reference only after iterate_supers_type() has dropped s_umount and its passive reference. Concurrent DFS automount expiry can therefore free the superblock before cifs_sb_active() uses it. A deterministic KASAN test reproduces the race as: BUG: KASAN: slab-use-after-free in cifs_sb_active+0x77/0x80 The same test passes with this change applied. Take the active reference in the callback while iterate_supers_type() still holds s_umount shared. cifs_put_tcp_super() remains the matching release. Fixes: bacd704a95ad ("cifs: handle prefix paths in reconnect") Cc: stable@vger.kernel.org Assisted-by: LLM Signed-off-by: Karl Mehltretter Signed-off-by: Paulo Alcantara --- fs/smb/client/misc.c | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c index 46e1382e8e04..d4db3f91a91f 100644 --- a/fs/smb/client/misc.c +++ b/fs/smb/client/misc.c @@ -891,8 +891,14 @@ static void tcon_super_cb(struct super_block *sb, void *arg) 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)) + dfs_src_pathname_equal(t2->origin_fullpath, t1->origin_fullpath)) { + /* + * Take the active reference while iterate_supers_type() still + * holds s_umount shared. + */ + cifs_sb_active(sb); sd->sb = sb; + } spin_unlock(&t2->tc_lock); } @@ -909,15 +915,8 @@ static struct super_block *__cifs_get_super(void (*f)(struct super_block *, void 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); + if (sd.sb) return sd.sb; - } } pr_warn_once("%s: could not find dfs superblock\n", __func__); return ERR_PTR(-EINVAL); From 42d3358bf145f27d32350037e67d3527e9098c1e Mon Sep 17 00:00:00 2001 From: Fredric Cover Date: Wed, 2 Sep 2026 17:49:01 -0700 Subject: [PATCH 05/18] smb: client: fill cache fields after populating cache in copy_ref_data() In copy_ref_data(), struct cache_entry *ce has its fields populated at the beginning of the function. Later, if alloc_target fails with an ERR_PTR, free_tgts() is called on the cache, leaving the cache metadata populated without any targets. Critically, this extends ce->etime, making the cache appear valid for longer without any targets. Also, free_tgts() does not set ce->numtgts to zero. On error, when the cache is freed, ce->numtgts is not zeroed, and other cache users may attempt to access nonexistent entries. Update fields after copying targets to prevent partial-state updates. Set ce->numtgts to zero at the end of free_tgts(). Signed-off-by: Fredric Cover Signed-off-by: Paulo Alcantara --- fs/smb/client/dfs_cache.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/fs/smb/client/dfs_cache.c b/fs/smb/client/dfs_cache.c index 86dba25b7a5a..611e18fe8204 100644 --- a/fs/smb/client/dfs_cache.c +++ b/fs/smb/client/dfs_cache.c @@ -123,6 +123,7 @@ static inline void free_tgts(struct cache_entry *ce) kfree(t); } + ce->numtgts = 0; WRITE_ONCE(ce->tgthint, NULL); } @@ -388,13 +389,6 @@ static int copy_ref_data(const struct dfs_info3_param *refs, int numrefs, struct cache_dfs_tgt *target; int i; - ce->ttl = max_t(int, refs[0].ttl, CACHE_MIN_TTL); - ce->etime = get_expire_time(ce->ttl); - ce->srvtype = refs[0].server_type; - ce->hdr_flags = refs[0].flags; - ce->ref_flags = refs[0].ref_flag; - ce->path_consumed = refs[0].path_consumed; - for (i = 0; i < numrefs; i++) { struct cache_dfs_tgt *t; @@ -409,12 +403,19 @@ static int copy_ref_data(const struct dfs_info3_param *refs, int numrefs, } else { list_add_tail(&t->list, &ce->tlist); } - ce->numtgts++; } target = list_first_entry_or_null(&ce->tlist, struct cache_dfs_tgt, list); + WRITE_ONCE(ce->tgthint, target); + ce->ttl = max_t(int, refs[0].ttl, CACHE_MIN_TTL); + ce->etime = get_expire_time(ce->ttl); + ce->srvtype = refs[0].server_type; + ce->hdr_flags = refs[0].flags; + ce->ref_flags = refs[0].ref_flag; + ce->path_consumed = refs[0].path_consumed; + ce->numtgts = numrefs; return 0; } @@ -634,7 +635,6 @@ static int update_cache_entry_locked(struct cache_entry *ce, const struct dfs_in } free_tgts(ce); - ce->numtgts = 0; rc = copy_ref_data(refs, numrefs, ce, th); From 9f2e63f1b2d5fc5b5423424902c091123e220e7e Mon Sep 17 00:00:00 2001 From: Bjoern Doebel Date: Thu, 3 Sep 2026 21:28:58 +0000 Subject: [PATCH 06/18] smb: client: avoid leaking refcount in cifs_queue_oplock_break() cifs_queue_oplock_break() unconditionally takes a reference on the target file before queueing cifs_oplock_break(). Only that work item decreases the reference counter again. If another oplock break arrives while that work is still queued, queue_work() will return false and not queue this second work item. As a result, we will never reach the point to drop the file reference again and are leaking this reference. This can be triggered when interacting with a slow-responding server. As a result, later unmount operations for this file system will fail with BUG: Dentry ... still in use (1) [unmount of cifs cifs] VFS: Busy inodes after unmount of cifs (cifs) kernel BUG at fs/super.c:777! Fix this by only incrementing the reference count if the work has been queued successfully. Taking it after queue_work() is safe because all three callers hold tcon->open_file_lock across the call and _cifsFileInfo_put() decrements under that same lock, so a worker that starts the handler in the window cannot drop the reference before it has been taken. Fixes: b98749cac4a69 ("CIFS: keep FileInfo handle live during oplock break") Cc: stable@vger.kernel.org Assisted-by: Kiro:claude-opus-5 Signed-off-by: Bjoern Doebel Reviewed-by: Namjae Jeon Signed-off-by: Paulo Alcantara --- fs/smb/client/misc.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/fs/smb/client/misc.c b/fs/smb/client/misc.c index d4db3f91a91f..945194fe7a97 100644 --- a/fs/smb/client/misc.c +++ b/fs/smb/client/misc.c @@ -378,10 +378,11 @@ void cifs_queue_oplock_break(struct cifsFileInfo *cfile) * open_file_lock to enforce the validity of it for the oplock * break handler. The matching put is done at the end of the * handler. + * + * Only take a reference if the work is actually queued. */ - cifsFileInfo_get(cfile); - - queue_work(cifsoplockd_wq, &cfile->oplock_break); + if (queue_work(cifsoplockd_wq, &cfile->oplock_break)) + cifsFileInfo_get(cfile); } void cifs_done_oplock_break(struct cifsInodeInfo *cinode) From 23b26f4408ac3f35a482d2e5cf6fc865d4201b71 Mon Sep 17 00:00:00 2001 From: Bjoern Doebel Date: Fri, 4 Sep 2026 10:42:36 +0000 Subject: [PATCH 07/18] smb: client: avoid leaking refcount when cifs_sb_tlink() fails cifs_oplock_break() takes over the reference that cifs_queue_oplock_break() acquired when it queued the work, and drops it with _cifsFileInfo_put() once the break has been processed. Only in setups with "-o multiuser", cifs_sb_tlink() may fail, at which point cifs_oplock_break() returns without putting the file reference, mirroring the reference leak we already fixed in the companion patch to cifs_queue_oplock_break(). This would trigger a crash due to busy inodes on the next unmount: BUG: Dentry ... still in use (1) [unmount of cifs cifs] VFS: Busy inodes after unmount of cifs (cifs) Drop the reference on that path as well. Doing so before the out label mirrors the normal path, which also puts the reference before cifs_done_oplock_break(). Found by Sashiko code review. The failure path was not exercised at runtime. Fixes: e8f5f849ffce2 ("cifs: fix potential oops in cifs_oplock_break") Cc: stable@vger.kernel.org Assisted-by: Kiro:claude-opus-5 Signed-off-by: Bjoern Doebel Reviewed-by: Namjae Jeon Signed-off-by: Paulo Alcantara --- fs/smb/client/file.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index d7b0a9512dfa..27b58d907203 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -3348,8 +3348,11 @@ void cifs_oplock_break(struct work_struct *work) TASK_UNINTERRUPTIBLE); tlink = cifs_sb_tlink(cifs_sb); - if (IS_ERR(tlink)) + if (IS_ERR(tlink)) { + /* drop the reference taken when the break was queued */ + _cifsFileInfo_put(cfile, false /* do not wait for ourself */, false); goto out; + } tcon = tlink_tcon(tlink); server = tcon->ses->server; From 5520e89a5a4f834bced64cf2ac927001cc513a40 Mon Sep 17 00:00:00 2001 From: Fan Wu Date: Fri, 4 Sep 2026 13:48:11 +0000 Subject: [PATCH 08/18] smb: client: fix cifsFileInfo reference leak in deferred close When cifs_close() defers a close, it hands the cifsFileInfo reference of the closing struct file to the queued work. Each execution of smb2_deferred_work_close() drops one such reference. deferred_close_scheduled can be false while the work is pending: the workqueue clears PENDING when the callback starts to run, before the callback clears the flag under deferred_lock. A close in that interval requeues the running work, and the callback then clears the flag, leaving the requeued work pending with the flag down. A later cifs_open() can reuse the handle and its cifs_close() reaches the same branch: queue_delayed_work() fails because the work is still pending, but cifs_close() returns without dropping the closing file's reference. The cifsFileInfo count stays pinned and its tlink, dentry and server handle are leaked. Check the return value and hand off the reference only when work was actually queued. Otherwise, use the shared _cifsFileInfo_put(), like the mod_delayed_work() branch above: the pending execution already owns its reference. This issue was found by an in-house static analysis tool. Fixes: c3f207ab29f7 ("cifs: Deferred close for files") Cc: stable@vger.kernel.org Assisted-by: Codex:gpt-5.6 Co-developed-by: Song Li Signed-off-by: Song Li Signed-off-by: Fan Wu Signed-off-by: Paulo Alcantara --- fs/smb/client/file.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index 27b58d907203..1aa4844f8b8a 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -1515,11 +1515,18 @@ int cifs_close(struct inode *inode, struct file *file) trace_smb3_close_cached(tcon->tid, tcon->ses->Suid, cfile->fid.persistent_fid, cifs_sb->ctx->closetimeo); - queue_delayed_work(deferredclose_wq, - &cfile->deferred, cifs_sb->ctx->closetimeo); - cfile->deferred_close_scheduled = true; - spin_unlock(&cinode->deferred_lock); - return 0; + /* + * Each queued execution owns one reference. + * If nothing was queued, the reference of + * the closing file is dropped below. + */ + if (queue_delayed_work(deferredclose_wq, + &cfile->deferred, + cifs_sb->ctx->closetimeo)) { + cfile->deferred_close_scheduled = true; + spin_unlock(&cinode->deferred_lock); + return 0; + } } spin_unlock(&cinode->deferred_lock); _cifsFileInfo_put(cfile, true, false); From cf4d35896621b7298eef51b7a465e5c0cb22f670 Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Sun, 6 Sep 2026 14:39:44 -0300 Subject: [PATCH 09/18] smb: client: fix uid/gid override in getattr with posix extensions When mounting with 'multiuser,posix' options, cifs_getattr() overrides the server-provided uid/gid with the current process's fsuid/fsgid. This is because the condition only checks for unix extensions (tcon->unix_ext) but not posix extensions (tcon->posix_extensions). With SMB3 POSIX extensions, the server provides real uid/gid values just like with unix extensions, so they should be preserved rather than replaced with the caller's credentials. Add a tcon->posix_extensions check to the condition so that uid/gid from the server are properly reported in stat results. Reported-by: Arthur Lesuisse Closes: https://lore.kernel.org/r/DB9P190MB2012266F6B8DECBE5D26A1798DB52@DB9P190MB2012.EURP190.PROD.OUTLOOK.COM Suggested-by: Arthur Lesuisse Reviewed-by: Namjae Jeon Signed-off-by: Paulo Alcantara Cc: Ronnie Sahlberg Cc: Shyam Prasad N Cc: Tom Talpey Cc: Bharath SM Cc: stable@vger.kernel.org --- fs/smb/client/inode.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c index 12ed8db10e00..49f9993ad567 100644 --- a/fs/smb/client/inode.c +++ b/fs/smb/client/inode.c @@ -2992,14 +2992,14 @@ int cifs_getattr(struct mnt_idmap *idmap, const struct path *path, stat->attributes |= STATX_ATTR_ENCRYPTED; /* - * If on a multiuser mount without unix extensions or cifsacl being - * enabled, and the admin hasn't overridden them, set the ownership - * to the fsuid/fsgid of the current process. + * If on a multiuser mount without unix extensions, posix extensions + * or cifsacl being enabled, and the admin hasn't overridden them, + * set the ownership to the fsuid/fsgid of the current process. */ sbflags = cifs_sb_flags(cifs_sb); if ((sbflags & CIFS_MOUNT_MULTIUSER) && !(sbflags & CIFS_MOUNT_CIFS_ACL) && - !tcon->unix_ext) { + !tcon->unix_ext && !tcon->posix_extensions) { if (!(sbflags & CIFS_MOUNT_OVERR_UID)) stat->uid = current_fsuid(); if (!(sbflags & CIFS_MOUNT_OVERR_GID)) From 18a72975e9f35aadecc75b031f693f2d1f49308f Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Sun, 6 Sep 2026 14:40:04 -0300 Subject: [PATCH 10/18] smb: client: honor forceuid/forcegid when mapping SIDs to uid/gid When the administrator mounts with forceuid or forcegid (uid=/gid= mount options), they expect all files to appear owned by the specified user/group. However, several code paths unconditionally called sid_to_id() to overwrite cf_uid/cf_gid with server-provided values, ignoring the administrator's explicit override: - smb311_posix_info_to_fattr() (stat via POSIX extensions) - cifs_posix_to_fattr() (readdir via POSIX extensions) - parse_sec_desc() (CIFS ACL ownership mapping) This allowed an untrusted server to dictate local file ownership even when the mount was configured to force specific uid/gid values. Fix all three call sites to check CIFS_MOUNT_OVERR_UID and CIFS_MOUNT_OVERR_GID before calling sid_to_id(), following the same pattern already used by cifs_unix_basic_to_fattr() for unix extensions. Closes: https://sashiko.dev/#/patchset/20260906155816.603278-1-pc%40manguebit.org Reviewed-by: Namjae Jeon Signed-off-by: Paulo Alcantara Cc: Ronnie Sahlberg Cc: Shyam Prasad N Cc: Tom Talpey Cc: Bharath SM Cc: stable@vger.kernel.org --- fs/smb/client/cifsacl.c | 29 ++++++++++++++++++----------- fs/smb/client/inode.c | 9 +++++++-- fs/smb/client/readdir.c | 9 +++++++-- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c index 213a421bf8e9..def8908dd7e9 100644 --- a/fs/smb/client/cifsacl.c +++ b/fs/smb/client/cifsacl.c @@ -1346,6 +1346,7 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb, { int rc = 0; struct smb_sid *owner_sid_ptr, *group_sid_ptr; + unsigned int sbflags = cifs_sb_flags(cifs_sb); struct smb_acl *dacl_ptr; /* no need for SACL ptr */ char *end_of_acl; __u32 dacloffset, osidoffset, gsidoffset; @@ -1364,17 +1365,21 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb, cifs_dbg(NOISY, "revision %d type 0x%x ooffset 0x%x goffset 0x%x sacloffset 0x%x dacloffset 0x%x\n", pntsd->revision, pntsd->type, osidoffset, gsidoffset, le32_to_cpu(pntsd->sacloffset), dacloffset); -/* cifs_dump_mem("owner_sid: ", owner_sid_ptr, 64); */ + fattr->cf_uid = cifs_sb->ctx->linux_uid; + fattr->cf_gid = cifs_sb->ctx->linux_gid; + rc = sid_from_sd(pntsd, acl_len, osidoffset, &owner_sid_ptr); if (rc) { cifs_dbg(FYI, "%s: Error %d parsing Owner SID\n", __func__, rc); return rc; } - rc = sid_to_id(cifs_sb, owner_sid_ptr, fattr, SIDOWNER); - if (rc) { - cifs_dbg(FYI, "%s: Error %d mapping Owner SID to uid\n", - __func__, rc); - return rc; + if (!(sbflags & CIFS_MOUNT_OVERR_UID)) { + rc = sid_to_id(cifs_sb, owner_sid_ptr, fattr, SIDOWNER); + if (rc) { + cifs_dbg(FYI, "%s: Error %d mapping Owner SID to uid\n", + __func__, rc); + return rc; + } } rc = sid_from_sd(pntsd, acl_len, gsidoffset, &group_sid_ptr); @@ -1383,11 +1388,13 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb, __func__, rc); return rc; } - rc = sid_to_id(cifs_sb, group_sid_ptr, fattr, SIDGROUP); - if (rc) { - cifs_dbg(FYI, "%s: Error %d mapping Group SID to gid\n", - __func__, rc); - return rc; + if (!(sbflags & CIFS_MOUNT_OVERR_GID)) { + rc = sid_to_id(cifs_sb, group_sid_ptr, fattr, SIDGROUP); + if (rc) { + cifs_dbg(FYI, "%s: Error %d mapping Group SID to gid\n", + __func__, rc); + return rc; + } } if (dacloffset) { diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c index 49f9993ad567..1fe0ef0a95db 100644 --- a/fs/smb/client/inode.c +++ b/fs/smb/client/inode.c @@ -851,6 +851,7 @@ static void smb311_posix_info_to_fattr(struct cifs_fattr *fattr, struct smb311_posix_qinfo *info = &data->posix_fi; struct cifs_sb_info *cifs_sb = CIFS_SB(sb); struct cifs_tcon *tcon = cifs_sb_master_tcon(cifs_sb); + unsigned int sbflags = cifs_sb_flags(cifs_sb); memset(fattr, 0, sizeof(*fattr)); @@ -895,8 +896,12 @@ static void smb311_posix_info_to_fattr(struct cifs_fattr *fattr, fattr->cf_symlink_target = data->symlink_target; data->symlink_target = NULL; } - sid_to_id(cifs_sb, &data->posix_owner, fattr, SIDOWNER); - sid_to_id(cifs_sb, &data->posix_group, fattr, SIDGROUP); + fattr->cf_uid = cifs_sb->ctx->linux_uid; + fattr->cf_gid = cifs_sb->ctx->linux_gid; + if (!(sbflags & CIFS_MOUNT_OVERR_UID)) + sid_to_id(cifs_sb, &data->posix_owner, fattr, SIDOWNER); + if (!(sbflags & CIFS_MOUNT_OVERR_GID)) + sid_to_id(cifs_sb, &data->posix_group, fattr, SIDGROUP); cifs_dbg(FYI, "POSIX query info: mode 0x%x uniqueid 0x%llx nlink %d\n", fattr->cf_mode, fattr->cf_uniqueid, fattr->cf_nlink); diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c index 32a75afca8f5..1ea84f4ada39 100644 --- a/fs/smb/client/readdir.c +++ b/fs/smb/client/readdir.c @@ -242,6 +242,7 @@ static void cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info, struct cifs_sb_info *cifs_sb) { + unsigned int sbflags = cifs_sb_flags(cifs_sb); struct smb2_posix_info_parsed parsed; posix_info_parse(info, NULL, &parsed); @@ -281,8 +282,12 @@ cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info, le32_to_cpu(info->ReparseTag), le32_to_cpu(info->Mode)); - sid_to_id(cifs_sb, &parsed.owner, fattr, SIDOWNER); - sid_to_id(cifs_sb, &parsed.group, fattr, SIDGROUP); + fattr->cf_uid = cifs_sb->ctx->linux_uid; + fattr->cf_gid = cifs_sb->ctx->linux_gid; + if (!(sbflags & CIFS_MOUNT_OVERR_UID)) + sid_to_id(cifs_sb, &parsed.owner, fattr, SIDOWNER); + if (!(sbflags & CIFS_MOUNT_OVERR_GID)) + sid_to_id(cifs_sb, &parsed.group, fattr, SIDGROUP); } static void __dir_info_to_fattr(struct cifs_fattr *fattr, const void *info) From cd2b2b57921d4caa7875e83198bb2aa71254328b Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Sun, 6 Sep 2026 14:40:23 -0300 Subject: [PATCH 11/18] smb: client: fix WSL reparse point uid/gid override wsl_to_fattr() unconditionally overwrites cf_uid/cf_gid with values from WSL extended attributes ($LXUID/$LXGID), ignoring the forceuid and forcegid mount options. Fix this by initializing cf_uid/cf_gid to the mount defaults and gating the $LXUID/$LXGID EA parsing on forceuid/forcegid. Closes: https://sashiko.dev/#/patchset/20260906190803.667489-1-pc%40manguebit.org Reviewed-by: Namjae Jeon Signed-off-by: Paulo Alcantara Cc: Ronnie Sahlberg Cc: Shyam Prasad N Cc: Tom Talpey Cc: Bharath SM Cc: stable@vger.kernel.org --- fs/smb/client/reparse.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c index 5cc5b0410d48..178da801e775 100644 --- a/fs/smb/client/reparse.c +++ b/fs/smb/client/reparse.c @@ -1137,10 +1137,14 @@ static bool wsl_to_fattr(struct cifs_open_info_data *data, struct cifs_sb_info *cifs_sb, u32 tag, struct cifs_fattr *fattr) { + unsigned int sbflags = cifs_sb_flags(cifs_sb); struct smb2_file_full_ea_info *ea; bool have_xattr_dev = false; u32 next = 0; + fattr->cf_uid = cifs_sb->ctx->linux_uid; + fattr->cf_gid = cifs_sb->ctx->linux_gid; + switch (tag) { case IO_REPARSE_TAG_LX_SYMLINK: fattr->cf_mode |= S_IFLNK; @@ -1177,11 +1181,13 @@ static bool wsl_to_fattr(struct cifs_open_info_data *data, nlen = ea->ea_name_length; v = (void *)((u8 *)ea->ea_data + ea->ea_name_length + 1); - if (!strncmp(name, SMB2_WSL_XATTR_UID, nlen)) - fattr->cf_uid = wsl_make_kuid(cifs_sb, v); - else if (!strncmp(name, SMB2_WSL_XATTR_GID, nlen)) - fattr->cf_gid = wsl_make_kgid(cifs_sb, v); - else if (!strncmp(name, SMB2_WSL_XATTR_MODE, nlen)) { + if (!strncmp(name, SMB2_WSL_XATTR_UID, nlen)) { + if (!(sbflags & CIFS_MOUNT_OVERR_UID)) + fattr->cf_uid = wsl_make_kuid(cifs_sb, v); + } else if (!strncmp(name, SMB2_WSL_XATTR_GID, nlen)) { + if (!(sbflags & CIFS_MOUNT_OVERR_GID)) + fattr->cf_gid = wsl_make_kgid(cifs_sb, v); + } else if (!strncmp(name, SMB2_WSL_XATTR_MODE, nlen)) { /* File type in reparse point tag and in xattr mode must match. */ if (S_DT(fattr->cf_mode) != S_DT(le32_to_cpu(*(__le32 *)v))) return false; From da6e25842431982d5a53cf00d925b98c690f4467 Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Sun, 6 Sep 2026 14:40:39 -0300 Subject: [PATCH 12/18] smb: client: avoid using uninitialized SIDs in cifs_posix_to_fattr() cifs_posix_to_fattr() ignores the return value of posix_info_parse(). When a malformed POSIX directory entry is encountered (e.g. invalid SID lengths from an untrusted server), posix_info_parse() returns -1 without populating the 'parsed' struct. The uninitialized stack memory in parsed.owner and parsed.group is then passed to sid_to_id(), which processes the garbage bytes and passes them to request_key() to construct a SID string, potentially leaking kernel stack contents to the userspace idmap daemon. Fix this by checking the return value and skipping the SID-to-id mapping when parsing fails. The remaining fattr fields (timestamps, mode, etc.) are populated directly from the 'info' pointer so they are unaffected. Closes: https://sashiko.dev/#/patchset/20260906172005.627163-1-pc%40manguebit.org Closes: https://sashiko.dev/#/patchset/20260906181540.647469-1-pc%40manguebit.org Reviewed-by: Namjae Jeon Signed-off-by: Paulo Alcantara Cc: Ronnie Sahlberg Cc: Shyam Prasad N Cc: Tom Talpey Cc: Bharath SM Cc: stable@vger.kernel.org --- fs/smb/client/readdir.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/fs/smb/client/readdir.c b/fs/smb/client/readdir.c index 1ea84f4ada39..9530e5b01564 100644 --- a/fs/smb/client/readdir.c +++ b/fs/smb/client/readdir.c @@ -244,8 +244,9 @@ cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info, { unsigned int sbflags = cifs_sb_flags(cifs_sb); struct smb2_posix_info_parsed parsed; + int rc; - posix_info_parse(info, NULL, &parsed); + rc = posix_info_parse(info, NULL, &parsed); memset(fattr, 0, sizeof(*fattr)); fattr->cf_uniqueid = le64_to_cpu(info->Inode); @@ -284,10 +285,15 @@ cifs_posix_to_fattr(struct cifs_fattr *fattr, struct smb2_posix_info *info, fattr->cf_uid = cifs_sb->ctx->linux_uid; fattr->cf_gid = cifs_sb->ctx->linux_gid; - if (!(sbflags & CIFS_MOUNT_OVERR_UID)) - sid_to_id(cifs_sb, &parsed.owner, fattr, SIDOWNER); - if (!(sbflags & CIFS_MOUNT_OVERR_GID)) - sid_to_id(cifs_sb, &parsed.group, fattr, SIDGROUP); + if (rc < 0) { + cifs_dbg(VFS, "%s: failed to parse SIDs: %d\n", + __func__, rc); + } else { + if (!(sbflags & CIFS_MOUNT_OVERR_UID)) + sid_to_id(cifs_sb, &parsed.owner, fattr, SIDOWNER); + if (!(sbflags & CIFS_MOUNT_OVERR_GID)) + sid_to_id(cifs_sb, &parsed.group, fattr, SIDGROUP); + } } static void __dir_info_to_fattr(struct cifs_fattr *fattr, const void *info) From fa7a2cfcf1e6117fc478cae6809c66c518740969 Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Sun, 6 Sep 2026 16:01:04 -0300 Subject: [PATCH 13/18] smb: client: fix file type corruption in wsl_to_fattr() Setting the file type in cf_mode without clearing the existing S_IFMT bits first is wrong as it corrupts the file type when cf_mode already has type bits set (e.g. S_IFREG | S_IFCHR == S_IFLNK). Clear S_IFMT before the switch statement. Closes: https://sashiko.dev/#/patchset/20260906172005.627163-1-pc%40manguebit.org Reviewed-by: Namjae Jeon Signed-off-by: Paulo Alcantara Cc: Ronnie Sahlberg Cc: Shyam Prasad N Cc: Tom Talpey Cc: Bharath SM Cc: stable@vger.kernel.org --- fs/smb/client/reparse.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c index 178da801e775..8a19dee564b8 100644 --- a/fs/smb/client/reparse.c +++ b/fs/smb/client/reparse.c @@ -1145,6 +1145,7 @@ static bool wsl_to_fattr(struct cifs_open_info_data *data, fattr->cf_uid = cifs_sb->ctx->linux_uid; fattr->cf_gid = cifs_sb->ctx->linux_gid; + fattr->cf_mode &= ~S_IFMT; switch (tag) { case IO_REPARSE_TAG_LX_SYMLINK: fattr->cf_mode |= S_IFLNK; From 65d5dbdc089be42fc48a6f77bc6b648307f34b17 Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Sun, 6 Sep 2026 16:01:16 -0300 Subject: [PATCH 14/18] smb: client: fix file type corruption in posix_reparse_to_fattr() Setting the file type in cf_mode without clearing the existing S_IFMT bits first is wrong as it corrupts the file type when cf_mode already has type bits set (e.g. S_IFREG | S_IFCHR == S_IFLNK). Use a local ftype variable to collect the new file type and apply it after validation succeeds, clearing S_IFMT and setting the new type in a single assignment. This avoids stripping cf_mode on malformed reparse points where the function returns false early. Closes: https://sashiko.dev/#/patchset/20260906172005.627163-1-pc%40manguebit.org Reviewed-by: Namjae Jeon Signed-off-by: Paulo Alcantara Cc: Ronnie Sahlberg Cc: Shyam Prasad N Cc: Tom Talpey Cc: Bharath SM Cc: stable@vger.kernel.org --- fs/smb/client/reparse.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c index 8a19dee564b8..616ca2dbfac4 100644 --- a/fs/smb/client/reparse.c +++ b/fs/smb/client/reparse.c @@ -1212,6 +1212,7 @@ static bool posix_reparse_to_fattr(struct cifs_sb_info *cifs_sb, struct cifs_open_info_data *data) { struct reparse_nfs_data_buffer *buf = (struct reparse_nfs_data_buffer *)data->reparse.buf; + umode_t ftype; if (buf == NULL) return true; @@ -1227,7 +1228,7 @@ static bool posix_reparse_to_fattr(struct cifs_sb_info *cifs_sb, WARN_ON_ONCE(1); return false; } - fattr->cf_mode |= S_IFCHR; + ftype = S_IFCHR; fattr->cf_rdev = reparse_mkdev(buf->DataBuffer); break; case NFS_SPECFILE_BLK: @@ -1235,22 +1236,23 @@ static bool posix_reparse_to_fattr(struct cifs_sb_info *cifs_sb, WARN_ON_ONCE(1); return false; } - fattr->cf_mode |= S_IFBLK; + ftype = S_IFBLK; fattr->cf_rdev = reparse_mkdev(buf->DataBuffer); break; case NFS_SPECFILE_FIFO: - fattr->cf_mode |= S_IFIFO; + ftype = S_IFIFO; break; case NFS_SPECFILE_SOCK: - fattr->cf_mode |= S_IFSOCK; + ftype = S_IFSOCK; break; case NFS_SPECFILE_LNK: - fattr->cf_mode |= S_IFLNK; + ftype = S_IFLNK; break; default: WARN_ON_ONCE(1); return false; } + fattr->cf_mode = (fattr->cf_mode & ~S_IFMT) | ftype; return true; } From 6bd360447941357e959414a525aa62576a448116 Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Sun, 6 Sep 2026 16:01:24 -0300 Subject: [PATCH 15/18] smb: client: fix file type corruption in cifs_reparse_point_to_fattr() Setting the file type in cf_mode without clearing the existing S_IFMT bits first is wrong as it corrupts the file type when cf_mode already has type bits set (e.g. S_IFREG | S_IFLNK == S_IFDIR | S_IFREG). Clear S_IFMT before setting S_IFLNK for native and SMB1 symlinks. Closes: https://sashiko.dev/#/patchset/20260906181540.647469-1-pc%40manguebit.org Reviewed-by: Namjae Jeon Signed-off-by: Paulo Alcantara Cc: Ronnie Sahlberg Cc: Shyam Prasad N Cc: Tom Talpey Cc: Bharath SM Cc: stable@vger.kernel.org --- fs/smb/client/reparse.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c index 616ca2dbfac4..b6bded042e78 100644 --- a/fs/smb/client/reparse.c +++ b/fs/smb/client/reparse.c @@ -1280,6 +1280,7 @@ bool cifs_reparse_point_to_fattr(struct cifs_sb_info *cifs_sb, break; case 0: /* SMB1 symlink */ case IO_REPARSE_TAG_SYMLINK: + fattr->cf_mode &= ~S_IFMT; fattr->cf_mode |= S_IFLNK; break; default: From 0ee150794c75bcd0be0e24ff3394f433cbae18cc Mon Sep 17 00:00:00 2001 From: Bjoern Doebel Date: Tue, 8 Sep 2026 16:10:00 +0000 Subject: [PATCH 16/18] smb: client: fix heap overflow in DACL owner/group rewrite When id_mode_to_cifs_acl rewrites an existing DACL, it allocates a buffer sized according to the on-disk DACL length reported by dacl_ptr->size. However, replace_sids_and_copy_aces may rewrite each ACE with a new owner/group SID obtained from the cifs.idmap upcall. Those SIDs can have up to SID_MAX_SUB_AUTHORITIES (15) sub-authorities, making each ACE up to 76 bytes (sizeof(struct smb_ace)). If the original DACL contains short SIDs (e.g., 1 sub-authority) while the replacement SIDs are long, the rewritten ACEs overflow the allocation. Fix this by always budgeting for worst-case SID expansion: allocate sizeof(struct smb_acl) plus num_aces * sizeof(struct smb_ace), which covers the smb_acl header and room for every ACE at maximum SID size. This replaces the previous split logic that used dacl_ptr->size for cifsacl mounts but num_aces * sizeof(struct smb_ace) for mode_from_sid mounts: both paths can trigger the same rewrite and need the same headroom. KASAN reports this as: BUG: KASAN: slab-out-of-bounds in build_sec_desc+0x1e8a/0x2680 [cifs] Write of size 4 at addr ffff8881a5e25374 by task chown/5298 ... The buggy address is located 0 bytes to the right of allocated 884-byte region [ffff8881a5e25000, ffff8881a5e25374) Cc: stable@vger.kernel.org Fixes: bc3e9dd9d104 ("cifs: Change SIDs in ACEs while transferring file ownership.") Assisted-by: Kiro:claude-opus-4.6 Signed-off-by: Bjoern Doebel Reviewed-by: Namjae Jeon Fixes: 5c3564852c58 ("cifs: Minimize the number of cifs_acl memory allocations") Signed-off-by: Paulo Alcantara --- fs/smb/client/cifsacl.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c index def8908dd7e9..3e96e151df35 100644 --- a/fs/smb/client/cifsacl.c +++ b/fs/smb/client/cifsacl.c @@ -1837,11 +1837,13 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode, cifs_put_tlink(tlink); return rc; } - if (mode_from_sid) - nsecdesclen += - le16_to_cpu(dacl_ptr->num_aces) * sizeof(struct smb_ace); - else /* cifsacl */ - nsecdesclen += le16_to_cpu(dacl_ptr->size); + /* + * Worst case: every ACE is rewritten with a new SID of + * SID_MAX_SUB_AUTHORITIES sub-auths -> sizeof(smb_ace) each, + * plus the smb_acl header replace_sids_and_copy_aces() emits. + */ + nsecdesclen += sizeof(struct smb_acl) + + le16_to_cpu(dacl_ptr->num_aces) * sizeof(struct smb_ace); } } From d05045177a855386bca5e1909e08d06290e6e3b3 Mon Sep 17 00:00:00 2001 From: Bjoern Doebel Date: Tue, 8 Sep 2026 16:10:01 +0000 Subject: [PATCH 17/18] smb: client: fail DACL rewrite when the new DACL exceeds 64K replace_sids_and_copy_aces() and set_chmod_dacl() accumulate the size of the DACL they build in a u16. That accumulator can wrap. validate_dacl() caps num_aces at (dacl_size - sizeof(struct smb_acl)) / 20, i.e. 3276 for a maximally sized DACL, while each rewritten ACE can grow to sizeof(struct smb_ace) (76 bytes) once its SID is replaced with one carrying SID_MAX_SUB_AUTHORITIES sub-authorities. The worst case is therefore sizeof(struct smb_acl) + 3276 * 76 = 248984 bytes, far beyond what a u16 can hold. A wraparound is reached with 863 ACEs. After the wraparound, ndacl_ptr->size becomes meaningless and the offset will point anywhere in the ACE array. As a result, we will see corruption of the DACL, which then gets sent to the server. This is not an out-of-bounds write as the allocation now covers the worst-case expansion, so writes will always go into the buffer. Adjust the code to use a u32 internally and return -EOVERFLOW in the overflow case. The operation must be refused, because a DACL can only hold 2^16-1 bytes on the wire and larger DACLs cannot be represented. set_chmod_dacl() carries the same pattern and is fixed the same way. It only wraps once the source DACL comes within roughly 380 bytes of the 64K ceiling, but the failure mode is identical. Suggested-by: Namjae Jeon Cc: stable@vger.kernel.org Fixes: f5065508897a ("cifs: Retain old ACEs when converting between mode bits and ACL.") Assisted-by: Kiro:claude-opus-5 Signed-off-by: Bjoern Doebel Reviewed-by: Namjae Jeon Signed-off-by: Paulo Alcantara --- fs/smb/client/cifsacl.c | 39 ++++++++++++++++++++++++++------------- 1 file changed, 26 insertions(+), 13 deletions(-) diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c index 3e96e151df35..c5e47a835f99 100644 --- a/fs/smb/client/cifsacl.c +++ b/fs/smb/client/cifsacl.c @@ -1096,13 +1096,13 @@ unsigned int setup_special_user_owner_ACE(struct smb_ace *pntace) static void populate_new_aces(char *nacl_base, struct smb_sid *pownersid, struct smb_sid *pgrpsid, - __u64 *pnmode, u16 *pnum_aces, u16 *pnsize, + __u64 *pnmode, u16 *pnum_aces, u32 *pnsize, bool modefromsid, bool posix) { __u64 nmode; u16 num_aces = 0; - u16 nsize = 0; + u32 nsize = 0; __u64 user_mode; __u64 group_mode; __u64 other_mode; @@ -1201,17 +1201,17 @@ static void populate_new_aces(char *nacl_base, *pnsize = nsize; } -static __u16 replace_sids_and_copy_aces(struct smb_acl *pdacl, struct smb_acl *pndacl, - struct smb_sid *pownersid, struct smb_sid *pgrpsid, - struct smb_sid *pnownersid, struct smb_sid *pngrpsid, - int *aclflag) +static int replace_sids_and_copy_aces(struct smb_acl *pdacl, struct smb_acl *pndacl, + struct smb_sid *pownersid, struct smb_sid *pgrpsid, + struct smb_sid *pnownersid, struct smb_sid *pngrpsid, + int *aclflag, u16 *pnsize) { int i; u16 size = 0; struct smb_ace *pntace = NULL; char *acl_base = NULL; u16 src_num_aces = 0; - u16 nsize = 0; + u32 nsize = 0; struct smb_ace *pnntace = NULL; char *nacl_base = NULL; u16 ace_size = 0; @@ -1240,9 +1240,12 @@ static __u16 replace_sids_and_copy_aces(struct smb_acl *pdacl, struct smb_acl *p size += le16_to_cpu(pntace->size); nsize += ace_size; + if (nsize > U16_MAX) + return -EOVERFLOW; } - return nsize; + *pnsize = nsize; + return 0; } static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl, @@ -1254,7 +1257,7 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl, struct smb_ace *pntace = NULL; char *acl_base = NULL; u16 src_num_aces = 0; - u16 nsize = 0; + u32 nsize = 0; struct smb_ace *pnntace = NULL; char *nacl_base = NULL; u16 num_aces = 0; @@ -1305,6 +1308,8 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl, nsize += cifs_copy_ace(pnntace, pntace, NULL); num_aces++; + if (nsize > U16_MAX) + return -EOVERFLOW; next_ace: size += le16_to_cpu(pntace->size); @@ -1321,6 +1326,10 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl, } finalize_dacl: + /* The DACL size field is 16-bit on the wire, see MS-DTYP 2.4.5 */ + if (nsize > U16_MAX) + return -EOVERFLOW; + pndacl->num_aces = cpu_to_le16(num_aces); pndacl->size = cpu_to_le16(nsize); @@ -1473,6 +1482,8 @@ static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd, rc = set_chmod_dacl(dacl_ptr, ndacl_ptr, owner_sid_ptr, group_sid_ptr, pnmode, mode_from_sid, posix); + if (rc) + return rc; sidsoffset = ndacloffset + le16_to_cpu(ndacl_ptr->size); /* copy the non-dacl portion of secdesc */ @@ -1548,10 +1559,12 @@ static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd, if (dacloffset) { /* Replace ACEs for old owner with new one */ - size = replace_sids_and_copy_aces(dacl_ptr, ndacl_ptr, - owner_sid_ptr, group_sid_ptr, - nowner_sid_ptr, ngroup_sid_ptr, - aclflag); + rc = replace_sids_and_copy_aces(dacl_ptr, ndacl_ptr, + owner_sid_ptr, group_sid_ptr, + nowner_sid_ptr, ngroup_sid_ptr, + aclflag, &size); + if (rc) + goto chown_chgrp_exit; ndacl_ptr->size = cpu_to_le16(size); } From cb26524ef4ac28fcfa554c0656e8dc412c38a8ff Mon Sep 17 00:00:00 2001 From: Paulo Alcantara Date: Wed, 9 Sep 2026 17:02:40 -0300 Subject: [PATCH 18/18] smb: client: fix one-byte OOB read in smb2_parse_native_symlink() When parsing a share-root relative native symlink, memcpy copies smb_target+1 (skipping the leading separator) but uses strlen(smb_target)+1 as the length, reading one byte past the allocated buffer. This fixes the following KASAN splat when accessing an SMB symlink with a target of '\a\b': BUG: KASAN: slab-out-of-bounds in smb2_parse_native_symlink+0x4f5/0xca0 Read of size 5 at addr ffff88800878fe21 by task netfsfuzz-execu/1 CPU: 1 UID: 0 PID: 1 Comm: netfsfuzz-execu Tainted: G N 7.2.0-11943-g2709dd5ae32f-dirty #1 PREEMPT(lazy) Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996) Call Trace: dump_stack_lvl+0x7b/0xa0 print_report+0xd0/0x630 kasan_report+0xe5/0x120 kasan_check_range+0x105/0x1b0 __asan_memcpy+0x23/0x60 smb2_parse_native_symlink+0x4f5/0xca0 parse_reparse_point+0x68a/0x1530 reparse_info_to_fattr+0x752/0xa20 cifs_get_fattr+0x873/0x15b0 cifs_get_inode_info+0xc0/0x310 cifs_lookup+0x308/0xa70 __lookup_slow+0x122/0x2b0 lookup_slow+0x50/0x70 path_lookupat+0x525/0xaf0 filename_lookup+0x1f2/0x550 vfs_statx+0xd1/0x1a0 vfs_fstatat+0x65/0xc0 __do_sys_newfstatat+0x9a/0x120 do_syscall_64+0xdd/0x4a0 entry_SYSCALL_64_after_hwframe+0x77/0x7f Reported-by: Yuanfu Xie Fixes: 723f4ef90452 ("cifs: Fix parsing native symlinks relative to the export") Suggested-by: Pali Rohar Reviewed-by: Pali Rohar Reviewed-by: Namjae Jeon Signed-off-by: Paulo Alcantara Cc: Ronnie Sahlberg Cc: Shyam Prasad N Cc: Tom Talpey Cc: Bharath SM Cc: stable@vger.kernel.org --- fs/smb/client/reparse.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/fs/smb/client/reparse.c b/fs/smb/client/reparse.c index b6bded042e78..8a1b9e8be5ba 100644 --- a/fs/smb/client/reparse.c +++ b/fs/smb/client/reparse.c @@ -971,7 +971,8 @@ int smb2_parse_native_symlink(char **target, const char *buf, unsigned int len, linux_target[i*3 + 1] = '.'; linux_target[i*3 + 2] = sep; } - memcpy(linux_target + levels*3, smb_target+1, smb_target_len); /* +1 to skip leading sep */ + /* +1 to skip leading sep */ + memcpy(linux_target + levels*3, smb_target+1, smb_target_len-1); } else { /* * This is either an absolute symlink in POSIX-style format