Skip to contentSkip to conversation

cman

Cluster-devel New Fabio M. Di Nitto 4 participants 52 messages Nov 23, 2011 – Nov 30, 2011

lore
Share & tools
lore
#0 lore raw

Before:

Analysis summary report:
------------------------
Files analyzed : 27
Total LoC input to cov-analyze : 32029
Functions analyzed : 380
Paths analyzed : 52124
New defects found : 315 Total

                                   3 BAD_SIZEOF
                                  16 CHECKED_RETURN
                                   4 CONSTANT_EXPRESSION_RESULT
                                   1 DEADCODE
                                   5 FORWARD_NULL
                                   2 INTEGER_OVERFLOW
                                   1 MISSING_BREAK
                                   1 NEGATIVE_RETURNS
                                   5 NO_EFFECT
                                 139 NULL_RETURNS
                                   2 PW.DECLARED_BUT_NOT_REFERENCED
                                   1 PW.MAIN_WRONG_NUM_PARAMS
                                  13 PW.MIXED_ENUM_TYPE
                                   1 PW.NOT_COMPATIBLE_WITH_PREVIOUS_DECL
                                  12 PW.SET_BUT_NOT_USED
                                   7 RESOURCE_LEAK
                                   3 REVERSE_INULL
                                  51 SECURE_CODING
                                   3 SIZEOF_MISMATCH
                                   1 STRING_NULL
                                  12 STRING_OVERFLOW
                                   6 TAINTED_SCALAR
                                   1 TAINTED_STRING
                                   4 TOCTOU
                                  20 UNINIT
                                   1 UNUSED_VALUE

After:

Analysis summary report:
------------------------
Files analyzed : 27
Total LoC input to cov-analyze : 32078
Functions analyzed : 379
Paths analyzed : 52565
New defects found : 205 Total
8 CHECKED_RETURN
2 INTEGER_OVERFLOW
2 NO_EFFECT
134 NULL_RETURNS
2 PW.DECLARED_BUT_NOT_REFERENCED
12 PW.MIXED_ENUM_TYPE
1 PW.NOT_COMPATIBLE_WITH_PREVIOUS_DECL 6 PW.SET_BUT_NOT_USED
4 RESOURCE_LEAK
10 SECURE_CODING
6 TAINTED_SCALAR
1 TAINTED_STRING
4 TOCTOU
13 UNINIT

Of the remaining warnings,
12 PW.MIXED_ENUM_TYPE
6 TAINTED_SCALAR
1 TAINTED_STRING
need further investigation as the warning is non obvious at all.

All the others can be ignored.

#1 lore raw

PATCH 1/41 libcman: fix bad flags check

Spotted by Coverity Scan

---
:100644 100644 a89c731... 9ea82c1... M cman/lib/libcman.c

 cman/lib/libcman.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
cman/lib/libcman.c +3 -3
index a89c731..9ea82c1 100644
@@ -467,7 +467,7 @@ int cman_dispatch(cman_handle_t handle, int flags)
 			len = smsg->msg->length;
 			free(smsg->msg);
 			free(smsg);
-			if (res || (flags & CMAN_DISPATCH_ONE))
+			if (res || (flags & CMAN_DISPATCH_TYPE_MASK) == CMAN_DISPATCH_ONE)
 				break;
 			else
 				continue;
@@ -481,7 +481,7 @@ int cman_dispatch(cman_handle_t handle, int flags)
 			len = smsg->msg->length;
 			free(smsg->msg);
 			free(smsg);
-			if (res || (flags & CMAN_DISPATCH_ONE))
+			if (res || (flags & CMAN_DISPATCH_TYPE_MASK) == CMAN_DISPATCH_ONE)
 				break;
 			else
 				continue;
@@ -495,7 +495,7 @@ int cman_dispatch(cman_handle_t handle, int flags)
 			len = smsg->msg->length;
 			free(smsg->msg);
 			free(smsg);
-			if (res || (flags & CMAN_DISPATCH_ONE))
+			if (res || (flags & CMAN_DISPATCH_TYPE_MASK) == CMAN_DISPATCH_ONE)
 				break;
 			else
 				continue;
Signature · 1 line
1.7.4.4
#2 lore raw

PATCH 2/41 libcman: fix fd usage

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 9ea82c1... 46f5c89... M cman/lib/libcman.c

 cman/lib/libcman.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
cman/lib/libcman.c +2 -2
index 9ea82c1..46f5c89 100644
@@ -318,8 +318,8 @@ static cman_handle_t open_socket(const char *name, int namelen, void *privdata)
 		free(h);
 		h = NULL;
 		errno = saved_errno;
-	}
-	fcntl(h->zero_fd, F_SETFD, 1); /* Set close-on-exec */
+	} else
+		fcntl(h->zero_fd, F_SETFD, 1); /* Set close-on-exec */
 
 	return (cman_handle_t)h;
 }
Signature · 1 line
1.7.4.4
#3 lore raw

PATCH 2/41 libcman: fix fd usage

Hi,

Quoted 13 lines · Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 9ea82c1... 46f5c89... M cman/lib/libcman.c

 cman/lib/libcman.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
cman/lib/libcman.c +0 -0
index 9ea82c1..46f5c89 100644

cman/lib/libcman.c static cman_handle_t open_socket(const char *name, int namelen, void *privdata) · line 322

Quoted 7 lines
cman/lib/libcman.c +2 -2
@@ -318,8 +318,8 @@ static cman_handle_t open_socket(const char *name, int namelen, void *privdata)
 		free(h);
 		h = NULL;
 		errno = saved_errno;
-	}
-	fcntl(h->zero_fd, F_SETFD, 1); /* Set close-on-exec */
+	} else
+		fcntl(h->zero_fd, F_SETFD, 1); /* Set close-on-exec */

FD_CLOEXEC would be a bit clearer than 1, even with the comment. Also its normal to do a F_GETFD first and OR the new flag so that you don't accidentally reset any other flags that are set on the fd beforehand. I'm not sure whether that makes sense in this particular case though,

Steve.

Quoted 3 lines
cman/lib/libcman.c +0 -0
 	return (cman_handle_t)h;
 }
#4 lore raw

PATCH 2/41 libcman: fix fd usage

Quoted 28 lines · Steven Whitehouse

Hi,

Quoted 13 lines · Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 9ea82c1... 46f5c89... M cman/lib/libcman.c

 cman/lib/libcman.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
cman/lib/libcman.c +0 -0
index 9ea82c1..46f5c89 100644

cman/lib/libcman.c static cman_handle_t open_socket(const char *name, int namelen, void *privdata) · line 322

Quoted 7 lines
cman/lib/libcman.c +2 -2
@@ -318,8 +318,8 @@ static cman_handle_t open_socket(const char *name, int namelen, void *privdata)
 		free(h);
 		h = NULL;
 		errno = saved_errno;
-	}
-	fcntl(h->zero_fd, F_SETFD, 1); /* Set close-on-exec */
+	} else
+		fcntl(h->zero_fd, F_SETFD, 1); /* Set close-on-exec */

FD_CLOEXEC would be a bit clearer than 1, even with the comment. Also its normal to do a F_GETFD first and OR the new flag so that you don't accidentally reset any other flags that are set on the fd beforehand. I'm not sure whether that makes sense in this particular case though,

Quoted 1 line

Steve.

Good catch... will change that for final submission

Fabio

#5 lore raw

PATCH 3/41 libcman: fix variable type

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 46f5c89... fbf5bf8... M cman/lib/libcman.c

 cman/lib/libcman.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
cman/lib/libcman.c +1 -1
index 46f5c89..fbf5bf8 100644
@@ -232,7 +232,7 @@ static int loopy_writev(int fd, struct iovec *iovptr, size_t iovlen)
 static int send_message(struct cman_handle *h, int msgtype, const void *inbuf, int inlen)
 {
 	struct sock_header header;
-	size_t len;
+	int len;
 	struct iovec iov[2];
 	size_t iovlen = 1;
 
Signature · 1 line
1.7.4.4
#6 lore raw

PATCH 4/41 libcman: fix possible memory leak

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 fbf5bf8... 5b015f0... M cman/lib/libcman.c

 cman/lib/libcman.c |   12 ++++++++++--
 1 files changed, 10 insertions(+), 2 deletions(-)
cman/lib/libcman.c +10 -2
index fbf5bf8..5b015f0 100644
@@ -533,16 +533,24 @@ int cman_dispatch(cman_handle_t handle, int flags)
 		{
 			len = read(h->fd, bufptr+offset, header->length-offset);
 			if (len == 0) {
+				if (bufptr != buf)
+					free(bufptr);
 				errno = EHOSTDOWN;
 				return -1;
 			}
 
 			if (len < 0 &&
-			    (errno == EINTR || errno == EAGAIN))
+			    (errno == EINTR || errno == EAGAIN)) {
+				if (bufptr != buf)
+					free(bufptr);
 				return 0;
+			}
 
-			if (len < 0)
+			if (len < 0) {
+				if (bufptr != buf)
+					free(bufptr);
 				return -1;
+			}
 			offset += len;
 		}
 
Signature · 1 line
1.7.4.4
#7 lore raw

PATCH 5/41 libcman: correctly check for vars before using them

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 5b015f0... ae6c709... M cman/lib/libcman.c

 cman/lib/libcman.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)
