From: Stefan Metzmacher <metze@samba.org>
Date: Wed, 23 Sep 2020 04:00:28 +0000 (+0200)
Subject: smbXsrv_session: set session->db_rec = NULL after session->db_rec = local_rec
X-Git-Tag: tevent-0.11.0~1670
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=638c4435a0eeb110719dd2e799f2a12758667326;p=thirdparty%2Fsamba.git

smbXsrv_session: set session->db_rec = NULL after session->db_rec = local_rec

This actually fixes crashes due to stale pointers.

With multi-channel and with 2 (or more) connections,
we'll call smbXsrv_session_disconnect_xconn() when a connection
gets disconnected, but we'll leave smbXsrv_client and all other
connections in place.

However smbXsrv_session_disconnect_xconn_callback() left
a stale session->db_rec pointer in place, which means
a following smbXsrv_session_logoff() will call
dbwrap_record_delete(local_rec) on a stale pointer.

BUG: https://bugzilla.samba.org/show_bug.cgi?id=14532

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Jeremy Allison <jra@samba.org>
---

diff --git a/selftest/knownfail.d/smb2.session.two_logoff b/selftest/knownfail.d/smb2.session.two_logoff
deleted file mode 100644
index 37cb69400b4..00000000000
--- a/selftest/knownfail.d/smb2.session.two_logoff
+++ /dev/null
@@ -1 +0,0 @@
-^samba3.smb2.session.*two_logoff
diff --git a/source3/smbd/smbXsrv_session.c b/source3/smbd/smbXsrv_session.c
index 05cf4a095ee..dd7f6fa5e2d 100644
--- a/source3/smbd/smbXsrv_session.c
+++ b/source3/smbd/smbXsrv_session.c
@@ -1831,8 +1831,8 @@ static int smbXsrv_session_logoff_all_callback(struct db_record *local_rec,
 	session = talloc_get_type_abort(ptr, struct smbXsrv_session);
 
 	session->db_rec = local_rec;
-
 	status = smbXsrv_session_clear_and_logoff(session);
+	session->db_rec = NULL;
 	if (!NT_STATUS_IS_OK(status)) {
 		if (NT_STATUS_IS_OK(state->first_status)) {
 			state->first_status = status;
@@ -1901,6 +1901,7 @@ static int smbXsrv_session_local_traverse_cb(struct db_record *local_rec,
 	TDB_DATA val;
 	void *ptr = NULL;
 	struct smbXsrv_session *session = NULL;
+	int ret;
 
 	val = dbwrap_record_get_value(local_rec);
 	if (val.dsize != sizeof(ptr)) {
@@ -1910,9 +1911,12 @@ static int smbXsrv_session_local_traverse_cb(struct db_record *local_rec,
 
 	memcpy(&ptr, val.dptr, val.dsize);
 	session = talloc_get_type_abort(ptr, struct smbXsrv_session);
+
 	session->db_rec = local_rec;
+	ret = state->caller_cb(session, state->caller_data);
+	session->db_rec = NULL;
 
-	return state->caller_cb(session, state->caller_data);
+	return ret;
 }
 
 struct smbXsrv_session_disconnect_xconn_state {
@@ -2019,6 +2023,7 @@ static int smbXsrv_session_disconnect_xconn_callback(struct db_record *local_rec
 				state->first_status = status;
 			}
 			state->errors++;
+			session->db_rec = NULL;
 			return 0;
 		}
 		ARRAY_DEL_ELEMENT(global->channels, n, global->num_channels);
@@ -2034,6 +2039,7 @@ static int smbXsrv_session_disconnect_xconn_callback(struct db_record *local_rec
 		state->errors++;
 	}
 
+	session->db_rec = NULL;
 	return 0;
 }