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

Pull smb server fixes from Steve French:

 - Fix several use-after-free races in durable handle reconnect,
   supersede, and oplock handling

 - Avoid holding the inode oplock lock while waiting for a lease break
   acknowledgement. This removes delays of up to 35 seconds when cifs.ko
   closes a deferred handle in response to a lease break

 - Fix malformed security descriptor handling, including an undersized
   DACL allocation issue and an out-of-bounds ACE SID read

 - Fix memory leaks in security descriptor and DOS attribute xattr
   encoding/decoding error paths

 - Fix outstanding SMB2 credit leaks on aborted requests and correct the
   QUERY_INFO credit charge calculation

 - Fix hard-link creation without replacement being incorrectly rejected
   when the handle lacks DELETE access

 - Avoid unnecessary zeroing of large SMB2 read buffers

 - Add an oplock list lockdep annotation and update the documented
   support status for durable handles and SMB3.1.1 compression

 - Durable handle fixes to address ownership and lifetime races during
   reconnect, session teardown, oplock handling, and superseding opens,
   preventing stale session and file references from being used by
   concurrent operations

* tag 'v7.2-rc1-smb3-server-fixes' of git://git.samba.org/ksmbd:
  ksmbd: fix app-instance durable supersede session UAF
  ksmbd: snapshot previous oplock state before durable checks
  ksmbd: close superseded durable handles through refcount handoff
  ksmbd: fix use-after-free of fp->owner.name in durable handle owner check
  smb/server: do not require delete access for non-replacing links
  ksmbd: don't hold ci->m_lock while waiting for a lease break ack
  ksmbd: doc: update feature support status for durable handles and compression
  ksmbd: annotate oplock list traversals under m_lock
  ksmbd: fix outstanding credit leak on abort and error paths
  ksmbd: fix credit charge calculation for SMB2 QUERY_INFO
  ksmbd: avoid zeroing the read buffer in smb2_read()
  ksmbd: validate num_subauth when copying ACE in set_ntacl_dacl
  ksmbd: reject undersized DACLs before parsing ACEs
  ksmbd: fix n.data memory leak in ksmbd_vfs_set_dos_attrib_xattr
  ksmbd: Fix acl.sd_buf memory leak and invalid sd_size error handling
  ksmbd: fix sd_ndr.data memory leak in ksmbd_vfs_set_sd_xattr

Author: torvalds

Documentation/filesystems/smb/ksmbd.rst

@@ -97,7 +97,7 @@ ACLs                           Partially Supported. only DACLs available, SACLs
                                to allow future support for running as a domain
                                member.
 Kerberos                       Supported.
-Durable handle v1,v2           Planned for future.
+Durable handle v1,v2           Supported.
 Persistent handle              Planned for future.
 SMB2 notify                    Planned for future.
 Sparse file support            Supported.
@@ -111,7 +111,7 @@ DCE/RPC support                Partially Supported. a few calls(NetShareEnumAll,
                                for Witness protocol e.g.)
 ksmbd/nfsd interoperability    Planned for future. The features that ksmbd
                                support are Leases, Notify, ACLs and Share modes.
-SMB3.1.1 Compression           Planned for future.
+SMB3.1.1 Compression           Supported.
 SMB3.1.1 over QUIC             Planned for future.
 Signing/Encryption over RDMA   Planned for future.
 SMB3.1.1 GMAC signing support  Planned for future.

fs/smb/server/ksmbd_work.h

@@ -67,6 +67,13 @@ struct ksmbd_work {
 	/* Number of granted credits */
 	unsigned int			credits_granted;
 
+	/*
+	 * Credit charge added to conn->outstanding_credits at receive time
+	 * for the SMB2 PDU currently being processed, pending release.  Zero
+	 * once the charge has been returned (on the response or error path).
+	 */
+	unsigned short			credit_charge;
+
 	/* response smb header size */
 	unsigned int                    response_sz;
 

fs/smb/server/oplock.c

@@ -278,10 +278,24 @@ struct oplock_info *opinfo_get(struct ksmbd_file *fp)
 	return opinfo;
 }
 
-static struct oplock_info *opinfo_get_list(struct ksmbd_inode *ci)
+struct oplock_snapshot {
+	bool durable_open;
+	bool durable_detached;
+	unsigned long long fid;
+};
+
+static struct oplock_info *opinfo_get_list(struct ksmbd_inode *ci,
+					   struct ksmbd_file *skip_fp,
+					   struct oplock_snapshot *snapshot)
 {
 	struct oplock_info *opinfo;
 
+	if (snapshot) {
+		snapshot->durable_open = false;
+		snapshot->durable_detached = false;
+		snapshot->fid = KSMBD_NO_FID;
+	}
+
 	down_read(&ci->m_lock);
 	opinfo = list_first_entry_or_null(&ci->m_op_list, struct oplock_info,
 					  op_entry);
@@ -295,6 +309,16 @@ static struct oplock_info *opinfo_get_list(struct ksmbd_inode *ci)
 				opinfo = NULL;
 			}
 		}
+
+		if (opinfo && snapshot && opinfo->o_fp &&
+		    opinfo->o_fp != skip_fp &&
+		    READ_ONCE(opinfo->o_fp->is_durable)) {
+			snapshot->durable_open = true;
+			snapshot->durable_detached =
+				!READ_ONCE(opinfo->o_fp->conn) ||
+				!READ_ONCE(opinfo->o_fp->tcon);
+			snapshot->fid = opinfo->fid;
+		}
 	}
 	up_read(&ci->m_lock);
 
@@ -314,7 +338,7 @@ void opinfo_put(struct oplock_info *opinfo)
 
 static bool ksmbd_inode_has_lease(struct ksmbd_inode *ci)
 {
-	struct oplock_info *opinfo = opinfo_get_list(ci);
+	struct oplock_info *opinfo = opinfo_get_list(ci, NULL, NULL);
 	bool is_lease;
 
 	if (!opinfo)
@@ -1180,6 +1204,36 @@ static int oplock_break(struct oplock_info *brk_opinfo, int req_op_level,
 	return err;
 }
 
+struct oplock_break_entry {
+	struct list_head	list;
+	struct oplock_info	*opinfo;
+};
+
+static int oplock_break_add(struct list_head *head, struct oplock_info *opinfo)
+{
+	struct oplock_break_entry *ent;
+
+	ent = kmalloc_obj(struct oplock_break_entry, KSMBD_DEFAULT_GFP);
+	if (!ent)
+		return -ENOMEM;
+
+	ent->opinfo = opinfo;
+	list_add_tail(&ent->list, head);
+	return 0;
+}
+
+static void oplock_break_drain_none(struct list_head *head)
+{
+	struct oplock_break_entry *ent, *tmp;
+
+	list_for_each_entry_safe(ent, tmp, head, list) {
+		oplock_break(ent->opinfo, SMB2_OPLOCK_LEVEL_NONE, NULL, false);
+		list_del(&ent->list);
+		opinfo_put(ent->opinfo);
+		kfree(ent);
+	}
+}
+
 void destroy_lease_table(struct ksmbd_conn *conn)
 {
 	struct lease_table *lb, *lbtmp;
@@ -1289,6 +1343,7 @@ void smb_send_parent_lease_break_noti(struct ksmbd_file *fp,
 {
 	struct oplock_info *opinfo;
 	struct ksmbd_inode *p_ci = NULL;
+	LIST_HEAD(brk_list);
 
 	if (lctx->version != 2)
 		return;
@@ -1314,19 +1369,22 @@ void smb_send_parent_lease_break_noti(struct ksmbd_file *fp,
 				continue;
 			}
 
-			oplock_break(opinfo, SMB2_OPLOCK_LEVEL_NONE, NULL, false);
-			opinfo_put(opinfo);
+			if (oplock_break_add(&brk_list, opinfo))
+				opinfo_put(opinfo);
 		}
 	}
 	up_read(&p_ci->m_lock);
 
+	oplock_break_drain_none(&brk_list);
+
 	ksmbd_inode_put(p_ci);
 }
 
 void smb_lazy_parent_lease_break_close(struct ksmbd_file *fp)
 {
 	struct oplock_info *opinfo;
 	struct ksmbd_inode *p_ci = NULL;
+	LIST_HEAD(brk_list);
 
 	rcu_read_lock();
 	opinfo = rcu_dereference(fp->f_opinfo);
@@ -1355,12 +1413,14 @@ void smb_lazy_parent_lease_break_close(struct ksmbd_file *fp)
 				continue;
 			}
 
-			oplock_break(opinfo, SMB2_OPLOCK_LEVEL_NONE, NULL, false);
-			opinfo_put(opinfo);
+			if (oplock_break_add(&brk_list, opinfo))
+				opinfo_put(opinfo);
 		}
 	}
 	up_read(&p_ci->m_lock);
 
+	oplock_break_drain_none(&brk_list);
+
 	ksmbd_inode_put(p_ci);
 }
 
@@ -1385,6 +1445,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
 	struct oplock_info *opinfo = NULL, *prev_opinfo = NULL;
 	struct ksmbd_inode *ci = fp->f_ci;
 	struct lease_table *new_lb = NULL;
+	struct oplock_snapshot prev_op_snapshot;
 	bool prev_op_has_lease;
 	bool prev_durable_open = false;
 	bool prev_durable_detached = false;
