Merge tag 'v7.2-rc3-smb3-server-fixes' of git://git.samba.org/ksmbd

Pull smb server fixes from Steve French:
 "ksmbd server fixes, mostly addressing malformed SMB request
  handling and connection/session lifetime issues, including
  two information-disclosure or memory-safety bugs in the SMB2
  request/response paths.

   - validate FILE_ALLOCATION_INFORMATION before block rounding to
     prevent a client-controlled overflow from truncating a file.

   - pin connections while asynchronous oplock and lease-break
     notifications are pending.

   - initialize compound SMB2 READ alignment padding, preventing
     disclosure of uninitialized heap bytes.

   - release the allocated alternate-stream xattr name after rename.

   - size multichannel binding session-key buffers for the largest
     permitted key, avoiding a stack buffer overflow.

   - remove a disconnecting connection's channels from every session,
     including channels whose binding state has since changed.

   - serialize binding preauthentication-session lookup and update
     against its teardown.

   - check that every compound request element contains StructureSize2
     before reading it"

* tag 'v7.2-rc3-smb3-server-fixes' of git://git.samba.org/ksmbd:
  ksmbd: validate compound request size before reading StructureSize2
  ksmbd: lock the binding preauth session in smb3_preauth_hash_rsp
  ksmbd: remove stale channels from all sessions on teardown
  ksmbd: fix stack buffer overflow in multichannel session-key copy
  ksmbd: fix memory leak of xattr_stream_name in smb2_rename()
  ksmbd: zero the smb2_read alignment tail to avoid an infoleak
  ksmbd: pin conn during async oplock break notification
  ksmbd: fix integer overflow in set_file_allocation_info()

5 files changed, 56 insertions, 32 deletions

