Fix missing locks on mosq->state.

Closes #1374. Thanks to Jeff Trull.
eclipse · Sep 17, 2019 · 40c1a97 · 40c1a97
1 parent 76d0b32
commit 40c1a97
Show file tree

Hide file tree

Showing 7 changed files with 45 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,14 +321,19 @@ int packet__read(struct mosquitto *mosq)
  uint8_t byte;
  ssize_t read_length;
  int rc = 0;
+ int state;
 
  if(!mosq){
  return MOSQ_ERR_INVAL;
  }
  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
@@ -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);