From ee3591d228efdcea7165fa5589c39af4be1fb878 Mon Sep 17 00:00:00 2001
From: "Roger A. Light" <roger@atchoo.org>
Date: Sun, 8 Sep 2019 21:11:20 +0100
Subject: [PATCH] Fix missing locks on `mosq->state`.

Closes #1374. Thanks to Jeff Trull.
---
 ChangeLog.txt           |  1 +
 lib/handle_ping.c       |  8 +++++++-
 lib/handle_pubackcomp.c |  7 ++++++-
 lib/handle_publish.c    |  7 ++++++-
 lib/handle_suback.c     |  7 ++++++-
 lib/packet_mosq.c       | 16 +++++++++++++---
 lib/util_mosq.c         |  7 ++++++-
 7 files changed, 45 insertions(+), 8 deletions(-)

diff --git a/ChangeLog.txt b/ChangeLog.txt
index 65817c9711..24c2c86a84 100644
--- a/ChangeLog.txt
+++ b/ChangeLog.txt
@@ -14,6 +14,7 @@ Broker:
 Client library:
 - Fix reconnect backoff for the situation where connections are dropped rather
   than refused. Closes #737.
+- Fix missing locks on `mosq->state`. Closes #1374.
 
 Documentation:
 - Improve details on global/per listener options in the mosquitto.conf man page.
