Skip to content

Commit 5005bcb

Browse files
namjaejeonsmfrench
authored andcommitted
ksmbd: validate session id and tree id in the compound request
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: [email protected] Reported-by: [email protected] # ZDI-CAN-21165 Signed-off-by: Namjae Jeon <[email protected]> Signed-off-by: Steve French <[email protected]>
1 parent 5fe7f7b commit 5005bcb

File tree

2 files changed

+59
-18
lines changed

2 files changed

+59
-18
lines changed

fs/smb/server/server.c

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -185,24 +185,31 @@ static void __handle_ksmbd_work(struct ksmbd_work *work,
185185
goto send;
186186
}
187187

188-
if (conn->ops->check_user_session) {
189-
rc = conn->ops->check_user_session(work);
190-
if (rc < 0) {
191-
command = conn->ops->get_cmd_val(work);
192-
conn->ops->set_rsp_status(work,
193-
STATUS_USER_SESSION_DELETED);
194-
goto send;
195-
} else if (rc > 0) {
196-
rc = conn->ops->get_ksmbd_tcon(work);
188+
do {
189+
if (conn->ops->check_user_session) {
190+
rc = conn->ops->check_user_session(work);
197191
if (rc < 0) {
198-
conn->ops->set_rsp_status(work,
199-
STATUS_NETWORK_NAME_DELETED);
192+
if (rc == -EINVAL)
193+
conn->ops->set_rsp_status(work,
194+
STATUS_INVALID_PARAMETER);
195+
else
196+
conn->ops->set_rsp_status(work,
197+
STATUS_USER_SESSION_DELETED);
200198
goto send;
199+
} else if (rc > 0) {
200+
rc = conn->ops->get_ksmbd_tcon(work);
201+
if (rc < 0) {
202+
if (rc == -EINVAL)
203+
conn->ops->set_rsp_status(work,
204+
STATUS_INVALID_PARAMETER);
205+
else
206+
conn->ops->set_rsp_status(work,
207+
STATUS_NETWORK_NAME_DELETED);
208+
goto send;
209+
}
201210
}
202211
}
203-
}
204212

205-
do {
206213
rc = __process_request(work, conn, &command);
207214
if (rc == SERVER_HANDLER_ABORT)
208215
break;

fs/smb/server/smb2pdu.c

Lines changed: 39 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,6 @@ int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
9191
unsigned int cmd = le16_to_cpu(req_hdr->Command);
9292
int tree_id;
9393

94-
work->tcon = NULL;
9594
if (cmd == SMB2_TREE_CONNECT_HE ||
9695
cmd == SMB2_CANCEL_HE ||
9796
cmd == SMB2_LOGOFF_HE) {
@@ -105,10 +104,28 @@ int smb2_get_ksmbd_tcon(struct ksmbd_work *work)
105104
}
106105

107106
tree_id = le32_to_cpu(req_hdr->Id.SyncId.TreeId);
107+
108+
/*
109+
* If request is not the first in Compound request,
110+
* Just validate tree id in header with work->tcon->id.
111+
*/
112+
if (work->next_smb2_rcv_hdr_off) {
113+
if (!work->tcon) {
114+
pr_err("The first operation in the compound does not have tcon\n");
115+
return -EINVAL;
116+
}
117+
if (work->tcon->id != tree_id) {
118+
pr_err("tree id(%u) is different with id(%u) in first operation\n",
119+
tree_id, work->tcon->id);
120+
return -EINVAL;
121+
}
122+
return 1;
123+
}
124+
108125
work->tcon = ksmbd_tree_conn_lookup(work->sess, tree_id);
109126
if (!work->tcon) {
110127
pr_err("Invalid tid %d\n", tree_id);
111-
return -EINVAL;
128+
return -ENOENT;
112129
}
113130

114131
return 1;
@@ -547,7 +564,6 @@ int smb2_check_user_session(struct ksmbd_work *work)
547564
unsigned int cmd = conn->ops->get_cmd_val(work);
548565
unsigned long long sess_id;
549566

550-
work->sess = NULL;
551567
/*
552568
* SMB2_ECHO, SMB2_NEGOTIATE, SMB2_SESSION_SETUP command do not
553569
* require a session id, so no need to validate user session's for
@@ -558,15 +574,33 @@ int smb2_check_user_session(struct ksmbd_work *work)
558574
return 0;
559575

560576
if (!ksmbd_conn_good(conn))
561-
return -EINVAL;
577+
return -EIO;
562578

563579
sess_id = le64_to_cpu(req_hdr->SessionId);
580+
581+
/*
582+
* If request is not the first in Compound request,
583+
* Just validate session id in header with work->sess->id.
584+
*/
585+
if (work->next_smb2_rcv_hdr_off) {
586+
if (!work->sess) {
587+
pr_err("The first operation in the compound does not have sess\n");
588+
return -EINVAL;
589+
}
590+
if (work->sess->id != sess_id) {
591+
pr_err("session id(%llu) is different with the first operation(%lld)\n",
592+
sess_id, work->sess->id);
593+
return -EINVAL;
594+
}
595+
return 1;
596+
}
597+
564598
/* Check for validity of user session */
565599
work->sess = ksmbd_session_lookup_all(conn, sess_id);
566600
if (work->sess)
567601
return 1;
568602
ksmbd_debug(SMB, "Invalid user session, Uid %llu\n", sess_id);
569-
return -EINVAL;
603+
return -ENOENT;
570604
}
571605

572606
static void destroy_previous_session(struct ksmbd_conn *conn,

0 commit comments

Comments
 (0)