From f71d68d2b473e47db260c27c98212829590d8bae Mon Sep 17 00:00:00 2001 From: Shyam Prasad N Date: Thu, 14 May 2026 23:38:07 +0530 Subject: [PATCH 01/14] cifs: invalidate cfid on unlink/rename/rmdir Today we do not invalidate the cached_dirent or the entire parent cfid when a dentry in a dir has been removed/moved. This change invalidates the parent cfid so that we don't serve directory contents from the cache. Cc: Signed-off-by: Shyam Prasad N Signed-off-by: Steve French --- fs/smb/client/inode.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/fs/smb/client/inode.c b/fs/smb/client/inode.c index 9472c0a6c187..0af93e881608 100644 --- a/fs/smb/client/inode.c +++ b/fs/smb/client/inode.c @@ -28,6 +28,23 @@ #include "cached_dir.h" #include "reparse.h" +static void cifs_invalidate_cached_dir(struct cifs_tcon *tcon, + struct dentry *parent) +{ + struct cached_fid *parent_cfid = NULL; + + if (!tcon || !parent) + return; + + if (!open_cached_dir_by_dentry(tcon, parent, &parent_cfid)) { + mutex_lock(&parent_cfid->dirents.de_mutex); + parent_cfid->dirents.is_valid = false; + parent_cfid->dirents.is_failed = true; + mutex_unlock(&parent_cfid->dirents.de_mutex); + close_cached_dir(parent_cfid); + } +} + /* * Set parameters for the netfs library */ @@ -2067,6 +2084,9 @@ static int __cifs_unlink(struct inode *dir, struct dentry *dentry, bool sillyren cifs_set_file_info(inode, attrs, xid, full_path, origattr); out_reval: + if (!rc && dentry->d_parent) + cifs_invalidate_cached_dir(tcon, dentry->d_parent); + if (inode) { cifs_inode = CIFS_I(inode); cifs_inode->time = 0; /* will force revalidate to get info @@ -2378,7 +2398,6 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry) } rc = server->ops->rmdir(xid, tcon, full_path, cifs_sb); - cifs_put_tlink(tlink); cifsInode = CIFS_I(d_inode(direntry)); @@ -2388,6 +2407,8 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry) i_size_write(d_inode(direntry), 0); clear_nlink(d_inode(direntry)); spin_unlock(&d_inode(direntry)->i_lock); + if (direntry->d_parent) + cifs_invalidate_cached_dir(tcon, direntry->d_parent); } /* force revalidate to go get info when needed */ @@ -2402,6 +2423,7 @@ int cifs_rmdir(struct inode *inode, struct dentry *direntry) inode_set_ctime_current(d_inode(direntry)); inode_set_mtime_to_ts(inode, inode_set_ctime_current(inode)); + cifs_put_tlink(tlink); rmdir_exit: free_dentry_path(page); @@ -2668,6 +2690,12 @@ cifs_rename2(struct mnt_idmap *idmap, struct inode *source_dir, } /* force revalidate to go get info when needed */ + if (!rc) { + cifs_invalidate_cached_dir(tcon, source_dentry->d_parent); + if (target_dentry->d_parent != source_dentry->d_parent) + cifs_invalidate_cached_dir(tcon, target_dentry->d_parent); + } + CIFS_I(source_dir)->time = CIFS_I(target_dir)->time = 0; cifs_rename_exit: From 10ce03879f935f756bc8a386b3fa3a1c7264d950 Mon Sep 17 00:00:00 2001 From: Henrique Carvalho Date: Wed, 27 May 2026 09:26:42 -0300 Subject: [PATCH 02/14] smb: client: fix conflicting option validation for new mount API Apply conflicting option validation consistently across all the new mount API paths, for both mount and remount. Some checks were only applied during initial mount validation, while others were handled during option parsing, causing mount and remount/reconfigure to behave differently. Move the conflicting option checks into smb3_handle_conflicting_options() and call it from the common validation paths, including for multichannel/max_channels handling. Fixes: 24e0a1eff9e2 ("cifs: switch to new mount api") Signed-off-by: Henrique Carvalho Signed-off-by: Steve French --- fs/smb/client/fs_context.c | 102 +++++++++++++++++++------------------ 1 file changed, 53 insertions(+), 49 deletions(-) diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c index 2f86158f85d7..fd4b13cd654d 100644 --- a/fs/smb/client/fs_context.c +++ b/fs/smb/client/fs_context.c @@ -693,6 +693,41 @@ static int smb3_handle_conflicting_options(struct fs_context *fc) { struct smb3_fs_context *ctx = smb3_fc2context(fc); + if (ctx->rdma && ctx->vals->protocol_id < SMB30_PROT_ID) { + cifs_errorf(fc, "SMB Direct requires Version >=3.0\n"); + return -EOPNOTSUPP; + } + + if (ctx->multiuser && !IS_ENABLED(CONFIG_KEYS)) { + cifs_errorf(fc, "Multiuser mounts require kernels with CONFIG_KEYS enabled\n"); + return -EOPNOTSUPP; + } + + if (ctx->multiuser && ctx->upcall_target == UPTARGET_MOUNT) { + cifs_errorf(fc, "multiuser mount option not supported with upcalltarget set as 'mount'\n"); + return -EINVAL; + } + + if (ctx->uid_specified && !ctx->forceuid_specified) { + ctx->override_uid = 1; + pr_notice("enabling forceuid mount option implicitly because uid= option is specified\n"); + } + + if (ctx->gid_specified && !ctx->forcegid_specified) { + ctx->override_gid = 1; + pr_notice("enabling forcegid mount option implicitly because gid= option is specified\n"); + } + + if (ctx->override_uid && !ctx->uid_specified) { + ctx->override_uid = 0; + pr_notice("ignoring forceuid mount option specified with no uid= option\n"); + } + + if (ctx->override_gid && !ctx->gid_specified) { + ctx->override_gid = 0; + pr_notice("ignoring forcegid mount option specified with no gid= option\n"); + } + if (ctx->multichannel_specified) { if (ctx->multichannel) { if (!ctx->max_channels_specified) { @@ -711,19 +746,14 @@ static int smb3_handle_conflicting_options(struct fs_context *fc) return -EINVAL; } } - } else { - if (ctx->max_channels_specified) { - if (ctx->max_channels > 1) - ctx->multichannel = true; - else - ctx->multichannel = false; - } else { + } else if (ctx->max_channels_specified) { + if (ctx->max_channels > 1) + ctx->multichannel = true; + else ctx->multichannel = false; - ctx->max_channels = 1; - } } - //resetting default values as remount doesn't initialize fs_context again + /* clear parse-time latches so they don't persist across remounts */ ctx->multichannel_specified = false; ctx->max_channels_specified = false; @@ -804,28 +834,23 @@ static int smb3_fs_context_parse_monolithic(struct fs_context *fc, if (ret < 0) break; } - return ret ?: smb3_handle_conflicting_options(fc); + return ret; } /* - * Validate the preparsed information in the config. + * smb3_fs_context_validate - check initial-mount-only constraints: + * UNC presence, address resolution, dialect warnings + * + * @fc: generic mount context */ static int smb3_fs_context_validate(struct fs_context *fc) { struct smb3_fs_context *ctx = smb3_fc2context(fc); + int rc; - if (ctx->rdma && ctx->vals->protocol_id < SMB30_PROT_ID) { - cifs_errorf(fc, "SMB Direct requires Version >=3.0\n"); - return -EOPNOTSUPP; - } - -#ifndef CONFIG_KEYS - /* Muliuser mounts require CONFIG_KEYS support */ - if (ctx->multiuser) { - cifs_errorf(fc, "Multiuser mounts require kernels with CONFIG_KEYS enabled\n"); - return -1; - } -#endif + rc = smb3_handle_conflicting_options(fc); + if (rc) + return rc; if (ctx->got_version == false) pr_warn_once("No dialect specified on mount. Default has changed to a more secure dialect, SMB2.1 or later (e.g. SMB3.1.1), from CIFS (SMB1). To use the less secure SMB1 dialect to access old servers which do not support SMB3.1.1 (or even SMB3 or SMB2.1) specify vers=1.0 on mount.\n"); @@ -860,26 +885,6 @@ static int smb3_fs_context_validate(struct fs_context *fc) /* set the port that we got earlier */ cifs_set_port((struct sockaddr *)&ctx->dstaddr, ctx->port); - if (ctx->uid_specified && !ctx->forceuid_specified) { - ctx->override_uid = 1; - pr_notice("enabling forceuid mount option implicitly because uid= option is specified\n"); - } - - if (ctx->gid_specified && !ctx->forcegid_specified) { - ctx->override_gid = 1; - pr_notice("enabling forcegid mount option implicitly because gid= option is specified\n"); - } - - if (ctx->override_uid && !ctx->uid_specified) { - ctx->override_uid = 0; - pr_notice("ignoring forceuid mount option specified with no uid= option\n"); - } - - if (ctx->override_gid && !ctx->gid_specified) { - ctx->override_gid = 0; - pr_notice("ignoring forcegid mount option specified with no gid= option\n"); - } - return 0; } @@ -1078,6 +1083,10 @@ static int smb3_reconfigure(struct fs_context *fc) if (rc) return rc; + rc = smb3_handle_conflicting_options(fc); + if (rc) + return rc; + old_ctx = kzalloc_obj(*old_ctx); if (!old_ctx) return -ENOMEM; @@ -1933,11 +1942,6 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, } /* case Opt_ignore: - is ignored as expected ... */ - if (ctx->multiuser && ctx->upcall_target == UPTARGET_MOUNT) { - cifs_errorf(fc, "multiuser mount option not supported with upcalltarget set as 'mount'\n"); - goto cifs_parse_mount_err; - } - return 0; cifs_parse_mount_err: From 6d9a4aaaa8b2612b5ef9d581e2f286a458b71ee1 Mon Sep 17 00:00:00 2001 From: Jian Zhang Date: Tue, 19 May 2026 18:44:22 +0800 Subject: [PATCH 03/14] cifs: remove all cifs files before kill super Cifs files may be put into fileinfo_put_wq during umounting cifs. After umount done, cifsFileInfo_put_final is called, which cause following BUG: BUG: kernel NULL pointer dereference, address: 0000000000000000 ... [ 134.222152] list_lru_add+0x64/0x1a0 [ 134.222399] ? cifs_put_tcon+0x171/0x340 [cifs] [ 134.222772] d_lru_add+0x44/0x60 [ 134.222997] dput+0x1fc/0x210 [ 134.223213] cifsFileInfo_put_final+0x11a/0x140 [cifs] [ 134.223576] process_one_work+0x17c/0x320 [ 134.223843] worker_thread+0x188/0x280 [ 134.224084] ? __pfx_worker_thread+0x10/0x10 [ 134.224366] kthread+0xcc/0x100 [ 134.224576] ? __pfx_kthread+0x10/0x10 [ 134.224827] ret_from_fork+0x30/0x50 [ 134.225063] ? __pfx_kthread+0x10/0x10 [ 134.225328] ret_from_fork_asm+0x1b/0x30 This can be reproduce by following: unshare -n bash -c " mkdir -p ${CIFS_MNT} ip netns attach root 1 ip link add eth0 type veth peer veth0 netns root ip link set eth0 up ip -n root link set veth0 up ip addr add 192.168.0.2/24 dev eth0 ip -n root addr add 192.168.0.1/24 dev veth0 ip route add default via 192.168.0.1 dev eth0 ip netns exec root sysctl net.ipv4.ip_forward=1 ip netns exec root iptables -t nat -A POSTROUTING -s 192.168.0.2 -o ${DEV} -j MASQUERADE mount -t cifs ${CIFS_PATH} ${CIFS_MNT} -o vers=3.0,sec=ntlmssp,credentials=${CIFS_CRED},rsize=65536,wsize=65536,cache=none,echo_interval=1 touch ${CIFS_MNT}/a.txt ip netns exec root iptables -t nat -D POSTROUTING -s 192.168.0.2 -o ${DEV} -j MASQUERADE " umount ${CIFS_MNT} Fixes: 340cea84f691 ("cifs: open files should not hold ref on superblock") Signed-off-by: Jian Zhang Signed-off-by: Steve French --- fs/smb/client/connect.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index dcde25da468d..cbeb5637eeb9 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -3996,6 +3996,9 @@ cifs_umount(struct cifs_sb_info *cifs_sb) } spin_unlock(&cifs_sb->tlink_tree_lock); + flush_workqueue(serverclose_wq); + flush_workqueue(fileinfo_put_wq); + kfree(cifs_sb->prepath); call_rcu(&cifs_sb->rcu, delayed_free); } From ec457f9afe5ae9538bdcd58fd4cb442b9787e183 Mon Sep 17 00:00:00 2001 From: Michael Bommarito Date: Sun, 17 May 2026 20:11:49 -0400 Subject: [PATCH 04/14] smb: client: resolve SWN tcon from live registrations cifs_swn_notify() looks up a witness registration by id under cifs_swnreg_idr_mutex, drops the mutex, and then uses the registration's cached tcon pointer. That pointer is not a lifetime reference, and it is not a stable representative once cifs_get_swn_reg() lets multiple tcons for the same net/share name share one registration id. A same-share second mount can keep the cifs_swn_reg alive after the first tcon unregisters and is freed. The registration then still points at the freed first tcon, so taking tc_lock or incrementing tc_count through swnreg->tcon only moves the use-after-free earlier. Taking tc_lock while holding cifs_swnreg_idr_mutex also violates the documented CIFS lock order. Fix this by making the registration store only the stable witness identity: id, net name, share name, and notify flags. When a notify arrives, copy that identity under cifs_swnreg_idr_mutex, drop the mutex, then find and pin a live witness tcon that currently matches the net/share pair under the normal cifs_tcp_ses_lock -> tc_lock order. The notification path uses that pinned tcon directly and drops the reference when done. Registration and unregister messages now use the live tcon passed by the caller instead of a cached tcon in the registration. The final unregister send is folded into cifs_swn_unregister() while the registration is still protected by cifs_swnreg_idr_mutex. This removes the previous find/drop/reacquire raw-pointer window. The release path only removes the idr entry and frees the stable identity strings. This preserves the intended one-registration/many-tcon behavior: a registration id represents a net/share pair, and notify handling acts on a live representative selected at use time. It also preserves CLIENT_MOVE ordering for the representative tcon because the old-IP unregister is sent before cifs_swn_register() sends the new-IP register. Fixes: fed979a7e082 ("cifs: Set witness notification handler for messages from userspace daemon") Cc: stable@vger.kernel.org Signed-off-by: Michael Bommarito Assisted-by: Claude:claude-opus-4-7 Signed-off-by: Steve French --- fs/smb/client/cifs_swn.c | 316 ++++++++++++++++++++++++++++++++------- fs/smb/client/trace.h | 2 + 2 files changed, 263 insertions(+), 55 deletions(-) diff --git a/fs/smb/client/cifs_swn.c b/fs/smb/client/cifs_swn.c index 9753a432d099..9951817d0d7f 100644 --- a/fs/smb/client/cifs_swn.c +++ b/fs/smb/client/cifs_swn.c @@ -28,10 +28,54 @@ struct cifs_swn_reg { bool net_name_notify; bool share_name_notify; bool ip_notify; - - struct cifs_tcon *tcon; }; +struct cifs_swn_reg_info { + int id; + unsigned int ref_count; + const char *net_name; + const char *share_name; + bool net_name_notify; + bool share_name_notify; + bool ip_notify; +}; + +static void cifs_swn_snapshot_reg(struct cifs_swn_reg *swnreg, + struct cifs_swn_reg_info *info) +{ + info->id = swnreg->id; + info->ref_count = kref_read(&swnreg->ref_count); + info->net_name = swnreg->net_name; + info->share_name = swnreg->share_name; + info->net_name_notify = swnreg->net_name_notify; + info->share_name_notify = swnreg->share_name_notify; + info->ip_notify = swnreg->ip_notify; +} + +static int cifs_swn_dup_reg(struct cifs_swn_reg *swnreg, + struct cifs_swn_reg_info *info) +{ + cifs_swn_snapshot_reg(swnreg, info); + + info->net_name = kstrdup(swnreg->net_name, GFP_KERNEL); + if (!info->net_name) + return -ENOMEM; + + info->share_name = kstrdup(swnreg->share_name, GFP_KERNEL); + if (!info->share_name) { + kfree(info->net_name); + return -ENOMEM; + } + + return 0; +} + +static void cifs_swn_free_reg_info(struct cifs_swn_reg_info *info) +{ + kfree(info->net_name); + kfree(info->share_name); +} + static int cifs_swn_auth_info_krb(struct cifs_tcon *tcon, struct sk_buff *skb) { int ret; @@ -73,7 +117,8 @@ static int cifs_swn_auth_info_ntlm(struct cifs_tcon *tcon, struct sk_buff *skb) * The authentication information to connect to the witness service is bundled * into the message. */ -static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg) +static int cifs_swn_send_register_message(struct cifs_swn_reg_info *swnreg, + struct cifs_tcon *tcon) { struct sk_buff *skb; struct genlmsghdr *hdr; @@ -109,10 +154,10 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg) * told to switch to it (client move message). In these cases we unregister from the * server address and register to the new address when we receive the notification. */ - if (swnreg->tcon->ses->server->use_swn_dstaddr) - addr = &swnreg->tcon->ses->server->swn_dstaddr; + if (tcon->ses->server->use_swn_dstaddr) + addr = &tcon->ses->server->swn_dstaddr; else - addr = &swnreg->tcon->ses->server->dstaddr; + addr = &tcon->ses->server->dstaddr; ret = nla_put(skb, CIFS_GENL_ATTR_SWN_IP, sizeof(struct sockaddr_storage), addr); if (ret < 0) @@ -136,10 +181,10 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg) goto nlmsg_fail; } - authtype = cifs_select_sectype(swnreg->tcon->ses->server, swnreg->tcon->ses->sectype); + authtype = cifs_select_sectype(tcon->ses->server, tcon->ses->sectype); switch (authtype) { case Kerberos: - ret = cifs_swn_auth_info_krb(swnreg->tcon, skb); + ret = cifs_swn_auth_info_krb(tcon, skb); if (ret < 0) { cifs_dbg(VFS, "%s: Failed to get kerberos auth info: %d\n", __func__, ret); goto nlmsg_fail; @@ -147,7 +192,7 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg) break; case NTLMv2: case RawNTLMSSP: - ret = cifs_swn_auth_info_ntlm(swnreg->tcon, skb); + ret = cifs_swn_auth_info_ntlm(tcon, skb); if (ret < 0) { cifs_dbg(VFS, "%s: Failed to get NTLM auth info: %d\n", __func__, ret); goto nlmsg_fail; @@ -176,7 +221,8 @@ static int cifs_swn_send_register_message(struct cifs_swn_reg *swnreg) /* * Sends an uregister message to the userspace daemon based on the registration */ -static int cifs_swn_send_unregister_message(struct cifs_swn_reg *swnreg) +static int cifs_swn_send_unregister_message(struct cifs_swn_reg_info *swnreg, + struct cifs_tcon *tcon) { struct sk_buff *skb; struct genlmsghdr *hdr; @@ -205,7 +251,7 @@ static int cifs_swn_send_unregister_message(struct cifs_swn_reg *swnreg) goto nlmsg_fail; ret = nla_put(skb, CIFS_GENL_ATTR_SWN_IP, sizeof(struct sockaddr_storage), - &swnreg->tcon->ses->server->dstaddr); + &tcon->ses->server->dstaddr); if (ret < 0) goto nlmsg_fail; @@ -241,6 +287,88 @@ static int cifs_swn_send_unregister_message(struct cifs_swn_reg *swnreg) return ret; } +/* + * Allocation-free mirror of extract_hostname() + extract_sharename() from + * fs/smb/client/unc.c. Those helpers kmalloc(GFP_KERNEL); this runs under + * cifs_tcp_ses_lock and tcon->tc_lock, both spinlocks, so we mirror their + * parsing in place against the caller's stable net_name/share_name strings. + * Keep in sync with unc.c. + */ +static bool cifs_swn_tcon_matches(struct cifs_tcon *tcon, + const char *net_name, + const char *share_name) +{ + const char *unc = tcon->tree_name; + const char *host, *share, *delim; + size_t host_len, share_len; + + if (!tcon->use_witness) + return false; + + /* extract_hostname: require strlen(unc) >= 3 */ + if (strnlen(unc, 3) < 3) + return false; + /* extract_hostname: skip all leading '\' characters */ + for (host = unc; *host == '\\'; host++) + ; + if (!*host) + return false; + delim = strchr(host, '\\'); + if (!delim) + return false; + host_len = delim - host; + if (strlen(net_name) != host_len || + strncasecmp(host, net_name, host_len)) + return false; + + /* extract_sharename: start at unc + 2, then first '\' onward */ + share = unc + 2; + delim = strchr(share, '\\'); + if (!delim) + return false; + share = delim + 1; + share_len = strlen(share); + + return strlen(share_name) == share_len && + !strncasecmp(share, share_name, share_len); +} + +/* + * One SWN registration id represents one net/share name pair. Multiple + * mounted tcons can therefore share the id. Pick a live representative at + * use time instead of caching the first tcon pointer in the registration. + */ +static struct cifs_tcon *cifs_swn_get_tcon(struct cifs_swn_reg_info *swnreg) +{ + struct TCP_Server_Info *server; + struct cifs_ses *ses; + struct cifs_tcon *tcon; + + spin_lock(&cifs_tcp_ses_lock); + list_for_each_entry(server, &cifs_tcp_ses_list, tcp_ses_list) { + list_for_each_entry(ses, &server->smb_ses_list, smb_ses_list) { + list_for_each_entry(tcon, &ses->tcon_list, tcon_list) { + spin_lock(&tcon->tc_lock); + if (tcon->status == TID_EXITING || + !cifs_swn_tcon_matches(tcon, swnreg->net_name, + swnreg->share_name)) { + spin_unlock(&tcon->tc_lock); + continue; + } + ++tcon->tc_count; + trace_smb3_tcon_ref(tcon->debug_id, + tcon->tc_count, + netfs_trace_tcon_ref_get_swn_notify); + spin_unlock(&tcon->tc_lock); + spin_unlock(&cifs_tcp_ses_lock); + return tcon; + } + } + } + spin_unlock(&cifs_tcp_ses_lock); + return NULL; +} + /* * Try to find a matching registration for the tcon's server name and share name. * Calls to this function must be protected by cifs_swnreg_idr_mutex. @@ -347,8 +475,6 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon) reg->net_name_notify = true; reg->share_name_notify = true; reg->ip_notify = (tcon->capabilities & SMB2_SHARE_CAP_SCALEOUT); - - reg->tcon = tcon; unlock: mutex_unlock(&cifs_swnreg_idr_mutex); @@ -368,11 +494,6 @@ static struct cifs_swn_reg *cifs_get_swn_reg(struct cifs_tcon *tcon) static void cifs_swn_reg_release(struct kref *ref) { struct cifs_swn_reg *swnreg = container_of(ref, struct cifs_swn_reg, ref_count); - int ret; - - ret = cifs_swn_send_unregister_message(swnreg); - if (ret < 0) - cifs_dbg(VFS, "%s: Failed to send unregister message: %d\n", __func__, ret); idr_remove(&cifs_swnreg_idr, swnreg->id); kfree(swnreg->net_name); @@ -380,23 +501,33 @@ static void cifs_swn_reg_release(struct kref *ref) kfree(swnreg); } -static void cifs_put_swn_reg(struct cifs_swn_reg *swnreg) +static void cifs_put_swn_reg_locked(struct cifs_swn_reg *swnreg, + struct cifs_tcon *tcon) { - mutex_lock(&cifs_swnreg_idr_mutex); + if (kref_read(&swnreg->ref_count) == 1) { + struct cifs_swn_reg_info swnreg_info; + int ret; + + cifs_swn_snapshot_reg(swnreg, &swnreg_info); + ret = cifs_swn_send_unregister_message(&swnreg_info, tcon); + if (ret < 0) + cifs_dbg(VFS, "%s: Failed to send unregister message: %d\n", + __func__, ret); + } + kref_put(&swnreg->ref_count, cifs_swn_reg_release); - mutex_unlock(&cifs_swnreg_idr_mutex); } -static int cifs_swn_resource_state_changed(struct cifs_swn_reg *swnreg, const char *name, int state) +static int cifs_swn_resource_state_changed(struct cifs_tcon *tcon, const char *name, int state) { switch (state) { case CIFS_SWN_RESOURCE_STATE_UNAVAILABLE: cifs_dbg(FYI, "%s: resource name '%s' become unavailable\n", __func__, name); - cifs_signal_cifsd_for_reconnect(swnreg->tcon->ses->server, true); + cifs_signal_cifsd_for_reconnect(tcon->ses->server, true); break; case CIFS_SWN_RESOURCE_STATE_AVAILABLE: cifs_dbg(FYI, "%s: resource name '%s' become available\n", __func__, name); - cifs_signal_cifsd_for_reconnect(swnreg->tcon->ses->server, true); + cifs_signal_cifsd_for_reconnect(tcon->ses->server, true); break; case CIFS_SWN_RESOURCE_STATE_UNKNOWN: cifs_dbg(FYI, "%s: resource name '%s' changed to unknown state\n", __func__, name); @@ -502,7 +633,7 @@ static int cifs_swn_reconnect(struct cifs_tcon *tcon, struct sockaddr_storage *a return ret; } -static int cifs_swn_client_move(struct cifs_swn_reg *swnreg, struct sockaddr_storage *addr) +static int cifs_swn_client_move(struct cifs_tcon *tcon, struct sockaddr_storage *addr) { struct sockaddr_in *ipv4 = (struct sockaddr_in *)addr; struct sockaddr_in6 *ipv6 = (struct sockaddr_in6 *)addr; @@ -512,14 +643,17 @@ static int cifs_swn_client_move(struct cifs_swn_reg *swnreg, struct sockaddr_sto else if (addr->ss_family == AF_INET6) cifs_dbg(FYI, "%s: move to %pI6\n", __func__, &ipv6->sin6_addr); - return cifs_swn_reconnect(swnreg->tcon, addr); + return cifs_swn_reconnect(tcon, addr); } int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info) { struct cifs_swn_reg *swnreg; + struct cifs_swn_reg_info swnreg_info; + struct cifs_tcon *tcon; char name[256]; int type; + int ret = 0; if (info->attrs[CIFS_GENL_ATTR_SWN_REGISTRATION_ID]) { int swnreg_id; @@ -527,21 +661,34 @@ int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info) swnreg_id = nla_get_u32(info->attrs[CIFS_GENL_ATTR_SWN_REGISTRATION_ID]); mutex_lock(&cifs_swnreg_idr_mutex); swnreg = idr_find(&cifs_swnreg_idr, swnreg_id); - mutex_unlock(&cifs_swnreg_idr_mutex); if (swnreg == NULL) { + mutex_unlock(&cifs_swnreg_idr_mutex); cifs_dbg(FYI, "%s: registration id %d not found\n", __func__, swnreg_id); return -EINVAL; } + ret = cifs_swn_dup_reg(swnreg, &swnreg_info); + mutex_unlock(&cifs_swnreg_idr_mutex); + if (ret) + return ret; } else { cifs_dbg(FYI, "%s: missing registration id attribute\n", __func__); return -EINVAL; } + tcon = cifs_swn_get_tcon(&swnreg_info); + if (!tcon) { + cifs_dbg(FYI, "%s: registration id %d has no live tcon\n", + __func__, swnreg_info.id); + ret = -ENODEV; + goto free_info; + } + if (info->attrs[CIFS_GENL_ATTR_SWN_NOTIFICATION_TYPE]) { type = nla_get_u32(info->attrs[CIFS_GENL_ATTR_SWN_NOTIFICATION_TYPE]); } else { cifs_dbg(FYI, "%s: missing notification type attribute\n", __func__); - return -EINVAL; + ret = -EINVAL; + goto out; } switch (type) { @@ -553,15 +700,18 @@ int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info) sizeof(name)); } else { cifs_dbg(FYI, "%s: missing resource name attribute\n", __func__); - return -EINVAL; + ret = -EINVAL; + goto out; } if (info->attrs[CIFS_GENL_ATTR_SWN_RESOURCE_STATE]) { state = nla_get_u32(info->attrs[CIFS_GENL_ATTR_SWN_RESOURCE_STATE]); } else { cifs_dbg(FYI, "%s: missing resource state attribute\n", __func__); - return -EINVAL; + ret = -EINVAL; + goto out; } - return cifs_swn_resource_state_changed(swnreg, name, state); + ret = cifs_swn_resource_state_changed(tcon, name, state); + break; } case CIFS_SWN_NOTIFICATION_CLIENT_MOVE: { struct sockaddr_storage addr; @@ -570,28 +720,36 @@ int cifs_swn_notify(struct sk_buff *skb, struct genl_info *info) nla_memcpy(&addr, info->attrs[CIFS_GENL_ATTR_SWN_IP], sizeof(addr)); } else { cifs_dbg(FYI, "%s: missing IP address attribute\n", __func__); - return -EINVAL; + ret = -EINVAL; + goto out; } - return cifs_swn_client_move(swnreg, &addr); + ret = cifs_swn_client_move(tcon, &addr); + break; } default: cifs_dbg(FYI, "%s: unknown notification type %d\n", __func__, type); break; } - return 0; +out: + cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_swn_notify); +free_info: + cifs_swn_free_reg_info(&swnreg_info); + return ret; } int cifs_swn_register(struct cifs_tcon *tcon) { struct cifs_swn_reg *swnreg; + struct cifs_swn_reg_info swnreg_info; int ret; swnreg = cifs_get_swn_reg(tcon); if (IS_ERR(swnreg)) return PTR_ERR(swnreg); - ret = cifs_swn_send_register_message(swnreg); + cifs_swn_snapshot_reg(swnreg, &swnreg_info); + ret = cifs_swn_send_register_message(&swnreg_info, tcon); if (ret < 0) { cifs_dbg(VFS, "%s: Failed to send swn register message: %d\n", __func__, ret); /* Do not put the swnreg or return error, the echo task will retry */ @@ -612,35 +770,68 @@ int cifs_swn_unregister(struct cifs_tcon *tcon) return PTR_ERR(swnreg); } + cifs_put_swn_reg_locked(swnreg, tcon); mutex_unlock(&cifs_swnreg_idr_mutex); - cifs_put_swn_reg(swnreg); - return 0; } -void cifs_swn_dump(struct seq_file *m) +/* + * Snapshot one registration under cifs_swnreg_idr_mutex and return. Callers + * intentionally do the per-registration network/genlmsg work without the + * mutex held, both to keep the critical section short and to avoid nesting + * cifs_swnreg_idr_mutex inside the higher tc_lock when a live tcon is then + * pinned for the send. + */ +static int cifs_swn_get_next_reg_info(int *id, struct cifs_swn_reg_info *info) { struct cifs_swn_reg *swnreg; + int ret = 0; + + mutex_lock(&cifs_swnreg_idr_mutex); + swnreg = idr_get_next(&cifs_swnreg_idr, id); + if (swnreg) { + ret = cifs_swn_dup_reg(swnreg, info); + if (!ret) { + *id = swnreg->id + 1; + ret = 1; + } + } + mutex_unlock(&cifs_swnreg_idr_mutex); + + return ret; +} + +void cifs_swn_dump(struct seq_file *m) +{ + struct cifs_swn_reg_info swnreg_info; + struct cifs_tcon *tcon; struct sockaddr_in *sa; struct sockaddr_in6 *sa6; - int id; + int id = 0; + int ret; seq_puts(m, "Witness registrations:"); - mutex_lock(&cifs_swnreg_idr_mutex); - idr_for_each_entry(&cifs_swnreg_idr, swnreg, id) { + while ((ret = cifs_swn_get_next_reg_info(&id, &swnreg_info)) > 0) { seq_printf(m, "\nId: %u Refs: %u Network name: '%s'%s Share name: '%s'%s Ip address: ", - id, kref_read(&swnreg->ref_count), - swnreg->net_name, swnreg->net_name_notify ? "(y)" : "(n)", - swnreg->share_name, swnreg->share_name_notify ? "(y)" : "(n)"); - switch (swnreg->tcon->ses->server->dstaddr.ss_family) { + swnreg_info.id, swnreg_info.ref_count, + swnreg_info.net_name, swnreg_info.net_name_notify ? "(y)" : "(n)", + swnreg_info.share_name, swnreg_info.share_name_notify ? "(y)" : "(n)"); + + tcon = cifs_swn_get_tcon(&swnreg_info); + if (!tcon) { + seq_puts(m, "(no live tcon)"); + goto next; + } + + switch (tcon->ses->server->dstaddr.ss_family) { case AF_INET: - sa = (struct sockaddr_in *) &swnreg->tcon->ses->server->dstaddr; + sa = (struct sockaddr_in *)&tcon->ses->server->dstaddr; seq_printf(m, "%pI4", &sa->sin_addr.s_addr); break; case AF_INET6: - sa6 = (struct sockaddr_in6 *) &swnreg->tcon->ses->server->dstaddr; + sa6 = (struct sockaddr_in6 *)&tcon->ses->server->dstaddr; seq_printf(m, "%pI6", &sa6->sin6_addr.s6_addr); if (sa6->sin6_scope_id) seq_printf(m, "%%%u", sa6->sin6_scope_id); @@ -648,23 +839,38 @@ void cifs_swn_dump(struct seq_file *m) default: seq_puts(m, "(unknown)"); } - seq_printf(m, "%s", swnreg->ip_notify ? "(y)" : "(n)"); + cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_swn_notify); +next: + seq_printf(m, "%s", swnreg_info.ip_notify ? "(y)" : "(n)"); + cifs_swn_free_reg_info(&swnreg_info); } - mutex_unlock(&cifs_swnreg_idr_mutex); + if (ret < 0) + seq_printf(m, "\nFailed to snapshot witness registration: %d", ret); seq_puts(m, "\n"); } void cifs_swn_check(void) { - struct cifs_swn_reg *swnreg; - int id; + struct cifs_swn_reg_info swnreg_info; + struct cifs_tcon *tcon; + int id = 0; int ret; - mutex_lock(&cifs_swnreg_idr_mutex); - idr_for_each_entry(&cifs_swnreg_idr, swnreg, id) { - ret = cifs_swn_send_register_message(swnreg); + while ((ret = cifs_swn_get_next_reg_info(&id, &swnreg_info)) > 0) { + tcon = cifs_swn_get_tcon(&swnreg_info); + if (!tcon) { + cifs_dbg(FYI, "%s: registration id %d has no live tcon\n", + __func__, swnreg_info.id); + goto free_info; + } + + ret = cifs_swn_send_register_message(&swnreg_info, tcon); if (ret < 0) cifs_dbg(FYI, "%s: Failed to send register message: %d\n", __func__, ret); + cifs_put_tcon(tcon, netfs_trace_tcon_ref_put_swn_notify); +free_info: + cifs_swn_free_reg_info(&swnreg_info); } - mutex_unlock(&cifs_swnreg_idr_mutex); + if (ret < 0) + cifs_dbg(FYI, "%s: Failed to snapshot registration: %d\n", __func__, ret); } diff --git a/fs/smb/client/trace.h b/fs/smb/client/trace.h index b99ec5a417fa..5b21ad3c15fb 100644 --- a/fs/smb/client/trace.h +++ b/fs/smb/client/trace.h @@ -181,6 +181,7 @@ EM(netfs_trace_tcon_ref_get_find, "GET Find ") \ EM(netfs_trace_tcon_ref_get_find_sess_tcon, "GET FndSes") \ EM(netfs_trace_tcon_ref_get_reconnect_server, "GET Reconn") \ + EM(netfs_trace_tcon_ref_get_swn_notify, "GET SwnNot") \ EM(netfs_trace_tcon_ref_new, "NEW ") \ EM(netfs_trace_tcon_ref_new_ipc, "NEW Ipc ") \ EM(netfs_trace_tcon_ref_new_reconnect_server, "NEW Reconn") \ @@ -192,6 +193,7 @@ EM(netfs_trace_tcon_ref_put_mnt_ctx, "PUT MntCtx") \ EM(netfs_trace_tcon_ref_put_dfs_refer, "PUT DfsRfr") \ EM(netfs_trace_tcon_ref_put_reconnect_server, "PUT Reconn") \ + EM(netfs_trace_tcon_ref_put_swn_notify, "PUT SwnNot") \ EM(netfs_trace_tcon_ref_put_tlink, "PUT Tlink ") \ EM(netfs_trace_tcon_ref_see_cancelled_close, "SEE Cn-Cls") \ EM(netfs_trace_tcon_ref_see_fscache_collision, "SEE FV-CO!") \ From 29f1005b8b4d3d3d8ac116d85f864a0b83bcf394 Mon Sep 17 00:00:00 2001 From: Qihang Date: Sun, 17 May 2026 16:25:27 +0800 Subject: [PATCH 05/14] cifs: validate full SID length in security descriptors parse_sid() only verified that the fixed SID header fit in the returned security descriptor, but did not verify that the full SID body described by num_subauth was present. A malicious server can return a truncated owner or group SID whose header lies within the descriptor buffer while sub_auth[] extends past the end of the allocation, leading to an out-of-bounds read when the client later parses or copies that SID. Validate the full SID body in parse_sid(), centralize owner/group SID lookup and bounds checking in sid_from_sd(), and use that validation in parse_sec_desc(), build_sec_desc(), and copy_sec_desc() before sub_auth[] is accessed. Signed-off-by: Qihang Signed-off-by: Steve French --- fs/smb/client/cifsacl.c | 196 ++++++++++++++++++++++++++-------------- 1 file changed, 129 insertions(+), 67 deletions(-) diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c index 786dbbc43c5b..42a3115359da 100644 --- a/fs/smb/client/cifsacl.c +++ b/fs/smb/client/cifsacl.c @@ -275,6 +275,73 @@ cifs_copy_sid(struct smb_sid *dst, const struct smb_sid *src) return size; } +static int parse_sid(const struct smb_sid *psid, const char *end_of_acl) +{ + unsigned int sid_len; + + /* SID must contain the fixed header before num_subauth is trusted. */ + if (end_of_acl < (const char *)psid + CIFS_SID_BASE_SIZE) { + cifs_dbg(VFS, "ACL too small to parse SID %p\n", psid); + return -EINVAL; + } + if (psid->num_subauth > SID_MAX_SUB_AUTHORITIES) { + cifs_dbg(VFS, "SID contains too many subauthorities %u\n", + psid->num_subauth); + return -EINVAL; + } + + sid_len = CIFS_SID_BASE_SIZE + psid->num_subauth * sizeof(__le32); + if (end_of_acl < (const char *)psid + sid_len) { + cifs_dbg(VFS, "ACL too small to parse SID %p\n", psid); + return -EINVAL; + } + +#ifdef CONFIG_CIFS_DEBUG2 + if (psid->num_subauth) { + int i; + + cifs_dbg(FYI, "SID revision %d num_auth %d\n", + psid->revision, psid->num_subauth); + + for (i = 0; i < psid->num_subauth; i++) { + cifs_dbg(FYI, "SID sub_auth[%d]: 0x%x\n", + i, le32_to_cpu(psid->sub_auth[i])); + } + + cifs_dbg(FYI, "RID 0x%x\n", + le32_to_cpu(psid->sub_auth[psid->num_subauth - 1])); + } +#endif + + return 0; +} + +static int sid_from_sd(const struct smb_ntsd *pntsd, __u32 secdesclen, + __u32 sid_offset, struct smb_sid **sid) +{ + struct smb_sid *psid; + char *end_of_acl; + + if (secdesclen < sizeof(struct smb_ntsd)) { + cifs_dbg(VFS, "ACL too small to parse security descriptor\n"); + return -EINVAL; + } + end_of_acl = (char *)pntsd + secdesclen; + + if (sid_offset < sizeof(struct smb_ntsd) || + sid_offset > secdesclen - CIFS_SID_BASE_SIZE) { + cifs_dbg(VFS, "Server returned illegal SID offset\n"); + return -EINVAL; + } + + psid = (struct smb_sid *)((char *)pntsd + sid_offset); + if (parse_sid(psid, end_of_acl)) + return -EINVAL; + + *sid = psid; + return 0; +} + static int id_to_sid(unsigned int cid, uint sidtype, struct smb_sid *ssid) { @@ -515,14 +582,14 @@ exit_cifs_idmap(void) } /* copy ntsd, owner sid, and group sid from a security descriptor to another */ -static __u32 copy_sec_desc(const struct smb_ntsd *pntsd, - struct smb_ntsd *pnntsd, - __u32 sidsoffset, - struct smb_sid *pownersid, - struct smb_sid *pgrpsid) +static int copy_sec_desc(const struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd, + __u32 sidsoffset, __u32 secdesclen, + __u32 *pnsecdesclen, struct smb_sid *pownersid, + struct smb_sid *pgrpsid) { struct smb_sid *owner_sid_ptr, *group_sid_ptr; struct smb_sid *nowner_sid_ptr, *ngroup_sid_ptr; + int rc; /* copy security descriptor control portion */ pnntsd->revision = pntsd->revision; @@ -533,28 +600,34 @@ static __u32 copy_sec_desc(const struct smb_ntsd *pntsd, pnntsd->gsidoffset = cpu_to_le32(sidsoffset + sizeof(struct smb_sid)); /* copy owner sid */ - if (pownersid) + if (pownersid) { owner_sid_ptr = pownersid; - else - owner_sid_ptr = (struct smb_sid *)((char *)pntsd + - le32_to_cpu(pntsd->osidoffset)); + } else { + rc = sid_from_sd(pntsd, secdesclen, + le32_to_cpu(pntsd->osidoffset), &owner_sid_ptr); + if (rc) + return rc; + } nowner_sid_ptr = (struct smb_sid *)((char *)pnntsd + sidsoffset); cifs_copy_sid(nowner_sid_ptr, owner_sid_ptr); /* copy group sid */ - if (pgrpsid) + if (pgrpsid) { group_sid_ptr = pgrpsid; - else - group_sid_ptr = (struct smb_sid *)((char *)pntsd + - le32_to_cpu(pntsd->gsidoffset)); + } else { + rc = sid_from_sd(pntsd, secdesclen, + le32_to_cpu(pntsd->gsidoffset), &group_sid_ptr); + if (rc) + return rc; + } ngroup_sid_ptr = (struct smb_sid *)((char *)pnntsd + sidsoffset + sizeof(struct smb_sid)); cifs_copy_sid(ngroup_sid_ptr, group_sid_ptr); - return sidsoffset + (2 * sizeof(struct smb_sid)); + *pnsecdesclen = sidsoffset + (2 * sizeof(struct smb_sid)); + return 0; } - /* change posix mode to reflect permissions pmode is the existing mode (we only want to overwrite part of this @@ -1232,38 +1305,6 @@ static int set_chmod_dacl(struct smb_acl *pdacl, struct smb_acl *pndacl, return 0; } -static int parse_sid(struct smb_sid *psid, char *end_of_acl) -{ - /* BB need to add parm so we can store the SID BB */ - - /* validate that we do not go past end of ACL - sid must be at least 8 - bytes long (assuming no sub-auths - e.g. the null SID */ - if (end_of_acl < (char *)psid + 8) { - cifs_dbg(VFS, "ACL too small to parse SID %p\n", psid); - return -EINVAL; - } - -#ifdef CONFIG_CIFS_DEBUG2 - if (psid->num_subauth) { - int i; - cifs_dbg(FYI, "SID revision %d num_auth %d\n", - psid->revision, psid->num_subauth); - - for (i = 0; i < psid->num_subauth; i++) { - cifs_dbg(FYI, "SID sub_auth[%d]: 0x%x\n", - i, le32_to_cpu(psid->sub_auth[i])); - } - - /* BB add length check to make sure that we do not have huge - num auths and therefore go off the end */ - cifs_dbg(FYI, "RID 0x%x\n", - le32_to_cpu(psid->sub_auth[psid->num_subauth-1])); - } -#endif - - return 0; -} - static bool dacl_offset_valid(unsigned int acl_len, __u32 dacloffset) { if (acl_len < sizeof(struct smb_acl)) @@ -1284,23 +1325,25 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb, int rc = 0; struct smb_sid *owner_sid_ptr, *group_sid_ptr; struct smb_acl *dacl_ptr; /* no need for SACL ptr */ - char *end_of_acl = ((char *)pntsd) + acl_len; - __u32 dacloffset; + char *end_of_acl; + __u32 dacloffset, osidoffset, gsidoffset; if (pntsd == NULL) return smb_EIO(smb_eio_trace_null_pointers); + if (acl_len < (int)sizeof(struct smb_ntsd)) { + cifs_dbg(VFS, "ACL too small to parse security descriptor\n"); + return -EINVAL; + } + end_of_acl = ((char *)pntsd) + acl_len; - owner_sid_ptr = (struct smb_sid *)((char *)pntsd + - le32_to_cpu(pntsd->osidoffset)); - group_sid_ptr = (struct smb_sid *)((char *)pntsd + - le32_to_cpu(pntsd->gsidoffset)); + osidoffset = le32_to_cpu(pntsd->osidoffset); + gsidoffset = le32_to_cpu(pntsd->gsidoffset); dacloffset = le32_to_cpu(pntsd->dacloffset); 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, le32_to_cpu(pntsd->osidoffset), - le32_to_cpu(pntsd->gsidoffset), + pntsd->revision, pntsd->type, osidoffset, gsidoffset, le32_to_cpu(pntsd->sacloffset), dacloffset); /* cifs_dump_mem("owner_sid: ", owner_sid_ptr, 64); */ - rc = parse_sid(owner_sid_ptr, end_of_acl); + 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; @@ -1312,9 +1355,9 @@ static int parse_sec_desc(struct cifs_sb_info *cifs_sb, return rc; } - rc = parse_sid(group_sid_ptr, end_of_acl); + rc = sid_from_sd(pntsd, acl_len, gsidoffset, &group_sid_ptr); if (rc) { - cifs_dbg(FYI, "%s: Error %d mapping Owner SID to gid\n", + cifs_dbg(FYI, "%s: Error %d parsing Group SID\n", __func__, rc); return rc; } @@ -1354,8 +1397,15 @@ static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd, struct smb_sid *nowner_sid_ptr = NULL, *ngroup_sid_ptr = NULL; struct smb_acl *dacl_ptr = NULL; /* no need for SACL ptr */ struct smb_acl *ndacl_ptr = NULL; /* no need for SACL ptr */ - char *end_of_acl = ((char *)pntsd) + secdesclen; + char *end_of_acl; u16 size = 0; + __u32 osidoffset, gsidoffset; + + if (secdesclen < sizeof(struct smb_ntsd)) { + cifs_dbg(VFS, "ACL too small to parse security descriptor\n"); + return -EINVAL; + } + end_of_acl = ((char *)pntsd) + secdesclen; dacloffset = le32_to_cpu(pntsd->dacloffset); if (dacloffset) { @@ -1370,10 +1420,18 @@ static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd, return rc; } - owner_sid_ptr = (struct smb_sid *)((char *)pntsd + - le32_to_cpu(pntsd->osidoffset)); - group_sid_ptr = (struct smb_sid *)((char *)pntsd + - le32_to_cpu(pntsd->gsidoffset)); + osidoffset = le32_to_cpu(pntsd->osidoffset); + gsidoffset = le32_to_cpu(pntsd->gsidoffset); + rc = sid_from_sd(pntsd, secdesclen, osidoffset, &owner_sid_ptr); + if (rc) { + cifs_dbg(FYI, "%s: Error %d parsing Owner SID\n", __func__, rc); + return rc; + } + rc = sid_from_sd(pntsd, secdesclen, gsidoffset, &group_sid_ptr); + if (rc) { + cifs_dbg(FYI, "%s: Error %d parsing Group SID\n", __func__, rc); + return rc; + } if (pnmode && *pnmode != NO_CHANGE_64) { /* chmod */ ndacloffset = sizeof(struct smb_ntsd); @@ -1389,8 +1447,10 @@ static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd, sidsoffset = ndacloffset + le16_to_cpu(ndacl_ptr->size); /* copy the non-dacl portion of secdesc */ - *pnsecdesclen = copy_sec_desc(pntsd, pnntsd, sidsoffset, - NULL, NULL); + rc = copy_sec_desc(pntsd, pnntsd, sidsoffset, secdesclen, + pnsecdesclen, NULL, NULL); + if (rc) + return rc; *aclflag |= CIFS_ACL_DACL; } else { @@ -1467,8 +1527,10 @@ static int build_sec_desc(struct smb_ntsd *pntsd, struct smb_ntsd *pnntsd, sidsoffset = ndacloffset + le16_to_cpu(ndacl_ptr->size); /* copy the non-dacl portion of secdesc */ - *pnsecdesclen = copy_sec_desc(pntsd, pnntsd, sidsoffset, - nowner_sid_ptr, ngroup_sid_ptr); + rc = copy_sec_desc(pntsd, pnntsd, sidsoffset, secdesclen, + pnsecdesclen, nowner_sid_ptr, ngroup_sid_ptr); + if (rc) + goto chown_chgrp_exit; chown_chgrp_exit: /* errors could jump here. So make sure we return soon after this */ From e8a5cf2ff5a13fefb228f2069e29dd7d8e37185d Mon Sep 17 00:00:00 2001 From: Fredric Cover Date: Mon, 1 Jun 2026 17:55:10 -0700 Subject: [PATCH 06/14] smb: client: fix races in cifsd thread creation The cifsd demultiplex thread can run and access tcp_ses before the parent thread has finished populating tcp_ses, which the worker thread accesses locklessly. Also, the kthread_run macro may start the thread before returning the thread pointer. Because the pointer is part of the structure that the thread can access, if the kernel is preempted after the thread is spawned, but before the thread pointer is populated and the thread attempts to exit, it will sleep, waiting for a SIGKILL signal. Fix this by moving creation of the thread to after all of tcp_ses'es fields are populated, and spawning the thread last, using a split kthread_create/wake_up_process logic. Signed-off-by: Fredric Cover Signed-off-by: Steve French --- fs/smb/client/connect.c | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index cbeb5637eeb9..104658e318b6 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -1871,14 +1871,6 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx, * this will succeed. No need for try_module_get(). */ __module_get(THIS_MODULE); - tcp_ses->tsk = kthread_run(cifs_demultiplex_thread, - tcp_ses, "cifsd"); - if (IS_ERR(tcp_ses->tsk)) { - rc = PTR_ERR(tcp_ses->tsk); - cifs_dbg(VFS, "error %d create cifsd thread\n", rc); - module_put(THIS_MODULE); - goto out_err_crypto_release; - } tcp_ses->min_offload = ctx->min_offload; tcp_ses->retrans = ctx->retrans; /* @@ -1886,9 +1878,7 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx, * to the struct since the kernel thread not created yet * no need to spinlock this update of tcpStatus */ - spin_lock(&tcp_ses->srv_lock); tcp_ses->tcpStatus = CifsNeedNegotiate; - spin_unlock(&tcp_ses->srv_lock); if ((ctx->max_credits < 20) || (ctx->max_credits > 60000)) tcp_ses->max_credits = SMB2_MAX_CREDITS_AVAILABLE; @@ -1897,7 +1887,16 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx, tcp_ses->nr_targets = 1; tcp_ses->ignore_signature = ctx->ignore_signature; - /* thread spawned, put it on the list */ + + tcp_ses->tsk = kthread_create(cifs_demultiplex_thread, + tcp_ses, "cifsd"); + if (IS_ERR(tcp_ses->tsk)) { + rc = PTR_ERR(tcp_ses->tsk); + cifs_dbg(VFS, "error %d create cifsd thread\n", rc); + module_put(THIS_MODULE); + goto out_err_crypto_release; + } + /* thread created, put it on the list */ spin_lock(&cifs_tcp_ses_lock); list_add(&tcp_ses->tcp_ses_list, &cifs_tcp_ses_list); spin_unlock(&cifs_tcp_ses_lock); @@ -1905,6 +1904,12 @@ cifs_get_tcp_session(struct smb3_fs_context *ctx, /* queue echo request delayed work */ queue_delayed_work(cifsiod_wq, &tcp_ses->echo, tcp_ses->echo_interval); + /* + * Use split create/wake logic to ensure that tcp_ses is fully populated + * and tcp_ses->tsk is valid + */ + wake_up_process(tcp_ses->tsk); + return tcp_ses; out_err_crypto_release: From af25ab681ea0cead031379c8dcb112678a2e1cf6 Mon Sep 17 00:00:00 2001 From: Huiwen He Date: Sat, 6 Jun 2026 00:35:17 +0800 Subject: [PATCH 07/14] smb/client: update i_blocks after contiguous writes When a lease allows CIFS to use cached inode attributes, getattr may return the locally cached attributes instead of revalidating them from the server. After local writes extend a file, the write path updates the file size, but i_blocks can remain based on the old allocation size. For example, while the file is still open after two contiguous writes, the local block count can remain smaller than the written range: after first write: st_size = 4096, st_blocks = 7 after second write: st_size = 12288, st_blocks = 21 after close: st_size = 12288, st_blocks = 24 This can make a fully written file look sparse: i_blocks * 512 < i_size and can cause swap activation to reject a valid write-created swapfile as having holes. This results in xfstests skipping swap-related tests on CIFS mounts: generic/472 [not run] swapfiles are not supported generic/494 [not run] swapfiles are not supported generic/497 [not run] swapfiles are not supported generic/569 [not run] swapfiles are not supported generic/636 [not run] swapfiles are not supported generic/643 [not run] swapfiles are not supported Update the local i_blocks estimate after successful writes, but only when the write starts at or before the currently known allocated range. This lets sequential writes grow i_blocks while avoiding treating write-past-EOF holes as allocated. Skip the local estimate for files that are already marked sparse, since their allocation needs to come from the server rather than from a contiguous-write estimate. Signed-off-by: Huiwen He Reviewed-by: ChenXiaoSong Signed-off-by: Steve French --- fs/smb/client/cifsfs.c | 4 ++-- fs/smb/client/cifsfs.h | 1 + fs/smb/client/cifsglob.h | 9 ++++--- fs/smb/client/file.c | 52 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 61 insertions(+), 5 deletions(-) diff --git a/fs/smb/client/cifsfs.c b/fs/smb/client/cifsfs.c index ce23924f01b3..6b97f7a91235 100644 --- a/fs/smb/client/cifsfs.c +++ b/fs/smb/client/cifsfs.c @@ -1615,7 +1615,7 @@ const struct file_operations cifs_file_strict_ops = { const struct file_operations cifs_file_direct_ops = { .read_iter = netfs_unbuffered_read_iter, - .write_iter = netfs_file_write_iter, + .write_iter = cifs_direct_write_iter, .open = cifs_open, .release = cifs_close, .lock = cifs_lock, @@ -1671,7 +1671,7 @@ const struct file_operations cifs_file_strict_nobrl_ops = { const struct file_operations cifs_file_direct_nobrl_ops = { .read_iter = netfs_unbuffered_read_iter, - .write_iter = netfs_file_write_iter, + .write_iter = cifs_direct_write_iter, .open = cifs_open, .release = cifs_close, .fsync = cifs_fsync, diff --git a/fs/smb/client/cifsfs.h b/fs/smb/client/cifsfs.h index c455b15f2778..c35074ce5aad 100644 --- a/fs/smb/client/cifsfs.h +++ b/fs/smb/client/cifsfs.h @@ -104,6 +104,7 @@ int cifs_closedir(struct inode *inode, struct file *file); ssize_t cifs_strict_readv(struct kiocb *iocb, struct iov_iter *to); ssize_t cifs_strict_writev(struct kiocb *iocb, struct iov_iter *from); ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from); +ssize_t cifs_direct_write_iter(struct kiocb *iocb, struct iov_iter *from); ssize_t cifs_loose_read_iter(struct kiocb *iocb, struct iov_iter *iter); int cifs_flock(struct file *file, int cmd, struct file_lock *fl); int cifs_lock(struct file *file, int cmd, struct file_lock *flock); diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index 82e0adc1dabd..943b7cd2c096 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -2387,9 +2387,12 @@ static inline int cifs_open_create_options(unsigned int oflags, int opts) } /* - * The number of blocks is not related to (i_size / i_blksize), but instead - * 512 byte (2**9) size is required for calculating num blocks. + * inode->i_blocks is counted in 512-byte units, independent of + * inode->i_blksize. */ -#define CIFS_INO_BLOCKS(size) DIV_ROUND_UP_ULL((u64)(size), 512) +#define CIFS_INO_BLOCK_SIZE 512ULL +#define CIFS_INO_BLOCKS(size) \ + DIV_ROUND_UP_ULL((u64)(size), CIFS_INO_BLOCK_SIZE) +#define CIFS_INO_BYTES(blocks) ((u64)(blocks) * CIFS_INO_BLOCK_SIZE) #endif /* _CIFS_GLOB_H */ diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index b60344125f27..58430ba51b10 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -2514,6 +2514,42 @@ int cifs_lock(struct file *file, int cmd, struct file_lock *flock) return rc; } +static void cifs_update_i_blocks_for_write(struct inode *inode, loff_t start, + loff_t end) +{ + struct cifsInodeInfo *cinode = CIFS_I(inode); + u64 allocated_end = CIFS_INO_BYTES(inode->i_blocks); + u64 blocks; + + if (cinode->cifsAttrs & FILE_ATTRIBUTE_SPARSE_FILE) + return; + + /* + * Grow the local estimate only across the currently known allocated + * prefix. A write beyond that may leave a hole. + */ + if ((u64)start > allocated_end) + return; + + blocks = CIFS_INO_BLOCKS(end); + if ((u64)inode->i_blocks < blocks) + inode->i_blocks = blocks; +} + +static void cifs_update_i_blocks_after_write(struct kiocb *iocb, + ssize_t written) +{ + struct inode *inode = file_inode(iocb->ki_filp); + loff_t end = iocb->ki_pos; + + if (written <= 0) + return; + + spin_lock(&inode->i_lock); + cifs_update_i_blocks_for_write(inode, end - written, end); + spin_unlock(&inode->i_lock); +} + void cifs_write_subrequest_terminated(struct cifs_io_subrequest *wdata, ssize_t result) { struct netfs_io_request *wreq = wdata->rreq; @@ -2532,6 +2568,8 @@ void cifs_write_subrequest_terminated(struct cifs_io_subrequest *wdata, ssize_t netfs_write_zero_point(inode, wrend); if (wrend > ictx->_remote_i_size) netfs_resize_file(ictx, wrend, true); + cifs_update_i_blocks_for_write(inode, wdata->subreq.start, + wrend); spin_unlock(&inode->i_lock); } @@ -2920,6 +2958,7 @@ cifs_writev(struct kiocb *iocb, struct iov_iter *from) } rc = netfs_buffered_write_iter_locked(iocb, from, NULL); + cifs_update_i_blocks_after_write(iocb, rc); out: up_read(&cinode->lock_sem); @@ -2949,6 +2988,7 @@ cifs_strict_writev(struct kiocb *iocb, struct iov_iter *from) (CIFS_UNIX_FCNTL_CAP & le64_to_cpu(tcon->fsUnixInfo.Capability)) && ((cifs_sb_flags(cifs_sb) & CIFS_MOUNT_NOPOSIXBRL) == 0)) { written = netfs_file_write_iter(iocb, from); + cifs_update_i_blocks_after_write(iocb, written); goto out; } written = cifs_writev(iocb, from); @@ -2961,6 +3001,7 @@ cifs_strict_writev(struct kiocb *iocb, struct iov_iter *from) * these pages but not on the region from pos to ppos+len-1. */ written = netfs_file_write_iter(iocb, from); + cifs_update_i_blocks_after_write(iocb, written); if (CIFS_CACHE_READ(cinode)) { /* * We have read level caching and we have just sent a write @@ -2979,6 +3020,15 @@ cifs_strict_writev(struct kiocb *iocb, struct iov_iter *from) return written; } +ssize_t cifs_direct_write_iter(struct kiocb *iocb, struct iov_iter *from) +{ + ssize_t written; + + written = netfs_file_write_iter(iocb, from); + cifs_update_i_blocks_after_write(iocb, written); + return written; +} + ssize_t cifs_loose_read_iter(struct kiocb *iocb, struct iov_iter *iter) { ssize_t rc; @@ -3003,6 +3053,7 @@ ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from) if (iocb->ki_filp->f_flags & O_DIRECT) { written = netfs_unbuffered_write_iter(iocb, from); + cifs_update_i_blocks_after_write(iocb, written); if (written > 0 && CIFS_CACHE_READ(cinode)) { cifs_zap_mapping(inode); cifs_dbg(FYI, @@ -3018,6 +3069,7 @@ ssize_t cifs_file_write_iter(struct kiocb *iocb, struct iov_iter *from) return written; written = netfs_file_write_iter(iocb, from); + cifs_update_i_blocks_after_write(iocb, written); if (!CIFS_CACHE_WRITE(CIFS_I(inode))) { rc = filemap_fdatawrite(inode->i_mapping); From 7acbaa16b99edaf8ef432229d4b7a6f3b666767d Mon Sep 17 00:00:00 2001 From: Huiwen He Date: Mon, 8 Jun 2026 23:57:29 +0800 Subject: [PATCH 08/14] smb/client: always return a value for FS_IOC_GETFLAGS Currently, repeated lsattr calls on a regular CIFS file without the compressed attribute may show random flags: $ touch test.bin $ lsattr test.bin s-S-ia-A-EjI---------m test.bin $ lsattr test.bin ------d-cEjI---------m test.bin The lsattr reproducer depends on the previous contents of its userspace buffer, so it may not reproduce on every setup. A deterministic reproducer is to initialize the ioctl argument before FS_IOC_GETFLAGS on a file without the compressed attribute: int flags = 0x7fffffff; ioctl(fd, FS_IOC_GETFLAGS, &flags); On an affected kernel, flags remains 0x7fffffff. With the fix, it is set to 0. This happens because when the cached inode does not have the compressed bit set, the CIFS fallback path in FS_IOC_GETFLAGS returns success without calling put_user() to write the zero flags value into the user buffer. As a result, the caller observes stale contents from its own buffer. Fix this by always writing the visible flags value back to the user buffer before returning success, even when the value is zero. Fixes: 64a5cfa6db94 ("Allow setting per-file compression via SMB2/3") Signed-off-by: Huiwen He Reviewed-by: ChenXiaoSong Signed-off-by: Steve French --- fs/smb/client/ioctl.c | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/fs/smb/client/ioctl.c b/fs/smb/client/ioctl.c index 17408bb8ab65..746d70091f3d 100644 --- a/fs/smb/client/ioctl.c +++ b/fs/smb/client/ioctl.c @@ -392,13 +392,11 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg) } #endif /* CONFIG_CIFS_ALLOW_INSECURE_LEGACY */ #endif /* CONFIG_CIFS_POSIX */ - rc = 0; - if (CIFS_I(inode)->cifsAttrs & ATTR_COMPRESSED) { - /* add in the compressed bit */ - ExtAttrBits = FS_COMPR_FL; - rc = put_user(ExtAttrBits & FS_FL_USER_VISIBLE, - (int __user *)arg); - } + if (CIFS_I(inode)->cifsAttrs & FILE_ATTRIBUTE_COMPRESSED) + ExtAttrBits |= FS_COMPR_FL; + + rc = put_user(ExtAttrBits & FS_FL_USER_VISIBLE, + (int __user *)arg); break; case FS_IOC_SETFLAGS: if (pSMBFile == NULL) From 5693347de107a26f68d1f43b25ff2e348c7229a9 Mon Sep 17 00:00:00 2001 From: Huiwen He Date: Mon, 8 Jun 2026 23:57:30 +0800 Subject: [PATCH 09/14] smb/client: use writable handle for FS_IOC_SETFLAGS compression Setting the compressed flag on a CIFS mount can fail with -EACCES: [compress_share] vfs objects = btrfs $ touch test.bin $ chattr +c test.bin chattr: Permission denied while setting flags on test.bin This can be reproduced against a Samba share backed by a filesystem that supports compression, such as btrfs. FS_IOC_SETFLAGS is issued on the file handle opened by userspace. chattr opens the target read-only before setting FS_COMPR_FL, so the SMB client currently sends FSCTL_SET_COMPRESSION on a handle that may not have FILE_WRITE_DATA access. Samba requires FILE_WRITE_DATA for FSCTL_SET_COMPRESSION and rejects the request. Use the current handle only if it already has FILE_WRITE_DATA. Otherwise try an existing writable handle for the inode. If none is available, open a temporary FILE_WRITE_DATA handle for the compression request. After FSCTL_SET_COMPRESSION succeeds, update the cached compressed attribute immediately, matching how smb2_set_sparse() updates FILE_ATTRIBUTE_SPARSE_FILE after a successful FSCTL_SET_SPARSE. Signed-off-by: Huiwen He Reviewed-by: ChenXiaoSong Signed-off-by: Steve French --- fs/smb/client/ioctl.c | 116 ++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 112 insertions(+), 4 deletions(-) diff --git a/fs/smb/client/ioctl.c b/fs/smb/client/ioctl.c index 746d70091f3d..886e6893a552 100644 --- a/fs/smb/client/ioctl.c +++ b/fs/smb/client/ioctl.c @@ -67,6 +67,110 @@ static long cifs_ioctl_query_info(unsigned int xid, struct file *filep, return rc; } +static int cifs_set_compression_by_path(unsigned int xid, struct file *filep, + struct cifs_tcon *tcon) +{ + struct inode *inode = file_inode(filep); + struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb); + struct TCP_Server_Info *server = tcon->ses->server; + struct cifs_open_parms oparms; + struct cifs_open_info_data data = {}; + struct cifsFileInfo *tmp_cfile = NULL; + struct cifs_fid fid = {}; + const char *full_path; + __u32 oplock = 0; + u64 uniqueid; + void *page; + int rc; + + if (!server->ops->open || !server->ops->close || + !server->ops->query_file_info) + return -EOPNOTSUPP; + + if (!(cifs_sb_flags(cifs_sb) & CIFS_MOUNT_SERVER_INUM) || + cifs_sb->mnt_cifs_serverino_autodisabled) + return -EOPNOTSUPP; + + if (d_unhashed(filep->f_path.dentry)) + return -ESTALE; + + page = alloc_dentry_path(); + full_path = build_path_from_dentry(filep->f_path.dentry, page); + if (IS_ERR(full_path)) { + free_dentry_path(page); + return PTR_ERR(full_path); + } + + oparms = CIFS_OPARMS(cifs_sb, tcon, full_path, FILE_WRITE_DATA | + FILE_READ_ATTRIBUTES, + FILE_OPEN, 0, ACL_NO_MODE); + oparms.fid = &fid; + + rc = server->ops->open(xid, &oparms, &oplock, NULL); + if (rc) + goto out; + + tmp_cfile = kzalloc_obj(*tmp_cfile); + if (!tmp_cfile) { + rc = -ENOMEM; + goto close; + } + + tmp_cfile->fid = fid; + rc = server->ops->query_file_info(xid, tcon, tmp_cfile, &data); + if (rc) + goto close; + + uniqueid = le64_to_cpu(data.fi.IndexNumber); + if (uniqueid != CIFS_I(inode)->uniqueid) { + rc = -ESTALE; + goto close; + } + + rc = server->ops->set_compression(xid, tcon, tmp_cfile); + +close: + server->ops->close(xid, tcon, &fid); + if (tmp_cfile) + kfree(tmp_cfile); + cifs_free_open_info(&data); +out: + free_dentry_path(page); + return rc; +} + +static int cifs_ioctl_set_compression(unsigned int xid, struct file *filep, + struct cifs_tcon *tcon, + struct cifsFileInfo *cfile) +{ + struct cifsFileInfo *wfile; + struct cifs_tcon *wtcon; + struct inode *inode = file_inode(filep); + int rc; + + if (!tcon->ses->server->ops->set_compression) + return -EOPNOTSUPP; + + if (cfile && (cfile->fid.access & FILE_WRITE_DATA)) { + rc = tcon->ses->server->ops->set_compression(xid, tcon, cfile); + if (rc != -EACCES) + return rc; + } + + rc = cifs_get_writable_file(CIFS_I(inode), FIND_FSUID_ONLY, &wfile); + if (!rc) { + wtcon = tlink_tcon(wfile->tlink); + rc = wtcon->ses->server->ops->set_compression(xid, wtcon, wfile); + cifsFileInfo_put(wfile); + if (rc != -EACCES) + return rc; + } else if (rc != -EBADF) { + return rc; + } + + return cifs_set_compression_by_path(xid, filep, tcon); +} + static long cifs_ioctl_copychunk(unsigned int xid, struct file *dst_file, unsigned long srcfd) { @@ -424,11 +528,15 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg) break; /* Try to set compress flag */ - if (tcon->ses->server->ops->set_compression) { - rc = tcon->ses->server->ops->set_compression( - xid, tcon, pSMBFile); - cifs_dbg(FYI, "set compress flag rc %d\n", rc); + rc = cifs_ioctl_set_compression(xid, filep, tcon, + pSMBFile); + if (rc == 0) { + spin_lock(&inode->i_lock); + CIFS_I(inode)->cifsAttrs |= + FILE_ATTRIBUTE_COMPRESSED; + spin_unlock(&inode->i_lock); } + cifs_dbg(FYI, "set compress flag rc %d\n", rc); break; case CIFS_IOC_COPYCHUNK_FILE: rc = cifs_ioctl_copychunk(xid, filep, arg); From 3ecad5de621ef538cbd63ae7075fddcc426dcd74 Mon Sep 17 00:00:00 2001 From: Huiwen He Date: Mon, 8 Jun 2026 23:57:31 +0800 Subject: [PATCH 10/14] smb/client: allow FS_IOC_SETFLAGS to clear compression The CIFS FS_IOC_SETFLAGS path can set FS_COMPR_FL now, but it cannot clear it again. This can be reproduced on a share backed by a filesystem that supports compression, for example btrfs exported by Samba: [compress_share] vfs objects = btrfs $ touch test.bin $ chattr +c test.bin $ lsattr test.bin $ chattr -c test.bin The final chattr -c fails with EOPNOTSUPP, and leaves the remote object with the compressed attribute still set, because the client always sends FSCTL_SET_COMPRESSION with COMPRESSION_FORMAT_DEFAULT. That is correct for setting FS_COMPR_FL, but clearing FS_COMPR_FL requires sending COMPRESSION_FORMAT_NONE. Fix this by passing the requested compression state through the set_compression operation. The SMB1 and SMB2 helpers no longer hard-code COMPRESSION_FORMAT_DEFAULT. When FS_COMPR_FL is set, send COMPRESSION_FORMAT_DEFAULT. When it is cleared, send COMPRESSION_FORMAT_NONE. If the server accepts the request, update the cached FILE_ATTRIBUTE_COMPRESSED bit under i_lock so FS_IOC_GETFLAGS reports the new state. Signed-off-by: Huiwen He Reviewed-by: ChenXiaoSong Signed-off-by: Steve French --- fs/smb/client/cifsglob.h | 2 +- fs/smb/client/cifssmb.c | 4 ++-- fs/smb/client/ioctl.c | 43 ++++++++++++++++++++++++++++----------- fs/smb/client/smb1ops.c | 5 +++-- fs/smb/client/smb1proto.h | 2 +- fs/smb/client/smb2ops.c | 4 ++-- fs/smb/client/smb2pdu.c | 6 +++--- fs/smb/client/smb2proto.h | 3 ++- 8 files changed, 45 insertions(+), 24 deletions(-) diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index 943b7cd2c096..a462c1590a9e 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -425,7 +425,7 @@ struct smb_version_operations { int (*set_file_info)(struct inode *, const char *, FILE_BASIC_INFO *, const unsigned int); int (*set_compression)(const unsigned int, struct cifs_tcon *, - struct cifsFileInfo *); + struct cifsFileInfo *, __u16); /* check if we can send an echo or nor */ bool (*can_echo)(struct TCP_Server_Info *); /* send echo request */ diff --git a/fs/smb/client/cifssmb.c b/fs/smb/client/cifssmb.c index 9e27bfa7376b..d39175cdf1b1 100644 --- a/fs/smb/client/cifssmb.c +++ b/fs/smb/client/cifssmb.c @@ -3207,7 +3207,7 @@ struct inode *cifs_create_reparse_inode(struct cifs_open_info_data *data, int CIFSSMB_set_compression(const unsigned int xid, struct cifs_tcon *tcon, - __u16 fid) + __u16 fid, __u16 compression_state) { int rc = 0; int bytes_returned; @@ -3222,7 +3222,7 @@ CIFSSMB_set_compression(const unsigned int xid, struct cifs_tcon *tcon, return rc; in_len = rc; - pSMB->compression_state = cpu_to_le16(COMPRESSION_FORMAT_DEFAULT); + pSMB->compression_state = cpu_to_le16(compression_state); pSMB->TotalParameterCount = 0; pSMB->TotalDataCount = cpu_to_le32(2); diff --git a/fs/smb/client/ioctl.c b/fs/smb/client/ioctl.c index 886e6893a552..9fa743be3652 100644 --- a/fs/smb/client/ioctl.c +++ b/fs/smb/client/ioctl.c @@ -68,7 +68,8 @@ static long cifs_ioctl_query_info(unsigned int xid, struct file *filep, } static int cifs_set_compression_by_path(unsigned int xid, struct file *filep, - struct cifs_tcon *tcon) + struct cifs_tcon *tcon, + __u16 compression_state) { struct inode *inode = file_inode(filep); struct cifs_sb_info *cifs_sb = CIFS_SB(inode->i_sb); @@ -127,7 +128,8 @@ static int cifs_set_compression_by_path(unsigned int xid, struct file *filep, goto close; } - rc = server->ops->set_compression(xid, tcon, tmp_cfile); + rc = server->ops->set_compression(xid, tcon, tmp_cfile, + compression_state); close: server->ops->close(xid, tcon, &fid); @@ -141,7 +143,8 @@ static int cifs_set_compression_by_path(unsigned int xid, struct file *filep, static int cifs_ioctl_set_compression(unsigned int xid, struct file *filep, struct cifs_tcon *tcon, - struct cifsFileInfo *cfile) + struct cifsFileInfo *cfile, + __u16 compression_state) { struct cifsFileInfo *wfile; struct cifs_tcon *wtcon; @@ -152,7 +155,8 @@ static int cifs_ioctl_set_compression(unsigned int xid, struct file *filep, return -EOPNOTSUPP; if (cfile && (cfile->fid.access & FILE_WRITE_DATA)) { - rc = tcon->ses->server->ops->set_compression(xid, tcon, cfile); + rc = tcon->ses->server->ops->set_compression(xid, tcon, cfile, + compression_state); if (rc != -EACCES) return rc; } @@ -160,7 +164,8 @@ static int cifs_ioctl_set_compression(unsigned int xid, struct file *filep, rc = cifs_get_writable_file(CIFS_I(inode), FIND_FSUID_ONLY, &wfile); if (!rc) { wtcon = tlink_tcon(wfile->tlink); - rc = wtcon->ses->server->ops->set_compression(xid, wtcon, wfile); + rc = wtcon->ses->server->ops->set_compression(xid, wtcon, wfile, + compression_state); cifsFileInfo_put(wfile); if (rc != -EACCES) return rc; @@ -168,7 +173,8 @@ static int cifs_ioctl_set_compression(unsigned int xid, struct file *filep, return rc; } - return cifs_set_compression_by_path(xid, filep, tcon); + return cifs_set_compression_by_path(xid, filep, tcon, + compression_state); } static long cifs_ioctl_copychunk(unsigned int xid, struct file *dst_file, @@ -460,6 +466,8 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg) struct tcon_link *tlink; struct cifs_sb_info *cifs_sb; __u64 ExtAttrBits = 0; + bool enable_compression; + __u16 compression_state; #ifdef CONFIG_CIFS_POSIX #ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY __u64 caps; @@ -523,17 +531,28 @@ long cifs_ioctl(struct file *filep, unsigned int command, unsigned long arg) * break; */ - /* Currently only flag we can set is compressed flag */ - if ((ExtAttrBits & FS_COMPR_FL) == 0) + /* Currently only flag we can set or clear is compressed. */ + if (ExtAttrBits & ~FS_COMPR_FL) { + rc = -EOPNOTSUPP; break; + } + + enable_compression = ExtAttrBits & FS_COMPR_FL; + compression_state = enable_compression ? + COMPRESSION_FORMAT_DEFAULT : + COMPRESSION_FORMAT_NONE; - /* Try to set compress flag */ rc = cifs_ioctl_set_compression(xid, filep, tcon, - pSMBFile); + pSMBFile, + compression_state); if (rc == 0) { spin_lock(&inode->i_lock); - CIFS_I(inode)->cifsAttrs |= - FILE_ATTRIBUTE_COMPRESSED; + if (enable_compression) + CIFS_I(inode)->cifsAttrs |= + FILE_ATTRIBUTE_COMPRESSED; + else + CIFS_I(inode)->cifsAttrs &= + ~FILE_ATTRIBUTE_COMPRESSED; spin_unlock(&inode->i_lock); } cifs_dbg(FYI, "set compress flag rc %d\n", rc); diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c index e198e3dda917..d34b3d99f6ed 100644 --- a/fs/smb/client/smb1ops.c +++ b/fs/smb/client/smb1ops.c @@ -1110,9 +1110,10 @@ smb_set_file_info(struct inode *inode, const char *full_path, static int cifs_set_compression(const unsigned int xid, struct cifs_tcon *tcon, - struct cifsFileInfo *cfile) + struct cifsFileInfo *cfile, __u16 compression_state) { - return CIFSSMB_set_compression(xid, tcon, cfile->fid.netfid); + return CIFSSMB_set_compression(xid, tcon, cfile->fid.netfid, + compression_state); } static int diff --git a/fs/smb/client/smb1proto.h b/fs/smb/client/smb1proto.h index 5f522d359952..80eaeb3dd2ec 100644 --- a/fs/smb/client/smb1proto.h +++ b/fs/smb/client/smb1proto.h @@ -117,7 +117,7 @@ struct inode *cifs_create_reparse_inode(struct cifs_open_info_data *data, struct kvec *reparse_iov, struct kvec *xattr_iov); int CIFSSMB_set_compression(const unsigned int xid, struct cifs_tcon *tcon, - __u16 fid); + __u16 fid, __u16 compression_state); int cifs_do_get_acl(const unsigned int xid, struct cifs_tcon *tcon, const unsigned char *searchName, struct posix_acl **acl, const int acl_type, const struct nls_table *nls_codepage, diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c index d4875f9532b4..a3257815e661 100644 --- a/fs/smb/client/smb2ops.c +++ b/fs/smb/client/smb2ops.c @@ -2246,10 +2246,10 @@ smb2_duplicate_extents(const unsigned int xid, static int smb2_set_compression(const unsigned int xid, struct cifs_tcon *tcon, - struct cifsFileInfo *cfile) + struct cifsFileInfo *cfile, __u16 compression_state) { return SMB2_set_compression(xid, tcon, cfile->fid.persistent_fid, - cfile->fid.volatile_fid); + cfile->fid.volatile_fid, compression_state); } static int diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c index fbeb2156ddb6..6a185b805c1e 100644 --- a/fs/smb/client/smb2pdu.c +++ b/fs/smb/client/smb2pdu.c @@ -3626,14 +3626,14 @@ SMB2_ioctl(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, int SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon, - u64 persistent_fid, u64 volatile_fid) + u64 persistent_fid, u64 volatile_fid, + __u16 compression_state) { int rc; struct compress_ioctl fsctl_input; char *ret_data = NULL; - fsctl_input.CompressionState = - cpu_to_le16(COMPRESSION_FORMAT_DEFAULT); + fsctl_input.CompressionState = cpu_to_le16(compression_state); rc = SMB2_ioctl(xid, tcon, persistent_fid, volatile_fid, FSCTL_SET_COMPRESSION, diff --git a/fs/smb/client/smb2proto.h b/fs/smb/client/smb2proto.h index 1ceb95b907e6..78a4e1c340f9 100644 --- a/fs/smb/client/smb2proto.h +++ b/fs/smb/client/smb2proto.h @@ -216,7 +216,8 @@ int SMB2_set_ea(const unsigned int xid, struct cifs_tcon *tcon, u64 persistent_fid, u64 volatile_fid, struct smb2_file_full_ea_info *buf, int len); int SMB2_set_compression(const unsigned int xid, struct cifs_tcon *tcon, - u64 persistent_fid, u64 volatile_fid); + u64 persistent_fid, u64 volatile_fid, + __u16 compression_state); int SMB2_oplock_break(const unsigned int xid, struct cifs_tcon *tcon, const u64 persistent_fid, const u64 volatile_fid, __u8 oplock_level); From 2e4f0d9815e93771a8002199fee2e3cd196811a7 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Thu, 11 Jun 2026 10:34:51 +0300 Subject: [PATCH 11/14] smb/client: clean up a type issue in cifs_xattr_get() The cifs_xattr_get() function returns type int, not ssize_t so declare "rc" as int as well. This has no effect on runtime. Signed-off-by: Dan Carpenter Signed-off-by: Steve French --- fs/smb/client/xattr.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/fs/smb/client/xattr.c b/fs/smb/client/xattr.c index 23227f2f9428..5091f6c0d7fe 100644 --- a/fs/smb/client/xattr.c +++ b/fs/smb/client/xattr.c @@ -272,7 +272,7 @@ static int cifs_xattr_get(const struct xattr_handler *handler, struct dentry *dentry, struct inode *inode, const char *name, void *value, size_t size) { - ssize_t rc = -EOPNOTSUPP; + int rc = -EOPNOTSUPP; unsigned int xid; struct super_block *sb = dentry->d_sb; struct cifs_sb_info *cifs_sb = CIFS_SB(sb); @@ -354,7 +354,7 @@ static int cifs_xattr_get(const struct xattr_handler *handler, inode, full_path, &acllen, extra_info); if (IS_ERR(pacl)) { rc = PTR_ERR(pacl); - cifs_dbg(VFS, "%s: error %zd getting sec desc\n", + cifs_dbg(VFS, "%s: error %d getting sec desc\n", __func__, rc); } else { if (value) { From 61f28012e5650c619223decdb7970e0d3162e949 Mon Sep 17 00:00:00 2001 From: Dan Carpenter Date: Thu, 11 Jun 2026 10:35:28 +0300 Subject: [PATCH 12/14] smb/client: Fix error code in smb2_aead_req_alloc() The "*num_sgs" variable is a u32 so "ERR_PTR(*num_sgs)" doesn't work. We would have to do something similar to the previous line where it's cast to int and then long. However, it's simpler to store the return in an int ret variable. This bug would eventually result in a crash when dereference the invalid error pointer. Fixes: d08089f649a0 ("cifs: Change the I/O paths to use an iterator rather than a page list") Cc: stable@kernel.org Signed-off-by: Dan Carpenter Signed-off-by: Steve French --- fs/smb/client/smb2ops.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/fs/smb/client/smb2ops.c b/fs/smb/client/smb2ops.c index a3257815e661..a8f8feeeccb5 100644 --- a/fs/smb/client/smb2ops.c +++ b/fs/smb/client/smb2ops.c @@ -4359,11 +4359,13 @@ static void *smb2_aead_req_alloc(struct crypto_aead *tfm, const struct smb_rqst unsigned int req_size = sizeof(**req) + crypto_aead_reqsize(tfm); unsigned int iv_size = crypto_aead_ivsize(tfm); unsigned int len; + int ret; u8 *p; - *num_sgs = cifs_get_num_sgs(rqst, num_rqst, sig); - if (IS_ERR_VALUE((long)(int)*num_sgs)) - return ERR_PTR(*num_sgs); + ret = cifs_get_num_sgs(rqst, num_rqst, sig); + if (ret < 0) + return ERR_PTR(ret); + *num_sgs = ret; len = iv_size; len += crypto_aead_alignmask(tfm) & ~(crypto_tfm_ctx_alignment() - 1); From c1266041e64571096e960c5507e176e88b4913e4 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Thu, 11 Jun 2026 17:33:39 +0200 Subject: [PATCH 13/14] smb: client: Use more common error handling code in smb3_reconfigure() Use an additional label so that a bit of exception handling can be better reused at the end of this function implementation. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Signed-off-by: Steve French --- fs/smb/client/fs_context.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c index fd4b13cd654d..9addc74ce57e 100644 --- a/fs/smb/client/fs_context.c +++ b/fs/smb/client/fs_context.c @@ -1092,10 +1092,8 @@ static int smb3_reconfigure(struct fs_context *fc) return -ENOMEM; rc = smb3_fs_context_dup(old_ctx, cifs_sb->ctx); - if (rc) { - kfree(old_ctx); - return rc; - } + if (rc) + goto free_old_ctx; /* * We can not change UNC/username/password/domainname/ @@ -1244,6 +1242,7 @@ static int smb3_reconfigure(struct fs_context *fc) kfree_sensitive(new_password2); smb3_cleanup_fs_context_contents(cifs_sb->ctx); memcpy(cifs_sb->ctx, old_ctx, sizeof(*old_ctx)); +free_old_ctx: kfree(old_ctx); return rc; From 0eb17dea51eff940230b5e562df29ce457124d41 Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Thu, 9 Oct 2025 10:37:39 +0200 Subject: [PATCH 14/14] smb: client: Use more common code in SMB2_tcon() Use an additional label so that a bit of common code can be better reused at the end of this function implementation. Signed-off-by: Markus Elfring Signed-off-by: Steve French --- fs/smb/client/smb2pdu.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c index 6a185b805c1e..4972cfe249f6 100644 --- a/fs/smb/client/smb2pdu.c +++ b/fs/smb/client/smb2pdu.c @@ -2129,8 +2129,8 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree, unc_path_len = cifs_strtoUTF16(unc_path, tree, strlen(tree), cp); if (unc_path_len <= 0) { - kfree(unc_path); - return -EINVAL; + rc = -EINVAL; + goto free_unc_path; } unc_path_len *= 2; @@ -2139,10 +2139,8 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree, atomic_set(&tcon->num_remote_opens, 0); rc = smb2_plain_req_init(SMB2_TREE_CONNECT, tcon, server, (void **) &req, &total_len); - if (rc) { - kfree(unc_path); - return rc; - } + if (rc) + goto free_unc_path; if (smb3_encryption_required(tcon)) flags |= CIFS_TRANSFORM_REQ; @@ -2233,6 +2231,7 @@ SMB2_tcon(const unsigned int xid, struct cifs_ses *ses, const char *tree, tcon_exit: free_rsp_buf(resp_buftype, rsp); +free_unc_path: kfree(unc_path); return rc;