summaryrefslogtreecommitdiff
path: root/fs/smb/client/connect.c
AgeCommit message (Collapse)AuthorFilesLines
2026-08-30smb: client: fix multiuser mount with krb5Paulo Alcantara1-5/+16
Customer reported that they could no longer mount their SMB shares with multiuser mount option and krb5. Turned out that the client wasn't duplicating username option when creating multiuser connections, therefore failing to retrieve credentials as cifs.upcall(8) couldn't find them in keytab. Fix this by duplicating username option (if set) from original fs context before creating multiuser connections with krb5. Reproducer: ``` $ ktutil ktutil: add_entry -password -p testuser -k 1 -e aes256-cts Password for testuser@ZELDA.TEST: ktutil: write_kt /etc/krb5.keytab ktutil: quit $ klist -ke Keytab name: FILE:/etc/krb5.keytab KVNO Principal ---- ---------------------------------------------------------------- 1 testuser@ZELDA.TEST (aes256-cts-hmac-sha1-96) $ mount.cifs //w22-root2/scratch /mnt/1 -o \ uid=1000,sec=krb5,username=testuser@ZELDA.TEST,multiuser mount error(13): Permission denied Refer to the mount.cifs(8) manual page (e.g. man mount.cifs) and kernel log messages (dmesg) ``` Reported-by: Jacob Shivers <jshivers@redhat.com> Fixes: 12b4c5d98cd7 ("smb: client: fix krb5 mount with username option") Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Shyam Prasad N <sprasad@microsoft.com> Cc: Tom Talpey <tom@talpey.com> Cc: Bharath SM <bharathsm@microsoft.com> Cc: Namjae Jeon <linkinjeon@kernel.org> Cc: stable@vger.kernel.org Reviewed-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Paulo Alcantara <pc@manguebit.org>
2026-08-24smb: client: Avoid leaking sensitive data to the heap in connect.cThomas Huth1-1/+1
TCP_Server_Info contains a preauth_sha_hash[] and a cryptkey[] array that might contain sensitive data. Thus free its memory with kfree_sensitive() to avoid that we are leaking this information to the heap. Signed-off-by: Thomas Huth <thuth@redhat.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Paulo Alcantara <pc@manguebit.org>
2026-08-24Revert "cifs: remove all cifs files before kill super"Zizhi Wo1-3/+0
This reverts commit 6d9a4aaaa8b2612b5ef9d581e2f286a458b71ee1. First, directly flushing fileinfo_put_wq in that commit cannot guarantee that all in-flight I/O has run its cleanup_work on system_dfl_wq and subsequently called queue_work(fileinfo_put_wq, ...). Flushing only the latter workqueue may therefore miss puts that have not yet been queued, so the fix is not reliable in the first place. Moreover, this fix flushes inside cifs_umount(), which means the busy-dentry warning can still be triggered when umount_check() is called inside kill_anon_super(), because kill_anon_super() is executed before cifs_umount(). Second, commit 75f5c412fa86 ("smb: client: fix busy dentry warning on unmount after DIO") already drains both serverclose_wq and fileinfo_put_wq in cifs_kill_sb(), before kill_anon_super(). By adding a per-superblock outstanding-rreq counter, it guarantees that all cleanup_work for this sb have run, and thus all relevant cfile puts are queued on fileinfo_put_wq or serverclose_wq. Third, no path between those drains and cifs_umount() can queue new work onto either workqueue. In the "cifs_sb->root == NULL" path there are no file-related workers either, so that case is safe as well. Therefore the busy-dentry and null-ptr-deref problems cannot arise, and the flush added by commit 6d9a4aaaa8b2 ("cifs: remove all cifs files before kill super") is redundant and can be removed. Signed-off-by: Zizhi Wo <wozizhi@huawei.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Paulo Alcantara <pc@manguebit.org>
2026-07-09cifs: Show reason why autodisabling serverino supportPali Rohár1-1/+1
Extend cifs_autodisable_serverino() function to print also text message why the function was called. The text message is printed just once for mount then autodisabling serverino support. Once the serverino support is disabled for mount it will not be re-enabled. So those text messages do not cause flooding logs. This change allows to debug issues why cifs.ko decide to turn off server inode number support and hence disable support for detection of hardlinks. Signed-off-by: Pali Rohár <pali@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2026-07-08smb: client: fix busy dentry warning on unmount after DIOZizhi Wo1-0/+1
Commit c68337442f03 ("cifs: Fix busy dentry used after unmounting") fixed the issue in cifs where deferred close of a file led to a dentry reference count not being released in umount, by flushing deferredclose_wq in cifs_kill_sb() to solve it. However, the cifs DIO path suffers from the same busy-dentry problem caused by a delayed dentry reference-count release: [dio] [cifsd] [close + umount] netfs_unbuffered_write_iter_locked ... cifs_demultiplex_thread netfs_unbuffered_write cifs_issue_write netfs_wait_for_in_progress_stream [1] ... netfs_write_subrequest_terminated netfs_subreq_clear_in_progress netfs_wake_collector // wake [1] netfs_put_subrequest netfs_put_request queue_work(system_dfl_wq, xxx) [2] // dio write return cifs_close _cifsFileInfo_put // cfile->count 2->1 --cfile->count [3] // umount cifs_kill_sb kill_anon_super // warning triggered! shrink_dcache_for_umount [4] [system_dfl_wq] [5] netfs_free_request ... _cifsFileInfo_put // cfile->count 1->0 --cfile->count queue_work(fileinfo_put_wq, xxx) [fileinfo_put_wq] [6] cifsFileInfo_put_work cifsFileInfo_put_final dput If the umount path is triggered before [5], it results warning: BUG: Dentry 00000000eab1f070{i=9a917b66ae404fec,n=test} still in use (1) [unmount of cifs cifs] The existing per-inode ictx->io_count wait in cifs_evict_inode() does not help: it lives in the inode eviction path, which runs after shrink_dcache_for_umount() has already warned about the busy dentries. Fix it by adding a per-superblock outstanding-rreq counter that is incremented in cifs_init_request() and decremented in cifs_free_request(). In cifs_kill_sb(), before kill_anon_super(), wait for this counter to reach 0 - which guarantees that all cleanup_work for this sb have run and thus all relevant cfile puts are queued on fileinfo_put_wq or serverclose_wq. Then drain the workqueue so the dentry refs are dropped. This is a targeted wait, not a flush of the system-wide system_dfl_wq. Fixes: 340cea84f691c ("cifs: open files should not hold ref on superblock") Signed-off-by: Zizhi Wo <wozizhi@huawei.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2026-06-24smb/client: use %pe to print error pointerFredric Cover1-2/+2
Currently, __reconnect_target_locked() prints the error pointer 'hostname' using PTR_ERR() and %ld, printing only the error code. Fix this by using by using %pe to print the error symbolic code instead, and remove PTR_ERR(). Signed-off-by: Fredric Cover <fredric.cover.lkernel@gmail.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2026-06-18Merge tag 'v7.2-rc-part1-smb3-client-fixes' of ↵Linus Torvalds1-11/+19
git://git.samba.org/sfrench/cifs-2.6 Pull smb client updates from Steve French: - Three cleanup patches - Fix error return value in smb2_aead_req_alloc - Three compression fixes - Update i_blocks after write (fixes various xfstests) - Fix races in cifsd thread creation - Fix potential out of bounds read parsing security descriptors - Witness protocol fix - Fix umount bug - Mount fix - Fix cached directory entries on unlink/rmdir/rename * tag 'v7.2-rc-part1-smb3-client-fixes' of git://git.samba.org/sfrench/cifs-2.6: smb: client: Use more common code in SMB2_tcon() smb: client: Use more common error handling code in smb3_reconfigure() smb/client: Fix error code in smb2_aead_req_alloc() smb/client: clean up a type issue in cifs_xattr_get() smb/client: allow FS_IOC_SETFLAGS to clear compression smb/client: use writable handle for FS_IOC_SETFLAGS compression smb/client: always return a value for FS_IOC_GETFLAGS smb/client: update i_blocks after contiguous writes smb: client: fix races in cifsd thread creation cifs: validate full SID length in security descriptors smb: client: resolve SWN tcon from live registrations cifs: remove all cifs files before kill super smb: client: fix conflicting option validation for new mount API cifs: invalidate cfid on unlink/rename/rmdir
2026-06-14smb: client: fix races in cifsd thread creationFredric Cover1-11/+16
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 <fredric.cover.lkernel@gmail.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2026-06-14cifs: remove all cifs files before kill superJian Zhang1-0/+3
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 <zhangjian496@huawei.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2026-06-03smb: client: convert cifs_smb3_do_mount() to sget_fc()Christian Brauner1-2/+3
The CIFS mount path already runs through fs_context: smb3_get_tree() calls smb3_get_tree_common() with a struct fs_context * in hand. But the fc is dropped on the way to sget(). Plumb it through to sget_fc() so the legacy sget() interface can go. cifs_smb3_do_mount() now takes (struct fs_context *, struct smb3_fs_context *). The old (fs_type, flags) pair is reconstructed from fc->fs_type and fc->sb_flags. The flags argument was always passed as 0 by the sole caller anyway. The cifs_dbg diagnostic now prints fc->sb_flags directly. cifs_match_super() and cifs_set_super() were the two void-data callbacks for sget(). The match callback now takes (struct super_block *, struct fs_context *) and reads struct cifs_mnt_data out of fc->sget_key. The set callback is gone entirely: sget_fc() pre-populates sb->s_fs_info from fc->s_fs_info before invoking set() so set_anon_super_fc() (which just allocates an anon bdev) is sufficient. Before sget_fc() we stash cifs_sb in fc->s_fs_info, the per-mount data in fc->sget_key and force fc->sb_flags to SB_NODIRATIME | SB_NOATIME to reproduce the previous hard-coded behaviour (alloc_super() reads fc->sb_flags). The original sb_flags is saved and restored around the call so the rest of the mount path sees the same fc semantics as before. mnt_data.flags keeps its historical value of 0 so the CIFS_MS_MASK comparison in compare_mount_options() returns the same (always-equal) result. No functional change. With this in place sget() has no remaining CIFS caller. Link: https://patch.msgid.link/20260529-work-sget-v2-3-57bbe08604e4@kernel.org Signed-off-by: Christian Brauner (Amutable) <brauner@kernel.org>
2026-04-16smb: client: fix dir separator in SMB1 UNIX mountsPaulo Alcantara1-5/+5
When calling cifs_mount_get_tcon() with SMB1 UNIX mounts, @cifs_sb->mnt_cifs_flags needs to be read or updated only after calling reset_cifs_unix_caps(), otherwise it might end up with missing CIFS_MOUNT_POSIXACL and CIFS_MOUNT_POSIX_PATHS bits. This fixes the wrong dir separator used in paths caused by the missing CIFS_MOUNT_POSIX_PATHS bit in cifs_sb_info::mnt_cifs_flags. Reported-by: "Kris Karas (Bug Reporting)" <bugs-a21@moonlit-rail.com> Closes: https://lore.kernel.org/r/f758f4ff-4d54-4244-931d-38f469c3ff14@moonlit-rail.com Fixes: 4fc3a433c139 ("smb: client: use atomic_t for mnt_cifs_flags") Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Cc: David Howells <dhowells@redhat.com> Cc: linux-cifs@vger.kernel.org Cc: stable@vger.kernel.org Signed-off-by: Steve French <stfrench@microsoft.com>
2026-04-05fs: remove unncessary pagevec.h includesTal Zussman1-1/+0
Remove unused pagevec.h includes from .c files. These were found with the following command: grep -rl '#include.*pagevec\.h' --include='*.c' | while read f; do grep -qE 'PAGEVEC_SIZE|folio_batch' "$f" || echo "$f" done There are probably more removal candidates in .h files, but those are more complex to analyze. Link: https://lkml.kernel.org/r/20260225-pagevec_cleanup-v2-2-716868cc2d11@columbia.edu Signed-off-by: Tal Zussman <tz2294@columbia.edu> Reviewed-by: Jan Kara <jack@suse.cz> Acked-by: Zi Yan <ziy@nvidia.com> Acked-by: Chris Li <chrisl@kernel.org> Reviewed-by: Lorenzo Stoakes (Oracle) <ljs@kernel.org> Cc: Christian Brauner <brauner@kernel.org> Cc: David Hildenbrand (Arm) <david@kernel.org> Cc: Matthew Wilcox (Oracle) <willy@infradead.org> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
2026-03-15smb: client: fix krb5 mount with username optionPaulo Alcantara1-0/+4
Customer reported that some of their krb5 mounts were failing against a single server as the client was trying to mount the shares with wrong credentials. It turned out the client was reusing SMB session from first mount to try mounting the other shares, even though a different username= option had been specified to the other mounts. By using username mount option along with sec=krb5 to search for principals from keytab is supported by cifs.upcall(8) since cifs-utils-4.8. So fix this by matching username mount option in match_session() even with Kerberos. For example, the second mount below should fail with -ENOKEY as there is no 'foobar' principal in keytab (/etc/krb5.keytab). The client ends up reusing SMB session from first mount to perform the second one, which is wrong. ``` $ ktutil ktutil: add_entry -password -p testuser -k 1 -e aes256-cts Password for testuser@ZELDA.TEST: ktutil: write_kt /etc/krb5.keytab ktutil: quit $ klist -ke Keytab name: FILE:/etc/krb5.keytab KVNO Principal ---- ---------------------------------------------------------------- 1 testuser@ZELDA.TEST (aes256-cts-hmac-sha1-96) $ mount.cifs //w22-root2/scratch /mnt/1 -o sec=krb5,username=testuser $ mount.cifs //w22-root2/scratch /mnt/2 -o sec=krb5,username=foobar $ mount -t cifs | grep -Po 'username=\K\w+' testuser testuser ``` Reported-by: Oscar Santos <ossantos@redhat.com> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Cc: David Howells <dhowells@redhat.com> Cc: linux-cifs@vger.kernel.org Cc: stable@vger.kernel.org Signed-off-by: Steve French <stfrench@microsoft.com>
2026-02-27smb: client: Use snprintf in cifs_set_cifscredsThorsten Blum1-7/+7
Replace unbounded sprintf() calls with the safer snprintf(). Avoid using magic numbers and use strlen() to calculate the key descriptor buffer size. Save the size in a local variable and reuse it for the bounded snprintf() calls. Remove CIFSCREDS_DESC_SIZE. Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2026-02-26smb: client: Don't log plaintext credentials in cifs_set_cifscredsThorsten Blum1-1/+0
When debug logging is enabled, cifs_set_cifscreds() logs the key payload and exposes the plaintext username and password. Remove the debug log to avoid exposing credentials. Fixes: 8a8798a5ff90 ("cifs: fetch credentials out of keyring for non-krb5 auth multiuser mounts") Cc: stable@vger.kernel.org Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Signed-off-by: Steve French <stfrench@microsoft.com>
2026-02-26smb: client: use atomic_t for mnt_cifs_flagsPaulo Alcantara1-28/+37
Use atomic_t for cifs_sb_info::mnt_cifs_flags as it's currently accessed locklessly and may be changed concurrently in mount/remount and reconnect paths. Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Reviewed-by: David Howells <dhowells@redhat.com> Cc: linux-cifs@vger.kernel.org Signed-off-by: Steve French <stfrench@microsoft.com>
2026-02-21Convert 'alloc_obj' family to use the new default GFP_KERNEL argumentLinus Torvalds1-5/+5
This was done entirely with mindless brute force, using git grep -l '\<k[vmz]*alloc_objs*(.*, GFP_KERNEL)' | xargs sed -i 's/\(alloc_objs*(.*\), GFP_KERNEL)/\1)/' to convert the new alloc_obj() users that had a simple GFP_KERNEL argument to just drop that argument. Note that due to the extreme simplicity of the scripting, any slightly more complex cases spread over multiple lines would not be triggered: they definitely exist, but this covers the vast bulk of the cases, and the resulting diff is also then easier to check automatically. For the same reason the 'flex' versions will be done as a separate conversion. Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
2026-02-21treewide: Replace kmalloc with kmalloc_obj for non-scalar typesKees Cook1-5/+5
This is the result of running the Coccinelle script from scripts/coccinelle/api/kmalloc_objs.cocci. The script is designed to avoid scalar types (which need careful case-by-case checking), and instead replace kmalloc-family calls that allocate struct or union object instances: Single allocations: kmalloc(sizeof(TYPE), ...) are replaced with: kmalloc_obj(TYPE, ...) Array allocations: kmalloc_array(COUNT, sizeof(TYPE), ...) are replaced with: kmalloc_objs(TYPE, COUNT, ...) Flex array allocations: kmalloc(struct_size(PTR, FAM, COUNT), ...) are replaced with: kmalloc_flex(*PTR, FAM, COUNT, ...) (where TYPE may also be *VAR) The resulting allocations no longer return "void *", instead returning "TYPE *". Signed-off-by: Kees Cook <kees@kernel.org>
2026-02-08cifs: SMB1 split: connect.cDavid Howells1-251/+0
Split SMB1-specific connection management stuff to smb1ops.c and move CIFSTCon() to cifssmb.c. Signed-off-by: David Howells <dhowells@redhat.com> cc: Steve French <sfrench@samba.org> cc: Paulo Alcantara <pc@manguebit.org> cc: Enzo Matsumiya <ematsumiya@suse.de> cc: linux-cifs@vger.kernel.org cc: linux-fsdevel@vger.kernel.org cc: linux-kernel@vger.kernel.org Acked-by: Enzo Matsumiya <ematsumiya@suse.de> Signed-off-by: Steve French <stfrench@microsoft.com>
2026-02-08cifs: SMB1 split: Split SMB1 protocol defs into smb1pdu.hDavid Howells1-1/+0
Split SMB1 protocol defs into smb1pdu.h. This should perhaps go in the common/ directory. Signed-off-by: David Howells <dhowells@redhat.com> cc: Steve French <sfrench@samba.org> cc: Paulo Alcantara <pc@manguebit.org> cc: Enzo Matsumiya <ematsumiya@suse.de> cc: linux-cifs@vger.kernel.org cc: linux-fsdevel@vger.kernel.org cc: linux-kernel@vger.kernel.org Acked-by: Enzo Matsumiya <ematsumiya@suse.de> Signed-off-by: Steve French <stfrench@microsoft.com>
2026-02-08smb: client: add multichannel async work for CONFIG_CIFS_DFS_UPCALL=nHenrique Carvalho1-0/+14
Multichannel support is independent of DFS configuration. Extend the async multichannel setup to non-DFS cifs.ko. Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2026-02-08smb: client: introduce multichannel async work during mountHenrique Carvalho1-3/+58
Mounts can experience large delays when servers advertise interfaces that are unreachable from the client. To fix this, decouple channel addition from the synchronous mount path by introducing struct mchan_mount and running channel setup as background work. Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2026-02-08smb: client: add proper locking around ses->iface_last_updateHenrique Carvalho1-0/+2
There is a missing ses->iface_lock in cifs_setup_session, around ses->iface_last_update. Signed-off-by: Henrique Carvalho <henrique.carvalho@suse.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-12-05smb: client: improve error message when creating SMB sessionPaulo Alcantara1-1/+6
When failing to create a new SMB session with 'sec=krb5' for example, the following error message isn't very useful CIFS: VFS: \\srv Send error in SessSetup = -126 Improve it by printing the following instead on dmesg CIFS: VFS: \\srv failed to create a new SMB session with Kerberos: -126 Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Cc: Pierguido Lambri <plambri@redhat.com> Reviewed-by: David Howells <dhowells@redhat.com> Cc: linux-cifs@vger.kernel.org Signed-off-by: Steve French <stfrench@microsoft.com>
2025-12-05smb: client: relax session and tcon reconnect attemptsPaulo Alcantara1-2/+2
When the client re-establishes connection to the server, it will queue a worker thread that will attempt to reconnect sessions and tcons on every two seconds, which is kinda overkill as it is a very common scenario when having expired passwords or KRB5 TGT tickets, or deleted shares. Use an exponential backoff strategy to handle session/tcon reconnect attempts in the worker thread to prevent the client from overloading the system when it is very unlikely to re-establish any session/tcon soon while client is idle. Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Reviewed-by: David Howells <dhowells@redhat.com> Cc: Pierguido Lambri <plambri@redhat.com> Cc: linux-cifs@vger.kernel.org Signed-off-by: Steve French <stfrench@microsoft.com>
2025-12-05cifs: client: allow changing multichannel mount options on remountRajasi Mandal1-1/+3
Previously, the client did not update a session's channel state when multichannel or max_channels mount options were changed via remount. This led to inconsistent behavior and prevented enabling or disabling multichannel support without a full unmount/remount cycle. Enable dynamic reconfiguration of multichannel and max_channels during remount by: - Introducing smb3_sync_ses_chan_max(), a centralized function for channel updates which synchronizes the session's channels with the updated configuration. - Replacing cifs_disable_secondary_channels() with cifs_decrease_secondary_channels(), which accepts a disable_mchan flag to support multichannel disable when the server stops supporting multichannel. - Updating remount logic to detect changes in multichannel or max_channels and trigger appropriate session/channel updates. Current limitation: - The query_interfaces worker runs even when max_channels=1 so that multichannel can be enabled later via remount without requiring an unmount. This is a temporary approach and may be refined in the future. Users can safely modify multichannel and max_channels on an existing mount. The client will correctly adjust the session's channel state to match the new configuration, preserving durability where possible and avoiding unnecessary disconnects. Reviewed-by: Shyam Prasad N <sprasad@microsoft.com> Signed-off-by: Rajasi Mandal <rajasimandal@microsoft.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-12-05cifs: Add a tracepoint to log EIO errorsDavid Howells1-5/+6
Add a tracepoint to log EIO errors and give it the capacity to convey up to two integers of information. This is then wrapped with three functions: int smb_EIO(enum smb_eio_trace trace) int smb_EIO1(enum smb_eio_trace trace, unsigned long info) int smb_EIO2(enum smb_eio_trace trace, unsigned long info, unsigned long info2) depending on how many bits of info are desired to be logged with any particular trace. The functions all return -EIO and can be used in place of -EIO. The trace argument is an enum value that gets translated to a string when the trace is printed. This makes is easier to log EIO instances when the client is under high load than turning on a printk wrapper such as cifs_dbg(). Granted, EIO could have its own separate EIO printing since EIO shouldn't happen. Signed-off-by: David Howells <dhowells@redhat.com> Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> cc: linux-cifs@vger.kernel.org cc: linux-fsdevel@vger.kernel.org Signed-off-by: Steve French <stfrench@microsoft.com>
2025-12-05cifs: Remove the server pointer from smb_messageDavid Howells1-17/+17
Remove the server pointer from smb_message and instead pass it down to all the things that access it. Signed-off-by: David Howells <dhowells@redhat.com> Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> cc: Shyam Prasad N <sprasad@microsoft.com> cc: Tom Talpey <tom@talpey.com> (RDMA, smbdirect) cc: linux-cifs@vger.kernel.org cc: netfs@lists.linux.dev cc: linux-fsdevel@vger.kernel.org Signed-off-by: Steve French <stfrench@microsoft.com>
2025-12-05cifs: Remove the RFC1002 header from smb_hdrDavid Howells1-19/+17
Remove the RFC1002 header from struct smb_hdr as used for SMB-1.0. This simplifies the SMB-1.0 code by simplifying a lot of places that have to add or subtract 4 to work around the fact that the RFC1002 header isn't really part of the message and the base for various offsets within the message is from the base of the smb_hdr, not the RFC1002 header. Further, clean up a bunch of places that require an extra kvec struct specifically pointing to the RFC1002 header, such that kvec[0].iov_base must be exactly 4 bytes before kvec[1].iov_base. This allows the header preamble size stuff to be removed too. The size of the request and response message are then handed around either directly or by summing the size of all the iov_len members in the kvec array for which we have a count. Also, this simplifies and cleans up the common transmission and receive paths for SMB1 and SMB2/3 as there no longer needs to be special handling casing for SMB1 messages as the RFC1002 header is now generated on the fly for SMB1 as it is for SMB2/3. Signed-off-by: David Howells <dhowells@redhat.com> Reviewed-by: Tom Talpey <tom@talpey.com> Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> cc: Shyam Prasad N <sprasad@microsoft.com> cc: linux-cifs@vger.kernel.org cc: netfs@lists.linux.dev cc: linux-fsdevel@vger.kernel.org Signed-off-by: Steve French <stfrench@microsoft.com>
2025-12-03Merge tag 'v6.19-rc-smb-fixes' of git://git.samba.org/ksmbdLinus Torvalds1-3/+3
Pull smb client and server updates from Steve French: - server fixes: - IPC use after free locking fix - fix locking bug in delete paths - fix use after free in disconnect - fix underflow in locking check - error mapping improvement - socket listening improvement - return code mapping fixes - crypto improvements (use default libraries) - cleanup patches: - netfs - client checkpatch cleanup - server cleanup - move server/client duplicate code to common code - fix some defines to better match protocol specification - smbdirect (RDMA) fixes - client debugging improvements for leases * tag 'v6.19-rc-smb-fixes' of git://git.samba.org/ksmbd: (44 commits) cifs: Use netfs_alloc/free_folioq_buffer() smb: client: show smb lease key in open_dirs output smb: client: show smb lease key in open_files output ksmbd: ipc: fix use-after-free in ipc_msg_send_request smb: client: relax WARN_ON_ONCE(SMBDIRECT_SOCKET_*) checks in recv_done() and smbd_conn_upcall() smb: server: relax WARN_ON_ONCE(SMBDIRECT_SOCKET_*) checks in recv_done() and smb_direct_cm_handler() smb: smbdirect: introduce SMBDIRECT_CHECK_STATUS_{WARN,DISCONNECT}() smb: smbdirect: introduce SMBDIRECT_DEBUG_ERR_PTR() helper ksmbd: vfs: fix race on m_flags in vfs_cache ksmbd: Replace strcpy + strcat to improve convert_to_nt_pathname smb: move FILE_SYSTEM_ATTRIBUTE_INFO to common/fscc.h ksmbd: implement error handling for STATUS_INFO_LENGTH_MISMATCH in smb server ksmbd: fix use-after-free in ksmbd_tree_connect_put under concurrency ksmbd: server: avoid busy polling in accept loop smb: move create_durable_reconn to common/smb2pdu.h smb: fix some warnings reported by scripts/checkpatch.pl smb: do some cleanups smb: move FILE_SYSTEM_SIZE_INFO to common/fscc.h smb: move some duplicate struct definitions to common/fscc.h smb: move list of FileSystemAttributes to common/fscc.h ...
2025-11-30smb: fix some warnings reported by scripts/checkpatch.plChenXiaoSong1-2/+2
Fix the following warnings: WARNING: __always_unused or __maybe_unused is preferred over \ __attribute__((__unused__)) WARNING: Prefer __packed over __attribute__((packed)) Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-11-30smb: move get_rfc1002_len() to common/smbglob.hZhangGuoDong1-1/+1
Rename get_rfc1002_length() to get_rfc1002_len(), then move duplicate definitions to common header file. Co-developed-by: ChenXiaoSong <chenxiaosong@kylinos.cn> Signed-off-by: ChenXiaoSong <chenxiaosong@kylinos.cn> Signed-off-by: ZhangGuoDong <zhangguodong@kylinos.cn> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-11-27Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/netJakub Kicinski1-0/+1
Conflicts: net/xdp/xsk.c 0ebc27a4c67d ("xsk: avoid data corruption on cq descriptor number") 8da7bea7db69 ("xsk: add indirect call for xsk_destruct_skb") 30ed05adca4a ("xsk: use a smaller new lock for shared pool case") https://lore.kernel.org/20251127105450.4a1665ec@canb.auug.org.au https://lore.kernel.org/eb4eee14-7e24-4d1b-b312-e9ea738fefee@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2025-11-25smb: client: fix memory leak in cifs_construct_tcon()Paulo Alcantara1-0/+1
When having a multiuser mount with domain= specified and using cifscreds, cifs_set_cifscreds() will end up setting @ctx->domainname, so it needs to be freed before leaving cifs_construct_tcon(). This fixes the following memory leak reported by kmemleak: mount.cifs //srv/share /mnt -o domain=ZELDA,multiuser,... su - testuser cifscreds add -d ZELDA -u testuser ... ls /mnt/1 ... umount /mnt echo scan > /sys/kernel/debug/kmemleak cat /sys/kernel/debug/kmemleak unreferenced object 0xffff8881203c3f08 (size 8): comm "ls", pid 5060, jiffies 4307222943 hex dump (first 8 bytes): 5a 45 4c 44 41 00 cc cc ZELDA... backtrace (crc d109a8cf): __kmalloc_node_track_caller_noprof+0x572/0x710 kstrdup+0x3a/0x70 cifs_sb_tlink+0x1209/0x1770 [cifs] cifs_get_fattr+0xe1/0xf50 [cifs] cifs_get_inode_info+0xb5/0x240 [cifs] cifs_revalidate_dentry_attr+0x2d1/0x470 [cifs] cifs_getattr+0x28e/0x450 [cifs] vfs_getattr_nosec+0x126/0x180 vfs_statx+0xf6/0x220 do_statx+0xab/0x110 __x64_sys_statx+0xd5/0x130 do_syscall_64+0xbb/0x380 entry_SYSCALL_64_after_hwframe+0x77/0x7f Fixes: f2aee329a68f ("cifs: set domainName when a domain-key is used in multiuser") Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Reviewed-by: David Howells <dhowells@redhat.com> Cc: Jay Shin <jaeshin@redhat.com> Cc: stable@vger.kernel.org Cc: linux-cifs@vger.kernel.org Signed-off-by: Steve French <stfrench@microsoft.com>
2025-11-06Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/netJakub Kicinski1-27/+19
Cross-merge networking fixes after downstream PR (net-6.18-rc5). Conflicts: drivers/net/wireless/ath/ath12k/mac.c 9222582ec524 ("Revert "wifi: ath12k: Fix missing station power save configuration"") 6917e268c433 ("wifi: ath12k: Defer vdev bring-up until CSA finalize to avoid stale beacon") https://lore.kernel.org/11cece9f7e36c12efd732baa5718239b1bf8c950.camel@sipsolutions.net Adjacent changes: drivers/net/ethernet/intel/Kconfig b1d16f7c0063 ("libie: depend on DEBUG_FS when building LIBIE_FWLOG") 93f53db9f9dc ("ice: switch to Page Pool") Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2025-11-04net: Convert proto_ops connect() callbacks to use sockaddr_unsizedKees Cook1-1/+1
Update all struct proto_ops connect() callback function prototypes from "struct sockaddr *" to "struct sockaddr_unsized *" to avoid lying to the compiler about object sizes. Calls into struct proto handlers gain casts that will be removed in the struct proto conversion patch. No binary changes expected. Signed-off-by: Kees Cook <kees@kernel.org> Link: https://patch.msgid.link/20251104002617.2752303-3-kees@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2025-11-04net: Convert proto_ops bind() callbacks to use sockaddr_unsizedKees Cook1-1/+1
Update all struct proto_ops bind() callback function prototypes from "struct sockaddr *" to "struct sockaddr_unsized *" to avoid lying to the compiler about object sizes. Calls into struct proto handlers gain casts that will be removed in the struct proto conversion patch. No binary changes expected. Signed-off-by: Kees Cook <kees@kernel.org> Link: https://patch.msgid.link/20251104002617.2752303-2-kees@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
2025-10-29smb: client: call smbd_destroy() in the same splace as ↵Stefan Metzmacher1-6/+2
kernel_sock_shutdown()/sock_release() With commit b0432201a11b ("smb: client: let destroy_mr_list() keep smbdirect_mr_io memory if registered") the changes from commit 214bab448476 ("cifs: Call MID callback before destroying transport") and commit 1d2a4f57cebd ("cifs:smbd When reconnecting to server, call smbd_destroy() after all MIDs have been called") are no longer needed. And it's better to use the same logic flow, so that the chance of smbdirect related problems is smaller. Fixes: 214bab448476 ("cifs: Call MID callback before destroying transport") Fixes: 1d2a4f57cebd ("cifs:smbd When reconnecting to server, call smbd_destroy() after all MIDs have been called") Cc: Steve French <smfrench@gmail.com> Cc: Tom Talpey <tom@talpey.com> Cc: Long Li <longli@microsoft.com> Cc: Namjae Jeon <linkinjeon@kernel.org> Cc: linux-cifs@vger.kernel.org Cc: samba-technical@lists.samba.org Signed-off-by: Stefan Metzmacher <metze@samba.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-10-29smb: client: handle lack of IPC in dfs_cache_refresh()Paulo Alcantara1-21/+17
In very rare cases, DFS mounts could end up with SMB sessions without any IPC connections. These mounts are only possible when having unexpired cached DFS referrals, hence not requiring any IPC connections during the mount process. Try to establish those missing IPC connections when refreshing DFS referrals. If the server is still rejecting it, then simply ignore and leave expired cached DFS referral for any potential DFS failovers. Reported-by: Jay Shin <jaeshin@redhat.com> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Cc: David Howells <dhowells@redhat.com> Cc: linux-cifs@vger.kernel.org Signed-off-by: Steve French <stfrench@microsoft.com>
2025-08-13smb: client: remove redundant lstrp update in negotiate protocolWang Zhaolong1-1/+0
Commit 34331d7beed7 ("smb: client: fix first command failure during re-negotiation") addressed a race condition by updating lstrp before entering negotiate state. However, this approach may have some unintended side effects. The lstrp field is documented as "when we got last response from this server", and updating it before actually receiving a server response could potentially affect other mechanisms that rely on this timestamp. For example, the SMB echo detection logic also uses lstrp as a reference point. In scenarios with frequent user operations during reconnect states, the repeated calls to cifs_negotiate_protocol() might continuously update lstrp, which could interfere with the echo detection timing. Additionally, commit 266b5d02e14f ("smb: client: fix race condition in negotiate timeout by using more precise timing") introduced a dedicated neg_start field specifically for tracking negotiate start time. This provides a more precise solution for the original race condition while preserving the intended semantics of lstrp. Since the race condition is now properly handled by the neg_start mechanism, the lstrp update in cifs_negotiate_protocol() is no longer necessary and can be safely removed. Fixes: 266b5d02e14f ("smb: client: fix race condition in negotiate timeout by using more precise timing") Cc: stable@vger.kernel.org Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-08-13smb: client: fix mid_q_entry memleak leak with per-mid lockingWang Zhaolong1-4/+4
This is step 4/4 of a patch series to fix mid_q_entry memory leaks caused by race conditions in callback execution. In compound_send_recv(), when wait_for_response() is interrupted by signals, the code attempts to cancel pending requests by changing their callbacks to cifs_cancelled_callback. However, there's a race condition between signal interruption and network response processing that causes both mid_q_entry and server buffer leaks: ``` User foreground process cifsd cifs_readdir open_cached_dir cifs_send_recv compound_send_recv smb2_setup_request smb2_mid_entry_alloc smb2_get_mid_entry smb2_mid_entry_alloc mempool_alloc // alloc mid kref_init(&temp->refcount); // refcount = 1 mid[0]->callback = cifs_compound_callback; mid[1]->callback = cifs_compound_last_callback; smb_send_rqst rc = wait_for_response wait_event_state TASK_KILLABLE cifs_demultiplex_thread allocate_buffers server->bigbuf = cifs_buf_get() standard_receive3 ->find_mid() smb2_find_mid __smb2_find_mid kref_get(&mid->refcount) // +1 cifs_handle_standard handle_mid /* bigbuf will also leak */ mid->resp_buf = server->bigbuf server->bigbuf = NULL; dequeue_mid /* in for loop */ mids[0]->callback cifs_compound_callback /* Signal interrupts wait: rc = -ERESTARTSYS */ /* if (... || midQ[i]->mid_state == MID_RESPONSE_RECEIVED) *? midQ[0]->callback = cifs_cancelled_callback; cancelled_mid[i] = true; /* The change comes too late */ mid->mid_state = MID_RESPONSE_READY release_mid // -1 /* cancelled_mid[i] == true causes mid won't be released in compound_send_recv cleanup */ /* cifs_cancelled_callback won't executed to release mid */ ``` The root cause is that there's a race between callback assignment and execution. Fix this by introducing per-mid locking: - Add spinlock_t mid_lock to struct mid_q_entry - Add mid_execute_callback() for atomic callback execution - Use mid_lock in cancellation paths to ensure atomicity This ensures that either the original callback or the cancellation callback executes atomically, preventing reference count leaks when requests are interrupted by signals. Link: https://bugzilla.kernel.org/show_bug.cgi?id=220404 Fixes: ee258d79159a ("CIFS: Move credit processing to mid callbacks for SMB3") Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-08-05smb: client: smb: client: eliminate mid_flags fieldWang Zhaolong1-5/+5
This is step 3/4 of a patch series to fix mid_q_entry memory leaks caused by race conditions in callback execution. Replace the mid_flags bitmask with dedicated boolean fields to simplify locking logic and improve code readability: - Replace MID_DELETED with bool deleted_from_q - Replace MID_WAIT_CANCELLED with bool wait_cancelled - Remove mid_flags field entirely The new boolean fields have clearer semantics: - deleted_from_q: whether mid has been removed from pending_mid_q - wait_cancelled: whether request was cancelled during wait This change reduces memory usage (from 4-byte bitmask to 2 boolean flags) and eliminates confusion about which lock protects which flag bits, preparing for per-mid locking in the next patch. Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> Acked-by: Enzo Matsumiya <ematsumiya@suse.de> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-08-05smb: client: add mid_counter_lock to protect the mid counter counterWang Zhaolong1-2/+3
This is step 2/4 of a patch series to fix mid_q_entry memory leaks caused by race conditions in callback execution. Add a dedicated mid_counter_lock to protect current_mid counter, separating it from mid_queue_lock which protects pending_mid_q operations. This reduces lock contention and prepares for finer- grained locking in subsequent patches. Changes: - Add TCP_Server_Info->mid_counter_lock spinlock - Rename CurrentMid to current_mid for consistency - Use mid_counter_lock to protect current_mid access - Update locking documentation in cifsglob.h This separation allows mid allocation to proceed without blocking queue operations, improving performance under heavy load. Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> Acked-by: Enzo Matsumiya <ematsumiya@suse.de> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-08-05smb: client: rename server mid_lock to mid_queue_lockWang Zhaolong1-10/+10
This is step 1/4 of a patch series to fix mid_q_entry memory leaks caused by race conditions in callback execution. The current mid_lock name is somewhat ambiguous about what it protects. To prepare for splitting this lock into separate, more granular locks, this patch renames mid_lock to mid_queue_lock to clearly indicate its specific responsibility for protecting the pending_mid_q list and related queue operations. No functional changes are made in this patch - it only prepares the codebase for the lock splitting that follows. - mid_queue_lock for queue operations - mid_counter_lock for mid counter operations - per-mid locks for individual mid state management Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> Acked-by: Enzo Matsumiya <ematsumiya@suse.de> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-07-27smb: client: fix netns refcount leak after net_passive changesWang Zhaolong1-6/+3
After commit 5c70eb5c593d ("net: better track kernel sockets lifetime"), kernel sockets now use net_passive reference counting. However, commit 95d2b9f693ff ("Revert "smb: client: fix TCP timers deadlock after rmmod"") restored the manual socket refcount manipulation without adapting to this new mechanism, causing a memory leak. The issue can be reproduced by[1]: 1. Creating a network namespace 2. Mounting and Unmounting CIFS within the namespace 3. Deleting the namespace Some memory leaks may appear after a period of time following step 3. unreferenced object 0xffff9951419f6b00 (size 256): comm "ip", pid 447, jiffies 4294692389 (age 14.730s) hex dump (first 32 bytes): 1b 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ 00 00 00 00 00 00 00 00 80 77 c2 44 51 99 ff ff .........w.DQ... backtrace: __kmem_cache_alloc_node+0x30e/0x3d0 __kmalloc+0x52/0x120 net_alloc_generic+0x1d/0x30 copy_net_ns+0x86/0x200 create_new_namespaces+0x117/0x300 unshare_nsproxy_namespaces+0x60/0xa0 ksys_unshare+0x148/0x360 __x64_sys_unshare+0x12/0x20 do_syscall_64+0x59/0x110 entry_SYSCALL_64_after_hwframe+0x78/0xe2 ... unreferenced object 0xffff9951442e7500 (size 32): comm "mount.cifs", pid 475, jiffies 4294693782 (age 13.343s) hex dump (first 32 bytes): 40 c5 38 46 51 99 ff ff 18 01 96 42 51 99 ff ff @.8FQ......BQ... 01 00 00 00 6f 00 c5 07 6f 00 d8 07 00 00 00 00 ....o...o....... backtrace: __kmem_cache_alloc_node+0x30e/0x3d0 kmalloc_trace+0x2a/0x90 ref_tracker_alloc+0x8e/0x1d0 sk_alloc+0x18c/0x1c0 inet_create+0xf1/0x370 __sock_create+0xd7/0x1e0 generic_ip_connect+0x1d4/0x5a0 [cifs] cifs_get_tcp_session+0x5d0/0x8a0 [cifs] cifs_mount_get_session+0x47/0x1b0 [cifs] dfs_mount_share+0xfa/0xa10 [cifs] cifs_mount+0x68/0x2b0 [cifs] cifs_smb3_do_mount+0x10b/0x760 [cifs] smb3_get_tree+0x112/0x2e0 [cifs] vfs_get_tree+0x29/0xf0 path_mount+0x2d4/0xa00 __se_sys_mount+0x165/0x1d0 Root cause: When creating kernel sockets, sk_alloc() calls net_passive_inc() for sockets with sk_net_refcnt=0. The CIFS code manually converts kernel sockets to user sockets by setting sk_net_refcnt=1, but doesn't call the corresponding net_passive_dec(). This creates an imbalance in the net_passive counter, which prevents the network namespace from being destroyed when its last user reference is dropped. As a result, the entire namespace and all its associated resources remain allocated. Timeline of patches leading to this issue: - commit ef7134c7fc48 ("smb: client: Fix use-after-free of network namespace.") in v6.12 fixed the original netns UAF by manually managing socket refcounts - commit e9f2517a3e18 ("smb: client: fix TCP timers deadlock after rmmod") in v6.13 attempted to use kernel sockets but introduced TCP timer issues - commit 5c70eb5c593d ("net: better track kernel sockets lifetime") in v6.14-rc5 introduced the net_passive mechanism with sk_net_refcnt_upgrade() for proper socket conversion - commit 95d2b9f693ff ("Revert "smb: client: fix TCP timers deadlock after rmmod"") in v6.15-rc3 reverted to manual refcount management without adapting to the new net_passive changes Fix this by using sk_net_refcnt_upgrade() which properly handles the net_passive counter when converting kernel sockets to user sockets. Link: https://bugzilla.kernel.org/show_bug.cgi?id=220343 [1] Fixes: 95d2b9f693ff ("Revert "smb: client: fix TCP timers deadlock after rmmod"") Cc: stable@vger.kernel.org Reviewed-by: Kuniyuki Iwashima <kuniyu@google.com> Reviewed-by: Enzo Matsumiya <ematsumiya@suse.de> Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-07-03smb: client: fix race condition in negotiate timeout by using more precise ↵Wang Zhaolong1-3/+4
timing When the SMB server reboots and the client immediately accesses the mount point, a race condition can occur that causes operations to fail with "Host is down" error. Reproduction steps: # Mount SMB share mount -t cifs //192.168.245.109/TEST /mnt/ -o xxxx ls /mnt # Reboot server ssh root@192.168.245.109 reboot ssh root@192.168.245.109 /path/to/cifs_server_setup.sh ssh root@192.168.245.109 systemctl stop firewalld # Immediate access fails ls /mnt ls: cannot access '/mnt': Host is down # But works if there is a delay The issue is caused by a race condition between negotiate and reconnect. The 20-second negotiate timeout mechanism can interfere with the normal recovery process when both are triggered simultaneously. ls cifsd --------------------------------------------------- cifs_getattr cifs_revalidate_dentry cifs_get_inode_info cifs_get_fattr smb2_query_path_info smb2_compound_op SMB2_open_init smb2_reconnect cifs_negotiate_protocol smb2_negotiate cifs_send_recv smb_send_rqst wait_for_response cifs_demultiplex_thread cifs_read_from_socket cifs_readv_from_socket server_unresponsive cifs_reconnect __cifs_reconnect cifs_abort_connection mid->mid_state = MID_RETRY_NEEDED cifs_wake_up_task cifs_sync_mid_result // case MID_RETRY_NEEDED rc = -EAGAIN; // In smb2_negotiate() rc = -EHOSTDOWN; The server_unresponsive() timeout triggers cifs_reconnect(), which aborts ongoing mid requests and causes the ls command to receive -EAGAIN, leading to -EHOSTDOWN. Fix this by introducing a dedicated `neg_start` field to precisely tracks when the negotiate process begins. The timeout check now uses this accurate timestamp instead of `lstrp`, ensuring that: 1. Timeout is only triggered after negotiate has actually run for 20s 2. The mechanism doesn't interfere with concurrent recovery processes 3. Uninitialized timestamps (value 0) don't trigger false timeouts Fixes: 7ccc1465465d ("smb: client: fix hang in wait_for_response() for negproto") Signed-off-by: Wang Zhaolong <wangzhaolong@huaweicloud.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-06-30cifs: all initializations for tcon should happen in tcon_info_allocShyam Prasad N1-7/+1
Today, a few work structs inside tcon are initialized inside cifs_get_tcon and not in tcon_info_alloc. As a result, if a tcon is obtained from tcon_info_alloc, but not called as a part of cifs_get_tcon, we may trip over. Cc: <stable@vger.kernel.org> Signed-off-by: Shyam Prasad N <sprasad@microsoft.com> Reviewed-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-06-26smb: client: fix potential deadlock when reconnecting channelsPaulo Alcantara1-22/+36
Fix cifs_signal_cifsd_for_reconnect() to take the correct lock order and prevent the following deadlock from happening ====================================================== WARNING: possible circular locking dependency detected 6.16.0-rc3-build2+ #1301 Tainted: G S W ------------------------------------------------------ cifsd/6055 is trying to acquire lock: ffff88810ad56038 (&tcp_ses->srv_lock){+.+.}-{3:3}, at: cifs_signal_cifsd_for_reconnect+0x134/0x200 but task is already holding lock: ffff888119c64330 (&ret_buf->chan_lock){+.+.}-{3:3}, at: cifs_signal_cifsd_for_reconnect+0xcf/0x200 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #2 (&ret_buf->chan_lock){+.+.}-{3:3}: validate_chain+0x1cf/0x270 __lock_acquire+0x60e/0x780 lock_acquire.part.0+0xb4/0x1f0 _raw_spin_lock+0x2f/0x40 cifs_setup_session+0x81/0x4b0 cifs_get_smb_ses+0x771/0x900 cifs_mount_get_session+0x7e/0x170 cifs_mount+0x92/0x2d0 cifs_smb3_do_mount+0x161/0x460 smb3_get_tree+0x55/0x90 vfs_get_tree+0x46/0x180 do_new_mount+0x1b0/0x2e0 path_mount+0x6ee/0x740 do_mount+0x98/0xe0 __do_sys_mount+0x148/0x180 do_syscall_64+0xa4/0x260 entry_SYSCALL_64_after_hwframe+0x76/0x7e -> #1 (&ret_buf->ses_lock){+.+.}-{3:3}: validate_chain+0x1cf/0x270 __lock_acquire+0x60e/0x780 lock_acquire.part.0+0xb4/0x1f0 _raw_spin_lock+0x2f/0x40 cifs_match_super+0x101/0x320 sget+0xab/0x270 cifs_smb3_do_mount+0x1e0/0x460 smb3_get_tree+0x55/0x90 vfs_get_tree+0x46/0x180 do_new_mount+0x1b0/0x2e0 path_mount+0x6ee/0x740 do_mount+0x98/0xe0 __do_sys_mount+0x148/0x180 do_syscall_64+0xa4/0x260 entry_SYSCALL_64_after_hwframe+0x76/0x7e -> #0 (&tcp_ses->srv_lock){+.+.}-{3:3}: check_noncircular+0x95/0xc0 check_prev_add+0x115/0x2f0 validate_chain+0x1cf/0x270 __lock_acquire+0x60e/0x780 lock_acquire.part.0+0xb4/0x1f0 _raw_spin_lock+0x2f/0x40 cifs_signal_cifsd_for_reconnect+0x134/0x200 __cifs_reconnect+0x8f/0x500 cifs_handle_standard+0x112/0x280 cifs_demultiplex_thread+0x64d/0xbc0 kthread+0x2f7/0x310 ret_from_fork+0x2a/0x230 ret_from_fork_asm+0x1a/0x30 other info that might help us debug this: Chain exists of: &tcp_ses->srv_lock --> &ret_buf->ses_lock --> &ret_buf->chan_lock Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&ret_buf->chan_lock); lock(&ret_buf->ses_lock); lock(&ret_buf->chan_lock); lock(&tcp_ses->srv_lock); *** DEADLOCK *** 3 locks held by cifsd/6055: #0: ffffffff857de398 (&cifs_tcp_ses_lock){+.+.}-{3:3}, at: cifs_signal_cifsd_for_reconnect+0x7b/0x200 #1: ffff888119c64060 (&ret_buf->ses_lock){+.+.}-{3:3}, at: cifs_signal_cifsd_for_reconnect+0x9c/0x200 #2: ffff888119c64330 (&ret_buf->chan_lock){+.+.}-{3:3}, at: cifs_signal_cifsd_for_reconnect+0xcf/0x200 Cc: linux-cifs@vger.kernel.org Reported-by: David Howells <dhowells@redhat.com> Fixes: d7d7a66aacd6 ("cifs: avoid use of global locks for high contention data") Reviewed-by: David Howells <dhowells@redhat.com> Tested-by: David Howells <dhowells@redhat.com> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Signed-off-by: David Howells <dhowells@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-06-21smb: client: fix first command failure during re-negotiationzhangjian1-0/+1
after fabc4ed200f9, server_unresponsive add a condition to check whether client need to reconnect depending on server->lstrp. When client failed to reconnect for some time and abort connection, server->lstrp is updated for the last time. In the following scene, server->lstrp is too old. This cause next command failure in re-negotiation rather than waiting for re-negotiation done. 1. mount -t cifs -o username=Everyone,echo_internal=10 //$server_ip/export /mnt 2. ssh $server_ip "echo b > /proc/sysrq-trigger &" 3. ls /mnt 4. sleep 21s 5. ssh $server_ip "service firewalld stop" 6. ls # return EHOSTDOWN If the interval between 5 and 6 is too small, 6 may trigger sending negotiation request. Before backgrounding cifsd thread try to receive negotiation response from server in cifs_readv_from_socket, server_unresponsive may trigger cifs_reconnect which cause 6 to be failed: ls thread ---------------- smb2_negotiate server->tcpStatus = CifsInNegotiate compound_send_recv wait_for_compound_request cifsd thread ---------------- cifs_readv_from_socket server_unresponsive server->tcpStatus == CifsInNegotiate && jiffies > server->lstrp + 20s cifs_reconnect cifs_abort_connection: mid_state = MID_RETRY_NEEDED ls thread ---------------- cifs_sync_mid_result return EAGAIN smb2_negotiate return EHOSTDOWN Though server->lstrp means last server response time, it is updated in cifs_abort_connection and cifs_get_tcp_session. We can also update server->lstrp before switching into CifsInNegotiate state to avoid failure in 6. Fixes: 7ccc1465465d ("smb: client: fix hang in wait_for_response() for negproto") Acked-by: Paulo Alcantara (Red Hat) <pc@manguebit.org> Acked-by: Meetakshi Setiya <msetiya@microsoft.com> Signed-off-by: zhangjian <zhangjian496@huawei.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-06-09smb: client: disable path remapping with POSIX extensionsPhilipp Kerling1-2/+8
If SMB 3.1.1 POSIX Extensions are available and negotiated, the client should be able to use all characters and not remap anything. Currently, the user has to explicitly request this behavior by specifying the "nomapposix" mount option. Link: https://lore.kernel.org/4195bb677b33d680e77549890a4f4dd3b474ceaf.camel@rx2.rx-server.de Signed-off-by: Philipp Kerling <pkerling@casix.org> Reviewed-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>