From e416ea62a9166e6075a07a970cc5bf79255d2700 Mon Sep 17 00:00:00 2001 From: Marios Makassikis Date: Thu, 16 Mar 2023 14:40:43 +0100 Subject: [PATCH 1/4] ksmbd: do not call kvmalloc() with __GFP_NORETRY | __GFP_NO_WARN Commit 83dcedd5540d ("ksmbd: fix infinite loop in ksmbd_conn_handler_loop()"), changes GFP modifiers passed to kvmalloc(). This cause xfstests generic/551 test to fail. We limit pdu length size according to connection status and maximum number of connections. In the rest, memory allocation of request is limited by credit management. so these flags are no longer needed. Fixes: 83dcedd5540d ("ksmbd: fix infinite loop in ksmbd_conn_handler_loop()") Cc: stable@vger.kernel.org Signed-off-by: Marios Makassikis Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/connection.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c index 115a67d2cf78..3f5dfebaa041 100644 --- a/fs/ksmbd/connection.c +++ b/fs/ksmbd/connection.c @@ -326,10 +326,7 @@ int ksmbd_conn_handler_loop(void *p) /* 4 for rfc1002 length field */ size = pdu_size + 4; - conn->request_buf = kvmalloc(size, - GFP_KERNEL | - __GFP_NOWARN | - __GFP_NORETRY); + conn->request_buf = kvmalloc(size, GFP_KERNEL); if (!conn->request_buf) break; From 2824861773eb512b37547516d81ef78108032cb2 Mon Sep 17 00:00:00 2001 From: Tom Rix Date: Fri, 24 Mar 2023 13:30:56 -0400 Subject: [PATCH 2/4] ksmbd: remove unused is_char_allowed function clang with W=1 reports fs/ksmbd/unicode.c:122:19: error: unused function 'is_char_allowed' [-Werror,-Wunused-function] static inline int is_char_allowed(char *ch) ^ This function is not used so remove it. Signed-off-by: Tom Rix Reviewed-by: Sergey Senozhatsky Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/unicode.c | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/fs/ksmbd/unicode.c b/fs/ksmbd/unicode.c index a0db699ddafd..9ae676906ed3 100644 --- a/fs/ksmbd/unicode.c +++ b/fs/ksmbd/unicode.c @@ -113,24 +113,6 @@ cifs_mapchar(char *target, const __u16 src_char, const struct nls_table *cp, goto out; } -/* - * is_char_allowed() - check for valid character - * @ch: input character to be checked - * - * Return: 1 if char is allowed, otherwise 0 - */ -static inline int is_char_allowed(char *ch) -{ - /* check for control chars, wildcards etc. */ - if (!(*ch & 0x80) && - (*ch <= 0x1f || - *ch == '?' || *ch == '"' || *ch == '<' || - *ch == '>' || *ch == '|')) - return 0; - - return 1; -} - /* * smb_from_utf16() - convert utf16le string to local charset * @to: destination buffer From 3a9b557f44ea8f216aab515a7db20e23f0eb51b9 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Fri, 31 Mar 2023 08:42:12 +0900 Subject: [PATCH 3/4] ksmbd: delete asynchronous work from list When smb2_lock request is canceled by smb2_cancel or smb2_close(), ksmbd is missing deleting async_request_entry async_requests list. Because calling init_smb2_rsp_hdr() in smb2_lock() mark ->synchronous as true and then it will not be deleted in ksmbd_conn_try_dequeue_request(). This patch add release_async_work() to release the ones allocated for async work. Cc: stable@vger.kernel.org Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/connection.c | 12 +++++------- fs/ksmbd/ksmbd_work.h | 2 +- fs/ksmbd/smb2pdu.c | 33 +++++++++++++++++++++------------ fs/ksmbd/smb2pdu.h | 1 + 4 files changed, 28 insertions(+), 20 deletions(-) diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c index 3f5dfebaa041..365ac32af505 100644 --- a/fs/ksmbd/connection.c +++ b/fs/ksmbd/connection.c @@ -112,10 +112,8 @@ void ksmbd_conn_enqueue_request(struct ksmbd_work *work) struct ksmbd_conn *conn = work->conn; struct list_head *requests_queue = NULL; - if (conn->ops->get_cmd_val(work) != SMB2_CANCEL_HE) { + if (conn->ops->get_cmd_val(work) != SMB2_CANCEL_HE) requests_queue = &conn->requests; - work->synchronous = true; - } if (requests_queue) { atomic_inc(&conn->req_running); @@ -136,14 +134,14 @@ int ksmbd_conn_try_dequeue_request(struct ksmbd_work *work) if (!work->multiRsp) atomic_dec(&conn->req_running); - spin_lock(&conn->request_lock); if (!work->multiRsp) { + spin_lock(&conn->request_lock); list_del_init(&work->request_entry); - if (!work->synchronous) - list_del_init(&work->async_request_entry); + spin_unlock(&conn->request_lock); + if (work->asynchronous) + release_async_work(work); ret = 0; } - spin_unlock(&conn->request_lock); wake_up_all(&conn->req_running_q); return ret; diff --git a/fs/ksmbd/ksmbd_work.h b/fs/ksmbd/ksmbd_work.h index 3234f2cf6327..f8ae6144c0ae 100644 --- a/fs/ksmbd/ksmbd_work.h +++ b/fs/ksmbd/ksmbd_work.h @@ -68,7 +68,7 @@ struct ksmbd_work { /* Request is encrypted */ bool encrypted:1; /* Is this SYNC or ASYNC ksmbd_work */ - bool synchronous:1; + bool asynchronous:1; bool need_invalidate_rkey:1; unsigned int remote_key; diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 97c9d1b5bcc0..3656ccac06e3 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -498,12 +498,6 @@ int init_smb2_rsp_hdr(struct ksmbd_work *work) rsp_hdr->SessionId = rcv_hdr->SessionId; memcpy(rsp_hdr->Signature, rcv_hdr->Signature, 16); - work->synchronous = true; - if (work->async_id) { - ksmbd_release_id(&conn->async_ida, work->async_id); - work->async_id = 0; - } - return 0; } @@ -644,7 +638,7 @@ int setup_async_work(struct ksmbd_work *work, void (*fn)(void **), void **arg) pr_err("Failed to alloc async message id\n"); return id; } - work->synchronous = false; + work->asynchronous = true; work->async_id = id; rsp_hdr->Id.AsyncId = cpu_to_le64(id); @@ -664,6 +658,24 @@ int setup_async_work(struct ksmbd_work *work, void (*fn)(void **), void **arg) return 0; } +void release_async_work(struct ksmbd_work *work) +{ + struct ksmbd_conn *conn = work->conn; + + spin_lock(&conn->request_lock); + list_del_init(&work->async_request_entry); + spin_unlock(&conn->request_lock); + + work->asynchronous = 0; + work->cancel_fn = NULL; + kfree(work->cancel_argv); + work->cancel_argv = NULL; + if (work->async_id) { + ksmbd_release_id(&conn->async_ida, work->async_id); + work->async_id = 0; + } +} + void smb2_send_interim_resp(struct ksmbd_work *work, __le32 status) { struct smb2_hdr *rsp_hdr; @@ -7045,13 +7057,9 @@ int smb2_lock(struct ksmbd_work *work) ksmbd_vfs_posix_lock_wait(flock); - spin_lock(&work->conn->request_lock); spin_lock(&fp->f_lock); list_del(&work->fp_entry); - work->cancel_fn = NULL; - kfree(argv); spin_unlock(&fp->f_lock); - spin_unlock(&work->conn->request_lock); if (work->state != KSMBD_WORK_ACTIVE) { list_del(&smb_lock->llist); @@ -7069,6 +7077,7 @@ int smb2_lock(struct ksmbd_work *work) work->send_no_response = 1; goto out; } + init_smb2_rsp_hdr(work); smb2_set_err_rsp(work); rsp->hdr.Status = @@ -7081,7 +7090,7 @@ int smb2_lock(struct ksmbd_work *work) spin_lock(&work->conn->llist_lock); list_del(&smb_lock->clist); spin_unlock(&work->conn->llist_lock); - + release_async_work(work); goto retry; } else if (!rc) { spin_lock(&work->conn->llist_lock); diff --git a/fs/ksmbd/smb2pdu.h b/fs/ksmbd/smb2pdu.h index 0c8a770fe318..9420dd2813fb 100644 --- a/fs/ksmbd/smb2pdu.h +++ b/fs/ksmbd/smb2pdu.h @@ -486,6 +486,7 @@ int find_matching_smb2_dialect(int start_index, __le16 *cli_dialects, struct file_lock *smb_flock_init(struct file *f); int setup_async_work(struct ksmbd_work *work, void (*fn)(void **), void **arg); +void release_async_work(struct ksmbd_work *work); void smb2_send_interim_resp(struct ksmbd_work *work, __le32 status); struct channel *lookup_chann_list(struct ksmbd_session *sess, struct ksmbd_conn *conn); From dc8289f912387c3bcfbc5d2db29c8947fa207c11 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Sat, 1 Apr 2023 13:21:04 +0900 Subject: [PATCH 4/4] ksmbd: fix slab-out-of-bounds in init_smb2_rsp_hdr When smb1 mount fails, KASAN detect slab-out-of-bounds in init_smb2_rsp_hdr like the following one. For smb1 negotiate(56bytes) , init_smb2_rsp_hdr() for smb2 is called. The issue occurs while handling smb1 negotiate as smb2 server operations. Add smb server operations for smb1 (get_cmd_val, init_rsp_hdr, allocate_rsp_buf, check_user_session) to handle smb1 negotiate so that smb2 server operation does not handle it. [ 411.400423] CIFS: VFS: Use of the less secure dialect vers=1.0 is not recommended unless required for access to very old servers [ 411.400452] CIFS: Attempting to mount \\192.168.45.139\homes [ 411.479312] ksmbd: init_smb2_rsp_hdr : 492 [ 411.479323] ================================================================== [ 411.479327] BUG: KASAN: slab-out-of-bounds in init_smb2_rsp_hdr+0x1e2/0x1f4 [ksmbd] [ 411.479369] Read of size 16 at addr ffff888488ed0734 by task kworker/14:1/199 [ 411.479379] CPU: 14 PID: 199 Comm: kworker/14:1 Tainted: G OE 6.1.21 #3 [ 411.479386] Hardware name: ASUSTeK COMPUTER INC. Z10PA-D8 Series/Z10PA-D8 Series, BIOS 3801 08/23/2019 [ 411.479390] Workqueue: ksmbd-io handle_ksmbd_work [ksmbd] [ 411.479425] Call Trace: [ 411.479428] [ 411.479432] dump_stack_lvl+0x49/0x63 [ 411.479444] print_report+0x171/0x4a8 [ 411.479452] ? kasan_complete_mode_report_info+0x3c/0x200 [ 411.479463] ? init_smb2_rsp_hdr+0x1e2/0x1f4 [ksmbd] [ 411.479497] kasan_report+0xb4/0x130 [ 411.479503] ? init_smb2_rsp_hdr+0x1e2/0x1f4 [ksmbd] [ 411.479537] kasan_check_range+0x149/0x1e0 [ 411.479543] memcpy+0x24/0x70 [ 411.479550] init_smb2_rsp_hdr+0x1e2/0x1f4 [ksmbd] [ 411.479585] handle_ksmbd_work+0x109/0x760 [ksmbd] [ 411.479616] ? _raw_spin_unlock_irqrestore+0x50/0x50 [ 411.479624] ? smb3_encrypt_resp+0x340/0x340 [ksmbd] [ 411.479656] process_one_work+0x49c/0x790 [ 411.479667] worker_thread+0x2b1/0x6e0 [ 411.479674] ? process_one_work+0x790/0x790 [ 411.479680] kthread+0x177/0x1b0 [ 411.479686] ? kthread_complete_and_exit+0x30/0x30 [ 411.479692] ret_from_fork+0x22/0x30 [ 411.479702] Fixes: 39b291b86b59 ("ksmbd: return unsupported error on smb1 mount") Cc: stable@vger.kernel.org Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/server.c | 5 +- fs/ksmbd/smb2pdu.c | 3 - fs/ksmbd/smb_common.c | 140 +++++++++++++++++++++++++++++++++--------- fs/ksmbd/smb_common.h | 2 +- 4 files changed, 112 insertions(+), 38 deletions(-) diff --git a/fs/ksmbd/server.c b/fs/ksmbd/server.c index 394b6ceac431..0d8242789dc8 100644 --- a/fs/ksmbd/server.c +++ b/fs/ksmbd/server.c @@ -289,10 +289,7 @@ static int queue_ksmbd_work(struct ksmbd_conn *conn) work->request_buf = conn->request_buf; conn->request_buf = NULL; - if (ksmbd_init_smb_server(work)) { - ksmbd_free_work_struct(work); - return -EINVAL; - } + ksmbd_init_smb_server(work); ksmbd_conn_enqueue_request(work); atomic_inc(&conn->r_count); diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 3656ccac06e3..8af939a181be 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -229,9 +229,6 @@ int init_smb2_neg_rsp(struct ksmbd_work *work) struct smb2_negotiate_rsp *rsp; struct ksmbd_conn *conn = work->conn; - if (conn->need_neg == false) - return -EINVAL; - *(__be32 *)work->response_buf = cpu_to_be32(conn->vals->header_size); diff --git a/fs/ksmbd/smb_common.c b/fs/ksmbd/smb_common.c index 9c1ce6d199ce..af0c2a9b8529 100644 --- a/fs/ksmbd/smb_common.c +++ b/fs/ksmbd/smb_common.c @@ -283,20 +283,121 @@ static int ksmbd_negotiate_smb_dialect(void *buf) return BAD_PROT_ID; } -int ksmbd_init_smb_server(struct ksmbd_work *work) -{ - struct ksmbd_conn *conn = work->conn; +#define SMB_COM_NEGOTIATE_EX 0x0 - if (conn->need_neg == false) +/** + * get_smb1_cmd_val() - get smb command value from smb header + * @work: smb work containing smb header + * + * Return: smb command value + */ +static u16 get_smb1_cmd_val(struct ksmbd_work *work) +{ + return SMB_COM_NEGOTIATE_EX; +} + +/** + * init_smb1_rsp_hdr() - initialize smb negotiate response header + * @work: smb work containing smb request + * + * Return: 0 on success, otherwise -EINVAL + */ +static int init_smb1_rsp_hdr(struct ksmbd_work *work) +{ + struct smb_hdr *rsp_hdr = (struct smb_hdr *)work->response_buf; + struct smb_hdr *rcv_hdr = (struct smb_hdr *)work->request_buf; + + /* + * Remove 4 byte direct TCP header. + */ + *(__be32 *)work->response_buf = + cpu_to_be32(sizeof(struct smb_hdr) - 4); + + rsp_hdr->Command = SMB_COM_NEGOTIATE; + *(__le32 *)rsp_hdr->Protocol = SMB1_PROTO_NUMBER; + rsp_hdr->Flags = SMBFLG_RESPONSE; + rsp_hdr->Flags2 = SMBFLG2_UNICODE | SMBFLG2_ERR_STATUS | + SMBFLG2_EXT_SEC | SMBFLG2_IS_LONG_NAME; + rsp_hdr->Pid = rcv_hdr->Pid; + rsp_hdr->Mid = rcv_hdr->Mid; + return 0; +} + +/** + * smb1_check_user_session() - check for valid session for a user + * @work: smb work containing smb request buffer + * + * Return: 0 on success, otherwise error + */ +static int smb1_check_user_session(struct ksmbd_work *work) +{ + unsigned int cmd = work->conn->ops->get_cmd_val(work); + + if (cmd == SMB_COM_NEGOTIATE_EX) return 0; - init_smb3_11_server(conn); + return -EINVAL; +} + +/** + * smb1_allocate_rsp_buf() - allocate response buffer for a command + * @work: smb work containing smb request + * + * Return: 0 on success, otherwise -ENOMEM + */ +static int smb1_allocate_rsp_buf(struct ksmbd_work *work) +{ + work->response_buf = kmalloc(MAX_CIFS_SMALL_BUFFER_SIZE, + GFP_KERNEL | __GFP_ZERO); + work->response_sz = MAX_CIFS_SMALL_BUFFER_SIZE; + + if (!work->response_buf) { + pr_err("Failed to allocate %u bytes buffer\n", + MAX_CIFS_SMALL_BUFFER_SIZE); + return -ENOMEM; + } - if (conn->ops->get_cmd_val(work) != SMB_COM_NEGOTIATE) - conn->need_neg = false; return 0; } +static struct smb_version_ops smb1_server_ops = { + .get_cmd_val = get_smb1_cmd_val, + .init_rsp_hdr = init_smb1_rsp_hdr, + .allocate_rsp_buf = smb1_allocate_rsp_buf, + .check_user_session = smb1_check_user_session, +}; + +static int smb1_negotiate(struct ksmbd_work *work) +{ + return ksmbd_smb_negotiate_common(work, SMB_COM_NEGOTIATE); +} + +static struct smb_version_cmds smb1_server_cmds[1] = { + [SMB_COM_NEGOTIATE_EX] = { .proc = smb1_negotiate, }, +}; + +static void init_smb1_server(struct ksmbd_conn *conn) +{ + conn->ops = &smb1_server_ops; + conn->cmds = smb1_server_cmds; + conn->max_cmds = ARRAY_SIZE(smb1_server_cmds); +} + +void ksmbd_init_smb_server(struct ksmbd_work *work) +{ + struct ksmbd_conn *conn = work->conn; + __le32 proto; + + if (conn->need_neg == false) + return; + + proto = *(__le32 *)((struct smb_hdr *)work->request_buf)->Protocol; + if (proto == SMB1_PROTO_NUMBER) + init_smb1_server(conn); + else + init_smb3_11_server(conn); +} + int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work, int info_level, struct ksmbd_file *dir, struct ksmbd_dir_info *d_info, @@ -444,20 +545,10 @@ static int smb_handle_negotiate(struct ksmbd_work *work) ksmbd_debug(SMB, "Unsupported SMB1 protocol\n"); - /* - * Remove 4 byte direct TCP header, add 2 byte bcc and - * 2 byte DialectIndex. - */ - *(__be32 *)work->response_buf = - cpu_to_be32(sizeof(struct smb_hdr) - 4 + 2 + 2); + /* Add 2 byte bcc and 2 byte DialectIndex. */ + inc_rfc1001_len(work->response_buf, 4); neg_rsp->hdr.Status.CifsError = STATUS_SUCCESS; - neg_rsp->hdr.Command = SMB_COM_NEGOTIATE; - *(__le32 *)neg_rsp->hdr.Protocol = SMB1_PROTO_NUMBER; - neg_rsp->hdr.Flags = SMBFLG_RESPONSE; - neg_rsp->hdr.Flags2 = SMBFLG2_UNICODE | SMBFLG2_ERR_STATUS | - SMBFLG2_EXT_SEC | SMBFLG2_IS_LONG_NAME; - neg_rsp->hdr.WordCount = 1; neg_rsp->DialectIndex = cpu_to_le16(work->conn->dialect); neg_rsp->ByteCount = 0; @@ -473,24 +564,13 @@ int ksmbd_smb_negotiate_common(struct ksmbd_work *work, unsigned int command) ksmbd_negotiate_smb_dialect(work->request_buf); ksmbd_debug(SMB, "conn->dialect 0x%x\n", conn->dialect); - if (command == SMB2_NEGOTIATE_HE) { - struct smb2_hdr *smb2_hdr = smb2_get_msg(work->request_buf); - - if (smb2_hdr->ProtocolId != SMB2_PROTO_NUMBER) { - ksmbd_debug(SMB, "Downgrade to SMB1 negotiation\n"); - command = SMB_COM_NEGOTIATE; - } - } - if (command == SMB2_NEGOTIATE_HE) { ret = smb2_handle_negotiate(work); - init_smb2_neg_rsp(work); return ret; } if (command == SMB_COM_NEGOTIATE) { if (__smb2_negotiate(conn)) { - conn->need_neg = true; init_smb3_11_server(conn); init_smb2_neg_rsp(work); ksmbd_debug(SMB, "Upgrade to SMB2 negotiation\n"); diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h index d30ce4c1a151..9130d2e3cd78 100644 --- a/fs/ksmbd/smb_common.h +++ b/fs/ksmbd/smb_common.h @@ -427,7 +427,7 @@ bool ksmbd_smb_request(struct ksmbd_conn *conn); int ksmbd_lookup_dialect_by_id(__le16 *cli_dialects, __le16 dialects_count); -int ksmbd_init_smb_server(struct ksmbd_work *work); +void ksmbd_init_smb_server(struct ksmbd_work *work); struct ksmbd_kstat; int ksmbd_populate_dot_dotdot_entries(struct ksmbd_work *work,