summaryrefslogtreecommitdiff
path: root/fs/smb/server
AgeCommit message (Collapse)Author
2025-03-10ksmbd: prevent connection release during oplock break notificationNamjae Jeon
ksmbd_work could be freed when after connection release. Increment r_count of ksmbd_conn to indicate that requests are not finished yet and to not release the connection. Cc: stable@vger.kernel.org Reported-by: Norbert Szetei <norbert@doyensec.com> Tested-by: Norbert Szetei <norbert@doyensec.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-03-10ksmbd: fix use-after-free in ksmbd_free_work_structNamjae Jeon
->interim_entry of ksmbd_work could be deleted after oplock is freed. We don't need to manage it with linked list. The interim request could be immediately sent whenever a oplock break wait is needed. Cc: stable@vger.kernel.org Reported-by: Norbert Szetei <norbert@doyensec.com> Tested-by: Norbert Szetei <norbert@doyensec.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-03-02ksmbd: fix incorrect validation for num_aces field of smb_aclNamjae Jeon
parse_dcal() validate num_aces to allocate posix_ace_state_array. if (num_aces > ULONG_MAX / sizeof(struct smb_ace *)) It is an incorrect validation that we can create an array of size ULONG_MAX. smb_acl has ->size field to calculate actual number of aces in request buffer size. Use this to check invalid num_aces. Reported-by: Igor Leite Ladessa <igor-ladessa@hotmail.com> Tested-by: Igor Leite Ladessa <igor-ladessa@hotmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-03-02smb: common: change the data type of num_aces to le16Namjae Jeon
2.4.5 in [MS-DTYP].pdf describe the data type of num_aces as le16. AceCount (2 bytes): An unsigned 16-bit integer that specifies the count of the number of ACE records in the ACL. Change it to le16 and add reserved field to smb_acl struct. Reported-by: Igor Leite Ladessa <igor-ladessa@hotmail.com> Tested-by: Igor Leite Ladessa <igor-ladessa@hotmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-03-02ksmbd: fix bug on trap in smb2_lockNamjae Jeon
If lock count is greater than 1, flags could be old value. It should be checked with flags of smb_lock, not flags. It will cause bug-on trap from locks_free_lock in error handling routine. Cc: stable@vger.kernel.org Reported-by: Norbert Szetei <norbert@doyensec.com> Tested-by: Norbert Szetei <norbert@doyensec.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-03-02ksmbd: fix use-after-free in smb2_lockNamjae Jeon
If smb_lock->zero_len has value, ->llist of smb_lock is not delete and flock is old one. It will cause use-after-free on error handling routine. Cc: stable@vger.kernel.org Reported-by: Norbert Szetei <norbert@doyensec.com> Tested-by: Norbert Szetei <norbert@doyensec.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-03-02ksmbd: fix type confusion via race condition when using ipc_msg_send_requestNamjae Jeon
req->handle is allocated using ksmbd_acquire_id(&ipc_ida), based on ida_alloc. req->handle from ksmbd_ipc_login_request and FSCTL_PIPE_TRANSCEIVE ioctl can be same and it could lead to type confusion between messages, resulting in access to unexpected parts of memory after an incorrect delivery. ksmbd check type of ipc response but missing add continue to check next ipc reponse. Cc: stable@vger.kernel.org Reported-by: Norbert Szetei <norbert@doyensec.com> Tested-by: Norbert Szetei <norbert@doyensec.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-03-02ksmbd: fix out-of-bounds in parse_sec_desc()Namjae Jeon
If osidoffset, gsidoffset and dacloffset could be greater than smb_ntsd struct size. If it is smaller, It could cause slab-out-of-bounds. And when validating sid, It need to check it included subauth array size. Cc: stable@vger.kernel.org Reported-by: Norbert Szetei <norbert@doyensec.com> Tested-by: Norbert Szetei <norbert@doyensec.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-01-23Merge tag 'v6.14-rc-ksmbd-server-fixes' of git://git.samba.org/ksmbdLinus Torvalds
Pull smb server updates from Steve French: "Three ksmbd server fixes: - Fix potential memory corruption in IPC calls - Support FSCTL_QUERY_INTERFACE_INFO for more configurations - Remove some unused functions" * tag 'v6.14-rc-ksmbd-server-fixes' of git://git.samba.org/ksmbd: ksmbd: fix integer overflows on 32 bit systems ksmbd: browse interfaces list on FSCTL_QUERY_INTERFACE_INFO IOCTL ksmbd: Remove unused functions
2025-01-20Merge tag 'kernel-6.14-rc1.cred' of ↵Linus Torvalds
git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs Pull cred refcount updates from Christian Brauner: "For the v6.13 cycle we switched overlayfs to a variant of override_creds() that doesn't take an extra reference. To this end the {override,revert}_creds_light() helpers were introduced. This generalizes the idea behind {override,revert}_creds_light() to the {override,revert}_creds() helpers. Afterwards overriding and reverting credentials is reference count free unless the caller explicitly takes a reference. All callers have been appropriately ported" * tag 'kernel-6.14-rc1.cred' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs: (30 commits) cred: fold get_new_cred_many() into get_cred_many() cred: remove unused get_new_cred() nfsd: avoid pointless cred reference count bump cachefiles: avoid pointless cred reference count bump dns_resolver: avoid pointless cred reference count bump trace: avoid pointless cred reference count bump cgroup: avoid pointless cred reference count bump acct: avoid pointless reference count bump io_uring: avoid pointless cred reference count bump smb: avoid pointless cred reference count bump cifs: avoid pointless cred reference count bump cifs: avoid pointless cred reference count bump ovl: avoid pointless cred reference count bump open: avoid pointless cred reference count bump nfsfh: avoid pointless cred reference count bump nfs/nfs4recover: avoid pointless cred reference count bump nfs/nfs4idmap: avoid pointless reference count bump nfs/localio: avoid pointless cred reference count bumps coredump: avoid pointless cred reference count bump binfmt_misc: avoid pointless cred reference count bump ...
2025-01-15ksmbd: fix integer overflows on 32 bit systemsDan Carpenter
On 32bit systems the addition operations in ipc_msg_alloc() can potentially overflow leading to memory corruption. Add bounds checking using KSMBD_IPC_MAX_PAYLOAD to avoid overflow. Fixes: 0626e6641f6b ("cifsd: add server handler for central processing and tranport layers") Cc: stable@vger.kernel.org Signed-off-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-01-15ksmbd: browse interfaces list on FSCTL_QUERY_INTERFACE_INFO IOCTLNamjae Jeon
ksmbd.mount will give each interfaces list and bind_interfaces_only flags to ksmbd server. Previously, the interfaces list was sent only when bind_interfaces_only was enabled. ksmbd server browse only interfaces list given from ksmbd.conf on FSCTL_QUERY_INTERFACE_INFO IOCTL. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-01-15ksmbd: Remove unused functionsDr. David Alan Gilbert
ksmbd_rpc_rap() was added in 2021 as part of commit 0626e6641f6b ("cifsd: add server handler for central processing and tranport layers") ksmbd_vfs_posix_lock_wait_timeout() was added in 2021 as part of commit f44158485826 ("cifsd: add file operations") both have remained unused. Remove them. Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-01-07ksmbd: Implement new SMB3 POSIX typeNamjae Jeon
As SMB3 posix extension specification, Give posix file type to posix mode. https://www.samba.org/~slow/SMB3_POSIX/fscc_posix_extensions.html#posix-file-type-definition Cc: stable@vger.kernel.org Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-01-07ksmbd: fix unexpectedly changed path in ksmbd_vfs_kern_path_lockedHe Wang
When `ksmbd_vfs_kern_path_locked` met an error and it is not the last entry, it will exit without restoring changed path buffer. But later this buffer may be used as the filename for creation. Fixes: c5a709f08d40 ("ksmbd: handle caseless file creation") Signed-off-by: He Wang <xw897002528@gmail.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-01-05ksmbd: Remove unneeded if check in ksmbd_rdma_capable_netdev()Thorsten Blum
Remove the unnecessary if check and assign the result directly. Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2025-01-05ksmbd: fix a missing return value check bugWentao Liang
In the smb2_send_interim_resp(), if ksmbd_alloc_work_struct() fails to allocate a node, it returns a NULL pointer to the in_work pointer. This can lead to an illegal memory write of in_work->response_buf when allocate_interim_rsp_buf() attempts to perform a kzalloc() on it. To address this issue, incorporating a check for the return value of ksmbd_alloc_work_struct() ensures that the function returns immediately upon allocation failure, thereby preventing the aforementioned illegal memory access. Fixes: 041bba4414cd ("ksmbd: fix wrong interim response on compound") Signed-off-by: Wentao Liang <liangwentao@iscas.ac.cn> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-12-15ksmbd: conn lock to serialize smb2 negotiateNamjae Jeon
If client send parallel smb2 negotiate request on same connection, ksmbd_conn can be racy. smb2 negotiate handling that are not performance-related can be serialized with conn lock. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-12-15ksmbd: fix broken transfers when exceeding max simultaneous operationsMarios Makassikis
Since commit 0a77d947f599 ("ksmbd: check outstanding simultaneous SMB operations"), ksmbd enforces a maximum number of simultaneous operations for a connection. The problem is that reaching the limit causes ksmbd to close the socket, and the client has no indication that it should have slowed down. This behaviour can be reproduced by setting "smb2 max credits = 128" (or lower), and transferring a large file (25GB). smbclient fails as below: $ smbclient //192.168.1.254/testshare -U user%pass smb: \> put file.bin cli_push returned NT_STATUS_USER_SESSION_DELETED putting file file.bin as \file.bin smb2cli_req_compound_submit: Insufficient credits. 0 available, 1 needed NT_STATUS_INTERNAL_ERROR closing remote file \file.bin smb: \> smb2cli_req_compound_submit: Insufficient credits. 0 available, 1 needed Windows clients fail with 0x8007003b (with smaller files even). Fix this by delaying reading from the socket until there's room to allocate a request. This effectively applies backpressure on the client, so the transfer completes, albeit at a slower rate. Fixes: 0a77d947f599 ("ksmbd: check outstanding simultaneous SMB operations") Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-12-15ksmbd: count all requests in req_running counterMarios Makassikis
This changes the semantics of req_running to count all in-flight requests on a given connection, rather than the number of elements in the conn->request list. The latter is used only in smb2_cancel, and the counter is not used Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-12-10ksmbd: set ATTR_CTIME flags when setting mtimeNamjae Jeon
David reported that the new warning from setattr_copy_mgtime is coming like the following. [ 113.215316] ------------[ cut here ]------------ [ 113.215974] WARNING: CPU: 1 PID: 31 at fs/attr.c:300 setattr_copy+0x1ee/0x200 [ 113.219192] CPU: 1 UID: 0 PID: 31 Comm: kworker/1:1 Not tainted 6.13.0-rc1+ #234 [ 113.220127] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.2-3-gd478f380-rebuilt.opensuse.org 04/01/2014 [ 113.221530] Workqueue: ksmbd-io handle_ksmbd_work [ksmbd] [ 113.222220] RIP: 0010:setattr_copy+0x1ee/0x200 [ 113.222833] Code: 24 28 49 8b 44 24 30 48 89 53 58 89 43 6c 5b 41 5c 41 5d 41 5e 41 5f 5d c3 cc cc cc cc 48 89 df e8 77 d6 ff ff e9 cd fe ff ff <0f> 0b e9 be fe ff ff 66 0 [ 113.225110] RSP: 0018:ffffaf218010fb68 EFLAGS: 00010202 [ 113.225765] RAX: 0000000000000120 RBX: ffffa446815f8568 RCX: 0000000000000003 [ 113.226667] RDX: ffffaf218010fd38 RSI: ffffa446815f8568 RDI: ffffffff94eb03a0 [ 113.227531] RBP: ffffaf218010fb90 R08: 0000001a251e217d R09: 00000000675259fa [ 113.228426] R10: 0000000002ba8a6d R11: ffffa4468196c7a8 R12: ffffaf218010fd38 [ 113.229304] R13: 0000000000000120 R14: ffffffff94eb03a0 R15: 0000000000000000 [ 113.230210] FS: 0000000000000000(0000) GS:ffffa44739d00000(0000) knlGS:0000000000000000 [ 113.231215] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 [ 113.232055] CR2: 00007efe0053d27e CR3: 000000000331a000 CR4: 00000000000006b0 [ 113.232926] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 [ 113.233812] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 [ 113.234797] Call Trace: [ 113.235116] <TASK> [ 113.235393] ? __warn+0x73/0xd0 [ 113.235802] ? setattr_copy+0x1ee/0x200 [ 113.236299] ? report_bug+0xf3/0x1e0 [ 113.236757] ? handle_bug+0x4d/0x90 [ 113.237202] ? exc_invalid_op+0x13/0x60 [ 113.237689] ? asm_exc_invalid_op+0x16/0x20 [ 113.238185] ? setattr_copy+0x1ee/0x200 [ 113.238692] btrfs_setattr+0x80/0x820 [btrfs] [ 113.239285] ? get_stack_info_noinstr+0x12/0xf0 [ 113.239857] ? __module_address+0x22/0xa0 [ 113.240368] ? handle_ksmbd_work+0x6e/0x460 [ksmbd] [ 113.240993] ? __module_text_address+0x9/0x50 [ 113.241545] ? __module_address+0x22/0xa0 [ 113.242033] ? unwind_next_frame+0x10e/0x920 [ 113.242600] ? __pfx_stack_trace_consume_entry+0x10/0x10 [ 113.243268] notify_change+0x2c2/0x4e0 [ 113.243746] ? stack_depot_save_flags+0x27/0x730 [ 113.244339] ? set_file_basic_info+0x130/0x2b0 [ksmbd] [ 113.244993] set_file_basic_info+0x130/0x2b0 [ksmbd] [ 113.245613] ? process_scheduled_works+0xbe/0x310 [ 113.246181] ? worker_thread+0x100/0x240 [ 113.246696] ? kthread+0xc8/0x100 [ 113.247126] ? ret_from_fork+0x2b/0x40 [ 113.247606] ? ret_from_fork_asm+0x1a/0x30 [ 113.248132] smb2_set_info+0x63f/0xa70 [ksmbd] ksmbd is trying to set the atime and mtime via notify_change without also setting the ctime. so This patch add ATTR_CTIME flags when setting mtime to avoid a warning. Reported-by: David Disseldorp <ddiss@suse.de> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-12-10ksmbd: fix racy issue from session lookup and expireNamjae Jeon
Increment the session reference count within the lock for lookup to avoid racy issue with session expire. Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-25737 Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-12-10ksmbd: retry iterate_dir in smb2_query_dirHobin Woo
Some file systems do not ensure that the single call of iterate_dir reaches the end of the directory. For example, FUSE fetches entries from a daemon using 4KB buffer and stops fetching if entries exceed the buffer. And then an actor of caller, KSMBD, is used to fill the entries from the buffer. Thus, pattern searching on FUSE, files located after the 4KB could not be found and STATUS_NO_SUCH_FILE was returned. Signed-off-by: Hobin Woo <hobin.woo@samsung.com> Reviewed-by: Sungjong Seo <sj1557.seo@samsung.com> Reviewed-by: Namjae Jeon <linkinjeon@kernel.org> Tested-by: Yoonho Shin <yoonho.shin@samsung.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-12-04ksmbd: align aux_payload_buf to avoid OOB reads in cryptographic operationsNorbert Szetei
The aux_payload_buf allocation in SMB2 read is performed without ensuring alignment, which could result in out-of-bounds (OOB) reads during cryptographic operations such as crypto_xor or ghash. This patch aligns the allocation of aux_payload_buf to prevent these issues. (Note that to add this patch to stable would require modifications due to recent patch "ksmbd: use __GFP_RETRY_MAYFAIL") Signed-off-by: Norbert Szetei <norbert@doyensec.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-12-02smb: avoid pointless cred reference count bumpChristian Brauner
The creds are allocated via prepare_kernel_cred() which has already taken a reference. This also removes a pointless check that gives the impression that override_creds() can ever be called on a task with current->cred NULL. That's not possible afaict. Remove the check to not imply that there can be a dangling pointer in current->cred. Link: https://lore.kernel.org/r/20241125-work-cred-v2-21-68b9d38bb5b2@kernel.org Reviewed-by: Jeff Layton <jlayton@kernel.org> Reviewed-by: Jens Axboe <axboe@kernel.dk> Signed-off-by: Christian Brauner <brauner@kernel.org>
2024-12-02tree-wide: s/revert_creds_light()/revert_creds()/gChristian Brauner
Rename all calls to revert_creds_light() back to revert_creds(). Link: https://lore.kernel.org/r/20241125-work-cred-v2-6-68b9d38bb5b2@kernel.org Reviewed-by: Jeff Layton <jlayton@kernel.org> Reviewed-by: Jens Axboe <axboe@kernel.dk> Signed-off-by: Christian Brauner <brauner@kernel.org>
2024-12-02tree-wide: s/override_creds_light()/override_creds()/gChristian Brauner
Rename all calls to override_creds_light() back to overrid_creds(). Link: https://lore.kernel.org/r/20241125-work-cred-v2-5-68b9d38bb5b2@kernel.org Reviewed-by: Jeff Layton <jlayton@kernel.org> Reviewed-by: Jens Axboe <axboe@kernel.dk> Signed-off-by: Christian Brauner <brauner@kernel.org>
2024-12-02tree-wide: s/revert_creds()/put_cred(revert_creds_light())/gChristian Brauner
Convert all calls to revert_creds() over to explicitly dropping reference counts in preparation for converting revert_creds() to revert_creds_light() semantics. Link: https://lore.kernel.org/r/20241125-work-cred-v2-3-68b9d38bb5b2@kernel.org Reviewed-by: Jeff Layton <jlayton@kernel.org> Reviewed-by: Jens Axboe <axboe@kernel.dk> Signed-off-by: Christian Brauner <brauner@kernel.org>
2024-12-02tree-wide: s/override_creds()/override_creds_light(get_new_cred())/gChristian Brauner
Convert all callers from override_creds() to override_creds_light(get_new_cred()) in preparation of making override_creds() not take a separate reference at all. Link: https://lore.kernel.org/r/20241125-work-cred-v2-1-68b9d38bb5b2@kernel.org Reviewed-by: Jeff Layton <jlayton@kernel.org> Reviewed-by: Jens Axboe <axboe@kernel.dk> Signed-off-by: Christian Brauner <brauner@kernel.org>
2024-12-01ksmbd: fix Out-of-Bounds Write in ksmbd_vfs_stream_writeJordy Zomer
An offset from client could be a negative value, It could allows to write data outside the bounds of the allocated buffer. Note that this issue is coming when setting 'vfs objects = streams_xattr parameter' in ksmbd.conf. Cc: stable@vger.kernel.org # v5.15+ Reported-by: Jordy Zomer <jordyzomer@google.com> Signed-off-by: Jordy Zomer <jordyzomer@google.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-12-01ksmbd: fix Out-of-Bounds Read in ksmbd_vfs_stream_readJordy Zomer
An offset from client could be a negative value, It could lead to an out-of-bounds read from the stream_buf. Note that this issue is coming when setting 'vfs objects = streams_xattr parameter' in ksmbd.conf. Cc: stable@vger.kernel.org # v5.15+ Reported-by: Jordy Zomer <jordyzomer@google.com> Signed-off-by: Jordy Zomer <jordyzomer@google.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-12-01smb: server: Fix building with GCC 15Brahmajit Das
GCC 15 introduces -Werror=unterminated-string-initialization by default, this results in the following build error fs/smb/server/smb_common.c:21:35: error: initializer-string for array of 'char' is too long [-Werror=unterminated-string-ini tialization] 21 | static const char basechars[43] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_-!@#$%"; | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ cc1: all warnings being treated as errors To this we are replacing char basechars[43] with a character pointer and then using strlen to get the length. Signed-off-by: Brahmajit Das <brahmajit.xyz@gmail.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-11-25ksmbd: fix use-after-free in SMB request handlingYunseong Kim
A race condition exists between SMB request handling in `ksmbd_conn_handler_loop()` and the freeing of `ksmbd_conn` in the workqueue handler `handle_ksmbd_work()`. This leads to a UAF. - KASAN: slab-use-after-free Read in handle_ksmbd_work - KASAN: slab-use-after-free in rtlock_slowlock_locked This race condition arises as follows: - `ksmbd_conn_handler_loop()` waits for `conn->r_count` to reach zero: `wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0);` - Meanwhile, `handle_ksmbd_work()` decrements `conn->r_count` using `atomic_dec_return(&conn->r_count)`, and if it reaches zero, calls `ksmbd_conn_free()`, which frees `conn`. - However, after `handle_ksmbd_work()` decrements `conn->r_count`, it may still access `conn->r_count_q` in the following line: `waitqueue_active(&conn->r_count_q)` or `wake_up(&conn->r_count_q)` This results in a UAF, as `conn` has already been freed. The discovery of this UAF can be referenced in the following PR for syzkaller's support for SMB requests. Link: https://github.com/google/syzkaller/pull/5524 Fixes: ee426bfb9d09 ("ksmbd: add refcnt to ksmbd_conn struct") Cc: linux-cifs@vger.kernel.org Cc: stable@vger.kernel.org # v6.6.55+, v6.10.14+, v6.11.3+ Cc: syzkaller@googlegroups.com Signed-off-by: Yunseong Kim <yskelg@gmail.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-11-25ksmbd: add debug print for pending request during server shutdownNamjae Jeon
We need to know how many pending requests are left at the end of server shutdown. That means we need to know how long the server will wait to process pending requests in case of a server shutdown. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-11-25ksmbd: add netdev-up/down event debug printNamjae Jeon
Add netdev-up/down event debug print to find what netdev is connected or disconnected. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-11-25ksmbd: add debug prints to know what smb2 requests were receivedNamjae Jeon
Add debug prints to know what smb2 requests were received. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-11-25ksmbd: add debug print for rdma capableNamjae Jeon
Add debug print to know if netdevice is RDMA-capable network adapter. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-11-25ksmbd: use msleep instaed of schedule_timeout_interruptible()Namjae Jeon
use msleep instaed of schedule_timeout_interruptible() to guarantee the task delays as expected. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-11-25ksmbd: use __GFP_RETRY_MAYFAILNamjae Jeon
Prefer to report ENOMEM rather than incur the oom for allocations in ksmbd. __GFP_NORETRY could not achieve that, It would fail the allocations just too easily. __GFP_RETRY_MAYFAIL will keep retrying the allocation until there is no more progress and fail the allocation instead go OOM and let the caller to deal with it. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-11-17ksmbd: fix malformed unsupported smb1 negotiate responseNamjae Jeon
When mounting with vers=1.0, ksmbd should return unsupported smb1 negotiate response. But this response is malformed. [ 6010.586702] CIFS: VFS: Bad protocol string signature header 0x25000000 [ 6010.586708] 00000000: 25000000 25000000 424d53ff 00000072 ...%...%.SMBr... [ 6010.586711] 00000010: c8408000 00000000 00000000 00000000 ..@............. [ 6010.586713] 00000020: 00 00 b9 32 00 00 01 00 01 ...2..... Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-11-05ksmbd: check outstanding simultaneous SMB operationsNamjae Jeon
If Client send simultaneous SMB operations to ksmbd, It exhausts too much memory through the "ksmbd_work_cache”. It will cause OOM issue. ksmbd has a credit mechanism but it can't handle this problem. This patch add the check if it exceeds max credits to prevent this problem by assuming that one smb request consumes at least one credit. Cc: stable@vger.kernel.org # v5.15+ Reported-by: Norbert Szetei <norbert@doyensec.com> Tested-by: Norbert Szetei <norbert@doyensec.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-11-05ksmbd: fix slab-use-after-free in smb3_preauth_hash_rspNamjae Jeon
ksmbd_user_session_put should be called under smb3_preauth_hash_rsp(). It will avoid freeing session before calling smb3_preauth_hash_rsp(). Cc: stable@vger.kernel.org # v5.15+ Reported-by: Norbert Szetei <norbert@doyensec.com> Tested-by: Norbert Szetei <norbert@doyensec.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-11-05ksmbd: fix slab-use-after-free in ksmbd_smb2_session_createNamjae Jeon
There is a race condition between ksmbd_smb2_session_create and ksmbd_expire_session. This patch add missing sessions_table_lock while adding/deleting session from global session table. Cc: stable@vger.kernel.org # v5.15+ Reported-by: Norbert Szetei <norbert@doyensec.com> Tested-by: Norbert Szetei <norbert@doyensec.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-10-28ksmbd: Fix the missing xa_store error checkJinjie Ruan
xa_store() can fail, it return xa_err(-EINVAL) if the entry cannot be stored in an XArray, or xa_err(-ENOMEM) if memory allocation failed, so check error for xa_store() to fix it. Cc: stable@vger.kernel.org Fixes: b685757c7b08 ("ksmbd: Implements sess->rpc_handle_list as xarray") Signed-off-by: Jinjie Ruan <ruanjinjie@huawei.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-10-11ksmbd: add support for supplementary groupsNamjae Jeon
Even though system user has a supplementary group, It gets NT_STATUS_ACCESS_DENIED when attempting to create file or directory. This patch add KSMBD_EVENT_LOGIN_REQUEST_EXT/RESPONSE_EXT netlink events to get supplementary groups list. The new netlink event doesn't break backward compatibility when using old ksmbd-tools. Co-developed-by: Atte Heikkilä <atteh.mailbox@gmail.com> Signed-off-by: Atte Heikkilä <atteh.mailbox@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-10-09ksmbd: fix user-after-free from session log offNamjae Jeon
There is racy issue between smb2 session log off and smb2 session setup. It will cause user-after-free from session log off. This add session_lock when setting SMB2_SESSION_EXPIRED and referece count to session struct not to free session while it is being used. Cc: stable@vger.kernel.org # v5.15+ Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-25282 Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-10-03Merge tag 'v6.12-rc1-ksmbd-fixes' of git://git.samba.org/ksmbdLinus Torvalds
Pull smb server fixes from Steve French: - small cleanup patches leveraging struct size to improve access bounds checking * tag 'v6.12-rc1-ksmbd-fixes' of git://git.samba.org/ksmbd: ksmbd: Use struct_size() to improve smb_direct_rdma_xmit() ksmbd: Annotate struct copychunk_ioctl_req with __counted_by_le() ksmbd: Use struct_size() to improve get_file_alternate_info()
2024-10-02move asm/unaligned.h to linux/unaligned.hAl Viro
asm/unaligned.h is always an include of asm-generic/unaligned.h; might as well move that thing to linux/unaligned.h and include that - there's nothing arch-specific in that header. auto-generated by the following: for i in `git grep -l -w asm/unaligned.h`; do sed -i -e "s/asm\/unaligned.h/linux\/unaligned.h/" $i done for i in `git grep -l -w asm-generic/unaligned.h`; do sed -i -e "s/asm-generic\/unaligned.h/linux\/unaligned.h/" $i done git mv include/asm-generic/unaligned.h include/linux/unaligned.h git mv tools/include/asm-generic/unaligned.h tools/include/linux/unaligned.h sed -i -e "/unaligned.h/d" include/asm-generic/Kbuild sed -i -e "s/__ASM_GENERIC/__LINUX/" include/linux/unaligned.h tools/include/linux/unaligned.h
2024-10-01ksmbd: Use struct_size() to improve smb_direct_rdma_xmit()Thorsten Blum
Use struct_size() to calculate the number of bytes to allocate for a new message. Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2024-10-01ksmbd: Annotate struct copychunk_ioctl_req with __counted_by_le()Thorsten Blum
Add the __counted_by_le compiler attribute to the flexible array member Chunks to improve access bounds-checking via CONFIG_UBSAN_BOUNDS and CONFIG_FORTIFY_SOURCE. Change the data type of the flexible array member Chunks from __u8[] to struct srv_copychunk[] for ChunkCount to match the number of elements in the Chunks array. (With __u8[], each srv_copychunk would occupy 24 array entries and the __counted_by compiler attribute wouldn't be applicable.) Use struct_size() to calculate the size of the copychunk_ioctl_req. Read Chunks[0] after checking that ChunkCount is not 0. Signed-off-by: Thorsten Blum <thorsten.blum@linux.dev> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>