[189] Mosquitto database corrupted on power-loss.

src/persist.c

@@ -350,9 +350,6 @@ int mqtt3_db_backup(struct mosquitto_db *db, bool shutdown)
 	char err[256];
 	char *outfile = NULL;
 	int len;
-#ifndef WIN32
-	int dir_fd;
-#endif
 
 	if(!db || !db->config || !db->config->persistence_filepath) return MOSQ_ERR_INVAL;
 	_mosquitto_log_printf(NULL, MOSQ_LOG_INFO, "Saving in-memory database to %s.", db->config->persistence_filepath);
@@ -365,6 +362,36 @@ int mqtt3_db_backup(struct mosquitto_db *db, bool shutdown)
 	}
 	snprintf(outfile, len, "%s.new", db->config->persistence_filepath);
 	outfile[len] = '\0';
+
+#ifndef WIN32
+	/**
+ 	*
+	* If a system lost power during the rename operation at the
+	* end of this file the filesystem could potentially be left
+	* with a directory that looks like this after powerup:
+	*
+	* 24094 -rw-r--r--    2 root     root          4099 May 30 16:27 mosquitto.db
+	* 24094 -rw-r--r--    2 root     root          4099 May 30 16:27 mosquitto.db.new
+	*
+	* The 24094 shows that mosquitto.db.new is hard-linked to the
+	* same file as mosquitto.db.  If fopen(outfile, "wb") is naively
+	* called then mosquitto.db will be truncated and the database
+	* potentially corrupted.
+	*
+	* Any existing mosquitto.db.new file must be removed prior to
+	* opening to guarantee that it is not hard-linked to
+	* mosquitto.db.
+	*
+	*/
+	rc = unlink(outfile);
+	if (rc != 0) {
+		if (errno != ENOENT) {
+			_mosquitto_log_printf(NULL, MOSQ_LOG_INFO, "Error saving in-memory database, unable to remove %s.", outfile);
+			goto error;
+		}
+	}
+#endif
+
 	db_fptr = _mosquitto_fopen(outfile, "wb");
 	if(db_fptr == NULL){
 		_mosquitto_log_printf(NULL, MOSQ_LOG_INFO, "Error saving in-memory database, unable to open %s for writing.", outfile);
@@ -399,17 +426,30 @@ int mqtt3_db_backup(struct mosquitto_db *db, bool shutdown)
 	mqtt3_db_subs_retain_write(db, db_fptr);
 
 #ifndef WIN32
+	/**
+	*
+	* Closing a file does not guarantee that the contents are
+	* written to disk.  Need to flush to send data from app to OS
+	* buffers, then fsync to deliver data from OS buffers to disk
+	* (as well as disk hardware permits).
+	* 
+	* man close (http://linux.die.net/man/2/close, 2016-06-20):
+	* 
+	*   "successful close does not guarantee that the data has
+	*   been successfully saved to disk, as the kernel defers
+	*   writes.  It is not common for a filesystem to flush
+	*   the  buffers  when  the stream is closed.  If you need
+	*   to be sure that the data is physically stored, use
+	*   fsync(2).  (It will depend on the disk hardware at this
+	*   point."
+	*
+	* This guarantees that the new state file will not overwrite
+	* the old state file before its contents are valid.
+	*
+	*/
+
+	fflush(db_fptr);
 	fsync(fileno(db_fptr));
-
-	if(db->config->persistence_location){
-		dir_fd = open(db->config->persistence_location, O_RDONLY);
-	}else{
-		dir_fd = open(".", O_RDONLY);
-	}
-	if(dir_fd > 0){
-		fsync(dir_fd);
-		close(dir_fd);
-	}
 #endif
 	fclose(db_fptr);