This is a note to let you know that I've just added the patch titled
ksmbd: validate session id and tree id in the compound request
to the 5.15-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
ksmbd-validate-session-id-and-tree-id-in-the-compound-request.patch
and it can be found in the queue-5.15 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@xxxxxxxxxxxxxxx> know about it.
>From stable-owner@xxxxxxxxxxxxxxx Thu Jul 20 15:25:40 2023
From: Namjae Jeon <linkinjeon@xxxxxxxxxx>
Date: Thu, 20 Jul 2023 22:23:31 +0900
Subject: ksmbd: validate session id and tree id in the compound request
To: stable@xxxxxxxxxxxxxxx
Cc: gregkh@xxxxxxxxxxxxxxxxxxx, stfrench@xxxxxxxxxxxxx, smfrench@xxxxxxxxx, Namjae Jeon <linkinjeon@xxxxxxxxxx>, zdi-disclosures@xxxxxxxxxxxxxx
Message-ID: <20230720132336.7614-5-linkinjeon@xxxxxxxxxx>
From: Namjae Jeon <linkinjeon@xxxxxxxxxx>
commit 5005bcb4219156f1bf7587b185080ec1da08518e upstream.
This patch validate session id and tree id in compound request.
If first operation in the compound is SMB2 ECHO request, ksmbd bypass
session and tree validation. So work->sess and work->tcon could be NULL.
If secound request in the compound access work->sess or tcon, It cause
NULL pointer dereferecing error.
Cc: stable@xxxxxxxxxxxxxxx
Reported-by: zdi-disclosures@xxxxxxxxxxxxxx # ZDI-CAN-21165
Signed-off-by: Namjae Jeon <linkinjeon@xxxxxxxxxx>
Signed-off-by: Steve French <stfrench@xxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
---
fs/ksmbd/server.c | 33 ++++++++++++++++++++-------------
fs/ksmbd/smb2pdu.c | 44 +++++++++++++++++++++++++++++++++++++++-----
2 files changed, 59 insertions(+), 18 deletions(-)
--- a/fs/ksmbd/server.c
+++ b/fs/ksmbd/server.c
@@ -184,24 +184,31 @@ static void __handle_ksmbd_work(struct k
goto send;
}
- if (conn->ops->check_user_session) {
- rc = conn->ops->check_user_session(work);
- if (rc < 0) {
- command = conn->ops->get_cmd_val(work);
- conn->ops->set_rsp_status(work,
- STATUS_USER_SESSION_DELETED);
- goto send;
- } else if (rc > 0) {
- rc = conn->ops->get_ksmbd_tcon(work);
+ do {
+ if (conn->ops->check_user_session) {
+ rc = conn->ops->check_user_session(work);
if (rc < 0) {
- conn->ops->set_rsp_status(work,
- STATUS_NETWORK_NAME_DELETED);
+ if (rc == -EINVAL)
+ conn->ops->set_rsp_status(work,
+ STATUS_INVALID_PARAMETER);
+ else
+ conn->ops->set_rsp_status(work,
+ STATUS_USER_SESSION_DELETED);
goto send;
+ } else if (rc > 0) {
+ rc = conn->ops->get_ksmbd_tcon(work);
+ if (rc < 0) {
+ if (rc == -EINVAL)
+ conn->ops->set_rsp_status(work,
+ STATUS_INVALID_PARAMETER);
+ else
+ conn->ops->set_rsp_status(work,
+ STATUS_NETWORK_NAME_DELETED);
+ goto send;
+ }
}
}
- }
- do {
rc = __process_request(work, conn, &command);
if (rc == SERVER_HANDLER_ABORT)
break;
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -97,7 +97,6 @@ int smb2_get_ksmbd_tcon(struct ksmbd_wor
struct smb2_hdr *req_hdr = work->request_buf;
int tree_id;
- work->tcon = NULL;
if (work->conn->ops->get_cmd_val(work) == SMB2_TREE_CONNECT_HE ||
work->conn->ops->get_cmd_val(work) == SMB2_CANCEL_HE ||
work->conn->ops->get_cmd_val(work) == SMB2_LOGOFF_HE) {
@@ -111,10 +110,28 @@ int smb2_get_ksmbd_tcon(struct ksmbd_wor
}
tree_id = le32_to_cpu(req_hdr->Id.SyncId.TreeId);
+
+ /*
+ * If request is not the first in Compound request,
+ * Just validate tree id in header with work->tcon->id.
+ */
+ if (work->next_smb2_rcv_hdr_off) {
+ if (!work->tcon) {
+ pr_err("The first operation in the compound does not have tcon\n");
+ return -EINVAL;
+ }
+ if (work->tcon->id != tree_id) {
+ pr_err("tree id(%u) is different with id(%u) in first operation\n",
+ tree_id, work->tcon->id);
+ return -EINVAL;
+ }
+ return 1;
+ }
+
work->tcon = ksmbd_tree_conn_lookup(work->sess, tree_id);
if (!work->tcon) {
pr_err("Invalid tid %d\n", tree_id);
- return -EINVAL;
+ return -ENOENT;
}
return 1;
@@ -569,7 +586,6 @@ int smb2_check_user_session(struct ksmbd
unsigned int cmd = conn->ops->get_cmd_val(work);
unsigned long long sess_id;
- work->sess = NULL;
/*
* SMB2_ECHO, SMB2_NEGOTIATE, SMB2_SESSION_SETUP command do not
* require a session id, so no need to validate user session's for
@@ -580,15 +596,33 @@ int smb2_check_user_session(struct ksmbd
return 0;
if (!ksmbd_conn_good(work))
- return -EINVAL;
+ return -EIO;
sess_id = le64_to_cpu(req_hdr->SessionId);
+
+ /*
+ * If request is not the first in Compound request,
+ * Just validate session id in header with work->sess->id.
+ */
+ if (work->next_smb2_rcv_hdr_off) {
+ if (!work->sess) {
+ pr_err("The first operation in the compound does not have sess\n");
+ return -EINVAL;
+ }
+ if (work->sess->id != sess_id) {
+ pr_err("session id(%llu) is different with the first operation(%lld)\n",
+ sess_id, work->sess->id);
+ return -EINVAL;
+ }
+ return 1;
+ }
+
/* Check for validity of user session */
work->sess = ksmbd_session_lookup_all(conn, sess_id);
if (work->sess)
return 1;
ksmbd_debug(SMB, "Invalid user session, Uid %llu\n", sess_id);
- return -EINVAL;
+ return -ENOENT;
}
static void destroy_previous_session(struct ksmbd_conn *conn,
Patches currently in stable-queue which might be from stable-owner@xxxxxxxxxxxxxxx are
queue-5.15/ksmbd-validate-session-id-and-tree-id-in-the-compound-request.patch
queue-5.15/ksmbd-fix-out-of-bound-read-in-smb2_write.patch
queue-5.15/ksmbd-use-ksmbd_req_buf_next-in-ksmbd_smb2_check_message.patch
queue-5.15/ksmbd-validate-command-payload-size.patch