cman/lib/libcman.c +10 -6
index 5b015f0..ae6c709 100644
@@ -1018,12 +1018,6 @@ static int cman_set_quorum_device(cman_handle_t handle,
 	char buf[strlen(name)+1 + sizeof(int)];
 	VALIDATE_HANDLE(h);
 
-	if ((!name) || (strlen(name) > MAX_CLUSTER_MEMBER_NAME_LEN) || (votes < 0))
-	{
-		errno = EINVAL;
-		return -1;
-	}
-
 	memcpy(buf, &votes, sizeof(int));
 	strcpy(buf+sizeof(int), name);
 	return info_call(h, ops, buf, strlen(name)+1+sizeof(int), NULL, 0);
@@ -1031,6 +1025,11 @@ static int cman_set_quorum_device(cman_handle_t handle,
 
 int cman_register_quorum_device(cman_handle_t handle, char *name, int votes)
 {
+	if ((!name) || (strlen(name) > MAX_CLUSTER_MEMBER_NAME_LEN) || (votes < 0))
+	{
+		errno = EINVAL;
+		return -1;
+	}
 	return cman_set_quorum_device(handle, CMAN_CMD_REG_QUORUMDEV, name, votes);
 }
 
@@ -1069,6 +1068,11 @@ int cman_get_quorum_device(cman_handle_t handle, struct cman_qdev_info *info)
 
 int cman_update_quorum_device(cman_handle_t handle, char *name, int votes)
 {
+	if ((!name) || (strlen(name) > MAX_CLUSTER_MEMBER_NAME_LEN) || (votes < 0))
+	{
+		errno = EINVAL;
+		return -1;
+	}
 	return cman_set_quorum_device(handle, CMAN_CMD_UPDATE_QUORUMDEV, name, votes);
 }
 
Signature · 1 line
1.7.4.4
#8 lore raw

PATCH 6/41 libcman: switch to strncpy to avoid possible buffer overflows

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 ae6c709... b49cf6f... M cman/lib/libcman.c

 cman/lib/libcman.c |   30 +++++++++++++++---------------
 1 files changed, 15 insertions(+), 15 deletions(-)
cman/lib/libcman.c +15 -15
index ae6c709..b49cf6f 100644
@@ -82,7 +82,7 @@ static void copy_node(cman_node_t *unode, struct cl_cluster_node *knode)
 {
 	unode->cn_nodeid = knode->node_id;
 	unode->cn_member = knode->state == NODESTATE_MEMBER?1:0;
-	strcpy(unode->cn_name, knode->name);
+	strncpy(unode->cn_name, knode->name, sizeof(unode->cn_name) - 1);
 	unode->cn_incarnation = knode->incarnation;
 	unode->cn_jointime = knode->jointime;
 
@@ -678,14 +678,14 @@ int cman_get_node(cman_handle_t handle, int nodeid, cman_node_t *node)
 	int status;
 	VALIDATE_HANDLE(h);
 
-	if (!node || strlen(node->cn_name) > sizeof(cman_node.name))
+	if (!node || strlen(node->cn_name) >= sizeof(cman_node.name))
 	{
 		errno = EINVAL;
 		return -1;
 	}
 
 	cman_node.node_id = nodeid;
-	strcpy(cman_node.name, node->cn_name);
+	strncpy(cman_node.name, node->cn_name, sizeof(cman_node.name) - 1);
 	status = info_call(h, CMAN_CMD_GETNODE, &cman_node, sizeof(struct cl_cluster_node),
 			   &cman_node, sizeof(struct cl_cluster_node));
 	if (status < 0)
@@ -905,14 +905,14 @@ int cman_barrier_register(cman_handle_t handle, const char *name, int flags, int
 	struct cl_barrier_info binfo;
 	VALIDATE_HANDLE(h);
 
-	if (strlen(name) > MAX_BARRIER_NAME_LEN)
+	if (strlen(name) >= MAX_BARRIER_NAME_LEN)
 	{
 		errno = EINVAL;
 		return -1;
 	}
 
 	binfo.cmd = BARRIER_CMD_REGISTER;
-	strcpy(binfo.name, name);
+	strncpy(binfo.name, name, sizeof(binfo.name) - 1);
 	binfo.arg = nodes;
 	binfo.flags = flags;
 
@@ -926,14 +926,14 @@ int cman_barrier_change(cman_handle_t handle, const char *name, int flags, int a
 	struct cl_barrier_info binfo;
 	VALIDATE_HANDLE(h);
 
-	if (strlen(name) > MAX_BARRIER_NAME_LEN)
+	if (strlen(name) >= MAX_BARRIER_NAME_LEN)
 	{
 		errno = EINVAL;
 		return -1;
 	}
 
 	binfo.cmd = BARRIER_CMD_CHANGE;
-	strcpy(binfo.name, name);
+	strncpy(binfo.name, name, sizeof(binfo.name) - 1);
 	binfo.arg = arg;
 	binfo.flags = flags;
 
@@ -947,14 +947,14 @@ int cman_barrier_wait(cman_handle_t handle, const char *name)
 	struct cl_barrier_info binfo;
 	VALIDATE_HANDLE(h);
 
-	if (strlen(name) > MAX_BARRIER_NAME_LEN)
+	if (strlen(name) >= MAX_BARRIER_NAME_LEN)
 	{
 		errno = EINVAL;
 		return -1;
 	}
 
 	binfo.cmd = BARRIER_CMD_WAIT;
-	strcpy(binfo.name, name);
+	strncpy(binfo.name, name, sizeof(binfo.name) - 1);
 
 	return info_call(h, CMAN_CMD_BARRIER, &binfo, sizeof(binfo), NULL, 0);
 }
@@ -965,14 +965,14 @@ int cman_barrier_delete(cman_handle_t handle, const char *name)
 	struct cl_barrier_info binfo;
 	VALIDATE_HANDLE(h);
 
-	if (strlen(name) > MAX_BARRIER_NAME_LEN)
+	if (strlen(name) >= MAX_BARRIER_NAME_LEN)
 	{
 		errno = EINVAL;
 		return -1;
 	}
 
 	binfo.cmd = BARRIER_CMD_DELETE;
-	strcpy(binfo.name, name);
+	strncpy(binfo.name, name, sizeof(binfo.name) - 1);
 
 	return info_call(h, CMAN_CMD_BARRIER, &binfo, sizeof(binfo), NULL, 0);
 }
@@ -1019,7 +1019,7 @@ static int cman_set_quorum_device(cman_handle_t handle,
 	VALIDATE_HANDLE(h);
 
 	memcpy(buf, &votes, sizeof(int));
-	strcpy(buf+sizeof(int), name);
+	strncpy(buf+sizeof(int), name, strlen(name)+1 + sizeof(int) - 1);
 	return info_call(h, ops, buf, strlen(name)+1+sizeof(int), NULL, 0);
 }
 
@@ -1059,7 +1059,7 @@ int cman_get_quorum_device(cman_handle_t handle, struct cman_qdev_info *info)
 	cman_node.node_id = CLUSTER_GETNODE_QUORUMDEV;
 	ret = info_call(h, CMAN_CMD_GETNODE, &cman_node, sizeof(cman_node), &cman_node, sizeof(cman_node));
 	if (!ret) {
-		strcpy(info->qi_name, cman_node.name);
+		strncpy(info->qi_name, cman_node.name, sizeof(info->qi_name) - 1);
 		info->qi_state = cman_node.state;
 		info->qi_votes = cman_node.votes;
 	}
@@ -1087,7 +1087,7 @@ int cman_get_fenceinfo(cman_handle_t handle, int nodeid, uint64_t *time, int *fe
 	if (!ret) {
 		*time = f.fence_time;
 		if (agent)
-			strcpy(agent, f.fence_agent);
+			strncpy(agent, f.fence_agent, sizeof(f.fence_agent) - 1);
 		*fenced = ((f.flags & FENCE_FLAGS_FENCED) != 0);
 	}
 	return ret;
@@ -1131,6 +1131,6 @@ int cman_node_fenced(cman_handle_t handle, int nodeid, uint64_t time, char *agen
 
 	f.nodeid = nodeid;
 	f.fence_time = time;
-	strcpy(f.fence_agent, agent);
+	strncpy(f.fence_agent, agent, sizeof(f.fence_agent) - 1);
 	return info_call(h, CMAN_CMD_UPDATE_FENCE_INFO, &f, sizeof(f), NULL, 0);
 }
Signature · 1 line
1.7.4.4
#9 lore raw

PATCH 7/41 cman_tool: make cman_error static and make it use it's arg instead of global errno

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 e1250ee... 60091c9... M cman/cman_tool/cman_tool.h
:100644 100644 a33c8b4... 872528b... M cman/cman_tool/join.c
:100644 100644 88612c4... a336c42... M cman/cman_tool/main.c

 cman/cman_tool/cman_tool.h |    2 --
 cman/cman_tool/join.c      |    1 +
 cman/cman_tool/main.c      |   37 +++++++++++++++++++------------------
 3 files changed, 20 insertions(+), 20 deletions(-)
cman/cman_tool/cman_tool.h +0 -2
index e1250ee..60091c9 100644
@@ -12,7 +12,6 @@
 #include <arpa/inet.h>
 #include <net/if.h>
 #include <stdio.h>
-#include <errno.h>
 #include <string.h>
 #include <stdlib.h>
 #include <stddef.h>
@@ -107,6 +106,5 @@ struct commandline
 typedef struct commandline commandline_t;
 
 int join(commandline_t *comline, char *envp[]);
-const char *cman_error(int err);
 
 #endif  /*  __CMAN_TOOL_DOT_H__  */
cman/cman_tool/join.c +1 -0
index a33c8b4..872528b 100644
@@ -1,3 +1,4 @@
+#include <errno.h>
 #include <sys/wait.h>
 #include <stdint.h>
 #include <signal.h>
cman/cman_tool/main.c +19 -18
index 88612c4..a336c42 100644
@@ -1,3 +1,4 @@
+#include <errno.h>
 #include <inttypes.h>
 #include <unistd.h>
 #include <signal.h>
@@ -194,6 +195,24 @@ static char *membership_state(char *buf, int buflen, int node_state)
 	return buf;
 }
 
+static const char *cman_error(int err)
+{
+	const char *die_error;
+
+	switch (err) {
+	case ENOTCONN:
+		die_error = "Cluster software not started";
+		break;
+	case ENOENT:
+		die_error = "Node is not yet a cluster member";
+		break;
+	default:
+		die_error = strerror(err);
+		break;
+	}
+	return die_error;
+}
+
 static void show_status(void)
 {
 	cman_cluster_t info;
@@ -557,24 +576,6 @@ static int show_services(void)
 }
 
 
-const char *cman_error(int err)
-{
-	const char *die_error;
-
-	switch (errno) {
-	case ENOTCONN:
-		die_error = "Cluster software not started";
-		break;
-	case ENOENT:
-		die_error = "Node is not yet a cluster member";
-		break;
-	default:
-		die_error = strerror(errno);
-		break;
-	}
-	return die_error;
-}
-
 static void leave(commandline_t *comline)
 {
 	cman_handle_t h;
Signature · 1 line
1.7.4.4
#10 lore raw

PATCH 8/41 cman_tool: don't use envp from main

In reply to Fabio M. Di Nitto

according to wikipedia it is a microsoft extensions. Use __environ directly from unistd.h

Spotted by Coverity Scan

---
:100644 100644 60091c9... 6c5744e... M cman/cman_tool/cman_tool.h
:100644 100644 872528b... b92090c... M cman/cman_tool/join.c
:100644 100644 a336c42... 6d8a1eb... M cman/cman_tool/main.c

 cman/cman_tool/cman_tool.h |    3 +--
 cman/cman_tool/join.c      |   13 +++++++------
 cman/cman_tool/main.c      |   10 +++++-----
 3 files changed, 13 insertions(+), 13 deletions(-)
cman/cman_tool/cman_tool.h +1 -2
index 60091c9..6c5744e 100644
@@ -19,7 +19,6 @@
 #include <fcntl.h>
 #include <netdb.h>
 #include <limits.h>
-#include <unistd.h>
 
 extern char *prog_name;
 
@@ -105,6 +104,6 @@ struct commandline
 };
 typedef struct commandline commandline_t;
 
-int join(commandline_t *comline, char *envp[]);
+int join(commandline_t *comline);
 
 #endif  /*  __CMAN_TOOL_DOT_H__  */
cman/cman_tool/join.c +7 -6
index 872528b..b92090c 100644
@@ -1,4 +1,5 @@
 #include <errno.h>
+#include <unistd.h>
 #include <sys/wait.h>
 #include <stdint.h>
 #include <signal.h>
@@ -119,7 +120,7 @@ static int check_corosync_status(pid_t pid)
 	return status;
 }
 
-int join(commandline_t *comline, char *main_envp[])
+int join(commandline_t *comline)
 {
 	int i, err;
 	int envptr = 0;
@@ -205,9 +206,9 @@ int join(commandline_t *comline, char *main_envp[])
 
 	/* Copy any COROSYNC_* env variables to the new daemon */
 	i=0;
-	while (i < MAX_ARGS && main_envp[i]) {
-		if (strncmp(main_envp[i], "COROSYNC_", 9) == 0)
-			envp[envptr++] = main_envp[i];
+	while (i < MAX_ARGS && __environ[i]) {
+		if (strncmp(__environ[i], "COROSYNC_", 9) == 0)
+			envp[envptr++] = __environ[i];
 		i++;
 	}
 
@@ -363,7 +364,7 @@ int join(commandline_t *comline, char *main_envp[])
 	res = confdb_object_create(confdb_handle, OBJECT_PARENT_HANDLE, "cman_private", strlen("cman_private"), &object_handle);
 	if (res == CS_OK) {
 		int envnum = 0;
-		const char *envvar = main_envp[envnum];
+		const char *envvar = __environ[envnum];
 		const char *equal;
 		char envname[PATH_MAX];
 
@@ -381,7 +382,7 @@ int join(commandline_t *comline, char *main_envp[])
 					}
 				}
 			}
-			envvar = main_envp[++envnum];
+			envvar = __environ[++envnum];
 		}
 	}
 	res = confdb_key_create_typed(confdb_handle, object_handle,
cman/cman_tool/main.c +5 -5
index a336c42..6d8a1eb 100644
@@ -1190,7 +1190,7 @@ static void check_arguments(commandline_t *comline)
 }
 
 