diff --git a/fs/smb/server/mgmt/user_session.c b/fs/smb/server/mgmt/user_session.c
index d6331184ebfc..f99c86284ba3 100644
--- a/fs/smb/server/mgmt/user_session.c
+++ b/fs/smb/server/mgmt/user_session.c
@@ -457,22 +457,19 @@ void ksmbd_sessions_deregister(struct ksmbd_conn *conn)
 {
 	struct ksmbd_session *sess;
 	unsigned long id;
+	struct hlist_node *tmp;
+	int bkt;
 
 	down_write(&sessions_table_lock);
-	if (conn->binding) {
-		int bkt;
-		struct hlist_node *tmp;
-
-		hash_for_each_safe(sessions_table, bkt, tmp, sess, hlist) {
-			if (!ksmbd_chann_del(conn, sess) &&
-			    xa_empty(&sess->ksmbd_chann_list)) {
-				hash_del(&sess->hlist);
-				down_write(&conn->session_lock);
-				xa_erase(&conn->sessions, sess->id);
-				up_write(&conn->session_lock);
-				if (atomic_dec_and_test(&sess->refcnt))
-					ksmbd_session_destroy(sess);
-			}
+	hash_for_each_safe(sessions_table, bkt, tmp, sess, hlist) {
+		if (!ksmbd_chann_del(conn, sess) &&
+		    xa_empty(&sess->ksmbd_chann_list)) {
+			hash_del(&sess->hlist);
+			down_write(&conn->session_lock);
+			xa_erase(&conn->sessions, sess->id);
+			up_write(&conn->session_lock);
+			if (atomic_dec_and_test(&sess->refcnt))
+				ksmbd_session_destroy(sess);
 		}
 	}
 
diff --git a/fs/smb/server/mgmt/user_session.h b/fs/smb/server/mgmt/user_session.h
index 8893a9aaede7..4637a8c8436d 100644
--- a/fs/smb/server/mgmt/user_session.h
+++ b/fs/smb/server/mgmt/user_session.h
@@ -19,7 +19,7 @@
 struct ksmbd_file_table;
 
 struct channel {
-	char			sess_key[SMB2_NTLMV2_SESSKEY_SIZE];
+	char			sess_key[CIFS_KEY_SIZE];
 	__u8			smb3signingkey[SMB3_SIGN_KEY_SIZE];
 	struct ksmbd_conn	*conn;
 };
diff --git a/fs/smb/server/oplock.c b/fs/smb/server/oplock.c
index 3c55ae5d6a11..79787099afdc 100644
--- a/fs/smb/server/oplock.c
+++ b/fs/smb/server/oplock.c
@@ -875,6 +875,7 @@ static void __smb2_oplock_break_noti(struct work_struct *wk)
 out:
 	ksmbd_free_work_struct(work);
 	ksmbd_conn_r_count_dec(conn);
+	ksmbd_conn_put(conn);
 }
 
 /**
@@ -910,7 +911,7 @@ static int smb2_oplock_break_noti(struct oplock_info *opinfo)
 	br_info->open_trunc = opinfo->open_trunc;
 
 	work->request_buf = (char *)br_info;
-	work->conn = conn;
+	work->conn = ksmbd_conn_get(conn);
 	work->sess = opinfo->sess;
 
 	ksmbd_conn_r_count_inc(conn);
@@ -985,6 +986,7 @@ static void __smb2_lease_break_noti(struct work_struct *wk)
 out:
 	ksmbd_free_work_struct(work);
 	ksmbd_conn_r_count_dec(conn);
+	ksmbd_conn_put(conn);
 }
 
 /**
@@ -1034,7 +1036,7 @@ static int smb2_lease_break_noti(struct oplock_info *opinfo, bool wait_ack,
 	memcpy(br_info->lease_key, lease->lease_key, SMB2_LEASE_KEY_SIZE);
 
 	work->request_buf = (char *)br_info;
-	work->conn = conn;
+	work->conn = ksmbd_conn_get(conn);
 	work->sess = opinfo->sess;
 
 	ksmbd_conn_r_count_inc(conn);
diff --git a/fs/smb/server/smb2misc.c b/fs/smb/server/smb2misc.c
index c0c4edd092c2..9f3629c86291 100644
--- a/fs/smb/server/smb2misc.c
+++ b/fs/smb/server/smb2misc.c
@@ -407,6 +407,11 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
 		return 1;
 	}
 
+	if (len < __SMB2_HEADER_STRUCTURE_SIZE + sizeof(__le16)) {
+		ksmbd_debug(SMB, "Message is too small for StructureSize2\n");
+		return 1;
+	}
+
 	if (smb2_req_struct_sizes[command] != pdu->StructureSize2) {
 		if (!(command == SMB2_OPLOCK_BREAK_HE &&
 		    (le16_to_cpu(pdu->StructureSize2) == OP_BREAK_STRUCT_SIZE_20 ||
diff --git a/fs/smb/server/smb2pdu.c b/fs/smb/server/smb2pdu.c
index b73167785e87..bec692bca1ca 100644
--- a/fs/smb/server/smb2pdu.c
+++ b/fs/smb/server/smb2pdu.c
@@ -1689,7 +1689,7 @@ static int ntlm_authenticate(struct ksmbd_work *work,
 	struct ksmbd_conn *conn = work->conn;
 	struct ksmbd_session *sess = work->sess;
 	struct ksmbd_user *user;
-	char channel_key[SMB2_NTLMV2_SESSKEY_SIZE] = {};
+	char channel_key[CIFS_KEY_SIZE] = {};
 	char *auth_key = conn->binding ? channel_key : sess->sess_key;
 	u64 prev_id;
 	bool binding = conn->binding;
@@ -1826,7 +1826,7 @@ static int krb5_authenticate(struct ksmbd_work *work,
 	struct ksmbd_conn *conn = work->conn;
 	struct ksmbd_session *sess = work->sess;
 	char *in_blob, *out_blob;
-	char channel_key[SMB2_NTLMV2_SESSKEY_SIZE] = {};
+	char channel_key[CIFS_KEY_SIZE] = {};
 	char *auth_key = conn->binding ? channel_key : sess->sess_key;
 	u64 prev_sess_id;
 	bool binding = conn->binding;
@@ -6597,9 +6597,8 @@ static int smb2_rename(struct ksmbd_work *work,
 			pr_err("failed to store stream name in xattr: %d\n",
 			       rc);
 			rc = -EINVAL;
-			goto out;
 		}
-
+		kfree(xattr_stream_name);
 		goto out;
 	}
 
@@ -6781,6 +6780,7 @@ static int set_file_allocation_info(struct ksmbd_work *work,
 	 */
 
 	loff_t alloc_blks;
+	u64 alloc_size;
 	struct inode *inode;
 	struct kstat stat;
 	int rc;
@@ -6796,7 +6796,19 @@ static int set_file_allocation_info(struct ksmbd_work *work,
 	if (rc)
 		return rc;
 
-	alloc_blks = (le64_to_cpu(file_alloc_info->AllocationSize) + 511) >> 9;
+	/*
+	 * AllocationSize is fully client-controlled (the caller only
+	 * validates the fixed 8-byte buffer length). Reject values that
+	 * would overflow the "round up to 512-byte blocks" conversion
+	 * below instead of silently wrapping it to a tiny block count,
+	 * which would truncate the file to a size the client never
+	 * asked for.
+	 */
+	alloc_size = le64_to_cpu(file_alloc_info->AllocationSize);
+	if (alloc_size > MAX_LFS_FILESIZE - 511)
+		return -EINVAL;
+
+	alloc_blks = (alloc_size + 511) >> 9;
 	inode = file_inode(fp->filp);
 
 	if (alloc_blks > stat.blocks) {
@@ -7433,6 +7445,15 @@ int smb2_read(struct ksmbd_work *work)
 		goto out;
 	}
 
+	/*
+	 * ksmbd_vfs_read() fills only nbytes; the [nbytes, ALIGN(nbytes, 8))
+	 * tail of the un-zeroed buffer is transmitted as compound-response
+	 * alignment padding, leaking uninitialized kernel memory to the
+	 * client.  Zero just that tail.
+	 */
+	if (nbytes & 7)
+		memset(aux_payload_buf + nbytes, 0, ALIGN(nbytes, 8) - nbytes);
+
 	if ((nbytes == 0 && length != 0) || nbytes < mincount) {
 		kvfree(aux_payload_buf);
 		rsp->hdr.Status = STATUS_END_OF_FILE;
@@ -9787,22 +9808,21 @@ void smb3_preauth_hash_rsp(struct ksmbd_work *work)
 	}
 
 	if (le16_to_cpu(rsp->Command) == SMB2_SESSION_SETUP_HE && sess) {
-		__u8 *hash_value;
+		ksmbd_conn_lock(conn);
 
 		if (conn->binding) {
 			struct preauth_session *preauth_sess;
 
 			preauth_sess = ksmbd_preauth_session_lookup(conn, sess->id);
-			if (!preauth_sess)
-				return;
-			hash_value = preauth_sess->Preauth_HashValue;
-		} else {
-			hash_value = sess->Preauth_HashValue;
-			if (!hash_value)
-				return;
+			if (preauth_sess)
+				ksmbd_gen_preauth_integrity_hash(conn,
+					work->response_buf,
+					preauth_sess->Preauth_HashValue);
+		} else if (sess->Preauth_HashValue) {
+			ksmbd_gen_preauth_integrity_hash(conn, work->response_buf,
+					 sess->Preauth_HashValue);
 		}
-		ksmbd_gen_preauth_integrity_hash(conn, work->response_buf,
-						 hash_value);
+		ksmbd_conn_unlock(conn);
 	}
 }