summaryrefslogtreecommitdiff
path: root/fs/ksmbd
AgeCommit message (Collapse)Author
2021-10-20ksmbd: add buffer validation in session setupMarios Makassikis
Make sure the security buffer's length/offset are valid with regards to the packet length. Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-20ksmbd: throttle session setup failures to avoid dictionary attacksNamjae Jeon
To avoid dictionary attacks (repeated session setups rapidly sent) to connect to server, ksmbd make a delay of a 5 seconds on session setup failure to make it harder to send enough random connection requests to break into a server if a user insert the wrong password 10 times in a row. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-20ksmbd: validate OutputBufferLength of QUERY_DIR, QUERY_INFO, IOCTL requestsHyunchul Lee
Validate OutputBufferLength of QUERY_DIR, QUERY_INFO, IOCTL requests and check the free size of response buffer for these requests. Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-15ksmbd: validate credit charge after validating SMB2 PDU body sizeRalph Boehme
smb2_validate_credit_charge() accesses fields in the SMB2 PDU body, but until smb2_calc_size() is called the PDU has not yet been verified to be large enough to access the PDU dynamic part length field. Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Ralph Boehme <slow@samba.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-15ksmbd: add buffer validation for smb directHyunchul Lee
Add buffer validation for smb direct. Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-15ksmbd: limit read/write/trans buffer size not to exceed 8MBNamjae Jeon
ksmbd limit read/write/trans buffer size not to exceed maximum 8MB. And set the minimum value of max response buffer size to 64KB. Windows client doesn't send session setup request if ksmbd set max trans/read/write size lower than 64KB in smb2 negotiate. It means windows allow at least 64 KB or more about this value. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-13ksmbd: validate compound response bufferNamjae Jeon
Add the check to validate compound response buffer. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-13ksmbd: fix potencial 32bit overflow from data area check in smb2_writeNamjae Jeon
DataOffset and Length validation can be potencial 32bit overflow. This patch fix it. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-13ksmbd: improve credits managementHyunchul Lee
* Requests except READ, WRITE, IOCTL, INFO, QUERY DIRECOTRY, CANCEL must consume one credit. * If client's granted credits are insufficient, refuse to handle requests. * Windows server 2016 or later grant up to 8192 credits to clients at once. Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-13ksmbd: add validation in smb2_ioctlNamjae Jeon
Add validation for request/response buffer size check in smb2_ioctl and fsctl_copychunk() take copychunk_ioctl_req pointer and the other arguments instead of smb2_ioctl_req structure and remove an unused smb2_ioctl_req argument of fsctl_validate_negotiate_info. Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Cc: Steve French <smfrench@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Acked-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Colin Ian King <colin.king@canonical.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-07ksmbd: fix oops from fuse driverNamjae Jeon
Marios reported kernel oops from fuse driver when ksmbd call mark_inode_dirty(). This patch directly update ->i_ctime after removing mark_inode_ditry() and notify_change will put inode to dirty list. Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Cc: Hyunchul Lee <hyc.lee@gmail.com> Reported-by: Marios Makassikis <mmakassikis@freebox.fr> Tested-by: Marios Makassikis <mmakassikis@freebox.fr> Acked-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-07ksmbd: fix version mismatch with out of treeNamjae Jeon
Fix version mismatch with out of tree, This updated version will be matched with ksmbd-tools. Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Cc: Steve French <smfrench@gmail.com> Cc: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-07ksmbd: use buf_data_size instead of recalculation in smb3_decrypt_req()Namjae Jeon
Tom suggested to use buf_data_size that is already calculated, to verify these offsets. Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Suggested-by: Tom Talpey <tom@talpey.com> Acked-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-07ksmbd: remove the leftover of smb2.0 dialect supportNamjae Jeon
Although ksmbd doesn't send SMB2.0 support in supported dialect list of smb negotiate response, There is the leftover of smb2.0 dialect. This patch remove it not to support SMB2.0 in ksmbd. Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Cc: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-07ksmbd: check strictly data area in ksmbd_smb2_check_message()Namjae Jeon
When invalid data offset and data length in request, ksmbd_smb2_check_message check strictly and doesn't allow to process such requests. Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Acked-by: Hyunchul Lee <hyc.lee@gmail.com> Reviewed-by: Ralph Boehme <slow@samba.org> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-10-06ksmbd: add the check to vaildate if stream protocol length exceeds maximum valueNamjae Jeon
This patch add MAX_STREAM_PROT_LEN macro and check if stream protocol length exceeds maximum value. opencode pdu size check in ksmbd_pdu_size_has_room(). Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Acked-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-30ksmbd: missing check for NULL in convert_to_nt_pathname()Dan Carpenter
The kmalloc() does not have a NULL check. This code can be re-written slightly cleaner to just use the kstrdup(). Fixes: 265fd1991c1d ("ksmbd: use LOOKUP_BENEATH to prevent the out of share access") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Acked-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-30ksmbd: fix transform header validationNamjae Jeon
Validate that the transform and smb request headers are present before checking OriginalMessageSize and SessionId fields. Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Reviewed-by: Tom Talpey <tom@talpey.com> Acked-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-30ksmbd: add buffer validation for SMB2_CREATE_CONTEXTHyunchul Lee
Add buffer validation for SMB2_CREATE_CONTEXT. Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Reviewed-by: Ralph Boehme <slow@samba.org> Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-30ksmbd: add validation in smb2 negotiateNamjae Jeon
This patch add validation to check request buffer check in smb2 negotiate and fix null pointer deferencing oops in smb3_preauth_hash_rsp() that found from manual test. Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Cc: Hyunchul Lee <hyc.lee@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Reviewed-by: Ralph Boehme <slow@samba.org> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-30ksmbd: add request buffer validation in smb2_set_infoNamjae Jeon
Add buffer validation in smb2_set_info, and remove unused variable in set_file_basic_info. and smb2_set_info infolevel functions take structure pointer argument. Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Acked-by: Hyunchul Lee <hyc.lee@gmail.com> Reviewed-by: Ralph Boehme <slow@samba.org> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-30ksmbd: use correct basic info level in set_file_basic_info()Namjae Jeon
Use correct basic info level in set/get_file_basic_info(). Reviewed-by: Ralph Boehme <slow@samba.org> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-29ksmbd: remove NTLMv1 authenticationNamjae Jeon
Remove insecure NTLMv1 authentication. Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Reviewed-by: Tom Talpey <tom@talpey.com> Acked-by: Steve French <smfrench@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-28ksmbd: fix documentation for 2 functionsEnzo Matsumiya
ksmbd_kthread_fn() and create_socket() returns 0 or error code, and not task_struct/ERR_PTR. Signed-off-by: Enzo Matsumiya <ematsumiya@suse.de> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-26ksmbd: fix invalid request buffer access in compoundNamjae Jeon
Ronnie reported invalid request buffer access in chained command when inserting garbage value to NextCommand of compound request. This patch add validation check to avoid this issue. Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Tested-by: Steve French <smfrench@gmail.com> Reviewed-by: Steve French <smfrench@gmail.com> Acked-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-26ksmbd: remove RFC1002 check in smb2 requestRonnie Sahlberg
In smb_common.c you have this function : ksmbd_smb_request() which is called from connection.c once you have read the initial 4 bytes for the next length+smb2 blob. It checks the first byte of this 4 byte preamble for valid values, i.e. a NETBIOSoverTCP SESSION_MESSAGE or a SESSION_KEEP_ALIVE. We don't need to check this for ksmbd since it only implements SMB2 over TCP port 445. The netbios stuff was only used in very old servers when SMB ran over TCP port 139. Now that we run over TCP port 445, this is actually not a NB header anymore and you can just treat it as a 4 byte length field that must be less than 16Mbyte. and remove the references to the RFC1002 constants that no longer applies. Cc: Tom Talpey <tom@talpey.com> Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Cc: Steve French <smfrench@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Acked-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-24ksmbd: use LOOKUP_BENEATH to prevent the out of share accessHyunchul Lee
instead of removing '..' in a given path, call kern_path with LOOKUP_BENEATH flag to prevent the out of share access. ran various test on this: smb2-cat-async smb://127.0.0.1/homes/../out_of_share smb2-cat-async smb://127.0.0.1/homes/foo/../../out_of_share smbclient //127.0.0.1/homes -c "mkdir ../foo2" smbclient //127.0.0.1/homes -c "rename bar ../bar" Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Boehme <slow@samba.org> Tested-by: Steve French <smfrench@gmail.com> Tested-by: Namjae Jeon <linkinjeon@kernel.org> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-22ksmbd: remove follow symlinks supportNamjae Jeon
Use LOOKUP_NO_SYMLINKS flags for default lookup to prohibit the middle of symlink component lookup and remove follow symlinks parameter support. We re-implement it as reparse point later. Test result: smbclient -Ulinkinjeon%1234 //172.30.1.42/share -c "get hacked/passwd passwd" NT_STATUS_OBJECT_NAME_NOT_FOUND opening remote file \hacked\passwd Cc: Ralph Böhme <slow@samba.org> Cc: Steve French <smfrench@gmail.com> Acked-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-22ksmbd: check protocol id in ksmbd_verify_smb_message()Namjae Jeon
When second smb2 pdu has invalid protocol id, ksmbd doesn't detect it and allow to process smb2 request. This patch add the check it in ksmbd_verify_smb_message() and don't use protocol id of smb2 request as protocol id of response. Reviewed-by: Ronnie Sahlberg <ronniesahlberg@gmail.com> Reviewed-by: Ralph Böhme <slow@samba.org> Reported-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-21ksmbd: add default data stream name in FILE_STREAM_INFORMATIONNamjae Jeon
Windows client expect to get default stream name(::DATA) in FILE_STREAM_INFORMATION response even if there is no stream data in file. This patch fix update failure when writing ppt or doc files. Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Reviewed-By: Tom Talpey <tom@talpey.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-21ksmbd: log that server is experimental at module loadSteve French
While we are working through detailed security reviews of ksmbd server code we should remind users that it is an experimental module by adding a warning when the module loads. Currently the module shows as experimental in Kconfig and is disabled by default, but we don't want to confuse users. Although ksmbd passes a wide variety of the important functional tests (since initial focus had been largely on functional testing such as smbtorture, xfstests etc.), and ksmbd has added key security features (e.g. GCM256 encryption, Kerberos support), there are ongoing detailed reviews of the code base for path processing and network buffer decoding, and this patch reminds users that the module should be considered "experimental." Reviewed-by: Namjae Jeon <linkinjeon@kernel.org> Reviewed-by: Paulo Alcantara (SUSE) <pc@cjr.nz> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-18ksmbd: add validation for FILE_FULL_EA_INFORMATION of smb2_get_infoNamjae Jeon
Add validation to check whether req->InputBufferLength is smaller than smb2_ea_info_req structure size. Cc: Ronnie Sahlberg <ronniesahlberg@gmail.com> Cc: Ralph Böhme <slow@samba.org> Cc: Steve French <smfrench@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-17ksmbd: prevent out of share accessHyunchul Lee
Because of .., files outside the share directory could be accessed. To prevent this, normalize the given path and remove all . and .. components. In addition to the usual large set of regression tests (smbtorture and xfstests), ran various tests on this to specifically check path name validation including libsmb2 tests to verify path normalization: ./examples/smb2-ls-async smb://172.30.1.15/homes2/../ ./examples/smb2-ls-async smb://172.30.1.15/homes2/foo/../ ./examples/smb2-ls-async smb://172.30.1.15/homes2/foo/../../ ./examples/smb2-ls-async smb://172.30.1.15/homes2/foo/../ ./examples/smb2-ls-async smb://172.30.1.15/homes2/foo/..bar/ ./examples/smb2-ls-async smb://172.30.1.15/homes2/foo/bar../ ./examples/smb2-ls-async smb://172.30.1.15/homes2/foo/bar.. ./examples/smb2-ls-async smb://172.30.1.15/homes2/foo/bar../../../../ Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-17ksmbd: transport_rdma: Don't include rwlock.h directlyMike Galbraith
rwlock.h specifically asks to not be included directly. In fact, the proper spinlock.h include isn't needed either, it comes with the huge pile that kthread.h ends up pulling in, so just drop it entirely. Signed-off-by: Mike Galbraith <efault@gmx.de> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-08ksmbd: fix control flow issues in sid_to_id()Namjae Jeon
Addresses-Coverity reported Control flow issues in sid_to_id() /fs/ksmbd/smbacl.c: 277 in sid_to_id() 271 272 if (sidtype == SIDOWNER) { 273 kuid_t uid; 274 uid_t id; 275 276 id = le32_to_cpu(psid->sub_auth[psid->num_subauth - 1]); >>> CID 1506810: Control flow issues (NO_EFFECT) >>> This greater-than-or-equal-to-zero comparison of an unsigned value >>> is always true. "id >= 0U". 277 if (id >= 0) { 278 /* 279 * Translate raw sid into kuid in the server's user 280 * namespace. 281 */ 282 uid = make_kuid(&init_user_ns, id); Addresses-Coverity: ("Control flow issues") Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-08ksmbd: fix read of uninitialized variable ret in set_file_basic_infoNamjae Jeon
Addresses-Coverity reported Uninitialized variables warninig : /fs/ksmbd/smb2pdu.c: 5525 in set_file_basic_info() 5519 if (!rc) { 5520 inode->i_ctime = ctime; 5521 mark_inode_dirty(inode); 5522 } 5523 inode_unlock(inode); 5524 } >>> CID 1506805: Uninitialized variables (UNINIT) >>> Using uninitialized value "rc". 5525 return rc; 5526 } 5527 5528 static int set_file_allocation_info(struct ksmbd_work *work, 5529 struct ksmbd_file *fp, char *buf) 5530 { Addresses-Coverity: ("Uninitialized variable") Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-08ksmbd: add missing assignments to ret on ndr_read_int64 read callsColin Ian King
Currently there are two ndr_read_int64 calls where ret is being checked for failure but ret is not being assigned a return value from the call. Static analyis is reporting the checks on ret as dead code. Fix this. Addresses-Coverity: ("Logical dead code") Reviewed-by: Sergey Senozhatsky <senozhatsky@chromium.org> Signed-off-by: Colin Ian King <colin.king@canonical.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-03ksmbd: add validation for ndr read/write functionsNamjae Jeon
If ndr->length is smaller than expected size, ksmbd can access invalid access in ndr->data. This patch add validation to check ndr->offset is over ndr->length. and added exception handling to check return value of ndr read/write function. Cc: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-03ksmbd: remove unused ksmbd_file_table_flush functionNamjae Jeon
ksmbd_file_table_flush is a leftover from SMB1. This function is no longer needed as SMB1 has been removed from ksmbd. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-03ksmbd: smbd: fix dma mapping error in smb_direct_post_send_dataHyunchul Lee
Becase smb direct header is mapped and msg->num_sge already is incremented, the decrement should be removed from the condition. Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-03ksmbd: Reduce error log 'speed is unknown' to debugPer Forlin
This log happens on servers with a network bridge since the bridge does not have a specified link speed. This is not a real error so change the error log to debug instead. Signed-off-by: Per Forlin <perfn@axis.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-03ksmbd: defer notify_change() callChristian Brauner
When ownership is changed we might in certain scenarios loose the ability to alter the inode after we changed ownership. This can e.g. happen when we are on an idmapped mount where uid 0 is mapped to uid 1000 and uid 1000 is mapped to uid 0. A caller with fs*id 1000 will be able to create files as *id 1000 on disk. They will also be able to change ownership of files owned by *id 0 to *id 1000 but they won't be able to change ownership in the other direction. This means acl operations following notify_change() would fail. Move the notify_change() call after the acls have been updated. This guarantees that we don't end up with spurious "hash value diff" warnings later on because we managed to change ownership but didn't manage to alter acls. Cc: Steve French <stfrench@microsoft.com> Cc: Christoph Hellwig <hch@infradead.org> Cc: Namjae Jeon <namjae.jeon@samsung.com> Cc: Hyunchul Lee <hyc.lee@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-03ksmbd: remove setattr preparations in set_file_basic_info()Christian Brauner
Permission checking and copying over ownership information is the task of the underlying filesystem not ksmbd. The order is also wrong here. This modifies the inode before notify_change(). If notify_change() fails this will have changed ownership nonetheless. All of this is unnecessary though since the underlying filesystem's ->setattr handler will do all this (if required) by itself. Cc: Steve French <stfrench@microsoft.com> Cc: Christoph Hellwig <hch@infradead.org> Cc: Namjae Jeon <namjae.jeon@samsung.com> Cc: Hyunchul Lee <hyc.lee@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-03ksmbd: ensure error is surfaced in set_file_basic_info()Christian Brauner
It seems the error was accidently ignored until now. Make sure it is surfaced. Cc: Steve French <stfrench@microsoft.com> Cc: Christoph Hellwig <hch@infradead.org> Cc: Namjae Jeon <namjae.jeon@samsung.com> Cc: Hyunchul Lee <hyc.lee@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-03ndr: fix translation in ndr_encode_posix_acl()Christian Brauner
The sid_to_id() helper encodes raw ownership information suitable for s*id handling. This is conceptually equivalent to reporting ownership information via stat to userspace. In this case the consumer is ksmbd instead of a regular user. So when encoding raw ownership information suitable for s*id handling later we need to map the id up according to the user namespace of ksmbd itself taking any idmapped mounts into account. Cc: Steve French <stfrench@microsoft.com> Cc: Christoph Hellwig <hch@infradead.org> Cc: Namjae Jeon <namjae.jeon@samsung.com> Cc: Hyunchul Lee <hyc.lee@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-03ksmbd: fix translation in sid_to_id()Christian Brauner
The sid_to_id() functions is relevant when changing ownership of filesystem objects based on acl information. In this case we need to first translate the relevant s*ids into k*ids in ksmbd's user namespace and account for any idmapped mounts. Requesting a change in ownership requires the inverse translation to be applied when we would report ownership to userspace. So k*id_from_mnt() must be used here. Cc: Steve French <stfrench@microsoft.com> Cc: Christoph Hellwig <hch@infradead.org> Cc: Namjae Jeon <namjae.jeon@samsung.com> Cc: Hyunchul Lee <hyc.lee@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-03ksmbd: fix subauth 0 handling in sid_to_id()Christian Brauner
It's not obvious why subauth 0 would be excluded from translation. This would lead to wrong results whenever a non-identity idmapping is used. Cc: Steve French <stfrench@microsoft.com> Cc: Christoph Hellwig <hch@infradead.org> Cc: Namjae Jeon <namjae.jeon@samsung.com> Cc: Hyunchul Lee <hyc.lee@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-03ksmbd: fix translation in acl entriesChristian Brauner
The ksmbd server performs translation of posix acls to smb acls. Currently the translation is wrong since the idmapping of the mount is used to map the ids into raw userspace ids but what is relevant is the user namespace of ksmbd itself. The user namespace of ksmbd itself which is the initial user namespace. The operation is similar to asking "What *ids would a userspace process see given that k*id in the relevant user namespace?". Before the final translation we need to apply the idmapping of the mount in case any is used. Add two simple helpers for ksmbd. Cc: Steve French <stfrench@microsoft.com> Cc: Christoph Hellwig <hch@infradead.org> Cc: Namjae Jeon <namjae.jeon@samsung.com> Cc: Hyunchul Lee <hyc.lee@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-03ksmbd: fix translation in ksmbd_acls_fattr()Christian Brauner
When creating new filesystem objects ksmbd translates between k*ids and s*ids. For this it often uses struct smb_fattr and stashes the k*ids in cf_uid and cf_gid. Let cf_uid and cf_gid always contain the final information taking any potential idmapped mounts into account. When finally translation cf_*id into s*ids translate them into the user namespace of ksmbd since that is the relevant user namespace here. Cc: Steve French <stfrench@microsoft.com> Cc: Christoph Hellwig <hch@infradead.org> Cc: Namjae Jeon <namjae.jeon@samsung.com> Cc: Hyunchul Lee <hyc.lee@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
2021-09-03ksmbd: fix translation in create_posix_rsp_buf()Christian Brauner
When transferring ownership information to the client the k*ids are translated into raw *ids before they are sent over the wire. The function currently erroneously translates the k*ids according to the mount's idmapping. Instead, reporting the owning *ids to userspace the underlying k*ids need to be mapped up in the caller's user namespace. This is how stat() works. The caller in this instance is ksmbd itself and ksmbd always runs in the initial user namespace. Translate according to that taking any potential idmapped mounts into account. Switch to from_k*id_munged() which ensures that the overflow*id is returned instead of the (*id_t)-1 when the k*id can't be translated. Cc: Steve French <stfrench@microsoft.com> Cc: Christoph Hellwig <hch@infradead.org> Cc: Namjae Jeon <namjae.jeon@samsung.com> Cc: Hyunchul Lee <hyc.lee@gmail.com> Cc: Sergey Senozhatsky <senozhatsky@chromium.org> Cc: linux-cifs@vger.kernel.org Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>