[PATCH] ksmbd: fix out-of-bounds read in expired-session LOCK check

[PATCH] ksmbd: fix out-of-bounds read in expired-session LOCK check

[Ian Bridges]

smb2_session_expired_cmd_allowed() runs from smb2_check_user_session()
before ksmbd_smb2_check_message() has validated the request. For a LOCK
on an expired Kerberos session it scans the lock elements, using the
unvalidated NextCommand field as the command length. A malformed request
makes it read past the request buffer.

  BUG: KASAN: slab-out-of-bounds in smb2_session_expired_cmd_allowed+0x260/0x2a0 [ksmbd]
  Call Trace:
   smb2_session_expired_cmd_allowed+0x260/0x2a0 [ksmbd]
   smb2_check_user_session+0x4ff/0x680 [ksmbd]
   handle_ksmbd_work+0x465/0x1cb0 [ksmbd]

Decide whether a command may run on an expired session from the command
alone. Move the unlock-only rule for LOCK into smb2_lock(), which runs
after the request has been validated. An expired session can still
release byte-range locks but cannot take new ones, as before.

A LOCK on an expired session now goes through the normal request checks
before smb2_lock() sees it, as CLOSE and LOGOFF already do. A malformed
one is rejected by those checks instead of returning
STATUS_NETWORK_SESSION_EXPIRED.

Fixes: a47634cd729b ("ksmbd: expire SMB sessions when Kerberos tickets expire")
Cc: stable@vger.kernel.org
Signed-off-by: Ian Bridges <icb@fastmail.org>
---

Notes:
    The bug needs an authenticated client whose Kerberos session has
    expired. The client then sends a malformed LOCK request.

    A reproducer is available on request.

    This fix was developed with LLM assistance.

    Tested under KASAN on a kernel whose fs/smb/server matches
    ksmbd-for-next ffde0bdfc69a. No Kerberos KDC was available, so
    session expiry was forced with a temporary hook in
    smb2_session_kerberos_expired() that is not part of this patch.
    Expiry of a real Kerberos ticket was not tested. With the hook, the
    malformed LOCK reads out of bounds before the patch and not after
    it. On an expired session an unlock still succeeds and releases the
    lock, and a new lock still returns STATUS_NETWORK_SESSION_EXPIRED,
    also inside a compound. READ is still refused, and CLOSE and LOGOFF
    still succeed. smbtorture gives the same results as on the base.

 fs/smb/server/smb2pdu.c | 57 +++++++++++++++++------------------------
 1 file changed, 23 insertions(+), 34 deletions(-)

diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index 92843d2944e4..ec87118e50b1 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -1014,39 +1014,15 @@ int smb2_allocate_rsp_buf(struct ksmbd_work *work)
 	return 0;
 }
 
-static bool smb2_session_expired_cmd_allowed(struct ksmbd_work *work,
-					      unsigned int cmd)
+/*
+ * MS-SMB2 3.3.5.2.9 lets LOGOFF, CLOSE and LOCK proceed on an expired
+ * session. CANCEL is allowed too. For LOCK, smb2_lock() allows only
+ * unlocks, after the request has been validated.
+ */
+static bool smb2_session_expired_cmd_allowed(unsigned int cmd)
 {
-	struct smb2_lock_req *req;
-	unsigned int len, lock_count, i;
-
-	if (cmd == SMB2_CANCEL_HE || cmd == SMB2_CLOSE_HE ||
-	    cmd == SMB2_LOGOFF_HE)
-		return true;
-	if (cmd != SMB2_LOCK_HE)
-		return false;
-
-	req = ksmbd_req_buf_next(work);
-	if (req->hdr.NextCommand)
-		len = le32_to_cpu(req->hdr.NextCommand);
-	else {
-		len = get_rfc1002_len(work->request_buf);
-		if (len < work->next_smb2_rcv_hdr_off)
-			return false;
-		len -= work->next_smb2_rcv_hdr_off;
-	}
-
-	lock_count = le16_to_cpu(req->LockCount);
-	if (!lock_count || len < offsetof(struct smb2_lock_req, locks) ||
-	    lock_count > (len - offsetof(struct smb2_lock_req, locks)) /
-			 sizeof(struct smb2_lock_element))
-		return false;
-
-	for (i = 0; i < lock_count; i++) {
-		if (le32_to_cpu(req->locks[i].Flags) != SMB2_LOCKFLAG_UNLOCK)
-			return false;
-	}
-	return true;
+	return cmd == SMB2_CANCEL_HE || cmd == SMB2_CLOSE_HE ||
+	       cmd == SMB2_LOGOFF_HE || cmd == SMB2_LOCK_HE;
 }
 
 static bool smb2_session_kerberos_expired(struct ksmbd_session *sess)
@@ -1124,7 +1100,7 @@ int smb2_check_user_session(struct ksmbd_work *work)
 			pr_err("compound request on a non-valid session (state %d)\n",
 					work->sess->state);
 			if (smb2_session_kerberos_expired(work->sess) &&
-			    smb2_session_expired_cmd_allowed(work, cmd))
+			    smb2_session_expired_cmd_allowed(cmd))
 				return 1;
 			return smb2_session_kerberos_expired(work->sess) ?
 				-EKEYEXPIRED : -EINVAL;
@@ -1136,7 +1112,7 @@ int smb2_check_user_session(struct ksmbd_work *work)
 	work->sess = ksmbd_session_lookup_all_states(conn, sess_id);
 	if (work->sess) {
 		if (smb2_session_kerberos_expired(work->sess)) {
-			return smb2_session_expired_cmd_allowed(work, cmd) ?
+			return smb2_session_expired_cmd_allowed(cmd) ?
 				1 : -EKEYEXPIRED;
 		}
 		if (work->sess->state != SMB2_SESSION_VALID) {
@@ -10881,6 +10857,19 @@ int smb2_lock(struct ksmbd_work *work)
 	if (smb2_compound_has_failed(work, &rsp->hdr))
 		return -EACCES;
 
+	/* An expired session may release byte-range locks, not take them. */
+	if (work->sess && smb2_session_kerberos_expired(work->sess)) {
+		lock_count = le16_to_cpu(req->LockCount);
+		for (i = 0; i < lock_count; i++) {
+			if (le32_to_cpu(req->locks[i].Flags) !=
+			    SMB2_LOCKFLAG_UNLOCK) {
+				rsp->hdr.Status = STATUS_NETWORK_SESSION_EXPIRED;
+				err = -EKEYEXPIRED;
+				goto out2;
+			}
+		}
+	}
+
 	if (work->next_smb2_rcv_hdr_off &&
 	    !has_file_id(req->VolatileFileId)) {
 		ksmbd_debug(SMB, "Compound request set FID = %llu\n",

base-commit: ffde0bdfc69a0f01a081d9c1a213e63d5719e625
-- 
2.47.3