[Cluster-devel] [coverity] config

[Cluster-devel] [coverity] config

[Fabio M. Di Nitto]

Original errors/warnings:

Analysis summary report:
------------------------
Files analyzed                 : 21
Total LoC input to cov-analyze : 29195
Functions analyzed             : 170
Paths analyzed                 : 10405
New defects found              : 136 Total
                                   3 CHECKED_RETURN
                                   2 FORWARD_NULL
                                   4 NO_EFFECT
                                  83 NULL_RETURNS
                                   6 PW.INCLUDE_RECURSION
                                   1 PW.MAIN_WRONG_NUM_PARAMS
                                   1 PW.NOT_COMPATIBLE_WITH_PREVIOUS_DECL
                                  10 RESOURCE_LEAK
                                   2 REVERSE_INULL
                                  13 SECURE_CODING
                                   3 SECURE_TEMP
                                   2 STRING_SIZE
                                   1 TOCTOU
                                   5 UNINIT

After fixes:

Analysis summary report:
------------------------
Files analyzed                 : 21
Total LoC input to cov-analyze : 29313
Functions analyzed             : 169
Paths analyzed                 : 10512
New defects found              : 81 Total
                                 66 NULL_RETURNS
                                  6 PW.INCLUDE_RECURSION
                                  1 PW.NOT_COMPATIBLE_WITH_PREVIOUS_DECL
                                  2 SECURE_CODING
                                  1 TOCTOU
                                  5 UNINIT

NOTES:

66 NULL_RETURNS

can be safely ignored (Coverity doesn't understand errno correctly)

 6 PW.INCLUDE_RECURSION

generated by libxml headers

 1 PW.NOT_COMPATIBLE_WITH_PREVIOUS_DECL

coverity internal declaration mismatch

 2 SECURE_CODING

can't be fixed without a public API change and the requirements
for the API are documented (caller must provide enough buffer)
Fits in the category of *bad* but not bad enough to require an API change

 1 TOCTOU

triggered by a possible race condition in ccs edit functionality. Not
worth fixing as it doesn't affect runtime.

 5 UNINIT

can be safely ignored, triggered by FILE * struct not being completely
initialized by fdopen/fopen.

[Cluster-devel] [PATCH 01/19] libccs: check return code consistently

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 577c74a... 703fa17... M	config/libs/libccsconfdb/fullxpath.c
 config/libs/libccsconfdb/fullxpath.c |    9 +++++----
 1 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/config/libs/libccsconfdb/fullxpath.c b/config/libs/libccsconfdb/fullxpath.c
index 577c74a..703fa17 100644
--- a/config/libs/libccsconfdb/fullxpath.c
+++ b/config/libs/libccsconfdb/fullxpath.c
@@ -326,6 +326,11 @@ char *_ccs_get_fullxpath(confdb_handle_t handle, hdb_handle_t connection_handle,
 			}
 		}
 
+		if (list)
+			if (set_previous_query(handle, connection_handle,
+					   (char *)query, OBJECT_PARENT_HANDLE))
+				goto fail;
+
 		rtn = malloc(size);
 
 		if (!rtn) {
@@ -342,10 +347,6 @@ char *_ccs_get_fullxpath(confdb_handle_t handle, hdb_handle_t connection_handle,
 				node->children ? node->children->
 				content : node->name);
 
-		if (list)
-			set_previous_query(handle, connection_handle,
-					   (char *)query, OBJECT_PARENT_HANDLE);
-
 	} else
 		errno = EINVAL;
 
-- 
1.7.4.4

[Cluster-devel] [PATCH 02/19] ccs_config_dump: remove unnecessary pointer to env

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 74c33c2... 022777f... M	config/tools/xml/ccs_config_dump.c
 config/tools/xml/ccs_config_dump.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/config/tools/xml/ccs_config_dump.c b/config/tools/xml/ccs_config_dump.c
index 74c33c2..022777f 100644
--- a/config/tools/xml/ccs_config_dump.c
+++ b/config/tools/xml/ccs_config_dump.c
@@ -154,7 +154,7 @@ static void read_arguments(int argc, char **argv)
 	}
 }
 
