ctdb-server: Coverity fixes

Signed-off-by: Amitay Isaacs <amitay@gmail.com>
Reviewed-by: Michael Adam <obnox@samba.org>

8 files changed, 49 insertions, 24 deletions

diff --git a/ctdb/server/ctdb_banning.c b/ctdb/server/ctdb_banning.c
index e6df4b9bfc4..13d97c84b13 100644
--- a/ctdb/server/ctdb_banning.c
+++ b/ctdb/server/ctdb_banning.c
@@ -84,7 +84,7 @@ int32_t ctdb_control_set_ban_state(struct ctdb_context *ctdb, TDB_DATA indata)
 	DEBUG(DEBUG_INFO,("SET BAN STATE\n"));
 
 	if (bantime->pnn != ctdb->pnn) {
-		if (bantime->pnn < 0 || bantime->pnn >= ctdb->num_nodes) {
+		if (bantime->pnn >= ctdb->num_nodes) {
 			DEBUG(DEBUG_ERR,(__location__ " ERROR: Invalid ban request. PNN:%d is invalid. Max nodes %d\n", bantime->pnn, ctdb->num_nodes));
 			return -1;
 		}
diff --git a/ctdb/server/ctdb_control.c b/ctdb/server/ctdb_control.c
index 99319ac523c..581c478d9dd 100644
--- a/ctdb/server/ctdb_control.c
+++ b/ctdb/server/ctdb_control.c
@@ -50,6 +50,12 @@ int32_t ctdb_dump_memory(struct ctdb_context *ctdb, TDB_DATA *outdata)
 	}
 	talloc_report_full(NULL, f);
 	fsize = ftell(f);
+	if (fsize == -1) {
+		DEBUG(DEBUG_ERR, (__location__ " Unable to get file size - %s\n",
+				  strerror(errno)));
+		fclose(f);
+		return -1;
+	}
 	rewind(f);
 	outdata->dptr = talloc_size(outdata, fsize);
 	if (outdata->dptr == NULL) {
diff --git a/ctdb/server/ctdb_daemon.c b/ctdb/server/ctdb_daemon.c
index cbe6b23ce0e..50b2de327ec 100644
--- a/ctdb/server/ctdb_daemon.c
+++ b/ctdb/server/ctdb_daemon.c
@@ -978,7 +978,7 @@ static int ux_socket_bind(struct ctdb_context *ctdb)
 
 	memset(&addr, 0, sizeof(addr));
 	addr.sun_family = AF_UNIX;
-	strncpy(addr.sun_path, ctdb->daemon.name, sizeof(addr.sun_path));
+	strncpy(addr.sun_path, ctdb->daemon.name, sizeof(addr.sun_path)-1);
 
 	/* First check if an old ctdbd might be running */
 	if (connect(ctdb->daemon.sd,
diff --git a/ctdb/server/ctdb_lock.c b/ctdb/server/ctdb_lock.c
index fc437b0fa7f..bb66f949632 100644
--- a/ctdb/server/ctdb_lock.c
+++ b/ctdb/server/ctdb_lock.c
@@ -259,7 +259,7 @@ static int ctdb_lockall_unmark(struct ctdb_context *ctdb)
 {
 	uint32_t priority;
 
-	for (priority=NUM_DB_PRIORITIES; priority>=0; priority--) {
+	for (priority=NUM_DB_PRIORITIES; priority>0; priority--) {
 		if (ctdb_db_iterator(ctdb, priority, db_lock_unmark_handler, NULL) != 0) {
 			return -1;
 		}
@@ -448,8 +448,11 @@ static void ctdb_lock_handler(struct tevent_context *ev,
 	}
 
 	/* Read the status from the child process */
-	read(lock_ctx->fd[0], &c, 1);
-	locked = (c == 0 ? true : false);
+	if (read(lock_ctx->fd[0], &c, 1) != 1) {
+		locked = false;
+	} else {
+		locked = (c == 0 ? true : false);
+	}
 
 	/* Update statistics */
 	CTDB_DECREMENT_STAT(lock_ctx->ctdb, locks.num_pending);
@@ -998,7 +1001,7 @@ struct lock_request *ctdb_lock_alldb_prio(struct ctdb_context *ctdb,
 					  void (*callback)(void *, bool),
 					  void *private_data)
 {
-	if (priority < 0 || priority > NUM_DB_PRIORITIES) {
+	if (priority < 1 || priority > NUM_DB_PRIORITIES) {
 		DEBUG(DEBUG_ERR, ("Invalid db priority: %u\n", priority));
 		return NULL;
 	}
diff --git a/ctdb/server/ctdb_logging.c b/ctdb/server/ctdb_logging.c
index 218186e7f04..17c5c35ae7e 100644
--- a/ctdb/server/ctdb_logging.c
+++ b/ctdb/server/ctdb_logging.c
@@ -38,7 +38,6 @@ struct ctdb_syslog_state {
 
 static int syslogd_is_started = 0;
 
-
 /* called when child is finished
  * this is for the syslog daemon, we can not use DEBUG here
  */
@@ -60,6 +59,10 @@ static void ctdb_syslog_handler(struct event_context *ev, struct fd_event *fde,
 		return;
 	}
 	msg = (struct syslog_message *)str;
+	if (msg->len >= (sizeof(str) - offsetof(struct syslog_message, message))) {
+		msg->len = (sizeof(str)-1) - offsetof(struct syslog_message, message);
+	}
+	msg->message[msg->len] = '\0';
 
 	syslog(msg->level, "%s", msg->message);
 }
@@ -401,8 +404,9 @@ static void write_to_log(struct ctdb_log_state *log,
 			do_debug("%*.*s\n", len, len, buf);
 		}
 		/* log it in the eventsystem as well */
-		if (log->logfn)
+		if (log && log->logfn) {
 			log->logfn(log->buf, len, log->logfn_private);
+		}
 	}
 }
 
diff --git a/ctdb/server/ctdb_recover.c b/ctdb/server/ctdb_recover.c
index 1cbcc59abec..414f5b1f8d9 100644
--- a/ctdb/server/ctdb_recover.c
+++ b/ctdb/server/ctdb_recover.c
@@ -1147,7 +1147,11 @@ static int store_tdb_record(struct ctdb_context *ctdb,
 
 	data2 = tdb_fetch(ctdb_db->ltdb->tdb, key);
 	if (data2.dptr == NULL || data2.dsize < sizeof(struct ctdb_ltdb_header)) {
-		tdb_store(ctdb_db->ltdb->tdb, key, data, 0);
+		if (tdb_store(ctdb_db->ltdb->tdb, key, data, 0) == -1) {
+			DEBUG(DEBUG_ERR, (__location__ "Failed to store record\n"));
+			ret = -1;
+			goto done;
+		}
 		DEBUG(DEBUG_INFO, (__location__ " Stored record\n"));
 		ret = 0;
 		goto done;
diff --git a/ctdb/server/ctdb_recoverd.c b/ctdb/server/ctdb_recoverd.c
index 5caf7c05eaa..8b4b6d0298b 100644
--- a/ctdb/server/ctdb_recoverd.c
+++ b/ctdb/server/ctdb_recoverd.c
@@ -535,15 +535,17 @@ static int create_missing_remote_databases(struct ctdb_context *ctdb, struct ctd
 				continue;
 			}
 			/* ok so we need to create this database */
-			ctdb_ctrl_getdbname(ctdb, CONTROL_TIMEOUT(), pnn, dbmap->dbs[db].dbid, 
-					    mem_ctx, &name);
+			ret = ctdb_ctrl_getdbname(ctdb, CONTROL_TIMEOUT(), pnn,
+						  dbmap->dbs[db].dbid, mem_ctx,
+						  &name);
 			if (ret != 0) {
 				DEBUG(DEBUG_ERR, (__location__ " Unable to get dbname from node %u\n", pnn));
 				return -1;
 			}
-			ctdb_ctrl_createdb(ctdb, CONTROL_TIMEOUT(), nodemap->nodes[j].pnn, 
-					   mem_ctx, name,
-					   dbmap->dbs[db].flags & CTDB_DB_FLAGS_PERSISTENT);
+			ret = ctdb_ctrl_createdb(ctdb, CONTROL_TIMEOUT(),
+						 nodemap->nodes[j].pnn,
+						 mem_ctx, name,
+						 dbmap->dbs[db].flags & CTDB_DB_FLAGS_PERSISTENT);
 			if (ret != 0) {
 				DEBUG(DEBUG_ERR, (__location__ " Unable to create remote db:%s\n", name));
 				return -1;
@@ -2035,7 +2037,12 @@ static int do_recovery(struct ctdb_recoverd *rec,
 
 	/* send a message to all clients telling them that the cluster 
 	   has been reconfigured */
-	ctdb_client_send_message(ctdb, CTDB_BROADCAST_CONNECTED, CTDB_SRVID_RECONFIGURE, tdb_null);
+	ret = ctdb_client_send_message(ctdb, CTDB_BROADCAST_CONNECTED,
+				       CTDB_SRVID_RECONFIGURE, tdb_null);
+	if (ret != 0) {
+		DEBUG(DEBUG_ERR, (__location__ " Failed to send reconfigure message\n"));
+		return -1;
+	}
 
 	DEBUG(DEBUG_NOTICE, (__location__ " Recovery complete\n"));
 
@@ -2206,9 +2213,7 @@ static int send_election_request(struct ctdb_recoverd *rec, uint32_t pnn)
 
 	/* send an election message to all active nodes */
 	DEBUG(DEBUG_INFO,(__location__ " Send election request to all active nodes\n"));
-	ctdb_client_send_message(ctdb, CTDB_BROADCAST_ALL, srvid, election_data);
-
-	return 0;
+	return ctdb_client_send_message(ctdb, CTDB_BROADCAST_ALL, srvid, election_data);
 }
 
 /*
@@ -2229,7 +2234,12 @@ static void unban_all_nodes(struct ctdb_context *ctdb)
 	for (i=0;i<nodemap->num;i++) {
 		if ( (!(nodemap->nodes[i].flags & NODE_FLAGS_DISCONNECTED))
 		  && (nodemap->nodes[i].flags & NODE_FLAGS_BANNED) ) {
-			ctdb_ctrl_modflags(ctdb, CONTROL_TIMEOUT(), nodemap->nodes[i].pnn, 0, NODE_FLAGS_BANNED);
+			ret = ctdb_ctrl_modflags(ctdb, CONTROL_TIMEOUT(),
+						 nodemap->nodes[i].pnn, 0,
+						 NODE_FLAGS_BANNED);
+			if (ret != 0) {
+				DEBUG(DEBUG_ERR, (__location__ " failed to reset ban state\n"));
+			}
 		}
 	}
 
@@ -2487,13 +2497,11 @@ static void disable_takeover_runs_handler(struct ctdb_context *ctdb,
 		DEBUG(DEBUG_ERR,(__location__ " Wrong size for data :%lu "
 				 "expecting %lu\n", (long unsigned)data.dsize,
 				 (long unsigned)sizeof(struct srvid_request)));
-		ret = -EINVAL;
-		goto done;
+		return;
 	}
 	if (data.dptr == NULL) {
 		DEBUG(DEBUG_ERR,(__location__ " No data received\n"));
-		ret = -EINVAL;
-		goto done;
+		return;
 	}
 
 	r = (struct srvid_request *)data.dptr;
diff --git a/ctdb/server/ctdb_takeover.c b/ctdb/server/ctdb_takeover.c
index 91f30302d77..04cdd6668d3 100644
--- a/ctdb/server/ctdb_takeover.c
+++ b/ctdb/server/ctdb_takeover.c
@@ -3386,7 +3386,7 @@ int32_t ctdb_control_get_public_ip_info(struct ctdb_context *ctdb,
 		if (vnn->iface == cur) {
 			info->active_idx = i;
 		}
-		strcpy(info->ifaces[i].name, cur->name);
+		strncpy(info->ifaces[i].name, cur->name, sizeof(info->ifaces[i].name)-1);
 		info->ifaces[i].link_state = cur->link_up;
 		info->ifaces[i].references = cur->references;
 	}