14831 NULL-pointer dereference in smb1_oplock_break_notification during lease break

Reviewed by: Albert Lee <trisk@forkgnu.org>
Reviewed by: Andrew Stormont <andyjstormont@gmail.com>
Reviewed by: Jerry Jelinek <gjelinek@gmail.com>
Reviewed by: Matt Barden <mbarden@tintri.com>
Approved by: Robert Mustacchi <rm@fingolfin.org>

4 files changed, 90 insertions, 77 deletions

diff --git a/usr/src/uts/common/fs/smbsrv/smb2_lease.c b/usr/src/uts/common/fs/smbsrv/smb2_lease.c
index a23f474cec..7b7247fad8 100644
--- a/usr/src/uts/common/fs/smbsrv/smb2_lease.c
+++ b/usr/src/uts/common/fs/smbsrv/smb2_lease.c
@@ -11,6 +11,7 @@
 
 /*
  * Copyright 2021 Tintri by DDN, Inc.  All rights reserved.
+ * Copyright 2021 RackTop Systems, Inc.
  */
 
 /*
@@ -241,13 +242,13 @@ smb2_lease_lookup(smb_server_t *sv, uint8_t *clnt_uuid, uint8_t *lease_key)
 
 /*
  * Find an smb_ofile_t in the current tree that shares the
- * specified lease and has some oplock breaking flags set.
+ * specified lease and holds the oplock for the lease.
  * If lease not found, NT_STATUS_OBJECT_NAME_NOT_FOUND.
- * If ofile not breaking NT_STATUS_UNSUCCESSFUL.
+ * If no ofile (on the lease) holds the oplock, NT_STATUS_UNSUCCESSFUL.
  * On success, ofile (held) in sr->fid_ofile.
  */
 static uint32_t