-int main(int argc, char *argv[], char *envp[])
+int main(int argc, char *argv[])
 {
 	confdb_handle_t handle = 0;
 	hdb_handle_t cluster_handle;
-- 
1.7.4.4

[Cluster-devel] [PATCH 03/19] ccs_tool: improve error checking on ccs_connect

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 28bbadc... a0fee9f... M	config/tools/ccs_tool/ccs_tool.c
 config/tools/ccs_tool/ccs_tool.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/config/tools/ccs_tool/ccs_tool.c b/config/tools/ccs_tool/ccs_tool.c
index 28bbadc..a0fee9f 100644
--- a/config/tools/ccs_tool/ccs_tool.c
+++ b/config/tools/ccs_tool/ccs_tool.c
@@ -90,6 +90,10 @@ static int test_main(int argc, char *argv[], int old_format){
       exit(EXIT_FAILURE);
     }
     desc = ccs_connect();
+    if (desc < 0) {
+      fprintf(stderr, "ccs_disconnect failed: unable to communicate with ccs\n");
+      exit(EXIT_FAILURE);
+    }
     if((error = ccs_disconnect(desc))){
       fprintf(stderr, "ccs_disconnect failed: %s\n", errstring(-error));
       exit(EXIT_FAILURE);
@@ -173,6 +177,10 @@ static int xpath_query(int argc, char **argv)
 	}
 
 	handle = ccs_connect();
+	if (handle < 0) {
+		fprintf(stderr, "Unable to connect to ccs\n");
+		exit(EXIT_FAILURE);
+	}
 
 	/* Process all the queries on the command-line */
 	for (i=1; i<argc; i++) {
-- 
1.7.4.4

[Cluster-devel] [PATCH 04/19] libccs: fix error checking

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 6a2b143... d9cddc0... M	config/libs/libccsconfdb/libccs.c
 config/libs/libccsconfdb/libccs.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/config/libs/libccsconfdb/libccs.c b/config/libs/libccsconfdb/libccs.c
index 6a2b143..d9cddc0 100644
--- a/config/libs/libccsconfdb/libccs.c
+++ b/config/libs/libccsconfdb/libccs.c
@@ -414,7 +414,7 @@ static int check_cluster_name(int ccs_handle, const char *cluster_name)
 	size_t datalen = 0;
 
 	handle = confdb_connect();
-	if (handle < 0)
+	if (handle == -1)
 		return -1;
 
 	if (confdb_object_find_start(handle, OBJECT_PARENT_HANDLE) != CS_OK) {
@@ -472,7 +472,7 @@ static int _ccs_get(int desc, const char *query, char **rtn, int list)
 	*rtn = NULL;
 
 	handle = confdb_connect();
-	if (handle < 0)
+	if (handle == -1)
 		return -1;
 
 	connection_handle = find_ccs_handle(handle, desc);
-- 
1.7.4.4

[Cluster-devel] [PATCH 05/19] configxml: clear variable usage

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 7dcf8ba... 9479bc2... M	config/plugins/xml/config.c
 config/plugins/xml/config.c |   11 +++++++----
 1 files changed, 7 insertions(+), 4 deletions(-)

diff --git a/config/plugins/xml/config.c b/config/plugins/xml/config.c
index 7dcf8ba..9479bc2 100644
--- a/config/plugins/xml/config.c
+++ b/config/plugins/xml/config.c
@@ -51,10 +51,12 @@ static void xml_comp_register(void)
 	lcr_component_register(&xml_comp_ver0);
 };
 
-static void addkeys(xmlAttrPtr tmpattr, struct objdb_iface_ver0 *objdb,
+static void addkeys(xmlAttrPtr tmpattr_in, struct objdb_iface_ver0 *objdb,
 		    hdb_handle_t object_handle)
 {
-	for (tmpattr = tmpattr; tmpattr; tmpattr = tmpattr->next) {
+	xmlAttrPtr tmpattr;
+
+	for (tmpattr = tmpattr_in; tmpattr; tmpattr = tmpattr->next) {
 		if (tmpattr->type == XML_ATTRIBUTE_NODE)
 			objdb->object_key_create_typed(object_handle,
 						 (char *)tmpattr->name,
@@ -66,12 +68,13 @@ static void addkeys(xmlAttrPtr tmpattr, struct objdb_iface_ver0 *objdb,
 	}
 }
 
-static void xml2objdb(xmlNodePtr tmpnode, struct objdb_iface_ver0 *objdb,
+static void xml2objdb(xmlNodePtr tmpnode_in, struct objdb_iface_ver0 *objdb,
 		      hdb_handle_t parent)
 {
 	hdb_handle_t object_handle = 0;
+	xmlNodePtr tmpnode;
 
-	for (tmpnode = tmpnode; tmpnode; tmpnode = tmpnode->next) {
+	for (tmpnode = tmpnode_in; tmpnode; tmpnode = tmpnode->next) {
 		if (tmpnode->type == XML_ELEMENT_NODE) {
 			objdb->object_create(parent, &object_handle,
 					     (char *)tmpnode->name,
-- 
1.7.4.4

[Cluster-devel] [PATCH 06/19] libccs: remove duplicate checks

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 703fa17... 3b2ab4b... M	config/libs/libccsconfdb/fullxpath.c
 config/libs/libccsconfdb/fullxpath.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/config/libs/libccsconfdb/fullxpath.c b/config/libs/libccsconfdb/fullxpath.c
index 703fa17..3b2ab4b 100644
--- a/config/libs/libccsconfdb/fullxpath.c
+++ b/config/libs/libccsconfdb/fullxpath.c
@@ -62,7 +62,7 @@ static int dump_objdb_buff(confdb_handle_t dump_handle, hdb_handle_t cluster_han
 		return -1;
 	}
 
-	if (!*buffer || ((*buffer) && !strlen(*buffer))) {
+	if (!strlen(*buffer)) {
 		snprintf(temp, PATH_MAX - 1,
 			 "<?xml version=\"1.0\"?>\n<cluster");
 		if (add_to_buffer(temp, buffer, bufsize))
-- 
1.7.4.4

[Cluster-devel] [PATCH 07/19] config: fix libccs return codes and errno handling

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 b0cf55b... 541fed7... M	config/libs/libccsconfdb/extras.c
:100644 100644 3b2ab4b... 6b2e138... M	config/libs/libccsconfdb/fullxpath.c
:100644 100644 d9cddc0... bfc2d29... M	config/libs/libccsconfdb/libccs.c
 config/libs/libccsconfdb/extras.c    |   34 +++++++++++++++++++---------------
 config/libs/libccsconfdb/fullxpath.c |    9 ++++-----
 config/libs/libccsconfdb/libccs.c    |   14 +++++++++++++-
 3 files changed, 36 insertions(+), 21 deletions(-)

diff --git a/config/libs/libccsconfdb/extras.c b/config/libs/libccsconfdb/extras.c
index b0cf55b..541fed7 100644
--- a/config/libs/libccsconfdb/extras.c
+++ b/config/libs/libccsconfdb/extras.c
@@ -38,8 +38,10 @@ int ccs_lookup_nodename(int cd, const char *nodename, char **retval)
 	size_t nodename_len;
 	struct addrinfo hints;
 
-	if (nodename == NULL)
-		return (-1);
+	if (nodename == NULL) {
+		errno = EINVAL;
+		return -1;
+	}
 
 	nodename_len = strlen(nodename);
 	ret = snprintf(path, sizeof(path),
@@ -47,19 +49,19 @@ int ccs_lookup_nodename(int cd, const char *nodename, char **retval)
 		       nodename);
 	if (ret < 0 || (size_t) ret >= sizeof(path)) {
 		errno = E2BIG;
-		return (-E2BIG);
+		return -1;
 	}
 
 	str = NULL;
 	error = ccs_get(cd, path, &str);
 	if (!error) {
 		*retval = str;
-		return (0);
+		return 0;
 	}
 
 	if (nodename_len >= sizeof(host_only)) {
 		errno = E2BIG;
-		return (-E2BIG);
+		return -1;
 	}
 
 	/* Try just the hostname */
@@ -71,14 +73,16 @@ int ccs_lookup_nodename(int cd, const char *nodename, char **retval)
 		ret = snprintf(path, sizeof(path),
 			       "/cluster/clusternodes/clusternode[@name=\"%s\"]/@name",
 			       host_only);
-		if (ret < 0 || (size_t) ret >= sizeof(path))
-			return (-E2BIG);
+		if (ret < 0 || (size_t) ret >= sizeof(path)) {
+			errno = E2BIG;
+			return -1;
+		}
 
 		str = NULL;
 		error = ccs_get(cd, path, &str);
 		if (!error) {
 			*retval = str;
-			return (0);
+			return 0;
 		}
 	}
 
@@ -128,7 +132,7 @@ int ccs_lookup_nodename(int cd, const char *nodename, char **retval)
 				if (strlen(str) >= sizeof(canonical_name)) {
 					free(str);
 					errno = E2BIG;
-					return (-E2BIG);
+					return -1;
 				}
 				strcpy(canonical_name, str);
 			}
@@ -136,7 +140,7 @@ int ccs_lookup_nodename(int cd, const char *nodename, char **retval)
 			if (strlen(str) >= sizeof(cur_node)) {
 				free(str);
 				errno = E2BIG;
-				return (-E2BIG);
+				return -1;
 			}
 
 			strcpy(cur_node, str);
@@ -153,9 +157,9 @@ int ccs_lookup_nodename(int cd, const char *nodename, char **retval)
 				*retval = strdup(canonical_name);
 				if (*retval == NULL) {
 					errno = ENOMEM;
-					return (-ENOMEM);
+					return -1;
 				}
-				return (0);
+				return 0;
 			}
 
 			if (getaddrinfo(str, NULL, &hints, &ai) == 0) {
@@ -178,9 +182,9 @@ int ccs_lookup_nodename(int cd, const char *nodename, char **retval)
 						    strdup(canonical_name);
 						if (*retval == NULL) {
 							errno = ENOMEM;
-							return (-ENOMEM);
+							return -1;
 						}
-						return (0);
+						return 0;
 					}
 				}
 				freeaddrinfo(ai);
@@ -195,7 +199,7 @@ int ccs_lookup_nodename(int cd, const char *nodename, char **retval)
 out_fail:
 	errno = EINVAL;
 	*retval = NULL;
-	return (-1);
+	return -1;
 }
 
 static int facility_id_get(char *name)
diff --git a/config/libs/libccsconfdb/fullxpath.c b/config/libs/libccsconfdb/fullxpath.c
index 3b2ab4b..6b2e138 100644
--- a/config/libs/libccsconfdb/fullxpath.c
+++ b/config/libs/libccsconfdb/fullxpath.c
@@ -148,10 +148,8 @@ static int dump_objdb_buff(confdb_handle_t dump_handle, hdb_handle_t cluster_han
 		res =
 		    dump_objdb_buff(dump_handle, cluster_handle, object_handle, buffer,
 				    bufsize);
-		if (res) {
-			errno = res;
+		if (res)
 			return res;
-		}
 
 		if (object_handle != parent_object_handle) {
 			snprintf(temp, PATH_MAX - 1, "</%s>\n", object_name);
@@ -254,8 +252,7 @@ char *_ccs_get_fullxpath(confdb_handle_t handle, hdb_handle_t connection_handle,
 	unsigned int xmllistindex = 0;
 	int prev = 0;
 	char *rtn = NULL;
-
-	errno = 0;
+	int myerrno;
 
 	if (strncmp(query, "/", 1)) {
 		errno = EINVAL;
@@ -351,8 +348,10 @@ char *_ccs_get_fullxpath(confdb_handle_t handle, hdb_handle_t connection_handle,
 		errno = EINVAL;
 
 fail:
+	myerrno = errno;
 	if (obj)
 		xmlXPathFreeObject(obj);
 
+	errno = myerrno;
 	return rtn;
 }
diff --git a/config/libs/libccsconfdb/libccs.c b/config/libs/libccsconfdb/libccs.c
index d9cddc0..bfc2d29 100644
--- a/config/libs/libccsconfdb/libccs.c
+++ b/config/libs/libccsconfdb/libccs.c
@@ -468,6 +468,7 @@ static int _ccs_get(int desc, const char *query, char **rtn, int list)
 	char data[128];
 	size_t datalen = 0;
 	int fullxpathint = 0;
+	int myerrno;
 
 	*rtn = NULL;
 
@@ -499,8 +500,11 @@ static int _ccs_get(int desc, const char *query, char **rtn, int list)
 		    _ccs_get_fullxpath(handle, connection_handle, query, list);
 
 fail:
+	myerrno = errno;
+
 	confdb_disconnect(handle);
 
+	errno = myerrno;
 	if (!*rtn)
 		return -1;
 
@@ -518,6 +522,7 @@ int ccs_connect(void)
 {
 	confdb_handle_t handle = 0;
 	int ccs_handle = 0;
+	int myerrno;
 
 	handle = confdb_connect();
 	if (handle == -1)
@@ -535,8 +540,12 @@ int ccs_connect(void)
 	}
 
 fail:
+	myerrno = errno;
+
 	confdb_disconnect(handle);
 
+	errno = myerrno;
+
 	return ccs_handle;
 }
 
@@ -584,9 +593,10 @@ int ccs_disconnect(int desc)
 	char data[128];
 	size_t datalen = 0;
 	int fullxpathint = 0;
+	int myerrno;
 
 	handle = confdb_connect();
-	if (handle <= 0)
+	if (handle == -1)
 		return handle;
 
 	connection_handle = find_ccs_handle(handle, desc);
@@ -611,7 +621,9 @@ int ccs_disconnect(int desc)
 	ret = destroy_ccs_handle(handle, connection_handle);
 
 fail:
+	myerrno = errno;
 	confdb_disconnect(handle);
+	errno = myerrno;
 	return ret;
 }
 
-- 
1.7.4.4

[Cluster-devel] [PATCH 08/19] config: improve string error checking in tokenizer

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 0a21f85... 710c176... M	config/libs/libccsconfdb/xpathlite.c
 config/libs/libccsconfdb/xpathlite.c |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/config/libs/libccsconfdb/xpathlite.c b/config/libs/libccsconfdb/xpathlite.c
index 0a21f85..710c176 100644
--- a/config/libs/libccsconfdb/xpathlite.c
+++ b/config/libs/libccsconfdb/xpathlite.c
@@ -15,13 +15,18 @@ static int tokenizer(char *current_query)
 	int tokens = 0;
 	char *curpos = current_query;
 	char *next = NULL;
-	char *end;
+	char *end = NULL;
 
 	end = current_query + strlen(current_query);
 
 	while (curpos <= end) {
 		tokens++;
 
+		if (!curpos) {
+			errno = EINVAL;
+			return -1;
+		}
+
 		if (strncmp(curpos, "/", 1)) {
 			errno = EINVAL;
 			return -1;
@@ -39,11 +44,16 @@ static int tokenizer(char *current_query)
 		if (!next)
 			return tokens;
 
-		if ((strstr(curpos, "[") > next) || !strstr(curpos, "["))
+		if ((strstr(curpos, "[") > next) || !strstr(curpos, "[")) {
 			curpos = next;
-		else
-			curpos = strstr(strstr(curpos, "]"), "/");
-
+		} else {
+			curpos = strstr(curpos, "]");
+			if (!curpos) {
+				errno = EINVAL;
+				return -1;
+			}
+			curpos = strstr(curpos, "/");
+		}
 	}
 	errno = EINVAL;
 	return -1;
-- 
1.7.4.4

[Cluster-devel] [PATCH 09/19] config: fix possible memory leak in libccs fullxpath usage

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 6b2e138... 8c4501f... M	config/libs/libccsconfdb/fullxpath.c
 config/libs/libccsconfdb/fullxpath.c |   14 ++++++--------
 1 files changed, 6 insertions(+), 8 deletions(-)

diff --git a/config/libs/libccsconfdb/fullxpath.c b/config/libs/libccsconfdb/fullxpath.c
index 6b2e138..8c4501f 100644
--- a/config/libs/libccsconfdb/fullxpath.c
+++ b/config/libs/libccsconfdb/fullxpath.c
@@ -174,10 +174,10 @@ static int dump_objdb_buff(confdb_handle_t dump_handle, hdb_handle_t cluster_han
 int xpathfull_init(confdb_handle_t handle)
 {
 	int size = XMLBUFSIZE;
-	char *buffer, *newbuf;
+	char *buffer;
 	hdb_handle_t cluster_handle;
 
-	newbuf = buffer = malloc(XMLBUFSIZE);
+	buffer = malloc(XMLBUFSIZE);
 	if (!buffer) {
 		errno = ENOMEM;
 		goto fail;
@@ -191,14 +191,9 @@ int xpathfull_init(confdb_handle_t handle)
 	if (confdb_object_find(handle, OBJECT_PARENT_HANDLE, "cluster", strlen("cluster"), &cluster_handle) != CS_OK)
 		goto fail;
 
-	if (dump_objdb_buff(handle, cluster_handle, cluster_handle, &newbuf, &size))
+	if (dump_objdb_buff(handle, cluster_handle, cluster_handle, &buffer, &size))
 		goto fail;
 
-	if (newbuf != buffer) {
-		buffer = newbuf;
-		newbuf = NULL;
-	}
-
 	doc = xmlParseMemory(buffer, strlen(buffer));
 	if (!doc)
 		goto fail;
@@ -214,6 +209,9 @@ int xpathfull_init(confdb_handle_t handle)
 	return 0;
 
 fail:
+	if (buffer)
+		free(buffer);
+
 	return -1;
 }
 
-- 
1.7.4.4

[Cluster-devel] [PATCH 10/19] config: fix extremely unlikely buffer overflow

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 541fed7... 12c5320... M	config/libs/libccsconfdb/extras.c
 config/libs/libccsconfdb/extras.c |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/config/libs/libccsconfdb/extras.c b/config/libs/libccsconfdb/extras.c
index 541fed7..12c5320 100644
--- a/config/libs/libccsconfdb/extras.c
+++ b/config/libs/libccsconfdb/extras.c
@@ -227,7 +227,7 @@ static int priority_id_get(char *name)
 }
 
 /* requires string buffer to be PATH_MAX */
-static void read_string(int fd, const char *path, char *string)
+static void read_string(int fd, const char *path, char *string, size_t string_s)
 {
 	char *str;
 	int error;
@@ -238,7 +238,7 @@ static void read_string(int fd, const char *path, char *string)
 	if (error || !str)
 		return;
 
-	strcpy(string, str);
+	strncpy(string, str, string_s - 1);
 
 	free(str);
 }
@@ -345,7 +345,8 @@ void ccs_read_logging(int fd, const char *name, int *debug, int *mode,
 	 */
 	create_daemon_path(name, "syslog_facility", path);
 
-	read_string(fd, "/cluster/logging/@syslog_facility", string);
+	read_string(fd, "/cluster/logging/@syslog_facility",
+		    string, sizeof(string));
 
 	if (string[0]) {
 		val = facility_id_get(string);
@@ -353,7 +354,7 @@ void ccs_read_logging(int fd, const char *name, int *debug, int *mode,
 			*syslog_facility = val;
 	}
 
-	read_string(fd, path, string);
+	read_string(fd, path, string, sizeof(string));
 
 	if (string[0]) {
 		val = facility_id_get(string);
@@ -366,7 +367,8 @@ void ccs_read_logging(int fd, const char *name, int *debug, int *mode,
 	 */
 	create_daemon_path(name, "syslog_priority", path);
 
-	read_string(fd, "/cluster/logging/@syslog_priority", string);
+	read_string(fd, "/cluster/logging/@syslog_priority",
+		    string, sizeof(string));
 
 	if (string[0]) {
 		val = priority_id_get(string);
@@ -374,7 +376,7 @@ void ccs_read_logging(int fd, const char *name, int *debug, int *mode,
 			*syslog_priority = val;
 	}
 
-	read_string(fd, path, string);
+	read_string(fd, path, string, sizeof(string));
 
 	if (string[0]) {
 		val = priority_id_get(string);
@@ -387,12 +389,12 @@ void ccs_read_logging(int fd, const char *name, int *debug, int *mode,
 	 */
 	create_daemon_path(name, "logfile", path);
 
-	read_string(fd, "/cluster/logging/@logfile", string);
+	read_string(fd, "/cluster/logging/@logfile", string, sizeof(string));
 
 	if (string[0])
 		strcpy(logfile, string);
 
-	read_string(fd, path, string);
+	read_string(fd, path, string, sizeof(string));
 
 	if (string[0])
 		strcpy(logfile, string);
@@ -432,7 +434,8 @@ void ccs_read_logging(int fd, const char *name, int *debug, int *mode,
 	 */
 	create_daemon_path(name, "logfile_priority", path);
 
-	read_string(fd, "/cluster/logging/@logfile_priority", string);
+	read_string(fd, "/cluster/logging/@logfile_priority",
+		    string, sizeof(string));
 
 	if (string[0]) {
 		val = priority_id_get(string);
@@ -440,7 +443,7 @@ void ccs_read_logging(int fd, const char *name, int *debug, int *mode,
 			*logfile_priority = val;
 	}
 
-	read_string(fd, path, string);
+	read_string(fd, path, string, sizeof(string));
 
 	if (string[0]) {
 		val = priority_id_get(string);
-- 
1.7.4.4

[Cluster-devel] [PATCH 11/19] config: fix a few theoretical buffer overflows

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 12c5320... 12536ae... M	config/libs/libccsconfdb/extras.c
 config/libs/libccsconfdb/extras.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/config/libs/libccsconfdb/extras.c b/config/libs/libccsconfdb/extras.c
index 12c5320..12536ae 100644
--- a/config/libs/libccsconfdb/extras.c
+++ b/config/libs/libccsconfdb/extras.c
@@ -65,7 +65,7 @@ int ccs_lookup_nodename(int cd, const char *nodename, char **retval)
 	}
 
 	/* Try just the hostname */
-	strcpy(host_only, nodename);
+	strncpy(host_only, nodename, sizeof(host_only) - 1);
 	p = strchr(host_only, '.');
 	if (p != NULL) {
 		*p = '\0';
@@ -134,7 +134,7 @@ int ccs_lookup_nodename(int cd, const char *nodename, char **retval)
 					errno = E2BIG;
 					return -1;
 				}
-				strcpy(canonical_name, str);
+				strncpy(canonical_name, str, sizeof(canonical_name) - 1);
 			}
 
 			if (strlen(str) >= sizeof(cur_node)) {
@@ -143,7 +143,7 @@ int ccs_lookup_nodename(int cd, const char *nodename, char **retval)
 				return -1;
 			}
 
-			strcpy(cur_node, str);
+			strncpy(cur_node, str, sizeof(cur_node) - 1);
 
 			p = strchr(cur_node, '.');
 			if (p != NULL)
-- 
1.7.4.4

[Cluster-devel] [PATCH 12/19] config: fix return checks in ldap loader

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 fb25af4... 49c30b7... M	config/plugins/ldap/configldap.c
 config/plugins/ldap/configldap.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/config/plugins/ldap/configldap.c b/config/plugins/ldap/configldap.c
index fb25af4..49c30b7 100644
--- a/config/plugins/ldap/configldap.c
+++ b/config/plugins/ldap/configldap.c
@@ -216,6 +216,8 @@ static int read_config_for(LDAP *ld, struct objdb_iface_ver0 *objdb, hdb_handle_
 				struct berval **val_ber;
 
 				val_ber = ldap_get_values_len(ld, e, attr);
+				if (!val_ber)
+					goto ldap_next;
 				i=0;
 				while (val_ber[i]) {
 					/*
@@ -244,9 +246,10 @@ static int read_config_for(LDAP *ld, struct objdb_iface_ver0 *objdb, hdb_handle_
 					}
 					i++;
 				}
+				ldap_value_free_len(val_ber);
+ldap_next:
 				ldap_memfree(attr);
 				attr = ldap_next_attribute(ld, e, attr_ber);
-				ldap_value_free_len(val_ber);
 			}
 			ldap_memfree(attr);
 			ber_free(attr_ber, 0);
-- 
1.7.4.4

[Cluster-devel] [PATCH 13/19] config: make sure error reporting does not overflow buffers

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 49c30b7... c9ba2bf... M	config/plugins/ldap/configldap.c
 config/plugins/ldap/configldap.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/config/plugins/ldap/configldap.c b/config/plugins/ldap/configldap.c
index 49c30b7..c9ba2bf 100644
--- a/config/plugins/ldap/configldap.c
+++ b/config/plugins/ldap/configldap.c
@@ -67,7 +67,7 @@ static int ldap_readconfig(struct objdb_iface_ver0 *objdb, const char **error_st
 
 	/* Read config tree from LDAP */
 	if (!(ret = init_config(objdb)))
-	    sprintf(error_reason, "%s", "Successfully read config from LDAP\n");
+	    snprintf(error_reason, sizeof(error_reason), "%s", "Successfully read config from LDAP\n");
 
         *error_string = error_reason;
 
@@ -156,13 +156,13 @@ static int read_config_for(LDAP *ld, struct objdb_iface_ver0 *objdb, hdb_handle_
 	hdb_handle_t parent_handle = OBJECT_PARENT_HANDLE;
 	hdb_handle_t object_handle;
 
-	sprintf(search_dn, "%s,%s", sub_dn, ldap_basedn);
+	snprintf(search_dn, sizeof(search_dn), "%s,%s", sub_dn, ldap_basedn);
 
 	/* Search the whole tree from the base DN provided */
 	rc = ldap_search_ext_s(ld, search_dn, LDAP_SCOPE_SUBTREE, "(objectClass=*)", NULL, 0,
 			       NULL, NULL, NULL, 0, &result);
 	if (rc != LDAP_SUCCESS) {
-		sprintf(error_reason, "ldap_search_ext_s: %s\n", ldap_err2string(rc));
+		snprintf(error_reason, sizeof(error_reason), "ldap_search_ext_s: %s\n", ldap_err2string(rc));
 		if (rc == LDAP_NO_SUCH_OBJECT)
 			return 0;
 		else
@@ -177,7 +177,7 @@ static int read_config_for(LDAP *ld, struct objdb_iface_ver0 *objdb, hdb_handle_
 
 			/* Make it parsable so we can discern the hierarchy */
 			if (ldap_str2dn(dn, &parsed_dn, LDAP_DN_PEDANTIC)) {
-				sprintf(error_reason, "ldap_str2dn failed: %s\n", ldap_err2string(rc));
+				snprintf(error_reason, sizeof(error_reason), "ldap_str2dn failed: %s\n", ldap_err2string(rc));
 				return -1;
 			}
 
@@ -273,7 +273,7 @@ static int init_config(struct objdb_iface_ver0 *objdb)
 
 	/* Connect to the LDAP server */
 	if (ldap_initialize(&ld, ldap_url)) {
-		sprintf(error_reason, "ldap_initialize failed: %s\n", strerror(errno));
+		snprintf(error_reason, sizeof(error_reason), "ldap_initialize failed: %s\n", strerror(errno));
 		return -1;
 	}
 	version = LDAP_VERSION3;
@@ -284,7 +284,7 @@ static int init_config(struct objdb_iface_ver0 *objdb)
 	 */
 	rc = ldap_simple_bind_s(ld, getenv("COROSYNC_LDAP_BINDDN"), getenv("COROSYNC_LDAP_BINDPWD"));
 	if (rc != LDAP_SUCCESS) {
-		sprintf(error_reason, "ldap_simple_bind failed: %s\n", ldap_err2string(rc));
+		snprintf(error_reason, sizeof(error_reason), "ldap_simple_bind failed: %s\n", ldap_err2string(rc));
 		return -1;
 	}
 
-- 
1.7.4.4

[Cluster-devel] [PATCH 14/19] config: make sure error reporting does not overflow buffers

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 9479bc2... a25df09... M	config/plugins/xml/config.c
 config/plugins/xml/config.c |    8 ++++----
 1 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/config/plugins/xml/config.c b/config/plugins/xml/config.c
index 9479bc2..a25df09 100644
--- a/config/plugins/xml/config.c
+++ b/config/plugins/xml/config.c
@@ -106,11 +106,11 @@ static int xml_readconfig(struct objdb_iface_ver0 *objdb, const char **error_str
 
 	/* Read low-level totem/aisexec etc config from cluster.conf */
 	if (!(ret = init_config(objdb, configfile, error_reason)))
-		sprintf(error_reason, "Successfully read config from %s\n",
-			configfile);
+		snprintf(error_reason, sizeof(error_reason),
+			 "Successfully read config from %s\n", configfile);
 	else
-		sprintf(error_reason, "Unable to read config from %s\n",
-			configfile);
+		snprintf(error_reason, sizeof(error_reason),
+			"Unable to read config from %s\n", configfile);
 
 	*error_string = error_reason;
 
-- 
1.7.4.4

[Cluster-devel] [PATCH 15/19] config: fix rng2ldif null reference

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 1de9181... 7a99417... M	config/tools/ldap/rng2ldif/value-list.c
 config/tools/ldap/rng2ldif/value-list.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/config/tools/ldap/rng2ldif/value-list.c b/config/tools/ldap/rng2ldif/value-list.c
index 1de9181..7a99417 100644
--- a/config/tools/ldap/rng2ldif/value-list.c
+++ b/config/tools/ldap/rng2ldif/value-list.c
@@ -158,6 +158,7 @@ id_readfile(struct idinfo *oi, char *filename)
 		assert(v->name);
 
 		c = valp;
+		assert(c);
 
 		valp = strchr(c, ',');
 		v->value = 0;
-- 
1.7.4.4

[Cluster-devel] [PATCH 16/19] config: rng2ldif bug fixes

[Fabio M. Di Nitto]

several differnt memory leaks, missing checks and other minor issues.

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 7607714... dd69401... M	config/tools/ldap/rng2ldif/genclass.c
:100644 100644 6b24dd2... 635d4c8... M	config/tools/ldap/rng2ldif/rng2ldif.c
:100644 100644 4c6c28f... 7c63124... M	config/tools/ldap/rng2ldif/tree.c
:100644 100644 7a99417... 010503a... M	config/tools/ldap/rng2ldif/value-list.c
 config/tools/ldap/rng2ldif/genclass.c   |    4 ++
 config/tools/ldap/rng2ldif/rng2ldif.c   |   10 ++++++
 config/tools/ldap/rng2ldif/tree.c       |   54 +++++++++++++++++++++++++-----
 config/tools/ldap/rng2ldif/value-list.c |   18 +++++++++-
 4 files changed, 75 insertions(+), 11 deletions(-)

diff --git a/config/tools/ldap/rng2ldif/genclass.c b/config/tools/ldap/rng2ldif/genclass.c
index 7607714..dd69401 100644
--- a/config/tools/ldap/rng2ldif/genclass.c
+++ b/config/tools/ldap/rng2ldif/genclass.c
@@ -1,6 +1,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/types.h>
+#include <sys/stat.h>
 #include <string.h>
 #include <unistd.h>
 
@@ -40,12 +41,15 @@ write_class_struct(char *csv, char *arg, struct idinfo *ids)
 	char filename[4096];
 	FILE *out = NULL;
 	int fd = -1;
+	mode_t oldumask;
 
 	if (!strcmp(arg, "-")) {
 		out = stdout;
 	} else {
+		oldumask=umask(S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
 		snprintf(filename, sizeof(filename), "%s.XXXXXX", arg);
 		fd = mkstemp(filename);
+		umask(oldumask);
 		if (fd < 0) {
 			perror("mkstemp");
 			return -1;
diff --git a/config/tools/ldap/rng2ldif/rng2ldif.c b/config/tools/ldap/rng2ldif/rng2ldif.c
index 6b24dd2..635d4c8 100644
--- a/config/tools/ldap/rng2ldif/rng2ldif.c
+++ b/config/tools/ldap/rng2ldif/rng2ldif.c
@@ -1,6 +1,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <sys/types.h>
+#include <sys/stat.h>
 #include <string.h>
 #include <unistd.h>
 #include <time.h>
@@ -118,6 +119,12 @@ open_relaxng(const char *filename)
 	}
 
 	n = xmlDocGetRootElement(p);
+	if (!n) {
+		printf("Unable to determine xml root element\n");
+		xmlFreeDoc(p);
+		return NULL;
+	}
+
 	if (xmlStrcmp(n->name, (xmlChar *)"grammar")) {
 		printf("%s is not a relaxng grammar\n", filename);
 		xmlFreeDoc(p);
@@ -141,12 +148,15 @@ write_ldap_schema(const char *rng, const char *arg,
 	time_t now;
 	struct tm now_tm;
 	int fd = -1;
+	mode_t oldumask;
 
 	if (!strcmp(arg, "-")) {
 		out_ldap = stdout;
 	} else {
+		oldumask=umask(S_IRGRP | S_IWGRP | S_IROTH | S_IWOTH);
 		snprintf(filename, sizeof(filename), "%s.XXXXXX", arg);
 		fd = mkstemp(filename);
+		umask(oldumask);
 		if (fd < 0) {
 			perror("mkstemp");
 			return -1;
diff --git a/config/tools/ldap/rng2ldif/tree.c b/config/tools/ldap/rng2ldif/tree.c
index 4c6c28f..7c63124 100644
--- a/config/tools/ldap/rng2ldif/tree.c
+++ b/config/tools/ldap/rng2ldif/tree.c
@@ -95,6 +95,10 @@ get_attr(xmlNodePtr curr_node, struct ldap_attr_node **attrs,
 	char *name, *normalized;
 
 	name = (char *)xmlGetProp(curr_node, (xmlChar *)"name");
+	if (!name) {
+		return NULL;
+	}
+
 	normalized = normalize_name((const char *)name);
 
 	n = find_attr_byname(*attrs, normalized);
@@ -152,8 +156,16 @@ find_ref(xmlNodePtr curr_node)
 
 	dbg_printf("Trying to parse ref tag\n");
 	name = (char *)xmlGetProp(curr_node, (xmlChar *)"name");
+	if (!name) {
+		fprintf(stderr, "Unable to determine xml name prop\n");
+		exit(1);
+	}
 
 	n = xmlDocGetRootElement(curr_node->doc);
+	if (!n) {
+		fprintf(stderr, "Unable to determine xml root element\n");
+		exit(1);
+	}
 	n = n->xmlChildrenNode;
 	for (; n; n = n->next) {
 		if (n->type != XML_ELEMENT_NODE)
@@ -189,7 +201,7 @@ find_optional_attributes(xmlNodePtr curr_node, int in_block,
 {
 	xmlNodePtr node;
 	struct ldap_attr_node *attr;
-	struct ldap_attr_meta_node *n;
+	struct ldap_attr_meta_node *n = NULL;
 
 	if (!curr_node || (curr_node->type == XML_ELEMENT_NODE &&
 	    (curr_node->name && !strcasecmp((char *)curr_node->name, "element")))) {
@@ -201,6 +213,13 @@ find_optional_attributes(xmlNodePtr curr_node, int in_block,
 	for (node = curr_node; node; node = node->next) {
 		if (node->type != XML_ELEMENT_NODE)
 			continue;
+
+		if (!node->name) 
+			continue;
+
+		if (strcmp((char *)node->name, "attribute"))
+			continue;
+
 		if (!strcasecmp((char *)node->name, "ref")) {
 			find_optional_attributes(
 				find_ref(node), 1, curr_obj, attrs, ids);
@@ -224,16 +243,14 @@ find_optional_attributes(xmlNodePtr curr_node, int in_block,
 			continue;
 		}
 
-		if (!node->name || strcmp((char *)node->name,
-			    "attribute")) {
-			continue;
-		}
-
 		if (!in_block)
 			continue;
 
 		attr = get_attr(node, attrs, ids);
-		n = zalloc(sizeof(*n));
+		if (!n) {
+			n = zalloc(sizeof(*n));
+			assert(n);
+		}
 
 		dbg_printf("opt attr '%s'\n", attr->idval->name);
 
@@ -251,6 +268,12 @@ find_optional_attributes(xmlNodePtr curr_node, int in_block,
 		n->node = attr;
 		n->next = curr_obj->optional_attrs;
 		curr_obj->optional_attrs = n;
+		n=NULL;
+	}
+
+	if (n) {
+		free(n);
+		n=NULL;
 	}
 	return 0;
 }
@@ -264,7 +287,7 @@ find_required_attributes(xmlNodePtr curr_node,
 {
 	xmlNodePtr node;
 	struct ldap_attr_node *attr;
-	struct ldap_attr_meta_node *n;
+	struct ldap_attr_meta_node *n = NULL;
 
 	dbg_printf("lookin for required\n");
 
@@ -275,7 +298,10 @@ find_required_attributes(xmlNodePtr curr_node,
 			continue;
 
 		attr = get_attr(node, attrs, ids);
-		n = zalloc(sizeof(*n));
+		if (!n) {
+			n = zalloc(sizeof(*n));
+			assert(n);
+		}
 
 		dbg_printf("req attr '%s'\n", attr->idval->name);
 
@@ -293,6 +319,12 @@ find_required_attributes(xmlNodePtr curr_node,
 		n->node = attr;
 		n->next = curr_obj->required_attrs;
 		curr_obj->required_attrs = n;
+		n=NULL;
+	}
+
+	if (n) {
+		free(n);
+		n=NULL;
 	}
 	return 0;
 }
@@ -311,6 +343,10 @@ parse_element_tag(xmlNodePtr curr_node,
 	
 	dbg_printf("Trying to parse element tag\n");
 	n = (char *)xmlGetProp(curr_node, (xmlChar *)"name");
+	if (!n) {
+		printf("Unable to parse element tag\n");
+		exit(1);
+	}
 	normalized = normalize_name(n);
 	v = id_find(ids, normalized, OBJ, 0);
 
diff --git a/config/tools/ldap/rng2ldif/value-list.c b/config/tools/ldap/rng2ldif/value-list.c
index 7a99417..010503a 100644
--- a/config/tools/ldap/rng2ldif/value-list.c
+++ b/config/tools/ldap/rng2ldif/value-list.c
@@ -3,6 +3,8 @@
 #include <assert.h>
 #include <unistd.h>
 #include <string.h>
+#include <sys/types.h>
+#include <sys/stat.h>
 #include "value-list.h"
 #include "zalloc.h"
 
@@ -71,11 +73,15 @@ int
 id_writefile(struct idinfo *oi, char *filename)
 {
 	char tmpfn[4096];
-	FILE *fp;
+	FILE *fp = NULL;
 	int fd;
+	mode_t oldumask;
+
+	oldumask=umask(S_IWGRP | S_IRGRP | S_IWOTH | S_IROTH);
 
 	snprintf(tmpfn, sizeof(tmpfn), "%s.XXXXXX", filename);
 	fd = mkstemp(tmpfn);
+	umask(oldumask);
 	if (fd < 0)
 		return -1;
 
@@ -109,8 +115,13 @@ id_readfile(struct idinfo *oi, char *filename)
 	char buf[4096];
 	int len, lineno = 0, entries = 0;
 
-	fp = fopen(filename, "r");
 	if (!filename) {
+		perror("no file?");
+		return 1;
+	}
+
+	fp = fopen(filename, "r");
+	if (!fp) {
 		perror("fopen");
 		return 1;
 	}
@@ -127,6 +138,7 @@ id_readfile(struct idinfo *oi, char *filename)
 			--len;
 		}
 		v = zalloc(sizeof(*v));
+		assert(v);
 
 		/* Attribute / object */
 		c = strchr(buf, ',');
@@ -184,6 +196,8 @@ id_readfile(struct idinfo *oi, char *filename)
 		}
 
 		++entries;
+		free(v);
+		v = NULL;
 	}
 
 	fclose(fp);
-- 
1.7.4.4

[Cluster-devel] [PATCH 17/19] config: fix confdb2ldif secure coding

[Fabio M. Di Nitto]

Spotted by Coverify Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 35f8c31... 1f83c2f... M	config/tools/ldap/confdb2ldif.c
 config/tools/ldap/confdb2ldif.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/config/tools/ldap/confdb2ldif.c b/config/tools/ldap/confdb2ldif.c
index 35f8c31..1f83c2f 100644
--- a/config/tools/ldap/confdb2ldif.c
+++ b/config/tools/ldap/confdb2ldif.c
@@ -20,7 +20,7 @@ static const char *ldap_attr_name(const char *attrname)
 	if (strcmp(attrname, "name") == 0)
 		return attrname;
 
-	sprintf(newname, "rhcs");
+	snprintf(newname, sizeof(newname) - 1, "rhcs");
 	for (i=0; i<strlen(attrname)+1; i++) {
 		if (i == 0)
 			newname[4+i] = attrname[i] & 0x5F;
@@ -99,16 +99,16 @@ static void print_config_tree(confdb_handle_t handle, hdb_handle_t parent_object
 		/* Check for "name", and create dummy parent object */
 		res = confdb_key_get(handle, object_handle, "name", strlen("name"), key_value, &key_value_len);
 		if (res == CS_OK) {
-			sprintf(cumulative_dn, "cn=%s,%s", object_name, fulldn);
+			snprintf(cumulative_dn, sizeof(cumulative_dn) - 1, "cn=%s,%s", object_name, fulldn);
 			printf("\n");
 			printf("dn: %s\n", cumulative_dn);
 			printf("cn: %s\n", object_name);
 			printf("objectclass: %s\n", "nsContainer");
 
-			sprintf(cumulative_dn, "name=%s,cn=%s,%s", key_value, object_name, fulldn);
+			snprintf(cumulative_dn, sizeof(cumulative_dn) - 1, "name=%s,cn=%s,%s", key_value, object_name, fulldn);
 		}
 		else {
-			sprintf(cumulative_dn, "cn=%s,%s", object_name, fulldn);
+			snprintf(cumulative_dn, sizeof(cumulative_dn) - 1, "cn=%s,%s", object_name, fulldn);
 		}
 
 		/* Down we go ... */
@@ -162,7 +162,7 @@ int main(int argc, char *argv[])
 		exit (1);
 	}
 
-	sprintf(basedn, "name=%s,%s", clusterroot, argv[1]);
+	snprintf(basedn, sizeof(basedn) - 1, "name=%s,%s", clusterroot, argv[1]);
 
 	/* Print a header */
 	printf("# This file was generated by confdb2ldif, from an existing cluster configuration\n");
-- 
1.7.4.4

[Cluster-devel] [PATCH 18/19] config: remove unused var in ccs_tool

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 a0fee9f... 34c4c1e... M	config/tools/ccs_tool/ccs_tool.c
 config/tools/ccs_tool/ccs_tool.c |   20 +++++---------------
 1 files changed, 5 insertions(+), 15 deletions(-)

diff --git a/config/tools/ccs_tool/ccs_tool.c b/config/tools/ccs_tool/ccs_tool.c
index a0fee9f..34c4c1e 100644
--- a/config/tools/ccs_tool/ccs_tool.c
+++ b/config/tools/ccs_tool/ccs_tool.c
@@ -9,15 +9,6 @@
 #include "ccs.h"
 
 
-/*
- * Old libccs retruned -error (mostly!) but didn't set errno (sigh)
- * New libccs sets errno correctly
- */
-static char *errstring(int retcode)
-{
-	return strerror(errno);
-}
-
 static void tool_print_usage(FILE *stream);
 
 int globalverbose=0;
@@ -27,7 +18,6 @@ static void test_print_usage(FILE *stream);
 static int test_main(int argc, char *argv[], int old_format){
   int desc=0;
   int i=0;
-  int error = 0;
   int force = 0, blocking = 0;
   char *str=NULL;
   char *cluster_name = NULL;
@@ -76,7 +66,7 @@ static int test_main(int argc, char *argv[], int old_format){
       desc = ccs_connect();
     }
     if(desc < 0){
-      fprintf(stderr, "ccs_connect failed: %s\n", errstring(-desc));
+      fprintf(stderr, "ccs_connect failed: %s\n", strerror(errno));
       exit(EXIT_FAILURE);
     } else {
       printf("Connect successful.\n");
@@ -94,8 +84,8 @@ static int test_main(int argc, char *argv[], int old_format){
       fprintf(stderr, "ccs_disconnect failed: unable to communicate with ccs\n");
       exit(EXIT_FAILURE);
     }
-    if((error = ccs_disconnect(desc))){
-      fprintf(stderr, "ccs_disconnect failed: %s\n", errstring(-error));
+    if(ccs_disconnect(desc)){
+      fprintf(stderr, "ccs_disconnect failed: %s\n", strerror(errno));
       exit(EXIT_FAILURE);
     } else {
       printf("Disconnect successful.\n");
@@ -107,8 +97,8 @@ static int test_main(int argc, char *argv[], int old_format){
       exit(EXIT_FAILURE);
     }
     desc = ccs_connect();
-    if((desc < 0) || (error = ccs_get(desc, argv[3], &str))){
-      fprintf(stderr, "ccs_get failed: %s\n", errstring(-error));
+    if((desc < 0) || ccs_get(desc, argv[3], &str)){
+      fprintf(stderr, "ccs_get failed: %s\n", strerror(errno));
       exit(EXIT_FAILURE);
     } else {
 	    if (old_format) {
-- 
1.7.4.4

[Cluster-devel] [PATCH 19/19] config: fix a few checks in ccs_tool edit functionalities

[Fabio M. Di Nitto]

Spotted by Coverity Scan

Signed-off-by: Fabio M. Di Nitto <fdinitto@redhat.com>
---
:100644 100644 fe52665... 9415506... M	config/tools/ccs_tool/editconf.c
 config/tools/ccs_tool/editconf.c |   40 +++++++++++++++++++++++++++++++------
 1 files changed, 33 insertions(+), 7 deletions(-)

diff --git a/config/tools/ccs_tool/editconf.c b/config/tools/ccs_tool/editconf.c
index fe52665..9415506 100644
--- a/config/tools/ccs_tool/editconf.c
+++ b/config/tools/ccs_tool/editconf.c
@@ -292,7 +292,7 @@ static void save_file(xmlDoc *doc, struct option_info *ninfo)
 	}
 	else
 	{
-		strcpy(tmpconffile, ninfo->outputfile);
+		strncpy(tmpconffile, ninfo->outputfile, sizeof(tmpconffile));
 	}
 
 	xmlKeepBlanksDefault(0);
@@ -431,6 +431,7 @@ static xmlNode *valid_fence_type(xmlNode *root, const char *fencetype)
 		if (cur_node->type == XML_ELEMENT_NODE && strcmp((char *)cur_node->name, "fencedevice") == 0)
 		{
 			xmlChar *name = xmlGetProp(cur_node, BAD_CAST "name");
+			assert(name);
 			if (strcmp((char *)name, fencetype) == 0)
 				return cur_node;
 		}
@@ -1268,6 +1269,7 @@ void add_nodeids(int argc, char **argv)
 	doc = open_configfile(&ninfo);
 
 	root_element = xmlDocGetRootElement(doc);
+	assert(root_element);
 
 	increment_version(root_element);
 
@@ -1332,10 +1334,11 @@ void add_nodeids(int argc, char **argv)
 		{
 			char tmp[80];
 			xmlChar *name = xmlGetProp(cur_node, BAD_CAST "name");
+			assert(name);
 
 			assert(strcmp((char*)nodenames[nodeidx], (char*)name) == 0);
 
-			sprintf(tmp, "%d", nodenumbers[nodeidx]);
+			snprintf(tmp, sizeof(tmp), "%d", nodenumbers[nodeidx]);
 			xmlSetProp(cur_node, BAD_CAST "nodeid", BAD_CAST tmp);
 			nodeidx++;
 		}
@@ -1404,6 +1407,7 @@ void add_node(int argc, char **argv)
 	doc = open_configfile(&ninfo);
 
 	root_element = xmlDocGetRootElement(doc);
+	assert(root_element);
 
 	increment_version(root_element);
 
@@ -1434,6 +1438,7 @@ void del_node(int argc, char **argv)
 	doc = open_configfile(&ninfo);
 
 	root_element = xmlDocGetRootElement(doc);
+	assert(root_element);
 
 	increment_version(root_element);
 
@@ -1486,7 +1491,7 @@ void list_nodes(int argc, char **argv)
 	doc = open_configfile(&ninfo);
 
 	root_element = xmlDocGetRootElement(doc);
-
+	assert(root_element);
 
 	printf("\nCluster name: %s, config_version: %s\n\n",
 	       (char *)cluster_name(root_element),
@@ -1604,6 +1609,7 @@ void add_service(int argc, char **argv)
 	doc = open_configfile(&ninfo);
 
 	root_element = xmlDocGetRootElement(doc);
+	assert(root_element);
 
 	increment_version(root_element);
 
@@ -1646,7 +1652,7 @@ void list_services(int argc, char **argv)
 	doc = open_configfile(&ninfo);
 
 	root_element = xmlDocGetRootElement(doc);
-
+	assert(root_element);
 
 	printf("\nCluster name: %s, config_version: %s\n\n",
 	       (char *)cluster_name(root_element),
@@ -1702,6 +1708,7 @@ void add_script(int argc, char **argv)
 	doc = open_configfile(&ninfo);
 
 	root_element = xmlDocGetRootElement(doc);
+	assert(root_element);
 
 	increment_version(root_element);
 
@@ -1750,6 +1757,7 @@ void add_ip(int argc, char **argv)
 	doc = open_configfile(&ninfo);
 
 	root_element = xmlDocGetRootElement(doc);
+	assert(root_element);
 
 	increment_version(root_element);
 
@@ -1838,6 +1846,7 @@ void add_fs(int argc, char **argv)
 	doc = open_configfile(&ninfo);
 
 	root_element = xmlDocGetRootElement(doc);
+	assert(root_element);
 
 	increment_version(root_element);
 
@@ -1887,8 +1896,14 @@ void add_fdomain(int argc, char **argv)
 	if (optind < argc - 1) {
 		ninfo.name = strdup(argv[optind]);
 		ninfo.failover_nodes = (const char **)malloc(sizeof(char *) * (argc - optind));
-		for (i = 0; i < argc - optind - 1; i++)
+		if (!ninfo.failover_nodes) {
+			fprintf(stdout, "Out of mem!\n");
+			exit(EXIT_FAILURE);
+		}
+		for (i = 0; i < argc - optind - 1; i++) {
 			ninfo.failover_nodes[i] = strdup(argv[i + optind + 1]);
+			assert(ninfo.failover_nodes[i]);
+		}
 		ninfo.failover_nodes[i] = NULL;
 	} else
 		addfdomain_usage(argv[0]);
@@ -1896,6 +1911,7 @@ void add_fdomain(int argc, char **argv)
 	doc = open_configfile(&ninfo);
 
 	root_element = xmlDocGetRootElement(doc);
+	assert(root_element);
 
 	increment_version(root_element);
 
@@ -1905,6 +1921,11 @@ void add_fdomain(int argc, char **argv)
 	save_file(doc, &ninfo);
 	/* Shutdown libxml */
 	xmlCleanupParser();
+
+	for (i = 0; i < argc - optind - 1; i++)
+		free((void *)ninfo.failover_nodes[i]);
+
+	free(ninfo.failover_nodes);
 }
 
 void create_skeleton(int argc, char **argv)
@@ -2018,6 +2039,7 @@ void add_fence(int argc, char **argv)
 
 	doc = open_configfile(&ninfo);
 	root_element = xmlDocGetRootElement(doc);
+	assert(root_element);
 
 	increment_version(root_element);
 
@@ -2065,6 +2087,7 @@ void del_fence(int argc, char **argv)
 
 	doc = open_configfile(&ninfo);
 	root_element = xmlDocGetRootElement(doc);
+	assert(root_element);
 	increment_version(root_element);
 
 	fencedevices = findnode(root_element, "fencedevices");
@@ -2109,6 +2132,7 @@ void list_fences(int argc, char **argv)
 	}
 	doc = open_configfile(&ninfo);
 	root_element = xmlDocGetRootElement(doc);
+	assert(root_element);
 
 	fencedevices = findnode(root_element, "fencedevices");
 	if (!fencedevices)
@@ -2159,6 +2183,7 @@ void list_scripts(int argc, char **argv)
 	}
 	doc = open_configfile(&ninfo);
 	root_element = xmlDocGetRootElement(doc);
+	assert(root_element);
 
 	rm = findnode(root_element, "rm");
 	if (!rm)
@@ -2210,7 +2235,7 @@ void list_ips(int argc, char **argv)
 	}
 	doc = open_configfile(&ninfo);
 	root_element = xmlDocGetRootElement(doc);
-
+	assert(root_element);
 	rm = findnode(root_element, "rm");
 	if (!rm)
 		die("Can't find \"rm\" in %s\n", ninfo.configfile);
@@ -2261,6 +2286,7 @@ void list_fs(int argc, char **argv)
 	}
 	doc = open_configfile(&ninfo);
 	root_element = xmlDocGetRootElement(doc);
+	assert(root_element);
 
 	rm = findnode(root_element, "rm");
 	if (!rm)
@@ -2328,7 +2354,7 @@ void list_fdomains(int argc, char **argv)
 	}
 	doc = open_configfile(&ninfo);
 	root_element = xmlDocGetRootElement(doc);
-
+	assert(root_element);
 	rm = findnode(root_element, "rm");
 	if (!rm)
 		die("Can't find \"rm\" in %s\n", ninfo.configfile);
-- 
1.7.4.4