@@ -1452,7 +1513,7 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
 			goto out;
 		}
 	}
-	prev_opinfo = opinfo_get_list(ci);
+	prev_opinfo = opinfo_get_list(ci, fp, &prev_op_snapshot);
 	if (!prev_opinfo ||
 	    (prev_opinfo->level == SMB2_OPLOCK_LEVEL_NONE && lctx)) {
 		opinfo_put(prev_opinfo);
@@ -1474,13 +1535,9 @@ int smb_grant_oplock(struct ksmbd_work *work, int req_op_level, u64 pid,
 		goto op_break_not_needed;
 	}
 
-	if (prev_opinfo->o_fp && prev_opinfo->o_fp != fp &&
-	    prev_opinfo->o_fp->is_durable) {
-		prev_durable_open = true;
-		prev_durable_detached = !prev_opinfo->o_fp->conn ||
-					!prev_opinfo->o_fp->tcon;
-		prev_fid = prev_opinfo->fid;
-	}
+	prev_durable_open = prev_op_snapshot.durable_open;
+	prev_durable_detached = prev_op_snapshot.durable_detached;
+	prev_fid = prev_op_snapshot.fid;
 
 	err = oplock_break(prev_opinfo, break_level, work,
 			   share_ret < 0 && prev_opinfo->is_lease);
@@ -1571,7 +1628,7 @@ static bool smb_break_all_write_oplock(struct ksmbd_work *work,
 	struct oplock_info *brk_opinfo;
 	bool sent_break = false;
 
-	brk_opinfo = opinfo_get_list(fp->f_ci);
+	brk_opinfo = opinfo_get_list(fp->f_ci, NULL, NULL);
 	if (!brk_opinfo)
 		return false;
 	if (brk_opinfo->level != SMB2_OPLOCK_LEVEL_BATCH &&
@@ -1602,9 +1659,11 @@ static void __smb_break_all_levII_oplock(struct ksmbd_work *work,
 					 bool send_interim, bool send_oplock_break)
 {
 	struct oplock_info *op, *brk_op;
+	struct oplock_break_entry *ent, *tmp;
 	struct ksmbd_inode *ci;
 	struct ksmbd_conn *conn = work->conn;
 	bool sent_interim = false;
+	LIST_HEAD(brk_list);
 
 	if (!test_share_config_flag(work->tcon->share_conf,
 				    KSMBD_SHARE_FLAG_OPLOCKS))
@@ -1646,6 +1705,22 @@ static void __smb_break_all_levII_oplock(struct ksmbd_work *work,
 			    SMB2_LEASE_KEY_SIZE))
 			goto next;
 		brk_op->open_trunc = is_trunc;
+
+		/*
+		 * Defer the break until ci->m_lock is released: oplock_break()
+		 * may block waiting for the lease break acknowledgment, and the
+		 * close that wakes that wait needs ci->m_lock for write.
+		 */
+		if (!oplock_break_add(&brk_list, brk_op))
+			continue;
+next:
+		opinfo_put(brk_op);
+	}
+	up_read(&ci->m_lock);
+
+	list_for_each_entry_safe(ent, tmp, &brk_list, list) {
+		brk_op = ent->opinfo;
+
 		if (!brk_op->is_lease && !send_oplock_break) {
 			brk_op->level = SMB2_OPLOCK_LEVEL_NONE;
 			brk_op->op_state = OPLOCK_STATE_NONE;
@@ -1657,10 +1732,10 @@ static void __smb_break_all_levII_oplock(struct ksmbd_work *work,
 				     false);
 		}
 		sent_interim = true;
-next:
+		list_del(&ent->list);
 		opinfo_put(brk_op);
+		kfree(ent);
 	}
-	up_read(&ci->m_lock);
 
 	if (op)
 		opinfo_put(op);

fs/smb/server/server.c

@@ -242,6 +242,20 @@ static void __handle_ksmbd_work(struct ksmbd_work *work,
 	} while (is_chained == true);
 
 send:
+	/*
+	 * Release any credit charge still outstanding for this request.  On
+	 * the normal path smb2_set_rsp_credits() already returned it, but the
+	 * abort, error and send-no-response paths skip that call, so the
+	 * charge would otherwise leak and eventually exhaust the connection's
+	 * outstanding credit window.
+	 */
+	if (work->credit_charge) {
+		spin_lock(&conn->credits_lock);
+		conn->outstanding_credits -= work->credit_charge;
+		work->credit_charge = 0;
+		spin_unlock(&conn->credits_lock);
+	}
+
 	if (work->tcon)
 		ksmbd_tree_connect_put(work->tcon);
 	smb3_preauth_hash_rsp(work);

fs/smb/server/smb2misc.c

@@ -261,8 +261,12 @@ static int smb2_calc_size(void *buf, unsigned int *len)
 
 static inline int smb2_query_info_req_len(struct smb2_query_info_req *h)
 {
-	return le32_to_cpu(h->InputBufferLength) +
-		le32_to_cpu(h->OutputBufferLength);
+	return le32_to_cpu(h->InputBufferLength);
+}
+
+static inline int smb2_query_info_resp_len(struct smb2_query_info_req *h)
+{
+	return le32_to_cpu(h->OutputBufferLength);
 }
 
 static inline int smb2_set_info_req_len(struct smb2_set_info_req *h)
@@ -297,9 +301,10 @@ static inline int smb2_ioctl_resp_len(struct smb2_ioctl_req *h)
 		le32_to_cpu(h->MaxOutputResponse);
 }
 
-static int smb2_validate_credit_charge(struct ksmbd_conn *conn,
+static int smb2_validate_credit_charge(struct ksmbd_work *work,
 				       struct smb2_hdr *hdr)
 {
+	struct ksmbd_conn *conn = work->conn;
 	unsigned int req_len = 0, expect_resp_len = 0, calc_credit_num, max_len;
 	unsigned short credit_charge = le16_to_cpu(hdr->CreditCharge);
 	void *__hdr = hdr;
@@ -308,6 +313,7 @@ static int smb2_validate_credit_charge(struct ksmbd_conn *conn,
 	switch (hdr->Command) {
 	case SMB2_QUERY_INFO:
 		req_len = smb2_query_info_req_len(__hdr);
+		expect_resp_len = smb2_query_info_resp_len(__hdr);
 		break;
 	case SMB2_SET_INFO:
 		req_len = smb2_set_info_req_len(__hdr);
@@ -356,8 +362,10 @@ static int smb2_validate_credit_charge(struct ksmbd_conn *conn,
 		ksmbd_debug(SMB, "Limits exceeding the maximum allowable outstanding requests, given : %u, pending : %u\n",
 			    credit_charge, conn->outstanding_credits);
 		ret = 1;
-	} else
+	} else {
 		conn->outstanding_credits += credit_charge;
+		work->credit_charge = credit_charge;
+	}
 
 	spin_unlock(&conn->credits_lock);
 
@@ -460,7 +468,7 @@ int ksmbd_smb2_check_message(struct ksmbd_work *work)
 
 validate_credit:
 	if ((work->conn->vals->req_capabilities & SMB2_GLOBAL_CAP_LARGE_MTU) &&
-	    smb2_validate_credit_charge(work->conn, hdr))
+	    smb2_validate_credit_charge(work, hdr))
 		return 1;
 
 	return 0;

fs/smb/server/smb2pdu.c

@@ -363,6 +363,7 @@ int smb2_set_rsp_credits(struct ksmbd_work *work)
 
 	conn->total_credits -= credit_charge;
 	conn->outstanding_credits -= credit_charge;
+	work->credit_charge = 0;
 	credits_requested = max_t(unsigned short,
 				  le16_to_cpu(req_hdr->CreditRequest), 1);
 
@@ -6920,16 +6921,18 @@ static int smb2_set_info_file(struct ksmbd_work *work, struct ksmbd_file *fp,
 	}
 	case FILE_LINK_INFORMATION:
 	{
-		if (!(fp->daccess & FILE_DELETE_LE)) {
-			pr_err("no right to delete : 0x%x\n", fp->daccess);
-			return -EACCES;
-		}
+		struct smb2_file_link_info *file_info;
 
 		if (buf_len < sizeof(struct smb2_file_link_info))
 			return -EMSGSIZE;
 
-		return smb2_create_link(work, work->tcon->share_conf,
-					(struct smb2_file_link_info *)buffer,
+		file_info = (struct smb2_file_link_info *)buffer;
+		if (file_info->ReplaceIfExists && !(fp->daccess & FILE_DELETE_LE)) {
+			pr_err("no right to delete : 0x%x\n", fp->daccess);
+			return -EACCES;
+		}
+
+		return smb2_create_link(work, work->tcon->share_conf, file_info,
 					buf_len, fp->filp,
 					work->conn->local_nls);
 	}
@@ -7324,7 +7327,7 @@ int smb2_read(struct ksmbd_work *work)
 	ksmbd_debug(SMB, "filename %pD, offset %lld, len %zu\n",
 		    fp->filp, offset, length);
 
-	aux_payload_buf = kvzalloc(ALIGN(length, 8), KSMBD_DEFAULT_GFP);
+	aux_payload_buf = kvmalloc(ALIGN(length, 8), KSMBD_DEFAULT_GFP);
 	if (!aux_payload_buf) {
 		err = -ENOMEM;
 		goto out;