summaryrefslogtreecommitdiff
path: root/fs/smb/server
AgeCommit message (Collapse)Author
2023-08-29ksmbd: replace one-element array with flex-array member in struct smb2_ea_infoNamjae Jeon
UBSAN complains about out-of-bounds array indexes on 1-element arrays in struct smb2_ea_info. UBSAN: array-index-out-of-bounds in fs/smb/server/smb2pdu.c:4335:15 index 1 is out of range for type 'char [1]' CPU: 1 PID: 354 Comm: kworker/1:4 Not tainted 6.5.0-rc4 #1 Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/22/2020 Workqueue: ksmbd-io handle_ksmbd_work [ksmbd] Call Trace: <TASK> __dump_stack linux/lib/dump_stack.c:88 dump_stack_lvl+0x48/0x70 linux/lib/dump_stack.c:106 dump_stack+0x10/0x20 linux/lib/dump_stack.c:113 ubsan_epilogue linux/lib/ubsan.c:217 __ubsan_handle_out_of_bounds+0xc6/0x110 linux/lib/ubsan.c:348 smb2_get_ea linux/fs/smb/server/smb2pdu.c:4335 smb2_get_info_file linux/fs/smb/server/smb2pdu.c:4900 smb2_query_info+0x63ae/0x6b20 linux/fs/smb/server/smb2pdu.c:5275 __process_request linux/fs/smb/server/server.c:145 __handle_ksmbd_work linux/fs/smb/server/server.c:213 handle_ksmbd_work+0x348/0x10b0 linux/fs/smb/server/server.c:266 process_one_work+0x85a/0x1500 linux/kernel/workqueue.c:2597 worker_thread+0xf3/0x13a0 linux/kernel/workqueue.c:2748 kthread+0x2b7/0x390 linux/kernel/kthread.c:389 ret_from_fork+0x44/0x90 linux/arch/x86/kernel/process.c:145 ret_from_fork_asm+0x1b/0x30 linux/arch/x86/entry/entry_64.S:304 </TASK> Cc: stable@vger.kernel.org Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-08-29ksmbd: fix slub overflow in ksmbd_decode_ntlmssp_auth_blob()Namjae Jeon
If authblob->SessionKey.Length is bigger than session key size(CIFS_KEY_SIZE), slub overflow can happen in key exchange codes. cifs_arc4_crypt copy to session key array from SessionKey from client. Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-21940 Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-08-29ksmbd: fix wrong DataOffset validation of create contextNamjae Jeon
If ->DataOffset of create context is 0, DataBuffer size is not correctly validated. This patch change wrong validation code and consider tag length in request. Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-21824 Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-08-29ksmbd: Fix one kernel-doc commentYang Li
Fix one kernel-doc comment to silence the warning: fs/smb/server/smb2pdu.c:4160: warning: Excess function parameter 'infoclass_size' description in 'buffer_check_err' Signed-off-by: Yang Li <yang.lee@linux.alibaba.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-08-29ksmbd: reduce descriptor size if remaining bytes is less than request sizeNamjae Jeon
Create 3 kinds of files to reproduce this problem. dd if=/dev/urandom of=127k.bin bs=1024 count=127 dd if=/dev/urandom of=128k.bin bs=1024 count=128 dd if=/dev/urandom of=129k.bin bs=1024 count=129 When copying files from ksmbd share to windows or cifs.ko, The following error message happen from windows client. "The file '129k.bin' is too large for the destination filesystem." We can see the error logs from ksmbd debug prints [48394.611537] ksmbd: RDMA r/w request 0x0: token 0x669d, length 0x20000 [48394.612054] ksmbd: smb_direct: RDMA write, len 0x20000, needed credits 0x1 [48394.612572] ksmbd: filename 129k.bin, offset 131072, len 131072 [48394.614189] ksmbd: nbytes 1024, offset 132096 mincount 0 [48394.614585] ksmbd: Failed to process 8 [-22] And we can reproduce it with cifs.ko, e.g. dd if=129k.bin of=/dev/null bs=128KB count=2 This problem is that ksmbd rdma return error if remaining bytes is less than Length of Buffer Descriptor V1 Structure. smb_direct_rdma_xmit() ... if (desc_buf_len == 0 || total_length > buf_len || total_length > t->max_rdma_rw_size) return -EINVAL; This patch reduce descriptor size with remaining bytes and remove the check for total_length and buf_len. Cc: stable@vger.kernel.org Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-08-29ksmbd: fix `force create mode' and `force directory mode'Atte Heikkilä
`force create mode' and `force directory mode' should be bitwise ORed with the perms after `create mask' and `directory mask' have been applied, respectively. Signed-off-by: Atte Heikkilä <atteh.mailbox@gmail.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-08-29ksmbd: fix wrong interim response on compoundNamjae Jeon
If smb2_lock or smb2_open request is compound, ksmbd could send wrong interim response to client. ksmbd allocate new interim buffer instead of using resonse buffer to support compound request. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-08-29ksmbd: add support for read compoundNamjae Jeon
MacOS sends a compound request including read to the server (e.g. open-read-close). So far, ksmbd has not handled read as a compound request. For compatibility between ksmbd and an OS that supports SMB, This patch provides compound support for read requests. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-08-29ksmbd: switch to use kmemdup_nul() helperYang Yingliang
Use kmemdup_nul() helper instead of open-coding to simplify the code. Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-08-28Merge tag 'v6.6-vfs.ctime' of ↵Linus Torvalds
git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs Pull vfs timestamp updates from Christian Brauner: "This adds VFS support for multi-grain timestamps and converts tmpfs, xfs, ext4, and btrfs to use them. This carries acks from all relevant filesystems. The VFS always uses coarse-grained timestamps when updating the ctime and mtime after a change. This has the benefit of allowing filesystems to optimize away a lot of metadata updates, down to around 1 per jiffy, even when a file is under heavy writes. Unfortunately, this has always been an issue when we're exporting via NFSv3, which relies on timestamps to validate caches. A lot of changes can happen in a jiffy, so timestamps aren't sufficient to help the client decide to invalidate the cache. Even with NFSv4, a lot of exported filesystems don't properly support a change attribute and are subject to the same problems with timestamp granularity. Other applications have similar issues with timestamps (e.g., backup applications). If we were to always use fine-grained timestamps, that would improve the situation, but that becomes rather expensive, as the underlying filesystem would have to log a lot more metadata updates. This introduces fine-grained timestamps that are used when they are actively queried. This uses the 31st bit of the ctime tv_nsec field to indicate that something has queried the inode for the mtime or ctime. When this flag is set, on the next mtime or ctime update, the kernel will fetch a fine-grained timestamp instead of the usual coarse-grained one. As POSIX generally mandates that when the mtime changes, the ctime must also change the kernel always stores normalized ctime values, so only the first 30 bits of the tv_nsec field are ever used. Filesytems can opt into this behavior by setting the FS_MGTIME flag in the fstype. Filesystems that don't set this flag will continue to use coarse-grained timestamps. Various preparatory changes, fixes and cleanups are included: - Fixup all relevant places where POSIX requires updating ctime together with mtime. This is a wide-range of places and all maintainers provided necessary Acks. - Add new accessors for inode->i_ctime directly and change all callers to rely on them. Plain accesses to inode->i_ctime are now gone and it is accordingly rename to inode->__i_ctime and commented as requiring accessors. - Extend generic_fillattr() to pass in a request mask mirroring in a sense the statx() uapi. This allows callers to pass in a request mask to only get a subset of attributes filled in. - Rework timestamp updates so it's possible to drop the @now parameter the update_time() inode operation and associated helpers. - Add inode_update_timestamps() and convert all filesystems to it removing a bunch of open-coding" * tag 'v6.6-vfs.ctime' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs: (107 commits) btrfs: convert to multigrain timestamps ext4: switch to multigrain timestamps xfs: switch to multigrain timestamps tmpfs: add support for multigrain timestamps fs: add infrastructure for multigrain timestamps fs: drop the timespec64 argument from update_time xfs: have xfs_vn_update_time gets its own timestamp fat: make fat_update_time get its own timestamp fat: remove i_version handling from fat_update_time ubifs: have ubifs_update_time use inode_update_timestamps btrfs: have it use inode_update_timestamps fs: drop the timespec64 arg from generic_update_time fs: pass the request_mask to generic_fillattr fs: remove silly warning from current_time gfs2: fix timestamp handling on quota inodes fs: rename i_ctime field to __i_ctime selinux: convert to ctime accessor functions security: convert to ctime accessor functions apparmor: convert to ctime accessor functions sunrpc: convert to ctime accessor functions ...
2023-08-09fs: pass the request_mask to generic_fillattrJeff Layton
generic_fillattr just fills in the entire stat struct indiscriminately today, copying data from the inode. There is at least one attribute (STATX_CHANGE_COOKIE) that can have side effects when it is reported, and we're looking at adding more with the addition of multigrain timestamps. Add a request_mask argument to generic_fillattr and have most callers just pass in the value that is passed to getattr. Have other callers (e.g. ksmbd) just pass in STATX_BASIC_STATS. Also move the setting of STATX_CHANGE_COOKIE into generic_fillattr. Acked-by: Joseph Qi <joseph.qi@linux.alibaba.com> Reviewed-by: Xiubo Li <xiubli@redhat.com> Reviewed-by: "Paulo Alcantara (SUSE)" <pc@manguebit.com> Reviewed-by: Jan Kara <jack@suse.cz> Signed-off-by: Jeff Layton <jlayton@kernel.org> Message-Id: <20230807-mgctime-v7-2-d1dec143a704@kernel.org> Signed-off-by: Christian Brauner <brauner@kernel.org>
2023-08-05ksmbd: fix wrong next length validation of ea buffer in smb2_set_ea()Namjae Jeon
There are multiple smb2_ea_info buffers in FILE_FULL_EA_INFORMATION request from client. ksmbd find next smb2_ea_info using ->NextEntryOffset of current smb2_ea_info. ksmbd need to validate buffer length Before accessing the next ea. ksmbd should check buffer length using buf_len, not next variable. next is the start offset of current ea that got from previous ea. Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-21598 Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-08-05ksmbd: validate command request sizeLong Li
In commit 2b9b8f3b68ed ("ksmbd: validate command payload size"), except for SMB2_OPLOCK_BREAK_HE command, the request size of other commands is not checked, it's not expected. Fix it by add check for request size of other commands. Cc: stable@vger.kernel.org Fixes: 2b9b8f3b68ed ("ksmbd: validate command payload size") Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Long Li <leo.lilong@huawei.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-07-24smb: convert to ctime accessor functionsJeff Layton
In later patches, we're going to change how the inode's ctime field is used. Switch to using accessor functions instead of raw accesses of inode->i_ctime. Acked-by: Tom Talpey <tom@talpey.com> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org> Signed-off-by: Jeff Layton <jlayton@kernel.org> Acked-by: Steve French <stfrench@microsoft.com> Message-Id: <20230705190309.579783-72-jlayton@kernel.org> Signed-off-by: Christian Brauner <brauner@kernel.org>
2023-07-23ksmbd: fix out of bounds in init_smb2_rsp_hdr()Namjae Jeon
If client send smb2 negotiate request and then send smb1 negotiate request, init_smb2_rsp_hdr is called for smb1 negotiate request since need_neg is set to false. This patch ignore smb1 packets after ->need_neg is set to false. Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-21541 Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-07-23ksmbd: no response from compound readNamjae Jeon
ksmbd doesn't support compound read. If client send read-read in compound to ksmbd, there can be memory leak from read buffer. Windows and linux clients doesn't send it to server yet. For now, No response from compound read. compound read will be supported soon. Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-21587, ZDI-CAN-21588 Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-07-23ksmbd: validate session id and tree id in compound requestNamjae Jeon
`smb2_get_msg()` in smb2_get_ksmbd_tcon() and smb2_check_user_session() will always return the first request smb2 header in a compound request. if `SMB2_TREE_CONNECT_HE` is the first command in compound request, will return 0, i.e. The tree id check is skipped. This patch use ksmbd_req_buf_next() to get current command in compound. Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-21506 Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-07-23ksmbd: fix out of bounds in smb3_decrypt_req()Namjae Jeon
smb3_decrypt_req() validate if pdu_length is smaller than smb2_transform_hdr size. Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-21589 Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-07-23ksmbd: check if a mount point is crossed during path lookupNamjae Jeon
Since commit 74d7970febf7 ("ksmbd: fix racy issue from using ->d_parent and ->d_name"), ksmbd can not lookup cross mount points. If last component is a cross mount point during path lookup, check if it is crossed to follow it down. And allow path lookup to cross a mount point when a crossmnt parameter is set to 'yes' in smb.conf. Cc: stable@vger.kernel.org Fixes: 74d7970febf7 ("ksmbd: fix racy issue from using ->d_parent and ->d_name") Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-07-12ksmbd: Fix unsigned expression compared with zeroWang Ming
The return value of the ksmbd_vfs_getcasexattr() is signed. However, the return value is being assigned to an unsigned variable and subsequently recasted, causing warnings. Use a signed type. Signed-off-by: Wang Ming <machel@vivo.com> Acked-by: Tom Talpey <tom@talpey.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-28ksmbd: avoid field overflow warningArnd Bergmann
clang warns about a possible field overflow in a memcpy: In file included from fs/smb/server/smb_common.c:7: include/linux/fortify-string.h:583:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning] __write_overflow_field(p_size_field, size); It appears to interpret the "&out[baselen + 4]" as referring to a single byte of the character array, while the equivalen "out + baselen + 4" is seen as an offset into the array. I don't see that kind of warning elsewhere, so just go with the simple rework. Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") Signed-off-by: Arnd Bergmann <arnd@arndb.de> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-26ksmbd: Replace one-element array with flexible-array memberGustavo A. R. Silva
One-element arrays are deprecated, and we are replacing them with flexible array members instead. So, replace one-element array with flexible-array member in struct smb_negotiate_req. This results in no differences in binary output. Link: https://github.com/KSPP/linux/issues/79 Link: https://github.com/KSPP/linux/issues/317 Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> Reviewed-by: Kees Cook <keescook@chromium.org> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-26ksmbd: Use struct_size() helper in ksmbd_negotiate_smb_dialect()Gustavo A. R. Silva
Prefer struct_size() over open-coded versions. Link: https://github.com/KSPP/linux/issues/160 Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org> Reviewed-by: Kees Cook <keescook@chromium.org> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-26ksmbd: add missing compound request handing in some commandsNamjae Jeon
This patch add the compound request handling to the some commands. Existing clients do not send these commands as compound requests, but ksmbd should consider that they may come. Cc: stable@vger.kernel.org Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-26ksmbd: fix out of bounds read in smb2_sess_setupNamjae Jeon
ksmbd does not consider the case of that smb2 session setup is in compound request. If this is the second payload of the compound, OOB read issue occurs while processing the first payload in the smb2_sess_setup(). Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-21355 Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-26ksmbd: Replace the ternary conditional operator with min()Lu Hongfei
It would be better to replace the traditional ternary conditional operator with min() in compare_sids. Signed-off-by: Lu Hongfei <luhongfei@vivo.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-26ksmbd: use kvzalloc instead of kvmallocNamjae Jeon
Use kvzalloc instead of kvmalloc. Reported-by: kernel test robot <lkp@intel.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-26ksmbd: Change the return value of ksmbd_vfs_query_maximal_access to voidLu Hongfei
The return value of ksmbd_vfs_query_maximal_access is meaningless, it is better to modify it to void. Signed-off-by: Lu Hongfei <luhongfei@vivo.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-26ksmbd: return a literal instead of 'err' in ksmbd_vfs_kern_path_locked()Namjae Jeon
Return a literal instead of 'err' in ksmbd_vfs_kern_path_locked(). Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-26ksmbd: use kzalloc() instead of __GFP_ZERONamjae Jeon
Use kzalloc() instead of __GFP_ZERO. Reported-by: Dan Carpenter <error27@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-26ksmbd: remove unused ksmbd_tree_conn_share functionNamjae Jeon
Remove unused ksmbd_tree_conn_share function. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-16ksmbd: validate session id and tree id in the compound requestNamjae Jeon
This patch validate session id and tree id in compound request. If first operation in the compound is SMB2 ECHO request, ksmbd bypass session and tree validation. So work->sess and work->tcon could be NULL. If secound request in the compound access work->sess or tcon, It cause NULL pointer dereferecing error. Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-21165 Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-16ksmbd: fix out-of-bound read in smb2_writeNamjae Jeon
ksmbd_smb2_check_message doesn't validate hdr->NextCommand. If ->NextCommand is bigger than Offset + Length of smb2 write, It will allow oversized smb2 write length. It will cause OOB read in smb2_write. Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-21164 Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-16ksmbd: add mnt_want_write to ksmbd vfs functionsNamjae Jeon
ksmbd is doing write access using vfs helpers. There are the cases that mnt_want_write() is not called in vfs helper. This patch add missing mnt_want_write() to ksmbd vfs functions. Cc: stable@vger.kernel.org Cc: Amir Goldstein <amir73il@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-16ksmbd: validate command payload sizeNamjae Jeon
->StructureSize2 indicates command payload size. ksmbd should validate this size with rfc1002 length before accessing it. This patch remove unneeded check and add the validation for this. [ 8.912583] BUG: KASAN: slab-out-of-bounds in ksmbd_smb2_check_message+0x12a/0xc50 [ 8.913051] Read of size 2 at addr ffff88800ac7d92c by task kworker/0:0/7 ... [ 8.914967] Call Trace: [ 8.915126] <TASK> [ 8.915267] dump_stack_lvl+0x33/0x50 [ 8.915506] print_report+0xcc/0x620 [ 8.916558] kasan_report+0xae/0xe0 [ 8.917080] kasan_check_range+0x35/0x1b0 [ 8.917334] ksmbd_smb2_check_message+0x12a/0xc50 [ 8.917935] ksmbd_verify_smb_message+0xae/0xd0 [ 8.918223] handle_ksmbd_work+0x192/0x820 [ 8.918478] process_one_work+0x419/0x760 [ 8.918727] worker_thread+0x2a2/0x6f0 [ 8.919222] kthread+0x187/0x1d0 [ 8.919723] ret_from_fork+0x1f/0x30 [ 8.919954] </TASK> Cc: stable@vger.kernel.org Reported-by: Chih-Yen Chang <cc85nod@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-02ksmbd: validate smb request protocol idNamjae Jeon
This patch add the validation for smb request protocol id. If it is not one of the four ids(SMB1_PROTO_NUMBER, SMB2_PROTO_NUMBER, SMB2_TRANSFORM_PROTO_NUM, SMB2_COMPRESSION_TRANSFORM_ID), don't allow processing the request. And this will fix the following KASAN warning also. [ 13.905265] BUG: KASAN: slab-out-of-bounds in init_smb2_rsp_hdr+0x1b9/0x1f0 [ 13.905900] Read of size 16 at addr ffff888005fd2f34 by task kworker/0:2/44 ... [ 13.908553] Call Trace: [ 13.908793] <TASK> [ 13.908995] dump_stack_lvl+0x33/0x50 [ 13.909369] print_report+0xcc/0x620 [ 13.910870] kasan_report+0xae/0xe0 [ 13.911519] kasan_check_range+0x35/0x1b0 [ 13.911796] init_smb2_rsp_hdr+0x1b9/0x1f0 [ 13.912492] handle_ksmbd_work+0xe5/0x820 Cc: stable@vger.kernel.org Reported-by: Chih-Yen Chang <cc85nod@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-02ksmbd: check the validation of pdu_size in ksmbd_conn_handler_loopNamjae Jeon
The length field of netbios header must be greater than the SMB header sizes(smb1 or smb2 header), otherwise the packet is an invalid SMB packet. If `pdu_size` is 0, ksmbd allocates a 4 bytes chunk to `conn->request_buf`. In the function `get_smb2_cmd_val` ksmbd will read cmd from `rcv_hdr->Command`, which is `conn->request_buf + 12`, causing the KASAN detector to print the following error message: [ 7.205018] BUG: KASAN: slab-out-of-bounds in get_smb2_cmd_val+0x45/0x60 [ 7.205423] Read of size 2 at addr ffff8880062d8b50 by task ksmbd:42632/248 ... [ 7.207125] <TASK> [ 7.209191] get_smb2_cmd_val+0x45/0x60 [ 7.209426] ksmbd_conn_enqueue_request+0x3a/0x100 [ 7.209712] ksmbd_server_process_request+0x72/0x160 [ 7.210295] ksmbd_conn_handler_loop+0x30c/0x550 [ 7.212280] kthread+0x160/0x190 [ 7.212762] ret_from_fork+0x1f/0x30 [ 7.212981] </TASK> Cc: stable@vger.kernel.org Reported-by: Chih-Yen Chang <cc85nod@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-02ksmbd: fix posix_acls and acls dereferencing possible ERR_PTR()Namjae Jeon
Dan reported the following error message: fs/smb/server/smbacl.c:1296 smb_check_perm_dacl() error: 'posix_acls' dereferencing possible ERR_PTR() fs/smb/server/vfs.c:1323 ksmbd_vfs_make_xattr_posix_acl() error: 'posix_acls' dereferencing possible ERR_PTR() fs/smb/server/vfs.c:1830 ksmbd_vfs_inherit_posix_acl() error: 'acls' dereferencing possible ERR_PTR() __get_acl() returns a mix of error pointers and NULL. This change it with IS_ERR_OR_NULL(). Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") Cc: stable@vger.kernel.org Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-02ksmbd: fix out-of-bound read in parse_lease_state()Namjae Jeon
This bug is in parse_lease_state, and it is caused by the missing check of `struct create_context`. When the ksmbd traverses the create_contexts, it doesn't check if the field of `NameOffset` and `Next` is valid, The KASAN message is following: [ 6.664323] BUG: KASAN: slab-out-of-bounds in parse_lease_state+0x7d/0x280 [ 6.664738] Read of size 2 at addr ffff888005c08988 by task kworker/0:3/103 ... [ 6.666644] Call Trace: [ 6.666796] <TASK> [ 6.666933] dump_stack_lvl+0x33/0x50 [ 6.667167] print_report+0xcc/0x620 [ 6.667903] kasan_report+0xae/0xe0 [ 6.668374] kasan_check_range+0x35/0x1b0 [ 6.668621] parse_lease_state+0x7d/0x280 [ 6.668868] smb2_open+0xbe8/0x4420 [ 6.675137] handle_ksmbd_work+0x282/0x820 Use smb2_find_context_vals() to find smb2 create request lease context. smb2_find_context_vals validate create context fields. Cc: stable@vger.kernel.org Reported-by: Chih-Yen Chang <cc85nod@gmail.com> Tested-by: Chih-Yen Chang <cc85nod@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-06-02ksmbd: fix out-of-bound read in deassemble_neg_contexts()Namjae Jeon
The check in the beginning is `clen + sizeof(struct smb2_neg_context) <= len_of_ctxts`, but in the end of loop, `len_of_ctxts` will subtract `((clen + 7) & ~0x7) + sizeof(struct smb2_neg_context)`, which causes integer underflow when clen does the 8 alignment. We should use `(clen + 7) & ~0x7` in the check to avoid underflow from happening. Then there are some variables that need to be declared unsigned instead of signed. [ 11.671070] BUG: KASAN: slab-out-of-bounds in smb2_handle_negotiate+0x799/0x1610 [ 11.671533] Read of size 2 at addr ffff888005e86cf2 by task kworker/0:0/7 ... [ 11.673383] Call Trace: [ 11.673541] <TASK> [ 11.673679] dump_stack_lvl+0x33/0x50 [ 11.673913] print_report+0xcc/0x620 [ 11.674671] kasan_report+0xae/0xe0 [ 11.675171] kasan_check_range+0x35/0x1b0 [ 11.675412] smb2_handle_negotiate+0x799/0x1610 [ 11.676217] ksmbd_smb_negotiate_common+0x526/0x770 [ 11.676795] handle_ksmbd_work+0x274/0x810 ... Cc: stable@vger.kernel.org Signed-off-by: Chih-Yen Chang <cc85nod@gmail.com> Tested-by: Chih-Yen Chang <cc85nod@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-05-26ksmbd: call putname after using the last componentNamjae Jeon
last component point filename struct. Currently putname is called after vfs_path_parent_lookup(). And then last component is used for lookup_one_qstr_excl(). name in last component is freed by previous calling putname(). And It cause file lookup failure when testing generic/464 test of xfstest. Fixes: 74d7970febf7 ("ksmbd: fix racy issue from using ->d_parent and ->d_name") Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-05-26ksmbd: fix incorrect AllocationSize set in smb2_get_infoNamjae Jeon
If filesystem support sparse file, ksmbd should return allocated size using ->i_blocks instead of stat->size. This fix generic/694 xfstests. Cc: stable@vger.kernel.org Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-05-26ksmbd: fix UAF issue from opinfo->connNamjae Jeon
If opinfo->conn is another connection and while ksmbd send oplock break request to cient on current connection, The connection for opinfo->conn can be disconnect and conn could be freed. When sending oplock break request, this ksmbd_conn can be used and cause user-after-free issue. When getting opinfo from the list, ksmbd check connection is being released. If it is not released, Increase ->r_count to wait that connection is freed. Cc: stable@vger.kernel.org Reported-by: Per Forlin <per.forlin@axis.com> Tested-by: Per Forlin <per.forlin@axis.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-05-26ksmbd: fix multiple out-of-bounds read during context decodingKuan-Ting Chen
Check the remaining data length before accessing the context structure to ensure that the entire structure is contained within the packet. Additionally, since the context data length `ctxt_len` has already been checked against the total packet length `len_of_ctxts`, update the comparison to use `ctxt_len`. Cc: stable@vger.kernel.org Signed-off-by: Kuan-Ting Chen <h3xrabbit@gmail.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-05-26ksmbd: fix slab-out-of-bounds read in smb2_handle_negotiateKuan-Ting Chen
Check request_buf length first to avoid out-of-bounds read by req->DialectCount. [ 3350.990282] BUG: KASAN: slab-out-of-bounds in smb2_handle_negotiate+0x35d7/0x3e60 [ 3350.990282] Read of size 2 at addr ffff88810ad61346 by task kworker/5:0/276 [ 3351.000406] Workqueue: ksmbd-io handle_ksmbd_work [ 3351.003499] Call Trace: [ 3351.006473] <TASK> [ 3351.006473] dump_stack_lvl+0x8d/0xe0 [ 3351.006473] print_report+0xcc/0x620 [ 3351.006473] kasan_report+0x92/0xc0 [ 3351.006473] smb2_handle_negotiate+0x35d7/0x3e60 [ 3351.014760] ksmbd_smb_negotiate_common+0x7a7/0xf00 [ 3351.014760] handle_ksmbd_work+0x3f7/0x12d0 [ 3351.014760] process_one_work+0xa85/0x1780 Cc: stable@vger.kernel.org Signed-off-by: Kuan-Ting Chen <h3xrabbit@gmail.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-05-26ksmbd: fix credit count leakageNamjae Jeon
This patch fix the failure from smb2.credits.single_req_credits_granted test. When client send 8192 credit request, ksmbd return 8191 credit granted. ksmbd should give maximum possible credits that must be granted within the range of not exceeding the max credit to client. Cc: stable@vger.kernel.org Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-05-26ksmbd: fix uninitialized pointer read in smb2_create_link()Namjae Jeon
There is a case that file_present is true and path is uninitialized. This patch change file_present is set to false by default and set to true when patch is initialized. Fixes: 74d7970febf7 ("ksmbd: fix racy issue from using ->d_parent and ->d_name") Reported-by: Coverity Scan <scan-admin@coverity.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-05-26ksmbd: fix uninitialized pointer read in ksmbd_vfs_rename()Namjae Jeon
Uninitialized rd.delegated_inode can be used in vfs_rename(). Fix this by setting rd.delegated_inode to NULL to avoid the uninitialized read. Fixes: 74d7970febf7 ("ksmbd: fix racy issue from using ->d_parent and ->d_name") Reported-by: Coverity Scan <scan-admin@coverity.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-05-24cifs: correct references in Documentation to old fs/cifs pathSteve French
The fs/cifs directory has moved to fs/smb/client, correct mentions of this in Documentation and comments. Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2023-05-24smb: move client and server files to common directory fs/smbSteve French
Move CIFS/SMB3 related client and server files (cifs.ko and ksmbd.ko and helper modules) to new fs/smb subdirectory: fs/cifs --> fs/smb/client fs/ksmbd --> fs/smb/server fs/smbfs_common --> fs/smb/common Suggested-by: Linus Torvalds <torvalds@linux-foundation.org> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>