From 1c90b54718fdea4f89e7e0c2415803f33f6d0b00 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Wed, 13 Jul 2022 09:14:29 +0900 Subject: ksmbd: remove unused ksmbd_share_configs_cleanup function remove unused ksmbd_share_configs_cleanup function. Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/mgmt/share_config.c | 14 -------------- fs/ksmbd/mgmt/share_config.h | 2 -- 2 files changed, 16 deletions(-) (limited to 'fs/ksmbd') diff --git a/fs/ksmbd/mgmt/share_config.c b/fs/ksmbd/mgmt/share_config.c index cb72d30f5b71..70655af93b44 100644 --- a/fs/ksmbd/mgmt/share_config.c +++ b/fs/ksmbd/mgmt/share_config.c @@ -222,17 +222,3 @@ bool ksmbd_share_veto_filename(struct ksmbd_share_config *share, } return false; } - -void ksmbd_share_configs_cleanup(void) -{ - struct ksmbd_share_config *share; - struct hlist_node *tmp; - int i; - - down_write(&shares_table_lock); - hash_for_each_safe(shares_table, i, tmp, share, hlist) { - hash_del(&share->hlist); - kill_share(share); - } - up_write(&shares_table_lock); -} diff --git a/fs/ksmbd/mgmt/share_config.h b/fs/ksmbd/mgmt/share_config.h index 953befc94e84..28bf3511763f 100644 --- a/fs/ksmbd/mgmt/share_config.h +++ b/fs/ksmbd/mgmt/share_config.h @@ -76,6 +76,4 @@ static inline void ksmbd_share_config_put(struct ksmbd_share_config *share) struct ksmbd_share_config *ksmbd_share_config_get(char *name); bool ksmbd_share_veto_filename(struct ksmbd_share_config *share, const char *filename); -void ksmbd_share_configs_cleanup(void); - #endif /* __SHARE_CONFIG_MANAGEMENT_H__ */ -- cgit v1.2.3 From e4d3e6b524c0c928f7fc9e03e047885c4951ae60 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Fri, 22 Jul 2022 10:15:10 +0900 Subject: ksmbd: replace sessions list in connection with xarray Replace sessions list in connection with xarray. Signed-off-by: Namjae Jeon Reviewed-by: Hyunchul Lee Signed-off-by: Steve French --- fs/ksmbd/connection.c | 3 ++- fs/ksmbd/connection.h | 2 +- fs/ksmbd/mgmt/user_session.c | 31 +++++++------------------------ fs/ksmbd/mgmt/user_session.h | 5 ++--- fs/ksmbd/smb2pdu.c | 13 +++++++++---- 5 files changed, 21 insertions(+), 33 deletions(-) (limited to 'fs/ksmbd') diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c index e8f476c5f189..ce23cc89046e 100644 --- a/fs/ksmbd/connection.c +++ b/fs/ksmbd/connection.c @@ -36,6 +36,7 @@ void ksmbd_conn_free(struct ksmbd_conn *conn) list_del(&conn->conns_list); write_unlock(&conn_list_lock); + xa_destroy(&conn->sessions); kvfree(conn->request_buf); kfree(conn->preauth_info); kfree(conn); @@ -66,12 +67,12 @@ struct ksmbd_conn *ksmbd_conn_alloc(void) init_waitqueue_head(&conn->req_running_q); INIT_LIST_HEAD(&conn->conns_list); - INIT_LIST_HEAD(&conn->sessions); INIT_LIST_HEAD(&conn->requests); INIT_LIST_HEAD(&conn->async_requests); spin_lock_init(&conn->request_lock); spin_lock_init(&conn->credits_lock); ida_init(&conn->async_ida); + xa_init(&conn->sessions); spin_lock_init(&conn->llist_lock); INIT_LIST_HEAD(&conn->lock_list); diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h index 98c1cbe45ec9..5b39f0bdeff8 100644 --- a/fs/ksmbd/connection.h +++ b/fs/ksmbd/connection.h @@ -55,7 +55,7 @@ struct ksmbd_conn { struct nls_table *local_nls; struct list_head conns_list; /* smb session 1 per user */ - struct list_head sessions; + struct xarray sessions; unsigned long last_active; /* How many request are running currently */ atomic_t req_running; diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c index 8d8ffd8c6f19..3a44e66456fc 100644 --- a/fs/ksmbd/mgmt/user_session.c +++ b/fs/ksmbd/mgmt/user_session.c @@ -152,8 +152,6 @@ void ksmbd_session_destroy(struct ksmbd_session *sess) if (!atomic_dec_and_test(&sess->refcnt)) return; - list_del(&sess->sessions_entry); - down_write(&sessions_table_lock); hash_del(&sess->hlist); up_write(&sessions_table_lock); @@ -181,42 +179,28 @@ static struct ksmbd_session *__session_lookup(unsigned long long id) return NULL; } -void ksmbd_session_register(struct ksmbd_conn *conn, - struct ksmbd_session *sess) +int ksmbd_session_register(struct ksmbd_conn *conn, + struct ksmbd_session *sess) { sess->conn = conn; - list_add(&sess->sessions_entry, &conn->sessions); + return xa_err(xa_store(&conn->sessions, sess->id, sess, GFP_KERNEL)); } void ksmbd_sessions_deregister(struct ksmbd_conn *conn) { struct ksmbd_session *sess; + unsigned long id; - while (!list_empty(&conn->sessions)) { - sess = list_entry(conn->sessions.next, - struct ksmbd_session, - sessions_entry); - + xa_for_each(&conn->sessions, id, sess) { + xa_erase(&conn->sessions, sess->id); ksmbd_session_destroy(sess); } } -static bool ksmbd_session_id_match(struct ksmbd_session *sess, - unsigned long long id) -{ - return sess->id == id; -} - struct ksmbd_session *ksmbd_session_lookup(struct ksmbd_conn *conn, unsigned long long id) { - struct ksmbd_session *sess = NULL; - - list_for_each_entry(sess, &conn->sessions, sessions_entry) { - if (ksmbd_session_id_match(sess, id)) - return sess; - } - return NULL; + return xa_load(&conn->sessions, id); } int get_session(struct ksmbd_session *sess) @@ -314,7 +298,6 @@ static struct ksmbd_session *__session_create(int protocol) goto error; set_session_flag(sess, protocol); - INIT_LIST_HEAD(&sess->sessions_entry); xa_init(&sess->tree_conns); INIT_LIST_HEAD(&sess->ksmbd_chann_list); INIT_LIST_HEAD(&sess->rpc_handle_list); diff --git a/fs/ksmbd/mgmt/user_session.h b/fs/ksmbd/mgmt/user_session.h index e241f16a3851..8b08189be3fc 100644 --- a/fs/ksmbd/mgmt/user_session.h +++ b/fs/ksmbd/mgmt/user_session.h @@ -57,7 +57,6 @@ struct ksmbd_session { __u8 smb3decryptionkey[SMB3_ENC_DEC_KEY_SIZE]; __u8 smb3signingkey[SMB3_SIGN_KEY_SIZE]; - struct list_head sessions_entry; struct ksmbd_file_table file_table; atomic_t refcnt; }; @@ -84,8 +83,8 @@ void ksmbd_session_destroy(struct ksmbd_session *sess); struct ksmbd_session *ksmbd_session_lookup_slowpath(unsigned long long id); struct ksmbd_session *ksmbd_session_lookup(struct ksmbd_conn *conn, unsigned long long id); -void ksmbd_session_register(struct ksmbd_conn *conn, - struct ksmbd_session *sess); +int ksmbd_session_register(struct ksmbd_conn *conn, + struct ksmbd_session *sess); void ksmbd_sessions_deregister(struct ksmbd_conn *conn); struct ksmbd_session *ksmbd_session_lookup_all(struct ksmbd_conn *conn, unsigned long long id); diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 353f047e783c..7e1620a27b85 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -588,7 +588,8 @@ int smb2_check_user_session(struct ksmbd_work *work) return -EINVAL; } -static void destroy_previous_session(struct ksmbd_user *user, u64 id) +static void destroy_previous_session(struct ksmbd_conn *conn, + struct ksmbd_user *user, u64 id) { struct ksmbd_session *prev_sess = ksmbd_session_lookup_slowpath(id); struct ksmbd_user *prev_user; @@ -607,6 +608,7 @@ static void destroy_previous_session(struct ksmbd_user *user, u64 id) } put_session(prev_sess); + xa_erase(&conn->sessions, prev_sess->id); ksmbd_session_destroy(prev_sess); } @@ -1439,7 +1441,7 @@ static int ntlm_authenticate(struct ksmbd_work *work) /* Check for previous session */ prev_id = le64_to_cpu(req->PreviousSessionId); if (prev_id && prev_id != sess->id) - destroy_previous_session(user, prev_id); + destroy_previous_session(conn, user, prev_id); if (sess->state == SMB2_SESSION_VALID) { /* @@ -1561,7 +1563,7 @@ static int krb5_authenticate(struct ksmbd_work *work) /* Check previous session */ prev_sess_id = le64_to_cpu(req->PreviousSessionId); if (prev_sess_id && prev_sess_id != sess->id) - destroy_previous_session(sess->user, prev_sess_id); + destroy_previous_session(conn, sess->user, prev_sess_id); if (sess->state == SMB2_SESSION_VALID) ksmbd_free_user(sess->user); @@ -1650,7 +1652,9 @@ int smb2_sess_setup(struct ksmbd_work *work) goto out_err; } rsp->hdr.SessionId = cpu_to_le64(sess->id); - ksmbd_session_register(conn, sess); + rc = ksmbd_session_register(conn, sess); + if (rc) + goto out_err; } else if (conn->dialect >= SMB30_PROT_ID && (server_conf.flags & KSMBD_GLOBAL_FLAG_SMB3_MULTICHANNEL) && req->Flags & SMB2_SESSION_REQ_FLAG_BINDING) { @@ -1828,6 +1832,7 @@ out_err: if (sess->user && sess->user->flags & KSMBD_USER_FLAG_DELAY_SESSION) try_delay = true; + xa_erase(&conn->sessions, sess->id); ksmbd_session_destroy(sess); work->sess = NULL; if (try_delay) -- cgit v1.2.3 From 8e06b31e348107c5d78e2c90bb7e69388cb97fb6 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Fri, 22 Jul 2022 10:15:57 +0900 Subject: ksmbd: add channel rwlock Add missing rwlock for channel list in session. Signed-off-by: Namjae Jeon Reviewed-by: Hyunchul Lee Signed-off-by: Steve French --- fs/ksmbd/mgmt/user_session.c | 3 +++ fs/ksmbd/mgmt/user_session.h | 1 + fs/ksmbd/smb2pdu.c | 20 ++++++++++++++++++-- 3 files changed, 22 insertions(+), 2 deletions(-) (limited to 'fs/ksmbd') diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c index 3a44e66456fc..25e9ba3b7550 100644 --- a/fs/ksmbd/mgmt/user_session.c +++ b/fs/ksmbd/mgmt/user_session.c @@ -32,11 +32,13 @@ static void free_channel_list(struct ksmbd_session *sess) { struct channel *chann, *tmp; + write_lock(&sess->chann_lock); list_for_each_entry_safe(chann, tmp, &sess->ksmbd_chann_list, chann_list) { list_del(&chann->chann_list); kfree(chann); } + write_unlock(&sess->chann_lock); } static void __session_rpc_close(struct ksmbd_session *sess, @@ -303,6 +305,7 @@ static struct ksmbd_session *__session_create(int protocol) INIT_LIST_HEAD(&sess->rpc_handle_list); sess->sequence_number = 1; atomic_set(&sess->refcnt, 1); + rwlock_init(&sess->chann_lock); switch (protocol) { case CIFDS_SESSION_FLAG_SMB2: diff --git a/fs/ksmbd/mgmt/user_session.h b/fs/ksmbd/mgmt/user_session.h index 8b08189be3fc..1ec659f0151b 100644 --- a/fs/ksmbd/mgmt/user_session.h +++ b/fs/ksmbd/mgmt/user_session.h @@ -48,6 +48,7 @@ struct ksmbd_session { char sess_key[CIFS_KEY_SIZE]; struct hlist_node hlist; + rwlock_t chann_lock; struct list_head ksmbd_chann_list; struct xarray tree_conns; struct ida tree_conn_ida; diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 7e1620a27b85..02a7df0eac4c 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -1512,7 +1512,9 @@ static int ntlm_authenticate(struct ksmbd_work *work) binding_session: if (conn->dialect >= SMB30_PROT_ID) { + read_lock(&sess->chann_lock); chann = lookup_chann_list(sess, conn); + read_unlock(&sess->chann_lock); if (!chann) { chann = kmalloc(sizeof(struct channel), GFP_KERNEL); if (!chann) @@ -1520,7 +1522,9 @@ binding_session: chann->conn = conn; INIT_LIST_HEAD(&chann->chann_list); + write_lock(&sess->chann_lock); list_add(&chann->chann_list, &sess->ksmbd_chann_list); + write_unlock(&sess->chann_lock); } } @@ -1594,7 +1598,9 @@ static int krb5_authenticate(struct ksmbd_work *work) } if (conn->dialect >= SMB30_PROT_ID) { + read_lock(&sess->chann_lock); chann = lookup_chann_list(sess, conn); + read_unlock(&sess->chann_lock); if (!chann) { chann = kmalloc(sizeof(struct channel), GFP_KERNEL); if (!chann) @@ -1602,7 +1608,9 @@ static int krb5_authenticate(struct ksmbd_work *work) chann->conn = conn; INIT_LIST_HEAD(&chann->chann_list); + write_lock(&sess->chann_lock); list_add(&chann->chann_list, &sess->ksmbd_chann_list); + write_unlock(&sess->chann_lock); } } @@ -8361,10 +8369,14 @@ int smb3_check_sign_req(struct ksmbd_work *work) if (le16_to_cpu(hdr->Command) == SMB2_SESSION_SETUP_HE) { signing_key = work->sess->smb3signingkey; } else { + read_lock(&work->sess->chann_lock); chann = lookup_chann_list(work->sess, conn); - if (!chann) + if (!chann) { + read_unlock(&work->sess->chann_lock); return 0; + } signing_key = chann->smb3signingkey; + read_unlock(&work->sess->chann_lock); } if (!signing_key) { @@ -8424,10 +8436,14 @@ void smb3_set_sign_rsp(struct ksmbd_work *work) le16_to_cpu(hdr->Command) == SMB2_SESSION_SETUP_HE) { signing_key = work->sess->smb3signingkey; } else { + read_lock(&work->sess->chann_lock); chann = lookup_chann_list(work->sess, work->conn); - if (!chann) + if (!chann) { + read_unlock(&work->sess->chann_lock); return; + } signing_key = chann->smb3signingkey; + read_unlock(&work->sess->chann_lock); } if (!signing_key) -- cgit v1.2.3 From 17ea92a9f6d0b9a97aaec5ab748e4591d70a562c Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Fri, 22 Jul 2022 10:17:06 +0900 Subject: ksmbd: fix kernel oops from idr_remove() There is a report that kernel oops happen from idr_remove(). kernel: BUG: kernel NULL pointer dereference, address: 0000000000000010 kernel: RIP: 0010:idr_remove+0x1/0x20 kernel: __ksmbd_close_fd+0xb2/0x2d0 [ksmbd] kernel: ksmbd_vfs_read+0x91/0x190 [ksmbd] kernel: ksmbd_fd_put+0x29/0x40 [ksmbd] kernel: smb2_read+0x210/0x390 [ksmbd] kernel: __process_request+0xa4/0x180 [ksmbd] kernel: __handle_ksmbd_work+0xf0/0x290 [ksmbd] kernel: handle_ksmbd_work+0x2d/0x50 [ksmbd] kernel: process_one_work+0x21d/0x3f0 kernel: worker_thread+0x50/0x3d0 kernel: rescuer_thread+0x390/0x390 kernel: kthread+0xee/0x120 kthread_complete_and_exit+0x20/0x20 kernel: ret_from_fork+0x22/0x30 While accessing files, If connection is disconnected, windows send session setup request included previous session destroy. But while still processing requests on previous session, this request destroy file table, which mean file table idr will be freed and set to NULL. So kernel oops happen from ft->idr in __ksmbd_close_fd(). This patch don't directly destroy session in destroy_previous_session(). It just set to KSMBD_SESS_EXITING so that connection will be terminated after finishing the rest of requests. Signed-off-by: Namjae Jeon Reviewed-by: Hyunchul Lee Signed-off-by: Steve French --- fs/ksmbd/mgmt/user_session.c | 2 ++ fs/ksmbd/smb2pdu.c | 8 ++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) (limited to 'fs/ksmbd') diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c index 25e9ba3b7550..b9acb6770b03 100644 --- a/fs/ksmbd/mgmt/user_session.c +++ b/fs/ksmbd/mgmt/user_session.c @@ -239,6 +239,8 @@ struct ksmbd_session *ksmbd_session_lookup_all(struct ksmbd_conn *conn, sess = ksmbd_session_lookup(conn, id); if (!sess && conn->binding) sess = ksmbd_session_lookup_slowpath(id); + if (sess && sess->state != SMB2_SESSION_VALID) + sess = NULL; return sess; } diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 02a7df0eac4c..5d01d4077e06 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -593,6 +593,7 @@ static void destroy_previous_session(struct ksmbd_conn *conn, { struct ksmbd_session *prev_sess = ksmbd_session_lookup_slowpath(id); struct ksmbd_user *prev_user; + struct channel *chann; if (!prev_sess) return; @@ -608,8 +609,11 @@ static void destroy_previous_session(struct ksmbd_conn *conn, } put_session(prev_sess); - xa_erase(&conn->sessions, prev_sess->id); - ksmbd_session_destroy(prev_sess); + prev_sess->state = SMB2_SESSION_EXPIRED; + write_lock(&prev_sess->chann_lock); + list_for_each_entry(chann, &prev_sess->ksmbd_chann_list, chann_list) + chann->conn->status = KSMBD_SESS_EXITING; + write_unlock(&prev_sess->chann_lock); } /** -- cgit v1.2.3 From a14c573870a664386adc10526a6c2648ea56dae1 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Thu, 28 Jul 2022 23:35:18 +0900 Subject: ksmbd: use wait_event instead of schedule_timeout() ksmbd threads eating masses of cputime when connection is disconnected. If connection is disconnected, ksmbd thread waits for pending requests to be processed using schedule_timeout. schedule_timeout() incorrectly is used, and it is more efficient to use wait_event/wake_up than to check r_count every time with timeout. Signed-off-by: Namjae Jeon Reviewed-by: Hyunchul Lee Signed-off-by: Steve French --- fs/ksmbd/connection.c | 6 +++--- fs/ksmbd/connection.h | 1 + fs/ksmbd/oplock.c | 35 ++++++++++++++++++++++------------- fs/ksmbd/server.c | 8 +++++++- 4 files changed, 33 insertions(+), 17 deletions(-) (limited to 'fs/ksmbd') diff --git a/fs/ksmbd/connection.c b/fs/ksmbd/connection.c index ce23cc89046e..756ad631c019 100644 --- a/fs/ksmbd/connection.c +++ b/fs/ksmbd/connection.c @@ -66,6 +66,7 @@ struct ksmbd_conn *ksmbd_conn_alloc(void) conn->outstanding_credits = 0; init_waitqueue_head(&conn->req_running_q); + init_waitqueue_head(&conn->r_count_q); INIT_LIST_HEAD(&conn->conns_list); INIT_LIST_HEAD(&conn->requests); INIT_LIST_HEAD(&conn->async_requests); @@ -165,7 +166,6 @@ int ksmbd_conn_write(struct ksmbd_work *work) struct kvec iov[3]; int iov_idx = 0; - ksmbd_conn_try_dequeue_request(work); if (!work->response_buf) { pr_err("NULL response header\n"); return -EINVAL; @@ -347,8 +347,8 @@ int ksmbd_conn_handler_loop(void *p) out: /* Wait till all reference dropped to the Server object*/ - while (atomic_read(&conn->r_count) > 0) - schedule_timeout(HZ); + wait_event(conn->r_count_q, atomic_read(&conn->r_count) == 0); + unload_nls(conn->local_nls); if (default_conn_ops.terminate_fn) diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h index 5b39f0bdeff8..2e4730457c92 100644 --- a/fs/ksmbd/connection.h +++ b/fs/ksmbd/connection.h @@ -65,6 +65,7 @@ struct ksmbd_conn { unsigned int outstanding_credits; spinlock_t credits_lock; wait_queue_head_t req_running_q; + wait_queue_head_t r_count_q; /* Lock to protect requests list*/ spinlock_t request_lock; struct list_head requests; diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c index 8b5560574d4c..3ef33ed4cdba 100644 --- a/fs/ksmbd/oplock.c +++ b/fs/ksmbd/oplock.c @@ -615,18 +615,13 @@ static void __smb2_oplock_break_noti(struct work_struct *wk) struct ksmbd_file *fp; fp = ksmbd_lookup_durable_fd(br_info->fid); - if (!fp) { - atomic_dec(&conn->r_count); - ksmbd_free_work_struct(work); - return; - } + if (!fp) + goto out; if (allocate_oplock_break_buf(work)) { pr_err("smb2_allocate_rsp_buf failed! "); - atomic_dec(&conn->r_count); ksmbd_fd_put(work, fp); - ksmbd_free_work_struct(work); - return; + goto out; } rsp_hdr = smb2_get_msg(work->response_buf); @@ -667,8 +662,16 @@ static void __smb2_oplock_break_noti(struct work_struct *wk) ksmbd_fd_put(work, fp); ksmbd_conn_write(work); + +out: ksmbd_free_work_struct(work); - atomic_dec(&conn->r_count); + /* + * Checking waitqueue to dropping pending requests on + * disconnection. waitqueue_active is safe because it + * uses atomic operation for condition. + */ + if (!atomic_dec_return(&conn->r_count) && waitqueue_active(&conn->r_count_q)) + wake_up(&conn->r_count_q); } /** @@ -731,9 +734,7 @@ static void __smb2_lease_break_noti(struct work_struct *wk) if (allocate_oplock_break_buf(work)) { ksmbd_debug(OPLOCK, "smb2_allocate_rsp_buf failed! "); - ksmbd_free_work_struct(work); - atomic_dec(&conn->r_count); - return; + goto out; } rsp_hdr = smb2_get_msg(work->response_buf); @@ -771,8 +772,16 @@ static void __smb2_lease_break_noti(struct work_struct *wk) inc_rfc1001_len(work->response_buf, 44); ksmbd_conn_write(work); + +out: ksmbd_free_work_struct(work); - atomic_dec(&conn->r_count); + /* + * Checking waitqueue to dropping pending requests on + * disconnection. waitqueue_active is safe because it + * uses atomic operation for condition. + */ + if (!atomic_dec_return(&conn->r_count) && waitqueue_active(&conn->r_count_q)) + wake_up(&conn->r_count_q); } /** diff --git a/fs/ksmbd/server.c b/fs/ksmbd/server.c index 4cd03d661df0..ce42bff42ef9 100644 --- a/fs/ksmbd/server.c +++ b/fs/ksmbd/server.c @@ -261,7 +261,13 @@ static void handle_ksmbd_work(struct work_struct *wk) ksmbd_conn_try_dequeue_request(work); ksmbd_free_work_struct(work); - atomic_dec(&conn->r_count); + /* + * Checking waitqueue to dropping pending requests on + * disconnection. waitqueue_active is safe because it + * uses atomic operation for condition. + */ + if (!atomic_dec_return(&conn->r_count) && waitqueue_active(&conn->r_count_q)) + wake_up(&conn->r_count_q); } /** -- cgit v1.2.3 From af7c39d971e43cd0af488729bca362427ad99488 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Mon, 25 Jul 2022 13:36:52 +0900 Subject: ksmbd: fix racy issue while destroying session on multichannel After multi-channel connection with windows, Several channels of session are connected. Among them, if there is a problem in one channel, Windows connects again after disconnecting the channel. In this process, the session is released and a kernel oop can occurs while processing requests to other channels. When the channel is disconnected, if other channels still exist in the session after deleting the channel from the channel list in the session, the session should not be released. Finally, the session will be released after all channels are disconnected. Signed-off-by: Namjae Jeon Reviewed-by: Hyunchul Lee Signed-off-by: Steve French --- fs/ksmbd/auth.c | 56 +++++++++++++++++++----------------- fs/ksmbd/auth.h | 11 +++++--- fs/ksmbd/connection.h | 7 ----- fs/ksmbd/mgmt/tree_connect.c | 5 ++-- fs/ksmbd/mgmt/tree_connect.h | 4 ++- fs/ksmbd/mgmt/user_session.c | 67 +++++++++++++++++++++++++++++--------------- fs/ksmbd/mgmt/user_session.h | 7 ++--- fs/ksmbd/oplock.c | 11 ++++---- fs/ksmbd/smb2pdu.c | 21 ++++++-------- fs/ksmbd/smb_common.h | 2 +- fs/ksmbd/vfs.c | 3 +- fs/ksmbd/vfs_cache.c | 2 +- 12 files changed, 110 insertions(+), 86 deletions(-) (limited to 'fs/ksmbd') diff --git a/fs/ksmbd/auth.c b/fs/ksmbd/auth.c index 911444d21267..c5a5c7b90d72 100644 --- a/fs/ksmbd/auth.c +++ b/fs/ksmbd/auth.c @@ -121,8 +121,8 @@ out: return rc; } -static int calc_ntlmv2_hash(struct ksmbd_session *sess, char *ntlmv2_hash, - char *dname) +static int calc_ntlmv2_hash(struct ksmbd_conn *conn, struct ksmbd_session *sess, + char *ntlmv2_hash, char *dname) { int ret, len, conv_len; wchar_t *domain = NULL; @@ -158,7 +158,7 @@ static int calc_ntlmv2_hash(struct ksmbd_session *sess, char *ntlmv2_hash, } conv_len = smb_strtoUTF16(uniname, user_name(sess->user), len, - sess->conn->local_nls); + conn->local_nls); if (conv_len < 0 || conv_len > len) { ret = -EINVAL; goto out; @@ -182,7 +182,7 @@ static int calc_ntlmv2_hash(struct ksmbd_session *sess, char *ntlmv2_hash, } conv_len = smb_strtoUTF16((__le16 *)domain, dname, len, - sess->conn->local_nls); + conn->local_nls); if (conv_len < 0 || conv_len > len) { ret = -EINVAL; goto out; @@ -215,8 +215,9 @@ out: * * Return: 0 on success, error number on error */ -int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, struct ntlmv2_resp *ntlmv2, - int blen, char *domain_name, char *cryptkey) +int ksmbd_auth_ntlmv2(struct ksmbd_conn *conn, struct ksmbd_session *sess, + struct ntlmv2_resp *ntlmv2, int blen, char *domain_name, + char *cryptkey) { char ntlmv2_hash[CIFS_ENCPWD_SIZE]; char ntlmv2_rsp[CIFS_HMAC_MD5_HASH_SIZE]; @@ -230,7 +231,7 @@ int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, struct ntlmv2_resp *ntlmv2, return -ENOMEM; } - rc = calc_ntlmv2_hash(sess, ntlmv2_hash, domain_name); + rc = calc_ntlmv2_hash(conn, sess, ntlmv2_hash, domain_name); if (rc) { ksmbd_debug(AUTH, "could not get v2 hash rc %d\n", rc); goto out; @@ -333,7 +334,8 @@ int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message *authblob, /* process NTLMv2 authentication */ ksmbd_debug(AUTH, "decode_ntlmssp_authenticate_blob dname%s\n", domain_name); - ret = ksmbd_auth_ntlmv2(sess, (struct ntlmv2_resp *)((char *)authblob + nt_off), + ret = ksmbd_auth_ntlmv2(conn, sess, + (struct ntlmv2_resp *)((char *)authblob + nt_off), nt_len - CIFS_ENCPWD_SIZE, domain_name, conn->ntlmssp.cryptkey); kfree(domain_name); @@ -659,8 +661,9 @@ struct derivation { bool binding; }; -static int generate_key(struct ksmbd_session *sess, struct kvec label, - struct kvec context, __u8 *key, unsigned int key_size) +static int generate_key(struct ksmbd_conn *conn, struct ksmbd_session *sess, + struct kvec label, struct kvec context, __u8 *key, + unsigned int key_size) { unsigned char zero = 0x0; __u8 i[4] = {0, 0, 0, 1}; @@ -720,8 +723,8 @@ static int generate_key(struct ksmbd_session *sess, struct kvec label, goto smb3signkey_ret; } - if (sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM || - sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM) + if (conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM || + conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM) rc = crypto_shash_update(CRYPTO_HMACSHA256(ctx), L256, 4); else rc = crypto_shash_update(CRYPTO_HMACSHA256(ctx), L128, 4); @@ -756,17 +759,17 @@ static int generate_smb3signingkey(struct ksmbd_session *sess, if (!chann) return 0; - if (sess->conn->dialect >= SMB30_PROT_ID && signing->binding) + if (conn->dialect >= SMB30_PROT_ID && signing->binding) key = chann->smb3signingkey; else key = sess->smb3signingkey; - rc = generate_key(sess, signing->label, signing->context, key, + rc = generate_key(conn, sess, signing->label, signing->context, key, SMB3_SIGN_KEY_SIZE); if (rc) return rc; - if (!(sess->conn->dialect >= SMB30_PROT_ID && signing->binding)) + if (!(conn->dialect >= SMB30_PROT_ID && signing->binding)) memcpy(chann->smb3signingkey, key, SMB3_SIGN_KEY_SIZE); ksmbd_debug(AUTH, "dumping generated AES signing keys\n"); @@ -820,30 +823,31 @@ struct derivation_twin { struct derivation decryption; }; -static int generate_smb3encryptionkey(struct ksmbd_session *sess, +static int generate_smb3encryptionkey(struct ksmbd_conn *conn, + struct ksmbd_session *sess, const struct derivation_twin *ptwin) { int rc; - rc = generate_key(sess, ptwin->encryption.label, + rc = generate_key(conn, sess, ptwin->encryption.label, ptwin->encryption.context, sess->smb3encryptionkey, SMB3_ENC_DEC_KEY_SIZE); if (rc) return rc; - rc = generate_key(sess, ptwin->decryption.label, + rc = generate_key(conn, sess, ptwin->decryption.label, ptwin->decryption.context, sess->smb3decryptionkey, SMB3_ENC_DEC_KEY_SIZE); if (rc) return rc; ksmbd_debug(AUTH, "dumping generated AES encryption keys\n"); - ksmbd_debug(AUTH, "Cipher type %d\n", sess->conn->cipher_type); + ksmbd_debug(AUTH, "Cipher type %d\n", conn->cipher_type); ksmbd_debug(AUTH, "Session Id %llu\n", sess->id); ksmbd_debug(AUTH, "Session Key %*ph\n", SMB2_NTLMV2_SESSKEY_SIZE, sess->sess_key); - if (sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM || - sess->conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM) { + if (conn->cipher_type == SMB2_ENCRYPTION_AES256_CCM || + conn->cipher_type == SMB2_ENCRYPTION_AES256_GCM) { ksmbd_debug(AUTH, "ServerIn Key %*ph\n", SMB3_GCM256_CRYPTKEY_SIZE, sess->smb3encryptionkey); ksmbd_debug(AUTH, "ServerOut Key %*ph\n", @@ -857,7 +861,8 @@ static int generate_smb3encryptionkey(struct ksmbd_session *sess, return 0; } -int ksmbd_gen_smb30_encryptionkey(struct ksmbd_session *sess) +int ksmbd_gen_smb30_encryptionkey(struct ksmbd_conn *conn, + struct ksmbd_session *sess) { struct derivation_twin twin; struct derivation *d; @@ -874,10 +879,11 @@ int ksmbd_gen_smb30_encryptionkey(struct ksmbd_session *sess) d->context.iov_base = "ServerIn "; d->context.iov_len = 10; - return generate_smb3encryptionkey(sess, &twin); + return generate_smb3encryptionkey(conn, sess, &twin); } -int ksmbd_gen_smb311_encryptionkey(struct ksmbd_session *sess) +int ksmbd_gen_smb311_encryptionkey(struct ksmbd_conn *conn, + struct ksmbd_session *sess) { struct derivation_twin twin; struct derivation *d; @@ -894,7 +900,7 @@ int ksmbd_gen_smb311_encryptionkey(struct ksmbd_session *sess) d->context.iov_base = sess->Preauth_HashValue; d->context.iov_len = 64; - return generate_smb3encryptionkey(sess, &twin); + return generate_smb3encryptionkey(conn, sess, &twin); } int ksmbd_gen_preauth_integrity_hash(struct ksmbd_conn *conn, char *buf, diff --git a/fs/ksmbd/auth.h b/fs/ksmbd/auth.h index 95629651cf26..25b772653de0 100644 --- a/fs/ksmbd/auth.h +++ b/fs/ksmbd/auth.h @@ -38,8 +38,9 @@ struct kvec; int ksmbd_crypt_message(struct ksmbd_conn *conn, struct kvec *iov, unsigned int nvec, int enc); void ksmbd_copy_gss_neg_header(void *buf); -int ksmbd_auth_ntlmv2(struct ksmbd_session *sess, struct ntlmv2_resp *ntlmv2, - int blen, char *domain_name, char *cryptkey); +int ksmbd_auth_ntlmv2(struct ksmbd_conn *conn, struct ksmbd_session *sess, + struct ntlmv2_resp *ntlmv2, int blen, char *domain_name, + char *cryptkey); int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message *authblob, int blob_len, struct ksmbd_conn *conn, struct ksmbd_session *sess); @@ -58,8 +59,10 @@ int ksmbd_gen_smb30_signingkey(struct ksmbd_session *sess, struct ksmbd_conn *conn); int ksmbd_gen_smb311_signingkey(struct ksmbd_session *sess, struct ksmbd_conn *conn); -int ksmbd_gen_smb30_encryptionkey(struct ksmbd_session *sess); -int ksmbd_gen_smb311_encryptionkey(struct ksmbd_session *sess); +int ksmbd_gen_smb30_encryptionkey(struct ksmbd_conn *conn, + struct ksmbd_session *sess); +int ksmbd_gen_smb311_encryptionkey(struct ksmbd_conn *conn, + struct ksmbd_session *sess); int ksmbd_gen_preauth_integrity_hash(struct ksmbd_conn *conn, char *buf, __u8 *pi_hash); int ksmbd_gen_sd_hash(struct ksmbd_conn *conn, char *sd_buf, int len, diff --git a/fs/ksmbd/connection.h b/fs/ksmbd/connection.h index 2e4730457c92..e7f7d5707951 100644 --- a/fs/ksmbd/connection.h +++ b/fs/ksmbd/connection.h @@ -20,13 +20,6 @@ #define KSMBD_SOCKET_BACKLOG 16 -/* - * WARNING - * - * This is nothing but a HACK. Session status should move to channel - * or to session. As of now we have 1 tcp_conn : 1 ksmbd_session, but - * we need to change it to 1 tcp_conn : N ksmbd_sessions. - */ enum { KSMBD_SESS_NEW = 0, KSMBD_SESS_GOOD, diff --git a/fs/ksmbd/mgmt/tree_connect.c b/fs/ksmbd/mgmt/tree_connect.c index 0d28e723a28c..b35ea6a6abc5 100644 --- a/fs/ksmbd/mgmt/tree_connect.c +++ b/fs/ksmbd/mgmt/tree_connect.c @@ -16,7 +16,8 @@ #include "user_session.h" struct ksmbd_tree_conn_status -ksmbd_tree_conn_connect(struct ksmbd_session *sess, char *share_name) +ksmbd_tree_conn_connect(struct ksmbd_conn *conn, struct ksmbd_session *sess, + char *share_name) { struct ksmbd_tree_conn_status status = {-EINVAL, NULL}; struct ksmbd_tree_connect_response *resp = NULL; @@ -41,7 +42,7 @@ ksmbd_tree_conn_connect(struct ksmbd_session *sess, char *share_name) goto out_error; } - peer_addr = KSMBD_TCP_PEER_SOCKADDR(sess->conn); + peer_addr = KSMBD_TCP_PEER_SOCKADDR(conn); resp = ksmbd_ipc_tree_connect_request(sess, sc, tree_conn, diff --git a/fs/ksmbd/mgmt/tree_connect.h b/fs/ksmbd/mgmt/tree_connect.h index 18e2a996e0aa..71e50271dccf 100644 --- a/fs/ksmbd/mgmt/tree_connect.h +++ b/fs/ksmbd/mgmt/tree_connect.h @@ -12,6 +12,7 @@ struct ksmbd_share_config; struct ksmbd_user; +struct ksmbd_conn; struct ksmbd_tree_connect { int id; @@ -40,7 +41,8 @@ static inline int test_tree_conn_flag(struct ksmbd_tree_connect *tree_conn, struct ksmbd_session; struct ksmbd_tree_conn_status -ksmbd_tree_conn_connect(struct ksmbd_session *sess, char *share_name); +ksmbd_tree_conn_connect(struct ksmbd_conn *conn, struct ksmbd_session *sess, + char *share_name); int ksmbd_tree_conn_disconnect(struct ksmbd_session *sess, struct ksmbd_tree_connect *tree_conn); diff --git a/fs/ksmbd/mgmt/user_session.c b/fs/ksmbd/mgmt/user_session.c index b9acb6770b03..3fa2139a0b30 100644 --- a/fs/ksmbd/mgmt/user_session.c +++ b/fs/ksmbd/mgmt/user_session.c @@ -151,9 +151,6 @@ void ksmbd_session_destroy(struct ksmbd_session *sess) if (!sess) return; - if (!atomic_dec_and_test(&sess->refcnt)) - return; - down_write(&sessions_table_lock); hash_del(&sess->hlist); up_write(&sessions_table_lock); @@ -184,16 +181,58 @@ static struct ksmbd_session *__session_lookup(unsigned long long id) int ksmbd_session_register(struct ksmbd_conn *conn, struct ksmbd_session *sess) { - sess->conn = conn; + sess->dialect = conn->dialect; + memcpy(sess->ClientGUID, conn->ClientGUID, SMB2_CLIENT_GUID_SIZE); return xa_err(xa_store(&conn->sessions, sess->id, sess, GFP_KERNEL)); } +static int ksmbd_chann_del(struct ksmbd_conn *conn, struct ksmbd_session *sess) +{ + struct channel *chann, *tmp; + + write_lock(&sess->chann_lock); + list_for_each_entry_safe(chann, tmp, &sess->ksmbd_chann_list, + chann_list) { + if (chann->conn == conn) { + list_del(&chann->chann_list); + kfree(chann); + write_unlock(&sess->chann_lock); + return 0; + } + } + write_unlock(&sess->chann_lock); + + return -ENOENT; +} + void ksmbd_sessions_deregister(struct ksmbd_conn *conn) { struct ksmbd_session *sess; - unsigned long id; - xa_for_each(&conn->sessions, id, sess) { + if (conn->binding) { + int bkt; + + down_write(&sessions_table_lock); + hash_for_each(sessions_table, bkt, sess, hlist) { + if (!ksmbd_chann_del(conn, sess)) { + up_write(&sessions_table_lock); + goto sess_destroy; + } + } + up_write(&sessions_table_lock); + } else { + unsigned long id; + + xa_for_each(&conn->sessions, id, sess) { + if (!ksmbd_chann_del(conn, sess)) + goto sess_destroy; + } + } + + return; + +sess_destroy: + if (list_empty(&sess->ksmbd_chann_list)) { xa_erase(&conn->sessions, sess->id); ksmbd_session_destroy(sess); } @@ -205,27 +244,12 @@ struct ksmbd_session *ksmbd_session_lookup(struct ksmbd_conn *conn, return xa_load(&conn->sessions, id); } -int get_session(struct ksmbd_session *sess) -{ - return atomic_inc_not_zero(&sess->refcnt); -} - -void put_session(struct ksmbd_session *sess) -{ - if (atomic_dec_and_test(&sess->refcnt)) - pr_err("get/%s seems to be mismatched.", __func__); -} - struct ksmbd_session *ksmbd_session_lookup_slowpath(unsigned long long id) { struct ksmbd_session *sess; down_read(&sessions_table_lock); sess = __session_lookup(id); - if (sess) { - if (!get_session(sess)) - sess = NULL; - } up_read(&sessions_table_lock); return sess; @@ -306,7 +330,6 @@ static struct ksmbd_session *__session_create(int protocol) INIT_LIST_HEAD(&sess->ksmbd_chann_list); INIT_LIST_HEAD(&sess->rpc_handle_list); sess->sequence_number = 1; - atomic_set(&sess->refcnt, 1); rwlock_init(&sess->chann_lock); switch (protocol) { diff --git a/fs/ksmbd/mgmt/user_session.h b/fs/ksmbd/mgmt/user_session.h index 1ec659f0151b..8934b8ee275b 100644 --- a/fs/ksmbd/mgmt/user_session.h +++ b/fs/ksmbd/mgmt/user_session.h @@ -33,8 +33,10 @@ struct preauth_session { struct ksmbd_session { u64 id; + __u16 dialect; + char ClientGUID[SMB2_CLIENT_GUID_SIZE]; + struct ksmbd_user *user; - struct ksmbd_conn *conn; unsigned int sequence_number; unsigned int flags; @@ -59,7 +61,6 @@ struct ksmbd_session { __u8 smb3signingkey[SMB3_SIGN_KEY_SIZE]; struct ksmbd_file_table file_table; - atomic_t refcnt; }; static inline int test_session_flag(struct ksmbd_session *sess, int bit) @@ -100,6 +101,4 @@ void ksmbd_release_tree_conn_id(struct ksmbd_session *sess, int id); int ksmbd_session_rpc_open(struct ksmbd_session *sess, char *rpc_name); void ksmbd_session_rpc_close(struct ksmbd_session *sess, int id); int ksmbd_session_rpc_method(struct ksmbd_session *sess, int id); -int get_session(struct ksmbd_session *sess); -void put_session(struct ksmbd_session *sess); #endif /* __USER_SESSION_MANAGEMENT_H__ */ diff --git a/fs/ksmbd/oplock.c b/fs/ksmbd/oplock.c index 3ef33ed4cdba..9046cff4374b 100644 --- a/fs/ksmbd/oplock.c +++ b/fs/ksmbd/oplock.c @@ -30,6 +30,7 @@ static DEFINE_RWLOCK(lease_list_lock); static struct oplock_info *alloc_opinfo(struct ksmbd_work *work, u64 id, __u16 Tid) { + struct ksmbd_conn *conn = work->conn; struct ksmbd_session *sess = work->sess; struct oplock_info *opinfo; @@ -38,7 +39,7 @@ static struct oplock_info *alloc_opinfo(struct ksmbd_work *work, return NULL; opinfo->sess = sess; - opinfo->conn = sess->conn; + opinfo->conn = conn; opinfo->level = SMB2_OPLOCK_LEVEL_NONE; opinfo->op_state = OPLOCK_STATE_NONE; opinfo->pending_break = 0; @@ -981,7 +982,7 @@ int find_same_lease_key(struct ksmbd_session *sess, struct ksmbd_inode *ci, } list_for_each_entry(lb, &lease_table_list, l_entry) { - if (!memcmp(lb->client_guid, sess->conn->ClientGUID, + if (!memcmp(lb->client_guid, sess->ClientGUID, SMB2_CLIENT_GUID_SIZE)) goto found; } @@ -997,7 +998,7 @@ found: rcu_read_unlock(); if (opinfo->o_fp->f_ci == ci) goto op_next; - err = compare_guid_key(opinfo, sess->conn->ClientGUID, + err = compare_guid_key(opinfo, sess->ClientGUID, lctx->lease_key); if (err) { err = -EINVAL; @@ -1131,7 +1132,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid, struct oplock_info *m_opinfo; /* is lease already granted ? */ - m_opinfo = same_client_has_lease(ci, sess->conn->ClientGUID, + m_opinfo = same_client_has_lease(ci, sess->ClientGUID, lctx); if (m_opinfo) { copy_lease(m_opinfo, opinfo); @@ -1249,7 +1250,7 @@ void smb_break_all_levII_oplock(struct ksmbd_work *work, struct ksmbd_file *fp, { struct oplock_info *op, *brk_op; struct ksmbd_inode *ci; - struct ksmbd_conn *conn = work->sess->conn; + struct ksmbd_conn *conn = work->conn; if (!test_share_config_flag(work->tcon->share_conf, KSMBD_SHARE_FLAG_OPLOCKS)) diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 5d01d4077e06..246ce4cd8469 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -603,12 +603,9 @@ static void destroy_previous_session(struct ksmbd_conn *conn, if (!prev_user || strcmp(user->name, prev_user->name) || user->passkey_sz != prev_user->passkey_sz || - memcmp(user->passkey, prev_user->passkey, user->passkey_sz)) { - put_session(prev_sess); + memcmp(user->passkey, prev_user->passkey, user->passkey_sz)) return; - } - put_session(prev_sess); prev_sess->state = SMB2_SESSION_EXPIRED; write_lock(&prev_sess->chann_lock); list_for_each_entry(chann, &prev_sess->ksmbd_chann_list, chann_list) @@ -1499,7 +1496,7 @@ static int ntlm_authenticate(struct ksmbd_work *work) if (smb3_encryption_negotiated(conn) && !(req->Flags & SMB2_SESSION_REQ_FLAG_BINDING)) { - rc = conn->ops->generate_encryptionkey(sess); + rc = conn->ops->generate_encryptionkey(conn, sess); if (rc) { ksmbd_debug(SMB, "SMB3 encryption key generation failed\n"); @@ -1590,7 +1587,7 @@ static int krb5_authenticate(struct ksmbd_work *work) sess->sign = true; if (smb3_encryption_negotiated(conn)) { - retval = conn->ops->generate_encryptionkey(sess); + retval = conn->ops->generate_encryptionkey(conn, sess); if (retval) { ksmbd_debug(SMB, "SMB3 encryption key generation failed\n"); @@ -1678,7 +1675,7 @@ int smb2_sess_setup(struct ksmbd_work *work) goto out_err; } - if (conn->dialect != sess->conn->dialect) { + if (conn->dialect != sess->dialect) { rc = -EINVAL; goto out_err; } @@ -1688,7 +1685,7 @@ int smb2_sess_setup(struct ksmbd_work *work) goto out_err; } - if (strncmp(conn->ClientGUID, sess->conn->ClientGUID, + if (strncmp(conn->ClientGUID, sess->ClientGUID, SMB2_CLIENT_GUID_SIZE)) { rc = -ENOENT; goto out_err; @@ -1890,7 +1887,7 @@ int smb2_tree_connect(struct ksmbd_work *work) ksmbd_debug(SMB, "tree connect request for tree %s treename %s\n", name, treename); - status = ksmbd_tree_conn_connect(sess, name); + status = ksmbd_tree_conn_connect(conn, sess, name); if (status.ret == KSMBD_TREE_CONN_STATUS_OK) rsp->hdr.Id.SyncId.TreeId = cpu_to_le32(status.tree_conn->id); else @@ -4875,7 +4872,7 @@ static int smb2_get_info_filesystem(struct ksmbd_work *work, struct smb2_query_info_rsp *rsp) { struct ksmbd_session *sess = work->sess; - struct ksmbd_conn *conn = sess->conn; + struct ksmbd_conn *conn = work->conn; struct ksmbd_share_config *share = work->tcon->share_conf; int fsinfoclass = 0; struct kstatfs stfs; @@ -5793,7 +5790,7 @@ static int set_rename_info(struct ksmbd_work *work, struct ksmbd_file *fp, } next: return smb2_rename(work, fp, user_ns, rename_info, - work->sess->conn->local_nls); + work->conn->local_nls); } static int set_file_disposition_info(struct ksmbd_file *fp, @@ -5925,7 +5922,7 @@ static int smb2_set_info_file(struct ksmbd_work *work, struct ksmbd_file *fp, return smb2_create_link(work, work->tcon->share_conf, (struct smb2_file_link_info *)req->Buffer, buf_len, fp->filp, - work->sess->conn->local_nls); + work->conn->local_nls); } case FILE_DISPOSITION_INFORMATION: { diff --git a/fs/ksmbd/smb_common.h b/fs/ksmbd/smb_common.h index e1369b4345a9..318c16fa81da 100644 --- a/fs/ksmbd/smb_common.h +++ b/fs/ksmbd/smb_common.h @@ -421,7 +421,7 @@ struct smb_version_ops { int (*check_sign_req)(struct ksmbd_work *work); void (*set_sign_rsp)(struct ksmbd_work *work); int (*generate_signingkey)(struct ksmbd_session *sess, struct ksmbd_conn *conn); - int (*generate_encryptionkey)(struct ksmbd_session *sess); + int (*generate_encryptionkey)(struct ksmbd_conn *conn, struct ksmbd_session *sess); bool (*is_transform_hdr)(void *buf); int (*decrypt_req)(struct ksmbd_work *work); int (*encrypt_resp)(struct ksmbd_work *work); diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c index 05efcdf7a4a7..c23793469645 100644 --- a/fs/ksmbd/vfs.c +++ b/fs/ksmbd/vfs.c @@ -481,12 +481,11 @@ int ksmbd_vfs_write(struct ksmbd_work *work, struct ksmbd_file *fp, char *buf, size_t count, loff_t *pos, bool sync, ssize_t *written) { - struct ksmbd_session *sess = work->sess; struct file *filp; loff_t offset = *pos; int err = 0; - if (sess->conn->connection_type) { + if (work->conn->connection_type) { if (!(fp->daccess & FILE_WRITE_DATA_LE)) { pr_err("no right to write(%pd)\n", fp->filp->f_path.dentry); diff --git a/fs/ksmbd/vfs_cache.c b/fs/ksmbd/vfs_cache.c index c4d59d2735f0..da9163b00350 100644 --- a/fs/ksmbd/vfs_cache.c +++ b/fs/ksmbd/vfs_cache.c @@ -569,7 +569,7 @@ struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work *work, struct file *filp) atomic_set(&fp->refcount, 1); fp->filp = filp; - fp->conn = work->sess->conn; + fp->conn = work->conn; fp->tcon = work->tcon; fp->volatile_id = KSMBD_NO_FID; fp->persistent_id = KSMBD_NO_FID; -- cgit v1.2.3 From aa7253c2393f6dcd6a1468b0792f6da76edad917 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Thu, 28 Jul 2022 21:56:19 +0900 Subject: ksmbd: fix memory leak in smb2_handle_negotiate The allocated memory didn't free under an error path in smb2_handle_negotiate(). Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-17815 Signed-off-by: Namjae Jeon Reviewed-by: Hyunchul Lee Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'fs/ksmbd') diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 246ce4cd8469..c545c41b0364 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -1142,12 +1142,16 @@ int smb2_handle_negotiate(struct ksmbd_work *work) status); rsp->hdr.Status = status; rc = -EINVAL; + kfree(conn->preauth_info); + conn->preauth_info = NULL; goto err_out; } rc = init_smb3_11_server(conn); if (rc < 0) { rsp->hdr.Status = STATUS_INVALID_PARAMETER; + kfree(conn->preauth_info); + conn->preauth_info = NULL; goto err_out; } -- cgit v1.2.3 From cf6531d98190fa2cf92a6d8bbc8af0a4740a223c Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Thu, 28 Jul 2022 21:57:08 +0900 Subject: ksmbd: fix use-after-free bug in smb2_tree_disconect smb2_tree_disconnect() freed the struct ksmbd_tree_connect, but it left the dangling pointer. It can be accessed again under compound requests. This bug can lead an oops looking something link: [ 1685.468014 ] BUG: KASAN: use-after-free in ksmbd_tree_conn_disconnect+0x131/0x160 [ksmbd] [ 1685.468068 ] Read of size 4 at addr ffff888102172180 by task kworker/1:2/4807 ... [ 1685.468130 ] Call Trace: [ 1685.468132 ] [ 1685.468135 ] dump_stack_lvl+0x49/0x5f [ 1685.468141 ] print_report.cold+0x5e/0x5cf [ 1685.468145 ] ? ksmbd_tree_conn_disconnect+0x131/0x160 [ksmbd] [ 1685.468157 ] kasan_report+0xaa/0x120 [ 1685.468194 ] ? ksmbd_tree_conn_disconnect+0x131/0x160 [ksmbd] [ 1685.468206 ] __asan_report_load4_noabort+0x14/0x20 [ 1685.468210 ] ksmbd_tree_conn_disconnect+0x131/0x160 [ksmbd] [ 1685.468222 ] smb2_tree_disconnect+0x175/0x250 [ksmbd] [ 1685.468235 ] handle_ksmbd_work+0x30e/0x1020 [ksmbd] [ 1685.468247 ] process_one_work+0x778/0x11c0 [ 1685.468251 ] ? _raw_spin_lock_irq+0x8e/0xe0 [ 1685.468289 ] worker_thread+0x544/0x1180 [ 1685.468293 ] ? __cpuidle_text_end+0x4/0x4 [ 1685.468297 ] kthread+0x282/0x320 [ 1685.468301 ] ? process_one_work+0x11c0/0x11c0 [ 1685.468305 ] ? kthread_complete_and_exit+0x30/0x30 [ 1685.468309 ] ret_from_fork+0x1f/0x30 Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-17816 Signed-off-by: Namjae Jeon Reviewed-by: Hyunchul Lee Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 1 + 1 file changed, 1 insertion(+) (limited to 'fs/ksmbd') diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index c545c41b0364..43bc027a50f2 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -2057,6 +2057,7 @@ int smb2_tree_disconnect(struct ksmbd_work *work) ksmbd_close_tree_conn_fds(work); ksmbd_tree_conn_disconnect(sess, tcon); + work->tcon = NULL; return 0; } -- cgit v1.2.3 From ac60778b87e45576d7bfdbd6f53df902654e6f09 Mon Sep 17 00:00:00 2001 From: Hyunchul Lee Date: Thu, 28 Jul 2022 23:41:51 +0900 Subject: ksmbd: prevent out of bound read for SMB2_WRITE OOB read memory can be written to a file, if DataOffset is 0 and Length is too large in SMB2_WRITE request of compound request. To prevent this, when checking the length of the data area of SMB2_WRITE in smb2_get_data_area_len(), let the minimum of DataOffset be the size of SMB2 header + the size of SMB2_WRITE header. This bug can lead an oops looking something like: [ 798.008715] BUG: KASAN: slab-out-of-bounds in copy_page_from_iter_atomic+0xd3d/0x14b0 [ 798.008724] Read of size 252 at addr ffff88800f863e90 by task kworker/0:2/2859 ... [ 798.008754] Call Trace: [ 798.008756] [ 798.008759] dump_stack_lvl+0x49/0x5f [ 798.008764] print_report.cold+0x5e/0x5cf [ 798.008768] ? __filemap_get_folio+0x285/0x6d0 [ 798.008774] ? copy_page_from_iter_atomic+0xd3d/0x14b0 [ 798.008777] kasan_report+0xaa/0x120 [ 798.008781] ? copy_page_from_iter_atomic+0xd3d/0x14b0 [ 798.008784] kasan_check_range+0x100/0x1e0 [ 798.008788] memcpy+0x24/0x60 [ 798.008792] copy_page_from_iter_atomic+0xd3d/0x14b0 [ 798.008795] ? pagecache_get_page+0x53/0x160 [ 798.008799] ? iov_iter_get_pages_alloc+0x1590/0x1590 [ 798.008803] ? ext4_write_begin+0xfc0/0xfc0 [ 798.008807] ? current_time+0x72/0x210 [ 798.008811] generic_perform_write+0x2c8/0x530 [ 798.008816] ? filemap_fdatawrite_wbc+0x180/0x180 [ 798.008820] ? down_write+0xb4/0x120 [ 798.008824] ? down_write_killable+0x130/0x130 [ 798.008829] ext4_buffered_write_iter+0x137/0x2c0 [ 798.008833] ext4_file_write_iter+0x40b/0x1490 [ 798.008837] ? __fsnotify_parent+0x275/0xb20 [ 798.008842] ? __fsnotify_update_child_dentry_flags+0x2c0/0x2c0 [ 798.008846] ? ext4_buffered_write_iter+0x2c0/0x2c0 [ 798.008851] __kernel_write+0x3a1/0xa70 [ 798.008855] ? __x64_sys_preadv2+0x160/0x160 [ 798.008860] ? security_file_permission+0x4a/0xa0 [ 798.008865] kernel_write+0xbb/0x360 [ 798.008869] ksmbd_vfs_write+0x27e/0xb90 [ksmbd] [ 798.008881] ? ksmbd_vfs_read+0x830/0x830 [ksmbd] [ 798.008892] ? _raw_read_unlock+0x2a/0x50 [ 798.008896] smb2_write+0xb45/0x14e0 [ksmbd] [ 798.008909] ? __kasan_check_write+0x14/0x20 [ 798.008912] ? _raw_spin_lock_bh+0xd0/0xe0 [ 798.008916] ? smb2_read+0x15e0/0x15e0 [ksmbd] [ 798.008927] ? memcpy+0x4e/0x60 [ 798.008931] ? _raw_spin_unlock+0x19/0x30 [ 798.008934] ? ksmbd_smb2_check_message+0x16af/0x2350 [ksmbd] [ 798.008946] ? _raw_spin_lock_bh+0xe0/0xe0 [ 798.008950] handle_ksmbd_work+0x30e/0x1020 [ksmbd] [ 798.008962] process_one_work+0x778/0x11c0 [ 798.008966] ? _raw_spin_lock_irq+0x8e/0xe0 [ 798.008970] worker_thread+0x544/0x1180 [ 798.008973] ? __cpuidle_text_end+0x4/0x4 [ 798.008977] kthread+0x282/0x320 [ 798.008982] ? process_one_work+0x11c0/0x11c0 [ 798.008985] ? kthread_complete_and_exit+0x30/0x30 [ 798.008989] ret_from_fork+0x1f/0x30 [ 798.008995] Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-17817 Signed-off-by: Hyunchul Lee Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2misc.c | 7 +++++-- fs/ksmbd/smb2pdu.c | 8 +++----- 2 files changed, 8 insertions(+), 7 deletions(-) (limited to 'fs/ksmbd') diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c index f8f456377a51..aa1e663d9deb 100644 --- a/fs/ksmbd/smb2misc.c +++ b/fs/ksmbd/smb2misc.c @@ -136,8 +136,11 @@ static int smb2_get_data_area_len(unsigned int *off, unsigned int *len, *len = le16_to_cpu(((struct smb2_read_req *)hdr)->ReadChannelInfoLength); break; case SMB2_WRITE: - if (((struct smb2_write_req *)hdr)->DataOffset) { - *off = le16_to_cpu(((struct smb2_write_req *)hdr)->DataOffset); + if (((struct smb2_write_req *)hdr)->DataOffset || + ((struct smb2_write_req *)hdr)->Length) { + *off = max_t(unsigned int, + le16_to_cpu(((struct smb2_write_req *)hdr)->DataOffset), + offsetof(struct smb2_write_req, Buffer)); *len = le32_to_cpu(((struct smb2_write_req *)hdr)->Length); break; } diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 43bc027a50f2..5943cb17a9b0 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -6514,14 +6514,12 @@ int smb2_write(struct ksmbd_work *work) writethrough = true; if (is_rdma_channel == false) { - if ((u64)le16_to_cpu(req->DataOffset) + length > - get_rfc1002_len(work->request_buf)) { - pr_err("invalid write data offset %u, smb_len %u\n", - le16_to_cpu(req->DataOffset), - get_rfc1002_len(work->request_buf)); + if (le16_to_cpu(req->DataOffset) < + offsetof(struct smb2_write_req, Buffer)) { err = -EINVAL; goto out; } + data_buf = (char *)(((char *)&req->hdr.ProtocolId) + le16_to_cpu(req->DataOffset)); -- cgit v1.2.3 From 824d4f64c20093275f72fc8101394d75ff6a249e Mon Sep 17 00:00:00 2001 From: Hyunchul Lee Date: Thu, 28 Jul 2022 21:58:53 +0900 Subject: ksmbd: prevent out of bound read for SMB2_TREE_CONNNECT if Status is not 0 and PathLength is long, smb_strndup_from_utf16 could make out of bound read in smb2_tree_connnect. This bug can lead an oops looking something like: [ 1553.882047] BUG: KASAN: slab-out-of-bounds in smb_strndup_from_utf16+0x469/0x4c0 [ksmbd] [ 1553.882064] Read of size 2 at addr ffff88802c4eda04 by task kworker/0:2/42805 ... [ 1553.882095] Call Trace: [ 1553.882098] [ 1553.882101] dump_stack_lvl+0x49/0x5f [ 1553.882107] print_report.cold+0x5e/0x5cf [ 1553.882112] ? smb_strndup_from_utf16+0x469/0x4c0 [ksmbd] [ 1553.882122] kasan_report+0xaa/0x120 [ 1553.882128] ? smb_strndup_from_utf16+0x469/0x4c0 [ksmbd] [ 1553.882139] __asan_report_load_n_noabort+0xf/0x20 [ 1553.882143] smb_strndup_from_utf16+0x469/0x4c0 [ksmbd] [ 1553.882155] ? smb_strtoUTF16+0x3b0/0x3b0 [ksmbd] [ 1553.882166] ? __kmalloc_node+0x185/0x430 [ 1553.882171] smb2_tree_connect+0x140/0xab0 [ksmbd] [ 1553.882185] handle_ksmbd_work+0x30e/0x1020 [ksmbd] [ 1553.882197] process_one_work+0x778/0x11c0 [ 1553.882201] ? _raw_spin_lock_irq+0x8e/0xe0 [ 1553.882206] worker_thread+0x544/0x1180 [ 1553.882209] ? __cpuidle_text_end+0x4/0x4 [ 1553.882214] kthread+0x282/0x320 [ 1553.882218] ? process_one_work+0x11c0/0x11c0 [ 1553.882221] ? kthread_complete_and_exit+0x30/0x30 [ 1553.882225] ret_from_fork+0x1f/0x30 [ 1553.882231] There is no need to check error request validation in server. This check allow invalid requests not to validate message. Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-17818 Signed-off-by: Hyunchul Lee Acked-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2misc.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'fs/ksmbd') diff --git a/fs/ksmbd/smb2misc.c b/fs/ksmbd/smb2misc.c index aa1e663d9deb..6e25ace36568 100644 --- a/fs/ksmbd/smb2misc.c +++ b/fs/ksmbd/smb2misc.c @@ -90,11 +90,6 @@ static int smb2_get_data_area_len(unsigned int *off, unsigned int *len, *off = 0; *len = 0; - /* error reqeusts do not have data area */ - if (hdr->Status && hdr->Status != STATUS_MORE_PROCESSING_REQUIRED && - (((struct smb2_err_rsp *)hdr)->StructureSize) == SMB2_ERROR_STRUCTURE_SIZE2_LE) - return ret; - /* * Following commands have data areas so we have to get the location * of the data buffer offset and data buffer length for the particular -- cgit v1.2.3 From 8f0541186e9ad1b62accc9519cc2b7a7240272a7 Mon Sep 17 00:00:00 2001 From: Namjae Jeon Date: Tue, 2 Aug 2022 07:28:51 +0900 Subject: ksmbd: fix heap-based overflow in set_ntacl_dacl() The testcase use SMB2_SET_INFO_HE command to set a malformed file attribute under the label `security.NTACL`. SMB2_QUERY_INFO_HE command in testcase trigger the following overflow. [ 4712.003781] ================================================================== [ 4712.003790] BUG: KASAN: slab-out-of-bounds in build_sec_desc+0x842/0x1dd0 [ksmbd] [ 4712.003807] Write of size 1060 at addr ffff88801e34c068 by task kworker/0:0/4190 [ 4712.003813] CPU: 0 PID: 4190 Comm: kworker/0:0 Not tainted 5.19.0-rc5 #1 [ 4712.003850] Workqueue: ksmbd-io handle_ksmbd_work [ksmbd] [ 4712.003867] Call Trace: [ 4712.003870] [ 4712.003873] dump_stack_lvl+0x49/0x5f [ 4712.003935] print_report.cold+0x5e/0x5cf [ 4712.003972] ? ksmbd_vfs_get_sd_xattr+0x16d/0x500 [ksmbd] [ 4712.003984] ? cmp_map_id+0x200/0x200 [ 4712.003988] ? build_sec_desc+0x842/0x1dd0 [ksmbd] [ 4712.004000] kasan_report+0xaa/0x120 [ 4712.004045] ? build_sec_desc+0x842/0x1dd0 [ksmbd] [ 4712.004056] kasan_check_range+0x100/0x1e0 [ 4712.004060] memcpy+0x3c/0x60 [ 4712.004064] build_sec_desc+0x842/0x1dd0 [ksmbd] [ 4712.004076] ? parse_sec_desc+0x580/0x580 [ksmbd] [ 4712.004088] ? ksmbd_acls_fattr+0x281/0x410 [ksmbd] [ 4712.004099] smb2_query_info+0xa8f/0x6110 [ksmbd] [ 4712.004111] ? psi_group_change+0x856/0xd70 [ 4712.004148] ? update_load_avg+0x1c3/0x1af0 [ 4712.004152] ? asym_cpu_capacity_scan+0x5d0/0x5d0 [ 4712.004157] ? xas_load+0x23/0x300 [ 4712.004162] ? smb2_query_dir+0x1530/0x1530 [ksmbd] [ 4712.004173] ? _raw_spin_lock_bh+0xe0/0xe0 [ 4712.004179] handle_ksmbd_work+0x30e/0x1020 [ksmbd] [ 4712.004192] process_one_work+0x778/0x11c0 [ 4712.004227] ? _raw_spin_lock_irq+0x8e/0xe0 [ 4712.004231] worker_thread+0x544/0x1180 [ 4712.004234] ? __cpuidle_text_end+0x4/0x4 [ 4712.004239] kthread+0x282/0x320 [ 4712.004243] ? process_one_work+0x11c0/0x11c0 [ 4712.004246] ? kthread_complete_and_exit+0x30/0x30 [ 4712.004282] ret_from_fork+0x1f/0x30 This patch add the buffer validation for security descriptor that is stored by malformed SMB2_SET_INFO_HE command. and allocate large response buffer about SMB2_O_INFO_SECURITY file info class. Fixes: e2f34481b24d ("cifsd: add server-side procedures for SMB3") Cc: stable@vger.kernel.org Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-17771 Reviewed-by: Hyunchul Lee Signed-off-by: Namjae Jeon Signed-off-by: Steve French --- fs/ksmbd/smb2pdu.c | 39 ++++++++++------ fs/ksmbd/smbacl.c | 130 ++++++++++++++++++++++++++++++++++++----------------- fs/ksmbd/smbacl.h | 2 +- fs/ksmbd/vfs.c | 5 +++ 4 files changed, 119 insertions(+), 57 deletions(-) (limited to 'fs/ksmbd') diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c index 5943cb17a9b0..9751cc92c111 100644 --- a/fs/ksmbd/smb2pdu.c +++ b/fs/ksmbd/smb2pdu.c @@ -535,9 +535,10 @@ int smb2_allocate_rsp_buf(struct ksmbd_work *work) struct smb2_query_info_req *req; req = smb2_get_msg(work->request_buf); - if (req->InfoType == SMB2_O_INFO_FILE && - (req->FileInfoClass == FILE_FULL_EA_INFORMATION || - req->FileInfoClass == FILE_ALL_INFORMATION)) + if ((req->InfoType == SMB2_O_INFO_FILE && + (req->FileInfoClass == FILE_FULL_EA_INFORMATION || + req->FileInfoClass == FILE_ALL_INFORMATION)) || + req->InfoType == SMB2_O_INFO_SECURITY) sz = large_sz; } @@ -2988,7 +2989,7 @@ int smb2_open(struct ksmbd_work *work) goto err_out; rc = build_sec_desc(user_ns, - pntsd, NULL, + pntsd, NULL, 0, OWNER_SECINFO | GROUP_SECINFO | DACL_SECINFO, @@ -3833,6 +3834,15 @@ static int verify_info_level(int info_level) return 0; } +static int smb2_resp_buf_len(struct ksmbd_work *work, unsigned short hdr2_len) +{ + int free_len; + + free_len = (int)(work->response_sz - + (get_rfc1002_len(work->response_buf) + 4)) - hdr2_len; + return free_len; +} + static int smb2_calc_max_out_buf_len(struct ksmbd_work *work, unsigned short hdr2_len, unsigned int out_buf_len) @@ -3842,9 +3852,7 @@ static int smb2_calc_max_out_buf_len(struct ksmbd_work *work, if (out_buf_len > work->conn->vals->max_trans_size) return -EINVAL; - free_len = (int)(work->response_sz - - (get_rfc1002_len(work->response_buf) + 4)) - - hdr2_len; + free_len = smb2_resp_buf_len(work, hdr2_len); if (free_len < 0) return -EINVAL; @@ -5107,10 +5115,10 @@ static int smb2_get_info_sec(struct ksmbd_work *work, struct smb_ntsd *pntsd = (struct smb_ntsd *)rsp->Buffer, *ppntsd = NULL; struct smb_fattr fattr = {{0}}; struct inode *inode; - __u32 secdesclen; + __u32 secdesclen = 0; unsigned int id = KSMBD_NO_FID, pid = KSMBD_NO_FID; int addition_info = le32_to_cpu(req->AdditionalInformation); - int rc; + int rc = 0, ppntsd_size = 0; if (addition_info & ~(OWNER_SECINFO | GROUP_SECINFO | DACL_SECINFO | PROTECTED_DACL_SECINFO | @@ -5156,11 +5164,14 @@ static int smb2_get_info_sec(struct ksmbd_work *work, if (test_share_config_flag(work->tcon->share_conf, KSMBD_SHARE_FLAG_ACL_XATTR)) - ksmbd_vfs_get_sd_xattr(work->conn, user_ns, - fp->filp->f_path.dentry, &ppntsd); - - rc = build_sec_desc(user_ns, pntsd, ppntsd, addition_info, - &secdesclen, &fattr); + ppntsd_size = ksmbd_vfs_get_sd_xattr(work->conn, user_ns, + fp->filp->f_path.dentry, + &ppntsd); + + /* Check if sd buffer size exceeds response buffer size */ + if (smb2_resp_buf_len(work, 8) > ppntsd_size) + rc = build_sec_desc(user_ns, pntsd, ppntsd, ppntsd_size, + addition_info, &secdesclen, &fattr); posix_acl_release(fattr.cf_acls); posix_acl_release(fattr.cf_dacls); kfree(ppntsd); diff --git a/fs/ksmbd/smbacl.c b/fs/ksmbd/smbacl.c index 38f23bf981ac..3781bca2c8fc 100644 --- a/fs/ksmbd/smbacl.c +++ b/fs/ksmbd/smbacl.c @@ -690,6 +690,7 @@ posix_default_acl: static void set_ntacl_dacl(struct user_namespace *user_ns, struct smb_acl *pndacl, struct smb_acl *nt_dacl, + unsigned int aces_size, const struct smb_sid *pownersid, const struct smb_sid *pgrpsid, struct smb_fattr *fattr) @@ -703,9 +704,19 @@ static void set_ntacl_dacl(struct user_namespace *user_ns, if (nt_num_aces) { ntace = (struct smb_ace *)((char *)nt_dacl + sizeof(struct smb_acl)); for (i = 0; i < nt_num_aces; i++) { - memcpy((char *)pndace + size, ntace, le16_to_cpu(ntace->size)); - size += le16_to_cpu(ntace->size); - ntace = (struct smb_ace *)((char *)ntace + le16_to_cpu(ntace->size)); + unsigned short nt_ace_size; + + if (offsetof(struct smb_ace, access_req) > aces_size) + break; + + nt_ace_size = le16_to_cpu(ntace->size); + if (nt_ace_size > aces_size) + break; + + memcpy((char *)pndace + size, ntace, nt_ace_size); + size += nt_ace_size; + aces_size -= nt_ace_size; + ntace = (struct smb_ace *)((char *)ntace + nt_ace_size); num_aces++; } } @@ -878,7 +889,7 @@ int parse_sec_desc(struct user_namespace *user_ns, struct smb_ntsd *pntsd, /* Convert permission bits from mode to equivalent CIFS ACL */ int build_sec_desc(struct user_namespace *user_ns, struct smb_ntsd *pntsd, struct smb_ntsd *ppntsd, - int addition_info, __u32 *secdesclen, + int ppntsd_size, int addition_info, __u32 *secdesclen, struct smb_fattr *fattr) { int rc = 0; @@ -938,15 +949,25 @@ int build_sec_desc(struct user_namespace *user_ns, if (!ppntsd) { set_mode_dacl(user_ns, dacl_ptr, fattr); - } else if (!ppntsd->dacloffset) { - goto out; } else { struct smb_acl *ppdacl_ptr; + unsigned int dacl_offset = le32_to_cpu(ppntsd->dacloffset); + int ppdacl_size, ntacl_size = ppntsd_size - dacl_offset; + + if (!dacl_offset || + (dacl_offset + sizeof(struct smb_acl) > ppntsd_size)) + goto out; + + ppdacl_ptr = (struct smb_acl *)((char *)ppntsd + dacl_offset); + ppdacl_size = le16_to_cpu(ppdacl_ptr->size); + if (ppdacl_size > ntacl_size || + ppdacl_size < sizeof(struct smb_acl)) + goto out; - ppdacl_ptr = (struct smb_acl *)((char *)ppntsd + - le32_to_cpu(ppntsd->dacloffset)); set_ntacl_dacl(user_ns, dacl_ptr, ppdacl_ptr, - nowner_sid_ptr, ngroup_sid_ptr, fattr); + ntacl_size - sizeof(struct smb_acl), + nowner_sid_ptr, ngroup_sid_ptr, + fattr); } pntsd->dacloffset = cpu_to_le32(offset); offset += le16_to_cpu(dacl_ptr->size); @@ -980,24 +1001,31 @@ int smb_inherit_dacl(struct ksmbd_conn *conn, struct smb_sid owner_sid, group_sid; struct dentry *parent = path->dentry->d_parent; struct user_namespace *user_ns = mnt_user_ns(path->mnt); - int inherited_flags = 0, flags = 0, i, ace_cnt = 0, nt_size = 0; - int rc = 0, num_aces, dacloffset, pntsd_type, acl_len; + int inherited_flags = 0, flags = 0, i, ace_cnt = 0, nt_size = 0, pdacl_size; + int rc = 0, num_aces, dacloffset, pntsd_type, pntsd_size, acl_len, aces_size; char *aces_base; bool is_dir = S_ISDIR(d_inode(path->dentry)->i_mode); - acl_len = ksmbd_vfs_get_sd_xattr(conn, user_ns, - parent, &parent_pntsd); - if (acl_len <= 0) + pntsd_size = ksmbd_vfs_get_sd_xattr(conn, user_ns, + parent, &parent_pntsd); + if (pntsd_size <= 0) return -ENOENT; dacloffset = le32_to_cpu(parent_pntsd->dacloffset); - if (!dacloffset) { + if (!dacloffset || (dacloffset + sizeof(struct smb_acl) > pntsd_size)) { rc = -EINVAL; goto free_parent_pntsd; } parent_pdacl = (struct smb_acl *)((char *)parent_pntsd + dacloffset); + acl_len = pntsd_size - dacloffset; num_aces = le32_to_cpu(parent_pdacl->num_aces); pntsd_type = le16_to_cpu(parent_pntsd->type); + pdacl_size = le16_to_cpu(parent_pdacl->size); + + if (pdacl_size > acl_len || pdacl_size < sizeof(struct smb_acl)) { + rc = -EINVAL; + goto free_parent_pntsd; + } aces_base = kmalloc(sizeof(struct smb_ace) * num_aces * 2, GFP_KERNEL); if (!aces_base) { @@ -1008,11 +1036,23 @@ int smb_inherit_dacl(struct ksmbd_conn *conn, aces = (struct smb_ace *)aces_base; parent_aces = (struct smb_ace *)((char *)parent_pdacl + sizeof(struct smb_acl)); + aces_size = acl_len - sizeof(struct smb_acl); if (pntsd_type & DACL_AUTO_INHERITED) inherited_flags = INHERITED_ACE; for (i = 0; i < num_aces; i++) { + int pace_size; + + if (offsetof(struct smb_ace, access_req) > aces_size) + break; + + pace_size = le16_to_cpu(parent_aces->size); + if (pace_size > aces_size) + break; + + aces_size -= pace_size; + flags = parent_aces->flags; if (!smb_inherit_flags(flags, is_dir)) goto pass; @@ -1057,8 +1097,7 @@ int smb_inherit_dacl(struct ksmbd_conn *conn, aces = (struct smb_ace *)((char *)aces + le16_to_cpu(aces->size)); ace_cnt++; pass: - parent_aces = - (struct smb_ace *)((char *)parent_aces + le16_to_cpu(parent_aces->size)); + parent_aces = (struct smb_ace *)((char *)parent_aces + pace_size); } if (nt_size > 0) { @@ -1153,7 +1192,7 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, struct path *path, struct smb_ntsd *pntsd = NULL; struct smb_acl *pdacl; struct posix_acl *posix_acls; - int rc = 0, acl_size; + int rc = 0, pntsd_size, acl_size, aces_size, pdacl_size, dacl_offset; struct smb_sid sid; int granted = le32_to_cpu(*pdaccess & ~FILE_MAXIMAL_ACCESS_LE); struct smb_ace *ace; @@ -1162,37 +1201,33 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, struct path *path, struct smb_ace *others_ace = NULL; struct posix_acl_entry *pa_entry; unsigned int sid_type = SIDOWNER; - char *end_of_acl; + unsigned short ace_size; ksmbd_debug(SMB, "check permission using windows acl\n"); - acl_size = ksmbd_vfs_get_sd_xattr(conn, user_ns, - path->dentry, &pntsd); - if (acl_size <= 0 || !pntsd || !pntsd->dacloffset) { - kfree(pntsd); - return 0; - } + pntsd_size = ksmbd_vfs_get_sd_xattr(conn, user_ns, + path->dentry, &pntsd); + if (pntsd_size <= 0 || !pntsd) + goto err_out; + + dacl_offset = le32_to_cpu(pntsd->dacloffset); + if (!dacl_offset || + (dacl_offset + sizeof(struct smb_acl) > pntsd_size)) + goto err_out; pdacl = (struct smb_acl *)((char *)pntsd + le32_to_cpu(pntsd->dacloffset)); - end_of_acl = ((char *)pntsd) + acl_size; - if (end_of_acl <= (char *)pdacl) { - kfree(pntsd); - return 0; - } + acl_size = pntsd_size - dacl_offset; + pdacl_size = le16_to_cpu(pdacl->size); - if (end_of_acl < (char *)pdacl + le16_to_cpu(pdacl->size) || - le16_to_cpu(pdacl->size) < sizeof(struct smb_acl)) { - kfree(pntsd); - return 0; - } + if (pdacl_size > acl_size || pdacl_size < sizeof(struct smb_acl)) + goto err_out; if (!pdacl->num_aces) { - if (!(le16_to_cpu(pdacl->size) - sizeof(struct smb_acl)) && + if (!(pdacl_size - sizeof(struct smb_acl)) && *pdaccess & ~(FILE_READ_CONTROL_LE | FILE_WRITE_DAC_LE)) { rc = -EACCES; goto err_out; } - kfree(pntsd); - return 0; + goto err_out; } if (*pdaccess & FILE_MAXIMAL_ACCESS_LE) { @@ -1200,11 +1235,16 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, struct path *path, DELETE; ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl)); + aces_size = acl_size - sizeof(struct smb_acl); for (i = 0; i < le32_to_cpu(pdacl->num_aces); i++) { + if (offsetof(struct smb_ace, access_req) > aces_size) + break; + ace_size = le16_to_cpu(ace->size); + if (ace_size > aces_size) + break; + aces_size -= ace_size; granted |= le32_to_cpu(ace->access_req); ace = (struct smb_ace *)((char *)ace + le16_to_cpu(ace->size)); - if (end_of_acl < (char *)ace) - goto err_out; } if (!pdacl->num_aces) @@ -1216,7 +1256,15 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, struct path *path, id_to_sid(uid, sid_type, &sid); ace = (struct smb_ace *)((char *)pdacl + sizeof(struct smb_acl)); + aces_size = acl_size - sizeof(struct smb_acl); for (i = 0; i < le32_to_cpu(pdacl->num_aces); i++) { + if (offsetof(struct smb_ace, access_req) > aces_size) + break; + ace_size = le16_to_cpu(ace->size); + if (ace_size > aces_size) + break; + aces_size -= ace_size; + if (!compare_sids(&sid, &ace->sid) || !compare_sids(&sid_unix_NFS_mode, &ace->sid)) { found = 1; @@ -1226,8 +1274,6 @@ int smb_check_perm_dacl(struct ksmbd_conn *conn, struct path *path, others_ace = ace; ace = (struct smb_ace *)((char *)ace + le16_to_cpu(ace->size)); - if (end_of_acl < (char *)ace) - goto err_out; } if (*pdaccess & FILE_MAXIMAL_ACCESS_LE && found) { diff --git a/fs/ksmbd/smbacl.h b/fs/ksmbd/smbacl.h index 811af3309429..fcb2c83f2992 100644 --- a/fs/ksmbd/smbacl.h +++ b/fs/ksmbd/smbacl.h @@ -193,7 +193,7 @@ struct posix_acl_state { int parse_sec_desc(struct user_namespace *user_ns, struct smb_ntsd *pntsd, int acl_len, struct smb_fattr *fattr); int build_sec_desc(struct user_namespace *user_ns, struct smb_ntsd *pntsd, - struct smb_ntsd *ppntsd, int addition_info, + struct smb_ntsd *ppntsd, int ppntsd_size, int addition_info, __u32 *secdesclen, struct smb_fattr *fattr); int init_acl_state(struct posix_acl_state *state, int cnt); void free_acl_state(struct posix_acl_state *state); diff --git a/fs/ksmbd/vfs.c b/fs/ksmbd/vfs.c index c23793469645..ea139078b4b7 100644 --- a/fs/ksmbd/vfs.c +++ b/fs/ksmbd/vfs.c @@ -1539,6 +1539,11 @@ int ksmbd_vfs_get_sd_xattr(struct ksmbd_conn *conn, } *pntsd = acl.sd_buf; + if (acl.sd_size < sizeof(struct smb_ntsd)) { + pr_err("sd size is invalid\n"); + goto out_free; + } + (*pntsd)->osidoffset = cpu_to_le32(le32_to_cpu((*pntsd)->osidoffset) - NDR_NTSD_OFFSETOF); (*pntsd)->gsidoffset = cpu_to_le32(le32_to_cpu((*pntsd)->gsidoffset) - -- cgit v1.2.3