-find_breaking_ofile(smb_request_t *sr, uint8_t *lease_key)
+find_oplock_ofile(smb_request_t *sr, uint8_t *lease_key)
 {
 	smb_tree_t	*tree = sr->tid_tree;
 	smb_lease_t	*lease;
@@ -265,6 +266,7 @@ find_breaking_ofile(smb_request_t *sr, uint8_t *lease_key)
 		ASSERT(o->f_magic == SMB_OFILE_MAGIC);
 		ASSERT(o->f_tree == tree);
 
+		DTRACE_PROBE1(every_ofile, smb_ofile_t *, o);
 		if ((lease = o->f_lease) == NULL)
 			continue; // no lease
 
@@ -273,14 +275,16 @@ find_breaking_ofile(smb_request_t *sr, uint8_t *lease_key)
 
 		/*
 		 * Now we know the lease exists, so if we don't
-		 * find an ofile with breaking flags, return:
+		 * find the ofile that has the oplock, return:
 		 */
 		status = NT_STATUS_UNSUCCESSFUL;
 
-		if (o->f_oplock.og_breaking == 0)
-			continue; // not breaking
+		DTRACE_PROBE2(lease_ofile, smb_lease_t *, lease,
+		    smb_ofile_t *, o);
+		if (lease->ls_oplock_ofile != o)
+			continue; // not oplock holder
 
-		/* Found breaking ofile. */
+		/* Found the ofile holding the oplock */
 		if (smb_ofile_hold(o)) {
 			sr->fid_ofile = o;
 			status = NT_STATUS_SUCCESS;
@@ -326,7 +330,7 @@ smb2_lease_break_ack(smb_request_t *sr)
 	if (rc != 0)
 		return (SDRC_ERROR);
 
-	status = find_breaking_ofile(sr, LeaseKey);
+	status = find_oplock_ofile(sr, LeaseKey);
 
 	DTRACE_SMB2_START(op__OplockBreak, smb_request_t *, sr);
 	if (status != 0)
@@ -336,6 +340,12 @@ smb2_lease_break_ack(smb_request_t *sr)
 	ofile = sr->fid_ofile;
 	lease = ofile->f_lease;
 
+	/* Either this ACK is unsolicited, or we timed out waiting. */
+	if (lease->ls_breaking == 0) {
+		status = NT_STATUS_UNSUCCESSFUL;
+		goto errout;
+	}
+
 	/*
 	 * Process the lease break ack.
 	 *
@@ -349,7 +359,6 @@ smb2_lease_break_ack(smb_request_t *sr)
 		goto errout;
 	}
 
-	ofile->f_oplock.og_breaking = 0;
 	lease->ls_breaking = 0;
 
 	LeaseState |= OPLOCK_LEVEL_GRANULAR;
@@ -360,9 +369,7 @@ smb2_lease_break_ack(smb_request_t *sr)
 		status = NT_STATUS_SUCCESS;
 	}
 
-	ofile->f_oplock.og_state = LeaseState;
-	lease->ls_state = LeaseState &
-	    OPLOCK_LEVEL_CACHE_MASK;
+	lease->ls_state = LeaseState & OPLOCK_LEVEL_CACHE_MASK;
 
 errout:
 	sr->smb2_status = status;
@@ -401,9 +408,7 @@ void
 smb2_lease_break_notification(smb_request_t *sr, uint32_t NewLevel,
     boolean_t AckReq)
 {
-	smb_ofile_t *ofile = sr->fid_ofile;
-	smb_oplock_grant_t *og = &ofile->f_oplock;
-	smb_lease_t *ls = ofile->f_lease;
+	smb_lease_t *ls = sr->fid_ofile->f_lease;
 	uint32_t oldcache;
 	uint32_t newcache;
 	uint16_t Epoch;
@@ -412,7 +417,7 @@ smb2_lease_break_notification(smb_request_t *sr, uint32_t NewLevel,
 	/*
 	 * Convert internal level to SMB2
 	 */
-	oldcache = og->og_state & OPLOCK_LEVEL_CACHE_MASK;
+	oldcache = ls->ls_state & OPLOCK_LEVEL_CACHE_MASK;
 	newcache = NewLevel & OPLOCK_LEVEL_CACHE_MASK;
 	if (ls->ls_version < 2)
 		Epoch = 0;
@@ -642,15 +647,9 @@ done:
 		ASSERT(status == NT_STATUS_SUCCESS);
 
 		/*
-		 * Keep track of what we got (in ofile->f_oplock.og_state)
-		 * so we'll know what we had when sending a break later.
-		 * Also update the lease with the new oplock state.
-		 * Also track which ofile on the lease owns the oplock.
-		 * The og_dialect here is the oplock dialect, not the
-		 * SMB dialect.  Leasing, so SMB 2.1 (or later).
+		 * Keep track of what we got (in lease->ls_state)
+		 * so we'll know what we last told the client.
 		 */
-		ofile->f_oplock.og_dialect = SMB_VERS_2_1;
-		ofile->f_oplock.og_state = op->op_oplock_state;
 		mutex_enter(&lease->ls_mutex);
 		lease->ls_state = op->op_oplock_state & CACHE_RWH;
 		lease->ls_epoch++;
@@ -703,7 +702,7 @@ smb2_lease_ofile_close(smb_ofile_t *ofile)
 			continue;
 		if (o->f_lease != lease)
 			continue;
-		if (o->f_oplock.og_closing)
+		if (o->f_oplock_closing)
 			continue;
 		/* If we can get a hold, use this ofile. */
 		if (smb_ofile_hold(o))
@@ -711,9 +710,11 @@ smb2_lease_ofile_close(smb_ofile_t *ofile)
 	}
 	if (o == NULL) {
 		/* Normal for last close on a lease. */
+		lease->ls_oplock_ofile = NULL;
 		return;
 	}
 	smb_oplock_move(node, ofile, o);
+	lease->ls_oplock_ofile = o;
 
 	smb_ofile_release(o);
 }
diff --git a/usr/src/uts/common/fs/smbsrv/smb_cmn_oplock.c b/usr/src/uts/common/fs/smbsrv/smb_cmn_oplock.c
index 8ec21f5f37..1b9ccce688 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_cmn_oplock.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_cmn_oplock.c
@@ -11,6 +11,7 @@
 
 /*
  * Copyright 2020 Nexenta by DDN, Inc.  All rights reserved.
+ * Copyright 2021 RackTop Systems, Inc.
  */
 
 /*
@@ -558,7 +559,7 @@ smb_oplock_req_excl(
 	/*
 	 * Don't allow grants on closing ofiles.
 	 */
-	if (ofile->f_oplock.og_closing)
+	if (ofile->f_oplock_closing)
 		return (status);
 
 	/*
@@ -1049,7 +1050,7 @@ smb_oplock_req_shared(
 	/*
 	 * Don't allow grants on closing ofiles.
 	 */
