From: Namjae Jeon <linkinjeon@kernel.org>
To: stable@vger.kernel.org
Cc: gregkh@linuxfoundation.org, stfrench@microsoft.com,
smfrench@gmail.com, Namjae Jeon <linkinjeon@kernel.org>,
zdi-disclosures@trendmicro.com
Subject: [5.15.y PATCH 4/4] ksmbd: validate session id and tree id in the compound request
Date: Thu, 20 Jul 2023 22:23:31 +0900 [thread overview]
Message-ID: <20230720132336.7614-5-linkinjeon@kernel.org> (raw)
In-Reply-To: <20230720132336.7614-1-linkinjeon@kernel.org>
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@vger.kernel.org
Reported-by: zdi-disclosures@trendmicro.com # ZDI-CAN-21165
Signed-off-by: Namjae Jeon <linkinjeon@kernel.org>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
fs/ksmbd/server.c | 33 ++++++++++++++++++++-------------
fs/ksmbd/smb2pdu.c | 44 +++++++++++++++++++++++++++++++++++++++-----
2 files changed, 59 insertions(+), 18 deletions(-)
diff --git a/fs/ksmbd/server.c b/fs/ksmbd/server.c
index 1c5e7e023058..eb45d56b3577 100644
--- a/fs/ksmbd/server.c
+++ b/fs/ksmbd/server.c
@@ -184,24 +184,31 @@ static void __handle_ksmbd_work(struct ksmbd_work *work,
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;
diff --git a/fs/ksmbd/smb2pdu.c b/fs/ksmbd/smb2pdu.c
index 266430a2a0e0..9f9d07caa57e 100644
--- a/fs/ksmbd/smb2pdu.c
+++ b/fs/ksmbd/smb2pdu.c
@@ -97,7 +97,6 @@ int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
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_work *work)
}
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_work *work)
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_work *work)
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,
--
2.25.1
next prev parent reply other threads:[~2023-07-20 13:25 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-07-20 13:23 [5.15.y PATCH 0/4] ksmbd: ZDI Vulnerability patches for 5.15.y Namjae Jeon
2023-07-20 13:23 ` [5.15.y PATCH 1/4] ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message() Namjae Jeon
2023-07-20 13:23 ` [5.15.y PATCH 2/4] ksmbd: validate command payload size Namjae Jeon
2023-07-20 13:23 ` [5.15.y PATCH 3/4] ksmbd: fix out-of-bound read in smb2_write Namjae Jeon
2023-07-20 13:23 ` Namjae Jeon [this message]
2023-07-20 13:23 ` [5.15.y PATCH 0/4] ksmbd: ZDI Vulnerability patches for 5.15.y Namjae Jeon
2023-07-20 13:43 ` Namjae Jeon
2023-07-20 13:23 ` [5.15.y PATCH 1/4] ksmbd: use ksmbd_req_buf_next() in ksmbd_smb2_check_message() Namjae Jeon
2023-07-20 13:44 ` Namjae Jeon
2023-07-20 17:54 ` [5.15.y PATCH 0/4] ksmbd: ZDI Vulnerability patches for 5.15.y Greg KH
2023-07-20 23:22 ` Namjae Jeon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20230720132336.7614-5-linkinjeon@kernel.org \
--to=linkinjeon@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=smfrench@gmail.com \
--cc=stable@vger.kernel.org \
--cc=stfrench@microsoft.com \
--cc=zdi-disclosures@trendmicro.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox