Commit Graph

111 Commits

Author SHA1 Message Date
Namjae Jeon
2db72604f3 ksmbd: fix version mismatch with out of tree
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-07 10:18:34 -05:00
Namjae Jeon
c7705eec78 ksmbd: use buf_data_size instead of recalculation in smb3_decrypt_req()
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-07 10:18:29 -05:00
Namjae Jeon
51a1387393 ksmbd: remove the leftover of smb2.0 dialect support
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-07 10:18:27 -05:00
Namjae Jeon
c2e99d4797 ksmbd: check strictly data area in ksmbd_smb2_check_message()
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-07 10:18:24 -05:00
Namjae Jeon
3639999011 ksmbd: add the check to vaildate if stream protocol length exceeds maximum value
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-10-06 00:23:00 -05:00
Dan Carpenter
87ffb310d5 ksmbd: missing check for NULL in convert_to_nt_pathname()
The kmalloc() does not have a NULL check.  This code can be re-written
slightly cleaner to just use the kstrdup().

Fixes: 265fd1991c ("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-30 20:00:05 -05:00
Namjae Jeon
4227f811cd ksmbd: fix transform header validation
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-30 09:58:07 -05:00
Hyunchul Lee
8f77150c15 ksmbd: add buffer validation for SMB2_CREATE_CONTEXT
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-30 09:58:07 -05:00
Namjae Jeon
442ff9ebeb ksmbd: add validation in smb2 negotiate
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-30 09:58:07 -05:00
Namjae Jeon
9496e268e3 ksmbd: add request buffer validation in smb2_set_info
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-30 09:58:06 -05:00
Namjae Jeon
88d300522c ksmbd: use correct basic info level in set_file_basic_info()
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-30 09:58:06 -05:00
Namjae Jeon
ce812992f2 ksmbd: remove NTLMv1 authentication
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-29 16:17:34 -05:00
Enzo Matsumiya
1018bf2455 ksmbd: fix documentation for 2 functions
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-28 20:51:32 -05:00
Namjae Jeon
d72a9c1588 ksmbd: fix invalid request buffer access in compound
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-26 16:47:14 -05:00
Ronnie Sahlberg
18d46769d5 ksmbd: remove RFC1002 check in smb2 request
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-26 16:47:14 -05:00
Hyunchul Lee
265fd1991c ksmbd: use LOOKUP_BENEATH to prevent the out of share access
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-24 21:25:23 -05:00
Namjae Jeon
4ea477988c ksmbd: remove follow symlinks support
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-22 23:37:38 -05:00
Namjae Jeon
18a015bccf ksmbd: check protocol id in ksmbd_verify_smb_message()
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-22 17:21:05 -05:00
Namjae Jeon
9f6323311c ksmbd: add default data stream name in FILE_STREAM_INFORMATION
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-21 12:53:23 -05:00
Steve French
e44fd5081c ksmbd: log that server is experimental at module load
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-21 12:35:20 -05:00
Namjae Jeon
6d56262c3d ksmbd: add validation for FILE_FULL_EA_INFORMATION of smb2_get_info
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-18 10:51:38 -05:00
Hyunchul Lee
f58eae6c5f ksmbd: prevent out of share access
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-17 17:18:48 -05:00
Mike Galbraith
a9b3043de4 ksmbd: transport_rdma: Don't include rwlock.h directly
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-17 08:58:43 -05:00
Namjae Jeon
4cf0ccd033 ksmbd: fix control flow issues in sid_to_id()
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-08 17:16:13 -05:00
Namjae Jeon
4ffd5264e8 ksmbd: fix read of uninitialized variable ret in set_file_basic_info
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-08 17:16:09 -05:00
Colin Ian King
36bbeb3365 ksmbd: add missing assignments to ret on ndr_read_int64 read calls
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-08 17:15:48 -05:00
Namjae Jeon
303fff2b8c ksmbd: add validation for ndr read/write functions
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-03 23:29:45 -05:00
Namjae Jeon
687c59e702 ksmbd: remove unused ksmbd_file_table_flush function
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-03 23:29:45 -05:00
Hyunchul Lee
72d6cbb533 ksmbd: smbd: fix dma mapping error in smb_direct_post_send_data
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-03 23:29:45 -05:00
Per Forlin
d475866eee ksmbd: Reduce error log 'speed is unknown' to debug
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-03 23:29:45 -05:00
Christian Brauner
28a5d3de9d ksmbd: defer notify_change() call
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-03 23:29:45 -05:00
Christian Brauner
db7fb6fe3d ksmbd: remove setattr preparations in set_file_basic_info()
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-03 23:29:45 -05:00
Christian Brauner
eb5784f0c6 ksmbd: ensure error is surfaced in set_file_basic_info()
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-03 23:29:45 -05:00
Christian Brauner
9467a0ce48 ndr: fix translation in ndr_encode_posix_acl()
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-03 23:29:45 -05:00
Christian Brauner
55cd04d75e ksmbd: fix translation in sid_to_id()
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-03 23:29:45 -05:00
Christian Brauner
f0bb29d5c6 ksmbd: fix subauth 0 handling in sid_to_id()
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-03 23:29:44 -05:00
Christian Brauner
0e844efebd ksmbd: fix translation in acl entries
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-03 23:29:44 -05:00
Christian Brauner
43205ca719 ksmbd: fix translation in ksmbd_acls_fattr()
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-03 23:29:44 -05:00
Christian Brauner
3cdc20e72c ksmbd: fix translation in create_posix_rsp_buf()
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>
2021-09-03 23:29:44 -05:00
Christian Brauner
475d6f9880 ksmbd: fix translation in smb2_populate_readdir_entry()
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.

The idmapping of the mount is already taken into account by the lower
filesystem and so kstat->*id will contain the mapped k*ids.

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>
2021-09-03 23:29:44 -05:00
Christian Brauner
da1e7ada5b ksmbd: fix lookup on idmapped mounts
It's great that the new in-kernel ksmbd server will support idmapped
mounts out of the box! However, lookup is currently broken. Lookup
helpers such as lookup_one_len() call inode_permission() internally to
ensure that the caller is privileged over the inode of the base dentry
they are trying to lookup under. So the permission checking here is
currently wrong.

Linux v5.15 will gain a new lookup helper lookup_one() that does take
idmappings into account. I've added it as part of my patch series to
make btrfs support idmapped mounts. The new helper is in linux-next as
part of David's (Sterba) btrfs for-next branch as commit
c972214c133b ("namei: add mapping aware lookup helper").

I've said it before during one of my first reviews: I would very much
recommend adding fstests to [1]. It already seems to have very
rudimentary cifs support. There is a completely generic idmapped mount
testsuite that supports idmapped mounts.

[1]: https://git.kernel.org/pub/scm/fs/xfs/xfsprogs-dev.git/
Cc: Colin Ian King <colin.king@canonical.com>
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: David Sterba <dsterba@suse.com>
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-03 23:29:44 -05:00
Namjae Jeon
7d5d8d7156 ksmbd: fix __write_overflow warning in ndr_read_string
Dan reported __write_overflow warning in ndr_read_string.

  CC [M]  fs/ksmbd/ndr.o
In file included from ./include/linux/string.h:253,
                 from ./include/linux/bitmap.h:11,
                 from ./include/linux/cpumask.h:12,
                 from ./arch/x86/include/asm/cpumask.h:5,
                 from ./arch/x86/include/asm/msr.h:11,
                 from ./arch/x86/include/asm/processor.h:22,
                 from ./arch/x86/include/asm/cpufeature.h:5,
                 from ./arch/x86/include/asm/thread_info.h:53,
                 from ./include/linux/thread_info.h:60,
                 from ./arch/x86/include/asm/preempt.h:7,
                 from ./include/linux/preempt.h:78,
                 from ./include/linux/spinlock.h:55,
                 from ./include/linux/wait.h:9,
                 from ./include/linux/wait_bit.h:8,
                 from ./include/linux/fs.h:6,
                 from fs/ksmbd/ndr.c:7:
In function memcpy,
    inlined from ndr_read_string at fs/ksmbd/ndr.c:86:2,
    inlined from ndr_decode_dos_attr at fs/ksmbd/ndr.c:167:2:
./include/linux/fortify-string.h:219:4: error: call to __write_overflow
declared with attribute error: detected write beyond size of object
    __write_overflow();
    ^~~~~~~~~~~~~~~~~~

This seems to be a false alarm because hex_attr size is always smaller
than n->length. This patch fix this warning by allocation hex_attr with
n->length.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
2021-08-27 14:03:49 -05:00
Namjae Jeon
e70e392fa7 ksmbd: fix permission check issue on chown and chmod
When commanding chmod and chown on cifs&ksmbd, ksmbd allows it without file
permissions check. There is code to check it in settattr_prepare.
Instead of setting the inode directly, update the mode and uid/gid
through notify_change.

Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
2021-08-21 23:26:34 +09:00
Namjae Jeon
a9a27d4ab3 ksmbd: don't set FILE DELETE and FILE_DELETE_CHILD in access mask by default
When there is no dacl in request, ksmbd send dacl that coverted by using
file permission. This patch don't set FILE DELETE and FILE_DELETE_CHILD
in access mask by default.

Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
2021-08-20 15:45:44 +09:00
Hyunchul Lee
323b1ea102 ksmbd: smbd: fix kernel oops during server shutdown
if server shutdown happens in the situation that
there are connections, workqueue could be destroyed
before queueing disconnect work.

Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
2021-08-13 08:18:13 +09:00
Namjae Jeon
777cad1604 ksmbd: remove select FS_POSIX_ACL in Kconfig
ksmbd is forcing to turn on FS_POSIX_ACL in Kconfig to use vfs acl
functions(posix_acl_alloc, get_acl, set_posix_acl). OpenWRT and other
platform doesn't use acl and this config is disable by default in
kernel. This patch use IS_ENABLED() to know acl config is enable and use
acl function if it is enable.

Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
2021-08-13 08:18:10 +09:00
Namjae Jeon
c6ce2b5716 ksmbd: use proper errno instead of -1 in smb2_get_ksmbd_tcon()
Use proper errno instead of -1 in smb2_get_ksmbd_tcon().

Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
2021-08-13 08:18:08 +09:00
Namjae Jeon
5ec3df8e98 ksmbd: update the comment for smb2_get_ksmbd_tcon()
Update the comment for smb2_get_ksmbd_tcon().

Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
2021-08-13 08:18:06 +09:00
Namjae Jeon
f4228b678b ksmbd: change int data type to boolean
Change data type of function that return only 0 or 1 to boolean.

Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
2021-08-13 08:18:03 +09:00
Marios Makassikis
eebff916f0 ksmbd: Fix multi-protocol negotiation
To negotiate either the SMB2 protocol or SMB protocol, a client must
send a SMB_COM_NEGOTIATE message containing the list of dialects it
supports, to which the server will respond with either a
SMB_COM_NEGOTIATE or a SMB2_NEGOTIATE response.

The current implementation responds with the highest common dialect,
rather than looking explicitly for "SMB 2.???" and "SMB 2.002", as
indicated in [MS-SMB2]:

  [MS-SMB2] 3.3.5.3.1:
    If the server does not implement the SMB 2.1 or 3.x dialect family,
    processing MUST continue as specified in 3.3.5.3.2.

    Otherwise, the server MUST scan the dialects provided for the dialect
    string "SMB 2.???". If the string is not present, continue to section
    3.3.5.3.2. If the string is present, the server MUST respond with an
    SMB2 NEGOTIATE Response as specified in 2.2.4.

  [MS-SMB2] 3.3.5.3.2:
    The server MUST scan the dialects provided for the dialect string "SMB
    2.002". If the string is present, the client understands SMB2, and the
    server MUST respond with an SMB2 NEGOTIATE Response.

This is an issue if a client attempts to negotiate SMB3.1.1 using
a SMB_COM_NEGOTIATE, as it will trigger the following NULL pointer
dereference:

  8<--- cut here ---
  Unable to handle kernel NULL pointer dereference at virtual address 00000000
  pgd = 1917455e
  [00000000] *pgd=00000000
  Internal error: Oops: 17 [#1] ARM
  CPU: 0 PID: 60 Comm: kworker/0:1 Not tainted 5.4.60-00027-g0518c02b5c5b #35
  Hardware name: Marvell Kirkwood (Flattened Device Tree)
  Workqueue: ksmbd-io handle_ksmbd_work
  PC is at ksmbd_gen_preauth_integrity_hash+0x24/0x190
  LR is at smb3_preauth_hash_rsp+0x50/0xa0
  pc : [<802b7044>] lr : [<802d6ac0>] psr: 40000013
  sp : bf199ed8 ip : 00000000 fp : 80d1edb0
  r10: 80a3471b r9 : 8091af16 r8 : 80d70640
  r7 : 00000072 r6 : be95e198 r5 : ca000000 r4 : b97fee00
  r3 : 00000000 r2 : 00000002 r1 : b97fea00 r0 : b97fee00
  Flags: nZcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
  Control: 0005317f Table: 3e7f4000 DAC: 00000055
  Process kworker/0:1 (pid: 60, stack limit = 0x3dd1fdb4)
  Stack: (0xbf199ed8 to 0xbf19a000)
  9ec0: b97fee00 00000000
  9ee0: be95e198 00000072 80d70640 802d6ac0 b3da2680 b97fea00 424d53ff be95e140
  9f00: b97fee00 802bd7b0 bf10fa58 80128a78 00000000 000001c8 b6220000 bf0b7720
  9f20: be95e198 80d0c410 bf7e2a00 00000000 00000000 be95e19c 80d0c370 80123b90
  9f40: bf0b7720 be95e198 bf0b7720 bf0b7734 80d0c410 bf198000 80d0c424 80d116e0
  9f60: bf10fa58 801240c0 00000000 bf10fa40 bf1463a0 bf198000 bf0b7720 80123ed0
  9f80: bf077ee4 bf10fa58 00000000 80127f80 bf1463a0 80127e88 00000000 00000000
  9fa0: 00000000 00000000 00000000 801010d0 00000000 00000000 00000000 00000000
  9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
  9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
  [<802b7044>] (ksmbd_gen_preauth_integrity_hash) from [<802d6ac0>] (smb3_preauth_hash_rsp+0x50/0xa0)
  [<802d6ac0>] (smb3_preauth_hash_rsp) from [<802bd7b0>] (handle_ksmbd_work+0x348/0x3f8)
  [<802bd7b0>] (handle_ksmbd_work) from [<80123b90>] (process_one_work+0x160/0x200)
  [<80123b90>] (process_one_work) from [<801240c0>] (worker_thread+0x1f0/0x2e4)
  [<801240c0>] (worker_thread) from [<80127f80>] (kthread+0xf8/0x10c)
  [<80127f80>] (kthread) from [<801010d0>] (ret_from_fork+0x14/0x24)
  Exception stack(0xbf199fb0 to 0xbf199ff8)
  9fa0: 00000000 00000000 00000000 00000000
  9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
  9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
  Code: e1855803 e5d13003 e1855c03 e5903094 (e1d330b0)
  ---[ end trace 8d03be3ed09e5699 ]---
  Kernel panic - not syncing: Fatal exception

smb3_preauth_hash_rsp() panics because conn->preauth_info is only allocated
when processing a SMB2 NEGOTIATE request.

Fix this by splitting the smb_protos array into two, each containing
only SMB1 and SMB2 dialects respectively.

While here, make ksmbd_negotiate_smb_dialect() static as it not
called from anywhere else.

Signed-off-by: Marios Makassikis <mmakassikis@freebox.fr>
Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
2021-08-13 08:18:00 +09:00