-	if (ofile->f_oplock.og_closing)
+	if (ofile->f_oplock_closing)
 		return (status);
 
 	/*
@@ -2289,10 +2290,6 @@ smb_oplock_break_CLOSE(smb_node_t *node, smb_ofile_t *ofile)
 	ASSERT(RW_READ_HELD(&node->n_ofile_list.ll_lock));
 	ASSERT(MUTEX_HELD(&node->n_oplock.ol_mutex));
 
-	if (ofile->f_oplock.og_closing)
-		return;
-	ofile->f_oplock.og_closing = B_TRUE;
-
 	/*
 	 * If Oplock.IIOplocks is not empty:
 	 *   For each Open ThisOpen in Oplock.IIOplocks:
diff --git a/usr/src/uts/common/fs/smbsrv/smb_ofile.c b/usr/src/uts/common/fs/smbsrv/smb_ofile.c
index 1d7a5c134f..70c546228c 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_ofile.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_ofile.c
@@ -23,6 +23,7 @@
  * Copyright 2016 Syneto S.R.L. All rights reserved.
  * Copyright (c) 2016 by Delphix. All rights reserved.
  * Copyright 2020 Tintri by DDN, Inc. All rights reserved.
+ * Copyright 2021 RackTop Systems, Inc.
  */
 
 /*
@@ -455,9 +456,13 @@ smb_ofile_close(smb_ofile_t *of, int32_t mtime_sec)
 		smb_llist_enter(&node->n_ofile_list, RW_READER);
 		mutex_enter(&node->n_oplock.ol_mutex);
 
-		if (of->f_lease != NULL)
-			smb2_lease_ofile_close(of);
-		smb_oplock_break_CLOSE(node, of);
+		if (of->f_oplock_closing == B_FALSE) {
+			of->f_oplock_closing = B_TRUE;
+
+			if (of->f_lease != NULL)
+				smb2_lease_ofile_close(of);
+			smb_oplock_break_CLOSE(node, of);
+		}
 
 		mutex_exit(&node->n_oplock.ol_mutex);
 		smb_llist_exit(&node->n_ofile_list);
diff --git a/usr/src/uts/common/fs/smbsrv/smb_srv_oplock.c b/usr/src/uts/common/fs/smbsrv/smb_srv_oplock.c
index d4811f6857..538b57b7ba 100644
--- a/usr/src/uts/common/fs/smbsrv/smb_srv_oplock.c
+++ b/usr/src/uts/common/fs/smbsrv/smb_srv_oplock.c
@@ -11,6 +11,7 @@
 
 /*
  * Copyright 2021 Tintri by DDN, Inc. All rights reserved.
+ * Copyright 2021 RackTop Systems, Inc.
  */
 
 /*
@@ -173,7 +174,7 @@ smb_oplock_ind_break_in_ack(smb_request_t *ack_sr, smb_ofile_t *ofile,
 	 * We're going to schedule a request that will have a
 	 * reference to this ofile. Get the hold first.
 	 */
-	if (ofile->f_oplock.og_closing ||
+	if (ofile->f_oplock_closing ||
 	    !smb_ofile_hold_olbrk(ofile)) {
 		/* It's closing (or whatever).  Nothing to do. */
 		return;
@@ -252,6 +253,9 @@ smb_oplock_ind_break(smb_ofile_t *ofile, uint32_t NewLevel,
 		break;
 
 	case STATUS_NEW_HANDLE:
+		/* nothing to do (keep for observability) */
+		return;
+
 	case NT_STATUS_OPLOCK_HANDLE_CLOSED:
 		smb_oplock_hdl_clear(ofile);
 		return;
@@ -265,7 +269,7 @@ smb_oplock_ind_break(smb_ofile_t *ofile, uint32_t NewLevel,
 	 * We're going to schedule a request that will have a
 	 * reference to this ofile. Get the hold first.
 	 */
-	if (ofile->f_oplock.og_closing ||
+	if (ofile->f_oplock_closing ||
 	    !smb_ofile_hold_olbrk(ofile)) {
 		/* It's closing (or whatever).  Nothing to do. */
 		return;
@@ -372,6 +376,19 @@ smb_oplock_async_break(void *arg)
 	smb_request_free(sr);
 }
 
+static void
+smb_oplock_update(smb_request_t *sr, smb_ofile_t *ofile, uint32_t NewLevel)
+{
+	if (ofile->f_lease != NULL)
+		ofile->f_lease->ls_state = NewLevel & CACHE_RWH;
+	else
+		ofile->f_oplock.og_state = NewLevel;
+
+	if (ofile->dh_persist) {
+		smb2_dh_update_oplock(sr, ofile);
+	}
+}
+
 #ifdef DEBUG
 int smb_oplock_debug_wait = 0;
 #endif
@@ -411,23 +428,24 @@ smb_oplock_send_brk(smb_request_t *sr)
 	 * Also updates the lease and NewLevel.
 	 */
 	sr->reply.max_bytes = MLEN;
-	if (ofile->f_oplock.og_dialect >= SMB_VERS_2_BASE) {
-		if (lease != NULL) {
-			/*
-			 * Oplock state has changed, so
-			 * update the epoch.
-			 */
-			mutex_enter(&lease->ls_mutex);
-			lease->ls_epoch++;
-			mutex_exit(&lease->ls_mutex);
-
-			/* Note, needs "old" state in og_state */
-			smb2_lease_break_notification(sr,
-			    (NewLevel & CACHE_RWH), AckReq);
-			NewLevel |= OPLOCK_LEVEL_GRANULAR;
-		} else {
-			smb2_oplock_break_notification(sr, NewLevel);
-		}
+	if (lease != NULL) {
+		/*
+		 * The ofile has as lease.  Must be SMB2+
+		 * Oplock state has changed, so update the epoch.
+		 */
+		mutex_enter(&lease->ls_mutex);
+		lease->ls_epoch++;
+		mutex_exit(&lease->ls_mutex);
+
+		/* Note, needs "old" state in ls_state */
+		smb2_lease_break_notification(sr,
+		    (NewLevel & CACHE_RWH), AckReq);
+		NewLevel |= OPLOCK_LEVEL_GRANULAR;
+	} else if (ofile->f_oplock.og_dialect >= SMB_VERS_2_BASE) {
+		/*
+		 * SMB2 using old-style oplock (no lease)
+		 */
+		smb2_oplock_break_notification(sr, NewLevel);
 	} else {
 		/*
 		 * SMB1 clients should only get Level II oplocks if they
@@ -443,8 +461,8 @@ smb_oplock_send_brk(smb_request_t *sr)
 	 * Keep track of what we last sent to the client,
 	 * preserving the GRANULAR flag (if a lease).
 	 * If we're expecting an ACK, set og_breaking
-	 * (and maybe lease->ls_breaking) so we can
-	 * later find the ofile with breaks pending.
+	 * (or maybe lease->ls_breaking) so we can
+	 * filter unsolicited ACKs.
 	 */
 	if (AckReq) {
 		uint32_t BreakTo;
@@ -459,17 +477,11 @@ smb_oplock_send_brk(smb_request_t *sr)
 				BreakTo = BREAK_TO_TWO;
 			else
 				BreakTo = BREAK_TO_NONE;
+			ofile->f_oplock.og_breaking = BreakTo;
 		}
-		/* Will update og_state in ack. */
-		ofile->f_oplock.og_breaking = BreakTo;
+		/* Will update ls/og_state in ack. */
 	} else {
-		if (lease != NULL)
-			lease->ls_state = NewLevel & CACHE_RWH;
-		ofile->f_oplock.og_state = NewLevel;
-
-		if (ofile->dh_persist) {
-			smb2_dh_update_oplock(sr, ofile);
-		}
+		smb_oplock_update(sr, ofile, NewLevel);
 	}
 
 	/*
@@ -587,9 +599,10 @@ smb_oplock_send_brk(smb_request_t *sr)
 	 * or a send failure for a durable handle type that we
 	 * preserve rather than just close.  Do local ack.
 	 */
-	ofile->f_oplock.og_breaking = 0;
 	if (lease != NULL)
 		lease->ls_breaking = 0;
+	else
+		ofile->f_oplock.og_breaking = 0;
 
 	status = smb_oplock_ack_break(sr, ofile, &NewLevel);
 	if (status == NT_STATUS_OPLOCK_BREAK_IN_PROGRESS) {
@@ -604,20 +617,13 @@ smb_oplock_send_brk(smb_request_t *sr)
 		    "status=0x%x", status);
 	}
 
-	/* Update og_state as if we heard from the client. */
-	ofile->f_oplock.og_state = NewLevel;
-	if (lease != NULL) {
-		lease->ls_state = NewLevel & CACHE_RWH;
-	}
-
-	if (ofile->dh_persist) {
-		smb2_dh_update_oplock(sr, ofile);
-	}
+	/* Update ls/og_state as if we heard from the client. */
+	smb_oplock_update(sr, ofile, NewLevel);
 }
 
 /*
- * See: NT_STATUS_OPLOCK_HANDLE_CLOSED above,
- * and: STATUS_NEW_HANDLE
+ * See: NT_STATUS_OPLOCK_HANDLE_CLOSED above and
+ * smb_ofile_close, smb_oplock_break_CLOSE.
  *
  * The FS-level oplock layer calls this to update the
  * SMB-level state when a handle loses its oplock.
@@ -629,7 +635,11 @@ smb_oplock_hdl_clear(smb_ofile_t *ofile)
 
 	if (lease != NULL) {
 		if (lease->ls_oplock_ofile == ofile) {
-			/* Last close on the lease. */
+			/*
+			 * smb2_lease_ofile_close should have
+			 * moved the oplock to another ofile.
+			 */
+			ASSERT(0);
 			lease->ls_oplock_ofile = NULL;
 		}
 	}