-static void do_join(commandline_t *comline, char *envp[])
+static void do_join(commandline_t *comline)
 {
 	int ret;
 
@@ -1212,18 +1212,18 @@ static void do_join(commandline_t *comline, char *envp[])
 			die("Not joining, configuration is not valid\n");
 	}
 
-	join(comline, envp);
+	join(comline);
 	if (comline->wait_opt || comline->wait_quorate_opt) {
 		do {
 			ret = cluster_wait(comline);
 			if (ret == ENOTCONN)
-				join(comline, envp);
+				join(comline);
 
 		} while (ret == ENOTCONN);
 	}
 }
 
-int main(int argc, char *argv[], char *envp[])
+int main(int argc, char *argv[])
 {
 	commandline_t comline;
 
@@ -1235,7 +1235,7 @@ int main(int argc, char *argv[], char *envp[])
 
 	switch (comline.operation) {
 	case OP_JOIN:
-		do_join(&comline, envp);
+		do_join(&comline);
 		break;
 
 	case OP_LEAVE:
Signature · 1 line
1.7.4.4
#11 lore raw

PATCH 8/41 cman_tool: don't use envp from main

Hi,

Quoted 56 lines · Fabio M. Di Nitto

according to wikipedia it is a microsoft extensions. Use __environ directly from unistd.h

Spotted by Coverity Scan

---
:100644 100644 60091c9... 6c5744e... M cman/cman_tool/cman_tool.h
:100644 100644 872528b... b92090c... M cman/cman_tool/join.c
:100644 100644 a336c42... 6d8a1eb... M cman/cman_tool/main.c

 cman/cman_tool/cman_tool.h |    3 +--
 cman/cman_tool/join.c      |   13 +++++++------
 cman/cman_tool/main.c      |   10 +++++-----
 3 files changed, 13 insertions(+), 13 deletions(-)
cman/cman_tool/cman_tool.h +1 -2
index 60091c9..6c5744e 100644
@@ -19,7 +19,6 @@
 #include <fcntl.h>
 #include <netdb.h>
 #include <limits.h>
-#include <unistd.h>
 
 extern char *prog_name;
 
@@ -105,6 +104,6 @@ struct commandline
 };
 typedef struct commandline commandline_t;
 
-int join(commandline_t *comline, char *envp[]);
+int join(commandline_t *comline);
 
 #endif  /*  __CMAN_TOOL_DOT_H__  */
cman/cman_tool/join.c +2 -1
index 872528b..b92090c 100644
@@ -1,4 +1,5 @@
 #include <errno.h>
+#include <unistd.h>
 #include <sys/wait.h>
 #include <stdint.h>
 #include <signal.h>
@@ -119,7 +120,7 @@ static int check_corosync_status(pid_t pid)
 	return status;
 }
 
-int join(commandline_t *comline, char *main_envp[])
+int join(commandline_t *comline)
 {
 	int i, err;
 	int envptr = 0;
@@ -205,9 +206,9 @@ int join(commandline_t *comline, char *main_envp[])
 

cman/cman_tool/join.c int join(commandline_t *comline, char *main_envp[]) · line 213

Quoted 10 lines
cman/cman_tool/join.c +3 -3
 	/* Copy any COROSYNC_* env variables to the new daemon */
 	i=0;
-	while (i < MAX_ARGS && main_envp[i]) {
-		if (strncmp(main_envp[i], "COROSYNC_", 9) == 0)
-			envp[envptr++] = main_envp[i];
+	while (i < MAX_ARGS && __environ[i]) {
+		if (strncmp(__environ[i], "COROSYNC_", 9) == 0)
+			envp[envptr++] = __environ[i];
 		i++;
 	}

Why not just use getenv() ?

Steve.

#12 lore raw

PATCH 8/41 cman_tool: don't use envp from main

Quoted 59 lines · Steven Whitehouse

Hi,

Quoted 56 lines · Fabio M. Di Nitto

according to wikipedia it is a microsoft extensions. Use __environ directly from unistd.h

Spotted by Coverity Scan

---
:100644 100644 60091c9... 6c5744e... M cman/cman_tool/cman_tool.h
:100644 100644 872528b... b92090c... M cman/cman_tool/join.c
:100644 100644 a336c42... 6d8a1eb... M cman/cman_tool/main.c

 cman/cman_tool/cman_tool.h |    3 +--
 cman/cman_tool/join.c      |   13 +++++++------
 cman/cman_tool/main.c      |   10 +++++-----
 3 files changed, 13 insertions(+), 13 deletions(-)
cman/cman_tool/cman_tool.h +1 -2
index 60091c9..6c5744e 100644
@@ -19,7 +19,6 @@
 #include <fcntl.h>
 #include <netdb.h>
 #include <limits.h>
-#include <unistd.h>
 
 extern char *prog_name;
 
@@ -105,6 +104,6 @@ struct commandline
 };
 typedef struct commandline commandline_t;
 
-int join(commandline_t *comline, char *envp[]);
+int join(commandline_t *comline);
 
 #endif  /*  __CMAN_TOOL_DOT_H__  */
cman/cman_tool/join.c +2 -1
index 872528b..b92090c 100644
@@ -1,4 +1,5 @@
 #include <errno.h>
+#include <unistd.h>
 #include <sys/wait.h>
 #include <stdint.h>
 #include <signal.h>
@@ -119,7 +120,7 @@ static int check_corosync_status(pid_t pid)
 	return status;
 }
 