diff --git a/lib/handle_ping.c b/lib/handle_ping.c
index 56d89a3c3f..69355be804 100644
--- a/lib/handle_ping.c
+++ b/lib/handle_ping.c
@@ -53,9 +53,15 @@ int handle__pingreq(struct mosquitto *mosq)
 
 int handle__pingresp(struct mosquitto *mosq)
 {
+	int state;
+
 	assert(mosq);
 
-	if(mosq->state != mosq_cs_connected){
+	pthread_mutex_lock(&mosq->state_mutex);
+	state = mosq->state;
+	pthread_mutex_unlock(&mosq->state_mutex);
+
+	if(state != mosq_cs_connected){
 		return MOSQ_ERR_PROTOCOL;
 	}
 
diff --git a/lib/handle_pubackcomp.c b/lib/handle_pubackcomp.c
index 67921494e2..774e948c2b 100644
--- a/lib/handle_pubackcomp.c
+++ b/lib/handle_pubackcomp.c
@@ -47,10 +47,15 @@ int handle__pubackcomp(struct mosquitto *mosq, const char *type)
 	int rc;
 	mosquitto_property *properties = NULL;
 	int qos;
+	int state;
 
 	assert(mosq);
 
-	if(mosq->state != mosq_cs_connected){
+	pthread_mutex_lock(&mosq->state_mutex);
+	state = mosq->state;
+	pthread_mutex_unlock(&mosq->state_mutex);
+
+	if(state != mosq_cs_connected){
 		return MOSQ_ERR_PROTOCOL;
 	}
 
diff --git a/lib/handle_publish.c b/lib/handle_publish.c
index 8c93e3cefb..485742fa7e 100644
--- a/lib/handle_publish.c
+++ b/lib/handle_publish.c
@@ -40,10 +40,15 @@ int handle__publish(struct mosquitto *mosq)
 	uint16_t mid;
 	int slen;
 	mosquitto_property *properties = NULL;
+	int state;
 
 	assert(mosq);
 
-	if(mosq->state != mosq_cs_connected){
+	pthread_mutex_lock(&mosq->state_mutex);
+	state = mosq->state;
+	pthread_mutex_unlock(&mosq->state_mutex);
+
+	if(state != mosq_cs_connected){
 		return MOSQ_ERR_PROTOCOL;
 	}
 
diff --git a/lib/handle_suback.c b/lib/handle_suback.c
index dadebcd79f..8dfdab63d1 100644
--- a/lib/handle_suback.c
+++ b/lib/handle_suback.c
@@ -40,10 +40,15 @@ int handle__suback(struct mosquitto *mosq)
 	int i = 0;
 	int rc;
 	mosquitto_property *properties = NULL;
+	int state;
 
 	assert(mosq);
 
-	if(mosq->state != mosq_cs_connected){
+	pthread_mutex_lock(&mosq->state_mutex);
+	state = mosq->state;
+	pthread_mutex_unlock(&mosq->state_mutex);
+
+	if(state != mosq_cs_connected){
 		return MOSQ_ERR_PROTOCOL;
 	}
 
diff --git a/lib/packet_mosq.c b/lib/packet_mosq.c
index dc54543532..101e2f5a4d 100644
--- a/lib/packet_mosq.c
+++ b/lib/packet_mosq.c
@@ -202,6 +202,7 @@ int packet__write(struct mosquitto *mosq)
 {
 	ssize_t write_length;
 	struct mosquitto__packet *packet;
+	int state;
 
 	if(!mosq) return MOSQ_ERR_INVAL;
 	if(mosq->sock == INVALID_SOCKET) return MOSQ_ERR_NO_CONN;
@@ -217,10 +218,14 @@ int packet__write(struct mosquitto *mosq)
 	}
 	pthread_mutex_unlock(&mosq->out_packet_mutex);
 
+	pthread_mutex_lock(&mosq->state_mutex);
+	state = mosq->state;
+	pthread_mutex_unlock(&mosq->state_mutex);
+
 #if defined(WITH_TLS) && !defined(WITH_BROKER)
-	if((mosq->state == mosq_cs_connect_pending) || mosq->want_connect){
+	if((state == mosq_cs_connect_pending) || mosq->want_connect){
 #else
-	if(mosq->state == mosq_cs_connect_pending){
+	if(state == mosq_cs_connect_pending){
 #endif
 		pthread_mutex_unlock(&mosq->current_out_packet_mutex);
 		return MOSQ_ERR_SUCCESS;
@@ -316,6 +321,7 @@ int packet__read(struct mosquitto *mosq)
 	uint8_t byte;
 	ssize_t read_length;
 	int rc = 0;
+	int state;
 
 	if(!mosq){
 		return MOSQ_ERR_INVAL;
@@ -323,7 +329,11 @@ int packet__read(struct mosquitto *mosq)
 	if(mosq->sock == INVALID_SOCKET){
 		return MOSQ_ERR_NO_CONN;
 	}
-	if(mosq->state == mosq_cs_connect_pending){
+	pthread_mutex_lock(&mosq->state_mutex);
+	state = mosq->state;
+	pthread_mutex_unlock(&mosq->state_mutex);
+
+	if(state == mosq_cs_connect_pending){
 		return MOSQ_ERR_SUCCESS;
 	}
 
diff --git a/lib/util_mosq.c b/lib/util_mosq.c
index 65fc2215ed..928d93cb45 100644
--- a/lib/util_mosq.c
+++ b/lib/util_mosq.c
@@ -68,6 +68,7 @@ int mosquitto__check_keepalive(struct mosquitto *mosq)
 #ifndef WITH_BROKER
 	int rc;
 #endif
+	int state;
 
 	assert(mosq);
 #if defined(WITH_BROKER) && defined(WITH_BRIDGE)
@@ -88,7 +89,11 @@ int mosquitto__check_keepalive(struct mosquitto *mosq)
 	if(mosq->keepalive && mosq->sock != INVALID_SOCKET &&
 			(now >= next_msg_out || now - last_msg_in >= mosq->keepalive)){
 
-		if(mosq->state == mosq_cs_connected && mosq->ping_t == 0){
+		pthread_mutex_lock(&mosq->state_mutex);
+		state = mosq->state;
+		pthread_mutex_unlock(&mosq->state_mutex);
+
+		if(state == mosq_cs_connected && mosq->ping_t == 0){
 			send__pingreq(mosq);
 			/* Reset last msg times to give the server time to send a pingresp */
 			pthread_mutex_lock(&mosq->msgtime_mutex);