-int join(commandline_t *comline, char *main_envp[])
+int join(commandline_t *comline)
 {
 	int i, err;
 	int envptr = 0;
@@ -205,9 +206,9 @@ int join(commandline_t *comline, char *main_envp[])
 
Quoted 11 lines

cman/cman_tool/join.c int join(commandline_t *comline, char *main_envp[]) · line 213

Quoted 10 lines
cman/cman_tool/join.c +3 -3
 	/* Copy any COROSYNC_* env variables to the new daemon */
 	i=0;
-	while (i < MAX_ARGS && main_envp[i]) {
-		if (strncmp(main_envp[i], "COROSYNC_", 9) == 0)
-			envp[envptr++] = main_envp[i];
+	while (i < MAX_ARGS && __environ[i]) {
+		if (strncmp(__environ[i], "COROSYNC_", 9) == 0)
+			envp[envptr++] = __environ[i];
 		i++;
 	}

Why not just use getenv() ?

getenv implies that you know the values you are looking for.

This function is generic and look for everything that starts with COROSYNC_ in the environment.

Fabio

#13 lore raw

PATCH 9/41 cman_tool: prevent buffer overrun

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 6d8a1eb... 5a5aa7b... M cman/cman_tool/main.c

 cman/cman_tool/main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
cman/cman_tool/main.c +2 -2
index 6d8a1eb..5a5aa7b 100644
@@ -641,7 +641,7 @@ static void set_votes(commandline_t *comline)
 	else {
 		/* Resolve node name into a number */
 		node.cn_nodeid = 0;
-		strcpy(node.cn_name, comline->nodenames[0]);
+		strncpy(node.cn_name, comline->nodenames[0], CMAN_MAX_NODENAME_LEN);
 		if (cman_get_node(h, node.cn_nodeid, &node))
 			die("Can't set votes for node %s : %s\n", node.cn_name, strerror(errno));
 		nodeid = node.cn_nodeid;
@@ -864,7 +864,7 @@ static void kill_node(commandline_t *comline)
 
 	    /* Resolve node name into a number */
 	    node.cn_nodeid = 0;
-	    strcpy(node.cn_name, comline->nodenames[i]);
+	    strncpy(node.cn_name, comline->nodenames[i], CMAN_MAX_NODENAME_LEN);
 	    if (cman_get_node(h, node.cn_nodeid, &node)) {
 		fprintf(stderr, "Can't kill node %s : %s\n", node.cn_name, strerror(errno));
 		continue;
Signature · 1 line
1.7.4.4
#14 lore raw

PATCH 10/41 cman_tool: check that memory is allocated

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 5a5aa7b... 46c315d... M cman/cman_tool/main.c

 cman/cman_tool/main.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)
cman/cman_tool/main.c +3 -0
index 5a5aa7b..46c315d 100644
@@ -539,6 +539,9 @@ static void show_nodes(commandline_t *comline)
 	/* Get Disallowed nodes, so we can show them as such */
 	dis_nodes = malloc(sizeof(cman_node_t) * count);
 
+	if (!dis_nodes)
+		die("cannot allocate memory for disallowed node\n");
+
 	if (cman_get_disallowed_nodes(h, count, &dis_count, dis_nodes) == 0) {
 		for (i = 0; i < numnodes; i++) {
 			for (j = 0; j < dis_count; j++) {
Signature · 1 line
1.7.4.4
#15 lore raw

PATCH 11/41 cman_tool: drop unused vars and fix value check

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 b92090c... a1c1c14... M cman/cman_tool/join.c
:100644 100644 46c315d... 0e8876a... M cman/cman_tool/main.c

 cman/cman_tool/join.c |   10 +++++-----
 cman/cman_tool/main.c |    6 ++----
 2 files changed, 7 insertions(+), 9 deletions(-)
cman/cman_tool/join.c +5 -5
index b92090c..a1c1c14 100644
@@ -122,7 +122,7 @@ static int check_corosync_status(pid_t pid)
 
 int join(commandline_t *comline)
 {
-	int i, err;
+	int i;
 	int envptr = 0;
 	int argvptr = 0;
 	char scratch[1024];
@@ -252,13 +252,13 @@ int join(commandline_t *comline)
 		be_daemon();
 
 		sprintf(scratch, "FORKED: %d\n", getpid());
-		err = write(p[1], scratch, strlen(scratch));
+		write(p[1], scratch, strlen(scratch));
 
 		execve(COROSYNCBIN, argv, envp);
 
 		/* exec failed - tell the parent process */
 		sprintf(scratch, "execve of " COROSYNCBIN " failed: %s", strerror(errno));
-		err = write(p[1], scratch, strlen(scratch));
+		write(p[1], scratch, strlen(scratch));
 		exit(1);
 		break;
 
@@ -290,7 +290,7 @@ int join(commandline_t *comline)
 			if ((len = read(p[0], message, sizeof(message)) > 0)) {
 
 				/* Forked OK - get the real corosync pid */
-				if (sscanf(messageptr, "FORKED: %d", &corosync_pid) == 1) {
+				if ((messageptr) && (sscanf(messageptr, "FORKED: %d", &corosync_pid) == 1)) {
 					if (comline->verbose & DEBUG_STARTUP_ONLY)
 						fprintf(stderr, "forked process ID is %d\n", corosync_pid);
 					status = 0;
@@ -303,7 +303,7 @@ int join(commandline_t *comline)
 						continue;
 				}
 				/* Success! get the new PID of double-forked corosync */
-				if (sscanf(messageptr, "SUCCESS: %d", &corosync_pid) == 1) {
+				if ((messageptr) && (sscanf(messageptr, "SUCCESS: %d", &corosync_pid) == 1)) {
 					if (comline->verbose & DEBUG_STARTUP_ONLY)
 						fprintf(stderr, "corosync running, process ID is %d\n", corosync_pid);
 					status = 0;
cman/cman_tool/main.c +2 -4
index 46c315d..0e8876a 100644
@@ -618,11 +618,10 @@ static void leave(commandline_t *comline)
 static void set_expected(commandline_t *comline)
 {
 	cman_handle_t h;
-	int result;
 
 	h = open_cman_handle(1);
 
-	if ((result = cman_set_expected_votes(h, comline->expected_votes)))
+	if (cman_set_expected_votes(h, comline->expected_votes))
 		die("can't set expected votes: %s", cman_error(errno));
 
 	cman_finish(h);
@@ -631,7 +630,6 @@ static void set_expected(commandline_t *comline)
 static void set_votes(commandline_t *comline)
 {
 	cman_handle_t h;
-	int result;
 	int nodeid;
 	struct cman_node node;
 
@@ -650,7 +648,7 @@ static void set_votes(commandline_t *comline)
 		nodeid = node.cn_nodeid;
 	}
 
-	if ((result = cman_set_votes(h, comline->votes, nodeid)))
+	if (cman_set_votes(h, comline->votes, nodeid))
 		die("can't set votes: %s", cman_error(errno));
 
 	cman_finish(h);
Signature · 1 line
1.7.4.4
#16 lore raw

PATCH 12/41 cman: add check to guarantee we found our own node name

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 16ef65f... f23f386... M cman/daemon/cmanconfig.c

 cman/daemon/cmanconfig.c |   10 +++++++++-
 1 files changed, 9 insertions(+), 1 deletions(-)
cman/daemon/cmanconfig.c +9 -1
index 16ef65f..f23f386 100644
@@ -71,7 +71,15 @@ int read_cman_nodes(struct corosync_api_v1 *corosync, unsigned int *config_versi
 	    objdb_get_int(corosync, object_handle, "expected_votes", &expected, 0);
 	    objdb_get_int(corosync, object_handle, "two_node", (unsigned int *)&two_node, 0);
 	    objdb_get_int(corosync, object_handle, "cluster_id", &cluster_id, 0);
-	    objdb_get_string(corosync, object_handle, "nodename", &our_nodename);
+	    if (objdb_get_string(corosync, object_handle, "nodename", &our_nodename) != 0) {
+		char message[132];
+		snprintf(message, sizeof(message),
+			 "cman was unable to determine our node name!\n");
+		log_printf(LOG_ERR, "%s", message);
+		write_cman_pipe(message);
+		error = -EINVAL;
+		goto out_err;
+	    }
 	    objdb_get_int(corosync, object_handle, "max_queued", &max_outstanding_messages, DEFAULT_MAX_QUEUED);
     }
     corosync->object_find_destroy(find_handle);
Signature · 1 line
1.7.4.4
#17 lore raw

PATCH 13/41 cman: drop dead code and fix code logic

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 f4362cf... 7fb3005... M cman/daemon/barrier.c

 cman/daemon/barrier.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)
cman/daemon/barrier.c +2 -4
index f4362cf..7fb3005 100644
@@ -177,11 +177,11 @@ void process_barrier_msg(struct cl_barriermsg *msg,
 {
 	struct cl_barrier *barrier;
 
-	barrier = find_barrier(msg->name);
-
 	/* Ignore other peoples' messages */
 	if (!we_are_a_cluster_member)
 		return;
+
+	barrier = find_barrier(msg->name);
 	if (!barrier)
 		return;
 
@@ -200,8 +200,6 @@ void process_barrier_msg(struct cl_barriermsg *msg,
 		break;
 
 	case BARRIER_COMPLETE:
-		if (!barrier)
-			return;
 		/* Once we receive COMPLETE, we know that everyone has completed.
 		   I love VS */
 		barrier_complete_phase2(barrier, 0);
Signature · 1 line
1.7.4.4
#18 lore raw

PATCH 14/41 cman_tool: fix a few possible buffer overflow

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 a1c1c14... 44c8d69... M cman/cman_tool/join.c
:100644 100644 0e8876a... b0a2116... M cman/cman_tool/main.c

 cman/cman_tool/join.c |    5 +++--
 cman/cman_tool/main.c |    6 +++---
 2 files changed, 6 insertions(+), 5 deletions(-)
cman/cman_tool/join.c +3 -2
index a1c1c14..44c8d69 100644
@@ -94,7 +94,7 @@ static const char *corosync_exit_reason(signed char status)
 		return "Another Corosync instance is already running";
 		break;
 	default:
-		sprintf(reason, "Error, reason code is %d", status);
+		snprintf(reason, sizeof(reason) - 1, "Error, reason code is %d", status);
 		return reason;
 		break;
 	}
@@ -287,7 +287,8 @@ int join(commandline_t *comline)
 		/* Did we get a cman-reported error? */
 		if (status == 1) {
 			int len;
-			if ((len = read(p[0], message, sizeof(message)) > 0)) {
+			if ((len = read(p[0], message, sizeof(message) - 1) > 0)) {
+				message[sizeof(message) - 1] = '\0';
 
 				/* Forked OK - get the real corosync pid */
 				if ((messageptr) && (sscanf(messageptr, "FORKED: %d", &corosync_pid) == 1)) {
cman/cman_tool/main.c +3 -3
index 0e8876a..b0a2116 100644
@@ -188,7 +188,7 @@ static char *membership_state(char *buf, int buflen, int node_state)
 		strncpy(buf, "Leaving", buflen);
 		break;
 	default:
-		sprintf(buf, "Unknown: code=%d", node_state);
+		snprintf(buf, buflen - 1, "Unknown: code=%d", node_state);
 		break;
 	}
 
@@ -414,7 +414,7 @@ static void print_node(commandline_t *comline, cman_handle_t h, int *format, str
 	if (node->cn_jointime.tv_sec && node->cn_member)
 		strftime(jstring, sizeof(jstring), "%F %H:%M:%S", jtime);
 	else
-		strcpy(jstring, "                   ");
+		strncpy(jstring, "                   ", sizeof(jstring));
 
 	if (!comline->format_opts) {
 		printf("%4u   %c  %5d   %s  %s\n",
@@ -1018,7 +1018,7 @@ static void decode_arguments(int argc, char *argv[], commandline_t *comline)
 			if (strlen(optarg) > MAX_NODE_NAME_LEN-1)
 				die("maximum cluster name length is %d",
 				    MAX_CLUSTER_NAME_LEN-1);
-			strcpy(comline->clustername, optarg);
+			strncpy(comline->clustername, optarg, sizeof(comline->clustername) - 1);
 			comline->clustername_opt = TRUE;
 			break;
 
Signature · 1 line
1.7.4.4
#19 lore raw

PATCH 15/41 cman: fix a few var checks and types

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 fb534a9... 9d52fde... M cman/daemon/cman-preconfig.c
:100644 100644 f23f386... d3009d0... M cman/daemon/cmanconfig.c
:100644 100644 02fe88d... 1489f7c... M cman/daemon/commands.c
:100644 100644 92cfd47... b85557d... M cman/daemon/daemon.c

 cman/daemon/cman-preconfig.c |   12 +++++++++---
 cman/daemon/cmanconfig.c     |    2 +-
 cman/daemon/commands.c       |   10 ++++++++--
 cman/daemon/daemon.c         |    2 +-
 4 files changed, 19 insertions(+), 7 deletions(-)
cman/daemon/cman-preconfig.c +9 -3
index fb534a9..9d52fde 100644
@@ -458,7 +458,7 @@ static int verify_nodename(struct objdb_iface_ver0 *objdb, char *node)
 	/* If nodename (from uname) is domain-less, try to match against
 	   cluster.conf names which may have domainname specified */
 	nodes_handle = nodeslist_init(objdb, cluster_parent_handle, &find_handle);
-	do {
+	while (nodes_handle) {
 		int len;
 
 		if (objdb_get_string(objdb, nodes_handle, "name", &str)) {
@@ -480,7 +480,7 @@ static int verify_nodename(struct objdb_iface_ver0 *objdb, char *node)
 			return 0;
 		}
 		nodes_handle = nodeslist_next(objdb, find_handle);
-	} while (nodes_handle);
+	}
 	objdb->object_find_destroy(find_handle);
 
 
@@ -706,6 +706,12 @@ static int get_nodename(struct objdb_iface_ver0 *objdb)
 	}
 	objdb->object_find_destroy(find_handle);
 
+	if (!nodeid_str) {
+		sprintf(error_reason, "This node has no nodeid in cluster.conf");
+		write_cman_pipe("This node has no nodeid in cluster.conf");
+		return -1;
+	}
+
 	nodeid = atoi(nodeid_str);
 	error = 0;
 
@@ -1024,7 +1030,7 @@ static void add_cman_overrides(struct objdb_iface_ver0 *objdb)
 			int keylen;
 			memset(tmp, 0, sizeof(tmp));
 
-			strcpy(tmp, cluster_name);
+			strncpy(tmp, cluster_name, sizeof(tmp) - 1);
 
 			/* Key length must be a multiple of 4 */
 			keylen = (strlen(cluster_name)+4) & 0xFC;
cman/daemon/cmanconfig.c +1 -1
index f23f386..d3009d0 100644
@@ -238,7 +238,7 @@ static int get_cman_join_info(struct corosync_api_v1 *corosync)
 	if (!votes) {
 		unsigned int votestmp=-1;
 		objdb_get_int(corosync, node_object, "votes", &votestmp, 1);
-		if (votestmp < 0 || votestmp > 255) {
+		if (votestmp > 255) {
 			log_printf(LOG_ERR, "invalid votes value %d", votestmp);
 			write_cman_pipe("Found invalid votes for node in CCS");
 			return -EINVAL;
cman/daemon/commands.c +8 -2
index 02fe88d..1489f7c 100644
@@ -457,8 +457,10 @@ int cman_join_cluster(struct corosync_api_v1 *api,
 	/* Make sure we have a node name */
 	if (nodename[0] == '\0') {
 		struct utsname un;
-		uname(&un);
-		strcpy(nodename, un.nodename);
+		if (uname(&un)) {
+			return -EINVAL;
+		}
+		strncpy(nodename, un.nodename, sizeof(nodename) - 1);
 	}
 
 	time(&join_time);
@@ -2314,6 +2316,10 @@ void add_ais_node(int nodeid, uint64_t incar, int total_members)
 		/* Emergency nodename */
 		sprintf(tempname, "Node%d", nodeid);
 		node = add_new_node(tempname, nodeid, 1, total_members, NODESTATE_DEAD);
+		if (!node) {
+			log_printf(LOG_ERR, "Unable to add newnode!\n");
+			return;
+		}
 	}
 
 	if (node->state == NODESTATE_DEAD || node->state == NODESTATE_LEAVING) {
cman/daemon/daemon.c +1 -1
index 92cfd47..b85557d 100644
@@ -353,7 +353,7 @@ static int process_rendezvous(hdb_handle_t handle, int fd, int revent, void *dat
 	return 0;
 }
 
-static int open_local_sock(const char *name, int name_len, mode_t mode, hdb_handle_t handle, int type)
+static int open_local_sock(const char *name, int name_len, mode_t mode, hdb_handle_t handle, con_type_t type)
 {
 	int local_socket;
 	struct sockaddr_un sockaddr;
Signature · 1 line
1.7.4.4
#20 lore raw

PATCH 16/41 cman: drop unrequired/unused vars and functions

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 1489f7c... 7641294... M cman/daemon/commands.c
:100644 100644 bbf2628... 4851387... M cman/daemon/nodelist.h

 cman/daemon/commands.c |   18 ------------------
 cman/daemon/nodelist.h |    4 +---
 2 files changed, 1 insertions(+), 21 deletions(-)
cman/daemon/commands.c +0 -18
index 1489f7c..7641294 100644
@@ -90,7 +90,6 @@ static int ccsd_timer_active = 0;
 static struct cluster_node *find_node_by_nodeid(int nodeid);
 static struct cluster_node *find_node_by_name(char *name);
 static int get_node_count(void);
-static int get_highest_nodeid(void);
 static int send_port_open_msg(unsigned char port);
 static int send_port_enquire(int nodeid);
 static void process_internal_message(char *data, int nodeid, int byteswap);
@@ -612,13 +611,10 @@ static int do_cmd_get_all_members(char *cmdbuf, char **retbuf, int retsize, int
 	char *outbuf = *retbuf + offset;
 	int num_nodes = 0;
 	int total_nodes = 0;
-	int highest_node;
 
 	if (!we_are_a_cluster_member)
 		return -ENOENT;
 
-	highest_node = get_highest_nodeid();
-
 	/* Count nodes */
 	list_iterate(nodelist, &cluster_members_list) {
 		total_nodes++;
@@ -1896,7 +1892,6 @@ void send_transition_msg(int last_memb_count, int first_trans)
 static void byteswap_internal_message(char *data)
 {
 	struct cl_protmsg *msg = (struct cl_protmsg *)data;
-	struct cl_barriermsg *barriermsg;
 	struct cl_killmsg *killmsg;
 	struct cl_leavemsg *leavemsg;
 	struct cl_transmsg *transmsg;
@@ -1934,7 +1929,6 @@ static void byteswap_internal_message(char *data)
 		break;
 
 	case CLUSTER_MSG_BARRIER:
-		barriermsg = (struct cl_barriermsg *)data;
 		break;
 
 	case CLUSTER_MSG_RECONFIGURE:
@@ -2388,18 +2382,6 @@ void del_ais_node(int nodeid)
 	}
 }
 
-static int get_highest_nodeid()
-{
-	int highest = 0;
-	struct cluster_node *node;
-
-	list_iterate_items(node, &cluster_members_list) {
-		if (node->node_id > highest)
-			highest = node->node_id;
-	}
-	return highest;
-}
-
 static int get_node_count()
 {
 	int count = 0;
cman/daemon/nodelist.h +1 -3
index bbf2628..4851387 100644
@@ -2,10 +2,8 @@
 static inline int objdb_get_string(OBJDB_API *corosync, hdb_handle_t object_service_handle,
 				   const char *key, char **value)
 {
-	int res;
-
 	*value = NULL;
-	if ( !(res = corosync->object_key_get(object_service_handle,
+	if ( !(corosync->object_key_get(object_service_handle,
 					      key,
 					      strlen(key),
 					      (void *)value,
Signature · 1 line
1.7.4.4
#21 lore raw

PATCH 17/41 cman: make 2 var const and allow backup defaults

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 9d52fde... 522994b... M cman/daemon/cman-preconfig.c

 cman/daemon/cman-preconfig.c |    8 ++++++--
 1 files changed, 6 insertions(+), 2 deletions(-)
cman/daemon/cman-preconfig.c +6 -2
index 9d52fde..522994b 100644
@@ -805,8 +805,8 @@ out:
 static void add_logging_overrides(struct objdb_iface_ver0 *objdb)
 {
 	char *logstr;
-	char *logfacility;
-	char *loglevel;
+	const char *logfacility;
+	const char *loglevel;
 	hdb_handle_t object_handle;
 	hdb_handle_t find_handle;
 
@@ -820,7 +820,11 @@ static void add_logging_overrides(struct objdb_iface_ver0 *objdb)
 	objdb->object_find_destroy(find_handle);
 
 	logfacility = facility_name_get(SYSLOGFACILITY);
+	if (!logfacility)
+		logfacility = "LOG_LOCAL4";
 	loglevel = priority_name_get(SYSLOGLEVEL);
+	if (!loglevel)
+		loglevel = "LOG_INFO";
 
 	/* enable timestamps on logging */
 	if (objdb_get_string(objdb, object_handle, "timestamp", &logstr)) {
Signature · 1 line
1.7.4.4
#22 lore raw

PATCH 18/41 cman: move check of null at beginning

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 7fb3005... 37283ba... M cman/daemon/barrier.c

 cman/daemon/barrier.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
cman/daemon/barrier.c +2 -2
index 7fb3005..37283ba 100644
@@ -256,7 +256,7 @@ static int barrier_setattr_enabled(struct cl_barrier *barrier,
 	int status;
 
 	/* Can't disable a barrier */
-	if (!arg) {
+	if ((!barrier) || (!arg)) {
 		return -EINVAL;
 	}
 
@@ -285,7 +285,7 @@ static int barrier_setattr_enabled(struct cl_barrier *barrier,
 			return status;
 		}
 	}
-	if (barrier && barrier->state == BARRIER_STATE_COMPLETE) {
+	if (barrier->state == BARRIER_STATE_COMPLETE) {
 		return barrier->endreason;
 	}
 	return 0;	/* Nothing to propogate */
Signature · 1 line
1.7.4.4
#23 lore raw

PATCH 19/41 cman: init structs before use

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 522994b... d632c17... M cman/daemon/cman-preconfig.c

 cman/daemon/cman-preconfig.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)
cman/daemon/cman-preconfig.c +4 -0
index 522994b..d632c17 100644
@@ -295,6 +295,10 @@ static int add_ifaddr(struct objdb_iface_ver0 *objdb, char *mcast, char *ifaddr,
 		[TX_MECH_RDMA] = "iba",
 	};
 
+	memset(&mcast_addr, 0, sizeof(mcast_addr));
+	memset(&localhost, 0, sizeof(localhost));
+	memset(&if_addr, 0, sizeof(if_addr));
+
 	/* Check the families match */
 	if (address_family(mcast, &mcast_addr, 0) !=
 	    address_family(ifaddr, &if_addr, mcast_addr.ss_family)) {
Signature · 1 line
1.7.4.4
#24 lore raw

PATCH 20/41 cman: simplify code

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 7641294... 284b351... M cman/daemon/commands.c

 cman/daemon/commands.c |    3 +--
 1 files changed, 1 insertions(+), 2 deletions(-)
cman/daemon/commands.c +1 -2
index 7641294..284b351 100644
@@ -270,13 +270,12 @@ static struct cluster_node *add_new_node(char *name, int nodeid, int votes, int
 			newnode->incarnation = incarnation;
 	}
 	if (!newnode->name) {
-		newnode->name = malloc(strlen(name)+1);
+		newnode->name = strdup(name);
 		if (!newnode->name) {
 			if (newalloc)
 				free(newnode);
 			return NULL;
 		}
-		strcpy(newnode->name, name);
 	}
 
 	if (!newnode->node_id) /* Don't clobber existing nodeid */
Signature · 1 line
1.7.4.4
#25 lore raw

PATCH 21/41 cman: fix free handle logic

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 284b351... 6b61332... M cman/daemon/commands.c

 cman/daemon/commands.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
cman/daemon/commands.c +3 -3
index 284b351..6b61332 100644
@@ -563,8 +563,6 @@ static int do_cmd_get_extrainfo(char *cmdbuf, char **retbuf, int retsize, int *r
 	corosync->object_find_create(OBJECT_PARENT_HANDLE, "totem", strlen("totem"), &totem_find_handle);
 	if (corosync->object_find_next(totem_find_handle, &totem_object_handle) == 0) {
 
-		corosync->object_find_destroy(totem_find_handle);
-
 		corosync->object_find_create(totem_object_handle, "interface", strlen("interface"), &iface_find_handle);
 		while (corosync->object_find_next(iface_find_handle, &object_handle) == 0) {
 
@@ -589,8 +587,10 @@ static int do_cmd_get_extrainfo(char *cmdbuf, char **retbuf, int retsize, int *r
 			}
 			ptr += sizeof(struct sockaddr_storage);
 		}
+
+		corosync->object_find_destroy(iface_find_handle);
 	}
-	corosync->object_find_destroy(iface_find_handle);
+	corosync->object_find_destroy(totem_find_handle);
 
 	for (i=0; i<num_interfaces; i++) {
 		ss = (struct sockaddr_storage *)ptr;
Signature · 1 line
1.7.4.4
#26 lore raw

PATCH 22/41 cman: fix several virtually impossible buffer overflows

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 6b5640a... ea5020c... M cman/daemon/ais.c
:100644 100644 37283ba... c356abe... M cman/daemon/barrier.c
:100644 100644 d632c17... baf94df... M cman/daemon/cman-preconfig.c
:100644 100644 d3009d0... 7eb52e4... M cman/daemon/cmanconfig.c
:100644 100644 6b61332... f397234... M cman/daemon/commands.c

 cman/daemon/ais.c            |    2 +-
 cman/daemon/barrier.c        |    6 +-
 cman/daemon/cman-preconfig.c |  114 +++++++++++++++++++++---------------------
 cman/daemon/cmanconfig.c     |    2 +-
 cman/daemon/commands.c       |   18 +++---
 5 files changed, 71 insertions(+), 71 deletions(-)
cman/daemon/ais.c +1 -1
index 6b5640a..ea5020c 100644
@@ -213,7 +213,7 @@ static int cman_exec_init_fn(struct corosync_api_v1 *api)
 	cman_init(api);
 
 	/* Let cman_tool know we are running and our PID */
-	sprintf(pipe_msg,"SUCCESS: %d", getpid());
+	snprintf(pipe_msg, sizeof(pipe_msg) - 1,"SUCCESS: %d", getpid());
 	write_cman_pipe(pipe_msg);
 	close(startup_pipe);
 	startup_pipe = 0;
cman/daemon/barrier.c +3 -3
index 37283ba..c356abe 100644
@@ -101,7 +101,7 @@ static void check_barrier_complete_phase1(struct cl_barrier *barrier)
 
 		bmsg.cmd = CLUSTER_MSG_BARRIER;
 		bmsg.subcmd = BARRIER_COMPLETE;
-		strcpy(bmsg.name, barrier->name);
+		strncpy(bmsg.name, barrier->name, MAX_BARRIER_NAME_LEN - 1);
 
 		log_printf(LOGSYS_LEVEL_DEBUG, "barrier: Sending COMPLETE for %s\n", barrier->name);
 		comms_send_message((char *) &bmsg, sizeof (bmsg),
@@ -160,7 +160,7 @@ static struct cl_barrier *alloc_barrier(char *name, int nodes)
 	}
 	memset(barrier, 0, sizeof (*barrier));
 
-	strcpy(barrier->name, name);
+	strncpy(barrier->name, name, MAX_BARRIER_NAME_LEN - 1);
 	barrier->flags = 0;
 	barrier->expected_nodes = nodes;
 	barrier->got_nodes = 0;
@@ -268,7 +268,7 @@ static int barrier_setattr_enabled(struct cl_barrier *barrier,
 		/* Send it to the rest of the cluster */
 		bmsg.cmd = CLUSTER_MSG_BARRIER;
 		bmsg.subcmd = BARRIER_WAIT;
-		strcpy(bmsg.name, barrier->name);
+		strncpy(bmsg.name, barrier->name, MAX_BARRIER_NAME_LEN - 1);
 
 		barrier->waitsent = 1;
 		barrier->phase = 1;
cman/daemon/cman-preconfig.c +57 -57

430 lines not on this page · raw message

cman/daemon/cmanconfig.c +1 -1
index d3009d0..7eb52e4 100644
@@ -175,7 +175,7 @@ static int get_cman_join_info(struct corosync_api_v1 *corosync)
 		goto out;
 	}
 
-	strcpy(cluster_name, cname);
+	strncpy(cluster_name, cname, sizeof(cluster_name) - 1);
 
 	expected_votes = 0;
 	if (getenv("CMAN_EXPECTEDVOTES")) {
cman/daemon/commands.c +9 -9
index 6b61332..f397234 100644
@@ -396,7 +396,7 @@ static void copy_to_usernode(struct cluster_node *node,
 	struct totem_ip_address node_ifs[INTERFACE_MAX];
 	/* totempg_ifaces_get always copies INTERFACE_MAX addresses */
 
-	strcpy(unode->name, node->name);
+	strncpy(unode->name, node->name, MAX_CLUSTER_MEMBER_NAME_LEN - 1);
 	unode->jointime = node->join_time;
 	unode->size = sizeof(struct cl_cluster_node);
 	unode->votes = node->votes;
@@ -421,7 +421,7 @@ int cman_set_nodename(char *name)
 	if (ais_running)
 		return -EALREADY;
 
-	strncpy(nodename, name, MAX_CLUSTER_MEMBER_NAME_LEN);
+	strncpy(nodename, name, MAX_CLUSTER_MEMBER_NAME_LEN - 1);
 	return 0;
 }
 
@@ -1401,7 +1401,7 @@ static int do_cmd_update_fence_info(char *cmdbuf)
 	fence_msg->nodeid = f->nodeid;
 	fence_msg->timesec = f->fence_time;
 	fence_msg->fenced = 1;
-	strcpy(fence_msg->agent, f->fence_agent);
+	strncpy(fence_msg->agent, f->fence_agent, MAX_FENCE_AGENT_NAME_LEN - 1);
 	comms_send_message(msg, sizeof(msg), 0,0, 0, 0);
 
 	log_printf(LOGSYS_LEVEL_DEBUG, "memb: node %d fenced by %s\n", f->nodeid, f->fence_agent);
@@ -1428,7 +1428,7 @@ static int do_cmd_get_fence_info(char *cmdbuf, char **retbuf, int retsize, int *
 	f->flags = node->flags&NODE_FLAGS_FENCED;
 
 	if (node->fence_agent)
-		strcpy(f->fence_agent, node->fence_agent);
+		strncpy(f->fence_agent, node->fence_agent, MAX_FENCE_AGENT_NAME_LEN - 1);
 	else
 		f->fence_agent[0] = '\0';
 	*retlen = sizeof(struct cl_fence_info);
@@ -1867,10 +1867,10 @@ void send_transition_msg(int last_memb_count, int first_trans)
 	msg->flags = us->flags;
 	msg->fence_time = us->fence_time;
 	msg->join_time = join_time;
-	strcpy(msg->clustername, cluster_name);
+	memcpy(msg->clustername, cluster_name, MAX_CLUSTER_NAME_LEN);
 	if (us->fence_agent)
 	{
-		strcpy(msg->fence_agent, us->fence_agent);
+		strncpy(msg->fence_agent, us->fence_agent, MAX_FENCE_AGENT_NAME_LEN - 1);
 		len += strlen(us->fence_agent)+1;
 	}
 	else
@@ -2128,7 +2128,7 @@ static void do_process_transition(int nodeid, char *data)
 		fence_msg->nodeid = nodeid;
 		fence_msg->timesec = node->fence_time;
 		fence_msg->fenced = 0;
-		strcpy(fence_msg->agent, node->fence_agent);
+		strncpy(fence_msg->agent, node->fence_agent, MAX_FENCE_AGENT_NAME_LEN - 1);
 		comms_send_message(fencemsg, sizeof(fencemsg), 0,0, nodeid, 0);
 	}
 }
@@ -2307,7 +2307,7 @@ void add_ais_node(int nodeid, uint64_t incar, int total_members)
 		log_printf(LOG_ERR, "Got node from AIS id %d with no config entry\n", nodeid);
 
 		/* Emergency nodename */
-		sprintf(tempname, "Node%d", nodeid);
+		snprintf(tempname, sizeof(tempname) - 1, "Node%d", nodeid);
 		node = add_new_node(tempname, nodeid, 1, total_members, NODESTATE_DEAD);
 		if (!node) {
 			log_printf(LOG_ERR, "Unable to add newnode!\n");
@@ -2432,7 +2432,7 @@ static const char *killmsg_reason(int reason)
 		return "we rejoined the cluster without a full restart";
 
 	default:
-		sprintf(msg, "we got kill message number %d", reason);
+		snprintf(msg, sizeof(msg) - 1, "we got kill message number %d", reason);
 		return msg;
 	}
 }
Signature · 1 line
1.7.4.4
#27 lore raw

PATCH 23/41 notifyd: check for pid errors and report them

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 133706f... 7b625c9... M cman/notifyd/main.c

 cman/notifyd/main.c |    7 +++++--
 1 files changed, 5 insertions(+), 2 deletions(-)
cman/notifyd/main.c +5 -2
index 133706f..7b625c9 100644
@@ -207,7 +207,7 @@ static void dispatch_notification(const char *str, int *quorum)
 	int envptr = 0;
 	int argvptr = 0;
 	char scratch[PATH_MAX];
-	pid_t notify_pid;
+	pid_t notify_pid, pid;
 	int pidstatus;
 	int err = 0;
 
@@ -248,7 +248,10 @@ static void dispatch_notification(const char *str, int *quorum)
 			break;
 
 		default: /* parent */
-			waitpid(notify_pid, &pidstatus, 0);
+			pid = waitpid(notify_pid, &pidstatus, 0);
+			if (pid < 0)
+				logt_print(LOG_ERR, "Error waiting for " SBINDIR "/cman_notify execution\n");
+
 			break;
 	}
 
Signature · 1 line
1.7.4.4
#28 lore raw

PATCH 24/41 notifyd: fix virtually impossible buffer overflows

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 7b625c9... 3091d2f... M cman/notifyd/main.c

 cman/notifyd/main.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)
cman/notifyd/main.c +2 -2
index 7b625c9..3091d2f 100644
@@ -136,7 +136,7 @@ static void lockfile(void)
 		exit(EXIT_FAILURE);
 	}
 
-	sprintf(buf, "%d\n", getpid());
+	snprintf(buf, sizeof(buf) - 1, "%d\n", getpid());
 
 	error = write(fd, buf, strlen(buf));
 	if (error <= 0) {
@@ -182,7 +182,7 @@ static void init_logging(int reconf)
 	int logfile_priority = SYSLOGLEVEL;
 
 	memset(logfile, 0, PATH_MAX);
-	sprintf(logfile, LOGDIR "/cmannotifyd.log");
+	snprintf(logfile, sizeof(logfile) - 1, LOGDIR "/cmannotifyd.log");
 
 	ccs_handle = ccs_connect();
 	if (ccs_handle > 0) {
Signature · 1 line
1.7.4.4
#29 lore raw

PATCH 25/41 qdiskd: use correct sizeof for memb_mask_t

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 d613d84... 01aa199... M cman/qdisk/main.c

 cman/qdisk/main.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)
cman/qdisk/main.c +3 -3
index d613d84..01aa199 100644
@@ -609,12 +609,12 @@ check_cman(qd_ctx *ctx, memb_mask_t mask, memb_mask_t master_mask)
 			   &retnodes, nodes) <0 )
 		return;
 
-	memset(master_mask, 0, sizeof(master_mask));
+	memset(master_mask, 0, sizeof(memb_mask_t));
 	for (x = 0; x < retnodes; x++) {
-		if (is_bit_set(mask, nodes[x].cn_nodeid-1, sizeof(mask)) &&
+		if (is_bit_set(mask, nodes[x].cn_nodeid-1, sizeof(memb_mask_t)) &&
 		    nodes[x].cn_member) {
 			set_bit(master_mask, nodes[x].cn_nodeid-1,
-				sizeof(master_mask));
+				sizeof(memb_mask_t));
 		} else {
 			/* Not in CMAN output = not allowed */
 			clear_bit(master_mask, (nodes[x].cn_nodeid-1),
Signature · 1 line
1.7.4.4
#30 lore raw

PATCH 26/41 qdiskd: report errors on cman_dispatch failures

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 01aa199... 6e9abea... M cman/qdisk/main.c

 cman/qdisk/main.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)
cman/qdisk/main.c +7 -3
index 01aa199..6e9abea 100644
@@ -551,7 +551,9 @@ quorum_init(qd_ctx *ctx, node_info_t *ni, int max, struct h_data *h, int maxh)
 
 		tv.tv_sec = ctx->qc_interval;
 		tv.tv_usec = 0;
-		cman_wait(ctx->qc_cman_user, &tv);
+		if (cman_wait(ctx->qc_cman_user, &tv) < 0)
+			logt_print(LOG_ERR, "cman_dispatch: %s\n",
+				   strerror(errno));
 	}
 
 	if (!_running) {
@@ -1246,10 +1248,12 @@ quorum_loop(qd_ctx *ctx, node_info_t *ni, int max)
 		} else {
 			error_cycles = 0;
 		}
-		
+
 		/* Could hit a watchdog timer here if we wanted to */
 		if (_running) {
-			cman_wait(ctx->qc_cman_user, &sleeptime);
+			if (cman_wait(ctx->qc_cman_user, &sleeptime) < 0)
+				logt_print(LOG_ERR, "cman_dispatch: %s\n",
+					   strerror(errno));
 		}
 	}
 
Signature · 1 line
1.7.4.4
#31 lore raw

PATCH 27/41 qdiskd: check for sscanf return codes

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 20a77f2... 6bbf311... M cman/qdisk/scandisk.c

 cman/qdisk/scandisk.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)
cman/qdisk/scandisk.c +9 -5
index 20a77f2..6bbf311 100644
@@ -260,8 +260,10 @@ static int scanprocpart(struct devlisthead *devlisthead)
 
 		if (strlen(line) > 128 + (22))
 			continue;
-		sscanf(line, "%4d %4d %10llu %s",
-		       &major, &minor, &blkcnt, device);
+
+		if (sscanf(line, "%4d %4d %10llu %s",
+		       &major, &minor, &blkcnt, device) < 4)
+			continue;
 
 		/* careful here.. if there is no device, we are scanning the
 		 * first two lines that are not useful to us
@@ -327,8 +329,9 @@ static int scanmdstat(struct devlisthead *devlisthead)
 		if (!(line[0] == 'm' && line[1] == 'd'))
 			continue;
 
-		sscanf(line, "%s %s %s %s %s",
-		       device, separator, status, personality, firstdevice);
+		if (sscanf(line, "%s %s %s %s %s",
+		       device, separator, status, personality, firstdevice) < 5)
+			continue;
 
 		/* scan only raids that are active */
 		if (strcmp(status, "active"))
@@ -408,7 +411,8 @@ static int scanmapper(struct devlisthead *devlisthead)
 		if (!start)
 			continue;
 
-		sscanf(line, "%s %s", major, device);
+		if (sscanf(line, "%s %s", major, device) < 2)
+			continue;
 
 		if (!strncmp(device, "device-mapper", 13)) {
 			maj = atoi(major);
Signature · 1 line
1.7.4.4
#32 lore raw

PATCH 28/41 qdisk: fix scandisk eval check

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 6bbf311... e962f74... M cman/qdisk/scandisk.c

 cman/qdisk/scandisk.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
cman/qdisk/scandisk.c +1 -1
index 6bbf311..e962f74 100644
@@ -224,7 +224,7 @@ static int lsdev(struct devlisthead *devlisthead, const char *path)
 
 				if (dev_is_block(&sb, newpath))
 					if (!add_lsdev_block
-					    (devlisthead, &sb, newpath) < 0)
+					    (devlisthead, &sb, newpath))
 						return -2;
 			}
 		}
Signature · 1 line
1.7.4.4
#33 lore raw

PATCH 29/41 qdiskd: add strlen check to avoid memory corruption

In reply to Fabio M. Di Nitto

and fix a few impossible buffer overflows

Spotted by Coverity Scan

---
:100644 100644 d5926f9... 929b152... M cman/qdisk/daemon_init.c

 cman/qdisk/daemon_init.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)
cman/qdisk/daemon_init.c +9 -5
index d5926f9..929b152 100644
@@ -48,11 +48,12 @@ check_pid_valid(pid_t pid, char *prog)
 	char dirpath[PATH_MAX];
 	char proc_cmdline[64];	/* yank this from kernel somewhere */
 	char *s = NULL;
+	size_t proc_cmdline_len = 0;
 
 	memset(filename, 0, PATH_MAX);
 	memset(dirpath, 0, PATH_MAX);
 
-	snprintf(dirpath, sizeof (dirpath), "/proc/%d", pid);
+	snprintf(dirpath, sizeof (dirpath) - 1, "/proc/%d", pid);
 	if ((dir = opendir(dirpath)) == NULL) {
 		closedir(dir);
 		return 0;	/* Pid has gone away. */
@@ -63,7 +64,7 @@ check_pid_valid(pid_t pid, char *prog)
 	 * proc-pid directory exists.  Now check to see if this
 	 * PID corresponds to the daemon we want to start.
 	 */
-	snprintf(filename, sizeof (filename), "/proc/%d/cmdline", pid);
+	snprintf(filename, sizeof (filename) - 1, "/proc/%d/cmdline", pid);
 	fp = fopen(filename, "r");
 	if (fp == NULL) {
 		perror("check_pid_valid");
@@ -83,9 +84,12 @@ check_pid_valid(pid_t pid, char *prog)
 	}
 	fclose(fp);
 
-	s = &(proc_cmdline[strlen(proc_cmdline)]);
-	if (*s == '\n')
-		*s = 0;
+	proc_cmdline_len = strlen(proc_cmdline);
+	if (proc_cmdline_len) {
+		s = &(proc_cmdline[proc_cmdline_len]);
+		if (*s == '\n')
+			*s = 0;
+	}
 
 	/*
 	 * Check to see if this is the same executable.
Signature · 1 line
1.7.4.4
#34 lore raw

PATCH 30/41 qdiskd: warn users when we cannot write eviction notice to disk

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 6e9abea... c4db713... M cman/qdisk/main.c

 cman/qdisk/main.c |   14 +++++++++-----
 1 files changed, 9 insertions(+), 5 deletions(-)
cman/qdisk/main.c +9 -5
index 6e9abea..c4db713 100644
@@ -300,8 +300,10 @@ check_transitions(qd_ctx *ctx, node_info_t *ni, int max, memb_mask_t mask)
 				logt_print(LOG_NOTICE,
 				       "Writing eviction notice for node %d\n",
 				       ni[x].ni_status.ps_nodeid);
-				qd_write_status(ctx, ni[x].ni_status.ps_nodeid,
-						S_EVICT, NULL, NULL, NULL);
+				if (qd_write_status(ctx, ni[x].ni_status.ps_nodeid,
+						S_EVICT, NULL, NULL, NULL) != 0)
+					logt_print(LOG_CRIT, "Unable to write eviction notice for node %d!\n",
+						   ni[x].ni_status.ps_nodeid);
 				if (ctx->qc_flags & RF_ALLOW_KILL) {
 					logt_print(LOG_DEBUG, "Telling CMAN to "
 						"kill the node\n");
@@ -335,8 +337,10 @@ check_transitions(qd_ctx *ctx, node_info_t *ni, int max, memb_mask_t mask)
 			logt_print(LOG_ALERT,
  			       "Writing eviction notice (again) for node %d\n",
 			       ni[x].ni_status.ps_nodeid);
-			qd_write_status(ctx, ni[x].ni_status.ps_nodeid,
-					S_EVICT, NULL, NULL, NULL);
+			if (qd_write_status(ctx, ni[x].ni_status.ps_nodeid,
+					S_EVICT, NULL, NULL, NULL) != 0)
+				logt_print(LOG_CRIT, "Unable to write eviction notice for node %d!\n",
+					   ni[x].ni_status.ps_nodeid);
 			ni[x].ni_status.ps_state = S_EVICT;
 
 			/* XXX Need to fence it again */
@@ -1850,7 +1854,7 @@ get_config_data(qd_ctx *ctx, struct h_data *h, int maxh, int *cfh)
 		ctx->qc_sched_prio = 1;
 		ctx->qc_max_error_cycles = 0;
 	}
-	
+
 	if (get_dynamic_config_data(ctx, ccsfd) < 0) {
 		goto out;
 	}
Signature · 1 line
1.7.4.4
#35 lore raw

PATCH 31/41 qdiskd: don�t deference null variable

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 929b152... 3cbac37... M cman/qdisk/daemon_init.c

 cman/qdisk/daemon_init.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)
cman/qdisk/daemon_init.c +3 -4
index 929b152..3cbac37 100644
@@ -54,10 +54,9 @@ check_pid_valid(pid_t pid, char *prog)
 	memset(dirpath, 0, PATH_MAX);
 
 	snprintf(dirpath, sizeof (dirpath) - 1, "/proc/%d", pid);
-	if ((dir = opendir(dirpath)) == NULL) {
-		closedir(dir);
-		return 0;	/* Pid has gone away. */
-	}
+	dir = opendir(dirpath);
+	if (!dir)
+		return 0; /* Pid has gone away. */
 	closedir(dir);
 
 	/*
Signature · 1 line
1.7.4.4
#36 lore raw

PATCH 32/41 qdiskd: clean code around case

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 c4db713... 51d9a39... M cman/qdisk/main.c

 cman/qdisk/main.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
cman/qdisk/main.c +2 -0
index c4db713..51d9a39 100644
@@ -146,9 +146,11 @@ check_self(qd_ctx *ctx, status_block_t *sb)
 	case S_EVICT:
 		/* Someone told us to die. */
 		reboot(RB_AUTOBOOT);
+		break;
 	default:
 		logt_print(LOG_EMERG, "Unhandled state: %d\n", sb->ps_state);
 		raise(SIGSTOP);
+		break;
 	}
 }
 
Signature · 1 line
1.7.4.4
#37 lore raw

PATCH 32/41 qdiskd: clean code around case

Quoted 13 lines · Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 c4db713... 51d9a39... M cman/qdisk/main.c

  cman/qdisk/main.c |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
cman/qdisk/main.c +0 -0
index c4db713..51d9a39 100644

cman/qdisk/main.c check_self(qd_ctx *ctx, status_block_t *sb) · line 149

Quoted 4 lines
cman/qdisk/main.c +1 -0
@@ -146,9 +146,11 @@ check_self(qd_ctx *ctx, status_block_t *sb)
  	case S_EVICT:
  		/* Someone told us to die. */
  		reboot(RB_AUTOBOOT);
+		break;

This fallthrough to 'default' was deliberate.

Quoted 7 lines
cman/qdisk/main.c +1 -0
  	default:
  		logt_print(LOG_EMERG, "Unhandled state: %d\n", sb->ps_state);
  		raise(SIGSTOP);
+		break;
  	}
  }
 
#38 lore raw

PATCH 32/41 qdiskd: clean code around case

Quoted 9 lines · Lon Hohberger
Quoted 8 lines · Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 c4db713... 51d9a39... M cman/qdisk/main.c

  cman/qdisk/main.c |    2 ++
  1 files changed, 2 insertions(+), 0 deletions(-)
Quoted 11 lines

cman/qdisk/main.c check_self(qd_ctx *ctx, status_block_t *sb) · line 149

Quoted 9 lines
cman/qdisk/main.c +1 -0
index c4db713..51d9a39 100644
@@ -146,9 +146,11 @@ check_self(qd_ctx *ctx, status_block_t *sb)
      case S_EVICT:
          /* Someone told us to die. */
          reboot(RB_AUTOBOOT);
+        break;

This fallthrough to 'default' was deliberate.

hmm ok, but isn't reboot going to make it useless either way?

Fabio

#39 lore raw

PATCH 32/41 qdiskd: clean code around case

Quoted 22 lines · Fabio M. Di Nitto
Quoted 9 lines · Lon Hohberger
Quoted 8 lines · Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 c4db713... 51d9a39... M cman/qdisk/main.c

   cman/qdisk/main.c |    2 ++
   1 files changed, 2 insertions(+), 0 deletions(-)
Quoted 11 lines

cman/qdisk/main.c check_self(qd_ctx *ctx, status_block_t *sb) · line 149

Quoted 9 lines
cman/qdisk/main.c +1 -0
index c4db713..51d9a39 100644
@@ -146,9 +146,11 @@ check_self(qd_ctx *ctx, status_block_t *sb)
       case S_EVICT:
           /* Someone told us to die. */
           reboot(RB_AUTOBOOT);
+        break;

This fallthrough to 'default' was deliberate.

Quoted 2 lines

hmm ok, but isn't reboot going to make it useless either way?

Yes, presuming reboot works. ;)

(There are cases where it doesn't; there was a bugzilla open about this when using KVM guests recently ...)

SIGSTOP
will prevent qdiskd from continuing and casting votes in this case.

-- Lon

#40 lore raw

PATCH 33/41 qdiskd: don't try the impossible if we can't open /dev/null

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 3cbac37... c260cb8... M cman/qdisk/daemon_init.c
:100644 100644 51d9a39... d21c209... M cman/qdisk/main.c

 cman/qdisk/daemon_init.c |    2 +-
 cman/qdisk/main.c        |    5 +++++
 2 files changed, 6 insertions(+), 1 deletions(-)
cman/qdisk/daemon_init.c +1 -1
index 3cbac37..c260cb8 100644
@@ -93,7 +93,7 @@ check_pid_valid(pid_t pid, char *prog)
 	/*
 	 * Check to see if this is the same executable.
 	 */
-	if ((s = strstr(proc_cmdline, prog)) == NULL) {
+	if (!strstr(proc_cmdline, prog)) {
 		return 0;
 	} else {
 		return 1;
cman/qdisk/main.c +5 -0
index 51d9a39..d21c209 100644
@@ -1995,6 +1995,11 @@ main(int argc, char **argv)
 		case 'Q':
 			/* Make qdisk very quiet */
 			nfd = open("/dev/null", O_RDWR);
+			if (nfd < 0) {
+				fprintf(stderr, "Could not open /dev/null!\n");
+				ret = -1;
+				goto out;
+			}
 			close(0);
 			close(1);
 			close(2);
Signature · 1 line
1.7.4.4
#41 lore raw

PATCH 34/41 qdiskd: avoid a potential crash in case config state is invalid

In reply to Fabio M. Di Nitto

Spotted by Coverify Scan

---
:100644 100644 d21c209... 079af01... M cman/qdisk/main.c

 cman/qdisk/main.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
cman/qdisk/main.c +3 -2
index d21c209..079af01 100644
@@ -73,6 +73,7 @@ static void update_local_status(qd_ctx *ctx, node_info_t *ni, int max, int score
 static int get_config_data(qd_ctx *ctx, struct h_data *h, int maxh, int *cfh);
 static int cman_wait(cman_handle_t ch, struct timeval *_tv);
 
+static int fake_cfh = 0;
 
 static void
 int_handler(int sig)
@@ -934,7 +935,7 @@ process_cman_event(cman_handle_t handle, void *private, int reason, int arg)
 		_cman_shutdown = 1;
 		break;
 	case CMAN_REASON_CONFIG_UPDATE:
-		get_config_data(ctx, NULL, 0, NULL);
+		get_config_data(ctx, NULL, 0, &fake_cfh);
 		break;
 	case CMAN_REASON_PORTCLOSED:
 		break;
@@ -978,7 +979,7 @@ quorum_loop(qd_ctx *ctx, node_info_t *ni, int max)
 	_running = 1;
 	while (_running) {
 		if (_reconfig) {
-			get_config_data(ctx, NULL, 0, NULL);
+			get_config_data(ctx, NULL, 0, &fake_cfh);
 			_reconfig = 0;
 		}
 
Signature · 1 line
1.7.4.4
#42 lore raw

PATCH 35/41 qdiskd: change variable type

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 9884ea7... d0d05f0... M cman/qdisk/disk.c

 cman/qdisk/disk.c |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
cman/qdisk/disk.c +3 -2
index 9884ea7..d0d05f0 100644
@@ -628,7 +628,8 @@ qdisk_write(target_info_t *disk, __off64_t offset, const void *buf, int count)
 	shared_header_t *hdrp;
 	void *ptr;
 	char *data;
-	size_t total = 0, rv = -1, psz = disk->d_blksz; //sysconf(_SC_PAGESIZE);
+	size_t total = 0, psz = disk->d_blksz; //sysconf(_SC_PAGESIZE);
+	int rv = -1;
 
 	maxsize = psz - (sizeof(shared_header_t));
 	if (count >= (maxsize + sizeof(shared_header_t))) {
@@ -677,7 +678,7 @@ qdisk_write(target_info_t *disk, __off64_t offset, const void *buf, int count)
 
 	if (rv == -1)
 		logt_print(LOG_ERR, "diskRawWriteShadow");
-	
+
 	free((char *)hdrp);
 	if (rv == -1)
 		return -1;
Signature · 1 line
1.7.4.4
#43 lore raw

PATCH 36/41 qdiskd: implement better string handling

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 e962f74... 477b8de... M cman/qdisk/scandisk.c

 cman/qdisk/scandisk.c |   20 +++++++++++++++-----
 1 files changed, 15 insertions(+), 5 deletions(-)
cman/qdisk/scandisk.c +15 -5
index e962f74..477b8de 100644
@@ -311,7 +311,7 @@ static int scanmdstat(struct devlisthead *devlisthead)
 	if (!fp)
 		return 0;
 
-	while (fgets(line, sizeof(line), fp) != NULL) {
+	while (fgets(line, sizeof(line) - 1, fp) != NULL) {
 
 		/* i like things to be absolutely clean */
 		memset(device, 0, sizeof(device));
@@ -321,7 +321,7 @@ static int scanmdstat(struct devlisthead *devlisthead)
 		memset(firstdevice, 0, sizeof(firstdevice));
 		memset(devices, 0, sizeof(devices));
 
-		if (strlen(line) > sizeof(line))
+		if (strlen(line) >= sizeof(line))
 			continue;
 
 		/* we only parse stuff that starts with ^md
@@ -347,8 +347,15 @@ static int scanmdstat(struct devlisthead *devlisthead)
 
 		/* trunkate the string from sdaX[Y] to sdaX and
 		 * copy the whole device string over */
-		memset(strstr(firstdevice, "["), 0, 1);
-		strcpy(devices, strstr(line, firstdevice));
+		tmp = strstr(firstdevice, "[");
+		if (!tmp)
+			continue;
+		memset(tmp, 0, 1);
+
+		tmp = strstr(line, firstdevice);
+		if (!tmp)
+			continue;
+		strncpy(devices, tmp, sizeof(devices) - 1);
 
 		/* if we don't find any slave (for whatever reason)
 		 * keep going */
@@ -357,8 +364,11 @@ static int scanmdstat(struct devlisthead *devlisthead)
 
 		tmp = devices;
 		while ((tmp) && ((next = strstr(tmp, " ")) || strlen(tmp))) {
+			char *tmp2;
 
-			memset(strstr(tmp, "["), 0, 1);
+			tmp2 = strstr(tmp, "[");
+			if (tmp2)
+				memset(tmp2, 0, 1);
 
 			startnode =
 			    find_dev_by_path(devlisthead->devnode, tmp, 1);
Signature · 1 line
1.7.4.4
#44 lore raw

PATCH 36/41 qdiskd: implement better string handling

Quoted 13 lines · Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 e962f74... 477b8de... M cman/qdisk/scandisk.c

  cman/qdisk/scandisk.c |   20 +++++++++++++++-----
  1 files changed, 15 insertions(+), 5 deletions(-)
cman/qdisk/scandisk.c +0 -0
index e962f74..477b8de 100644

cman/qdisk/scandisk.c static int scanmdstat(struct devlisthead *devlisthead) · line 314

Quoted 5 lines
cman/qdisk/scandisk.c +1 -1
@@ -311,7 +311,7 @@ static int scanmdstat(struct devlisthead *devlisthead)
  	if (!fp)
  		return 0;
 
-	while (fgets(line, sizeof(line), fp) != NULL) {
+	while (fgets(line, sizeof(line) - 1, fp) != NULL) {

fgets reads size-1, this isn't needed.

-- Lon

#45 lore raw

PATCH 37/41 qdiskd: add failure paths to check_process_running and drop duplicate check

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 c260cb8... 2ba6fa0... M cman/qdisk/daemon_init.c
:100644 100644 079af01... 175bdb6... M cman/qdisk/main.c

 cman/qdisk/daemon_init.c |   27 ++++++++++++---------------
 cman/qdisk/main.c        |    9 ++++++++-
 2 files changed, 20 insertions(+), 16 deletions(-)
cman/qdisk/daemon_init.c +12 -15
index c260cb8..2ba6fa0 100644
@@ -113,6 +113,11 @@ check_process_running(char *prog, pid_t * pid)
 
 	*pid = -1;
 
+	if (!prog) {
+		errno = EINVAL;
+		return -1;
+	}
+
 	/*
 	 * Now see if there is a pidfile associated with this cmd in /var/run
 	 */
@@ -120,7 +125,10 @@ check_process_running(char *prog, pid_t * pid)
 	memset(filename, 0, PATH_MAX);
 
 	cmd = basename(prog);
-	snprintf(filename, sizeof (filename), "/var/run/%s.pid", cmd);
+	if (!cmd)
+		return -1;
+
+	snprintf(filename, sizeof (filename) - 1, "/var/run/%s.pid", cmd);
 
 	ret = stat(filename, &st);
 	if ((ret < 0) || (!st.st_size))
@@ -130,9 +138,8 @@ check_process_running(char *prog, pid_t * pid)
 	 * Read the pid from the file.
 	 */
 	fp = fopen(filename, "r");
-	if (fp == NULL) {	/* error */
+	if (fp == NULL)		/* error */
 		return 0;
-	}
 
 	ret = fscanf(fp, "%d\n", &oldpid);
 	fclose(fp);
@@ -144,6 +151,7 @@ check_process_running(char *prog, pid_t * pid)
 		*pid = oldpid;
 		return 1;
 	}
+
 	return 0;
 }
 
@@ -200,22 +208,12 @@ setup_sigmask(void)
 void
 daemon_init(char *prog)
 {
-	uid_t uid;
-	pid_t pid;
-
-	uid = getuid();
-	if (uid) {
+	if (getuid()) {
 		logt_print(LOG_ERR,
 			"daemon_init: Sorry, only root wants to run this.\n");
 		exit(1);
 	}
 
-	if (check_process_running(prog, &pid) && (pid != getpid())) {
-		logt_print(LOG_ERR,
-			"daemon_init: Process \"%s\" already running.\n",
-			prog);
-		exit(1);
-	}
 	if (setup_sigmask() < 0) {
 		logt_print(LOG_ERR, "daemon_init: Unable to set signal mask.\n");
 		exit(1);
@@ -226,7 +224,6 @@ daemon_init(char *prog)
 		exit(1);
 	}
 
-
 	update_pidfile(prog);
 }
 
cman/qdisk/main.c +8 -1
index 079af01..175bdb6 100644
@@ -1980,7 +1980,14 @@ main(int argc, char **argv)
 	quorum_header_t qh;
 	qd_priv_t qp;
 
-	if (check_process_running(argv[0], &pid) && pid !=getpid()) {
+
+	rv = check_process_running(argv[0], &pid);
+	if (rv < 0) {
+		fprintf(stderr, "Unable to determin if %s is already running: %s\n",
+			argv[0], strerror(errno));
+		return -1;
+	}
+	if (rv && pid !=getpid()) {
 		printf("QDisk services already running\n");
 		return 0;
 	}
Signature · 1 line
1.7.4.4
#46 lore raw

PATCH 37/41 qdiskd: add failure paths to check_process_running and drop duplicate check

cman/qdisk/main.c main(int argc, char **argv) · line 1987

Quoted 2 lines · Fabio M. Di Nitto
cman/qdisk/main.c +2 -0
+		fprintf(stderr, "Unable to determin if %s is already running: %s\n",
+			argv[0], strerror(errno));

Typo (determine)

-- Lon

#47 lore raw

PATCH 38/41 qdiskd: fix possible resource leak in scandisk

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 477b8de... b99a672... M cman/qdisk/scandisk.c

 cman/qdisk/scandisk.c |   12 ++++++++----
 1 files changed, 8 insertions(+), 4 deletions(-)
cman/qdisk/scandisk.c +8 -4
index 477b8de..b99a672 100644
@@ -252,11 +252,12 @@ static int scanprocpart(struct devlisthead *devlisthead)
 	unsigned long long blkcnt;
 	char device[128];
 	struct devnode *startnode;
+
 	fp = fopen("/proc/partitions", "r");
 	if (!fp)
 		return 0;
-	while (fgets(line, sizeof(line), fp)
-	       != NULL) {
+
+	while (fgets(line, sizeof(line) - 1, fp) != NULL) {
 
 		if (strlen(line) > 128 + (22))
 			continue;
@@ -270,16 +271,19 @@ static int scanprocpart(struct devlisthead *devlisthead)
 		 */
 		if (!strlen(device))
 			continue;
+
 		startnode =
 		    find_dev_by_majmin(devlisthead->devnode, major, minor);
 		if (!startnode) {
 			startnode = alloc_list_obj(devlisthead, major, minor);
-			if (!startnode)
+			if (!startnode) {
+				fclose(fp);
 				return -2;
+			}
 		}
 
 		startnode->procpart = 1;
-		strcpy(startnode->procname, device);
+		strncpy(startnode->procname, device, sizeof(startnode->procname) - 1);
 	}
 
 	fclose(fp);
Signature · 1 line
1.7.4.4
#48 lore raw

PATCH 38/41 qdiskd: fix possible resource leak in scandisk

cman/qdisk/scandisk.c static int scanprocpart(struct devlisthead *devlisthead) · line 260

Quoted 7 lines · Fabio M. Di Nitto

Spotted by Coverity Scan

cman/qdisk/scandisk.c +2 -2
-	while (fgets(line, sizeof(line), fp)
-	       != NULL) {
+
+	while (fgets(line, sizeof(line) - 1, fp) != NULL) {

fgets takes at most size-1, this isn't needed

-- Lon

#49 lore raw

PATCH 39/41 qdiskd: don't leak memory if we fail to read from disk

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 d0d05f0... 7c6e114... M cman/qdisk/disk.c

 cman/qdisk/disk.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
cman/qdisk/disk.c +1 -0
index d0d05f0..7c6e114 100644
@@ -604,6 +604,7 @@ qdisk_read(target_info_t *disk, __off64_t offset, void *bufin, int count)
 	rv = diskRawReadShadow(disk, offset, (char *)hdrp, disk->d_blksz);
 	
 	if (rv == -1) {
+		free(hdrp);
 		return -1;
 	}
 	
Signature · 1 line
1.7.4.4
#50 lore raw

PATCH 40/41 qdiskd: fix uninitialized values

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 b99a672... b80046b... M cman/qdisk/scandisk.c

 cman/qdisk/scandisk.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)
cman/qdisk/scandisk.c +1 -1
index b99a672..b80046b 100644
@@ -642,7 +642,7 @@ static int sysfs_is_disk(char *path)
 static int scansysfs(struct devlisthead *devlisthead, const char *path, int level, int parent_holder)
 {
 	struct devnode *startnode;
-	int i, n, maj, min, has_holder;
+	int i, n, maj = -1, min = -1, has_holder;
 	struct dirent **namelist;
 	struct stat sb;
 	char newpath[MAXPATHLEN];
Signature · 1 line
1.7.4.4
#51 lore raw

PATCH 41/41 qdiskd: fix more uninizialized values

In reply to Fabio M. Di Nitto

Spotted by Coverity Scan

---
:100644 100644 7c6e114... ec47a5f... M cman/qdisk/disk.c

 cman/qdisk/disk.c |   13 ++-----------
 1 files changed, 2 insertions(+), 11 deletions(-)
cman/qdisk/disk.c +2 -11
index 7c6e114..ec47a5f 100644
@@ -756,19 +756,10 @@ qdisk_init(char *partname, char *label)
 
 	time(&t);
 
+	memset(&ps, 0, sizeof(status_block_t));
+
 	ps.ps_magic = STATE_MAGIC_NUMBER;
-	ps.ps_updatenode = 0;
-	ps.pad0 = 0;
 	ps.ps_timestamp = (uint64_t)t;
-	ps.ps_state = (uint8_t)S_NONE;
-	ps.pad1[0] = 0;
-	ps.ps_flags = 0;
-	ps.ps_score = 0;
-	ps.ps_scoremax = 0;
-	ps.ps_ca_sec = 0;
-	ps.ps_ca_usec = 0;
-	ps.ps_lc_sec = 0;
-	ps.ps_ca_usec = 0;
 
 	/* Node IDs 1..N */
 	for (nid = 1; nid <= MAX_NODES_DISK; nid++) {
Signature · 1 line
1.7.4.4