[ofa-general] Re: [PATCH] ibsim/sim.h: Fix NodeDescription size

[Sasha Khapyorsky]

On 09:28 Mon 12 May     , Hal Rosenstock wrote:
> 
> I made NODEIDLEN 65 rather than 64 due to the +1 in the original define.
> How about defining it as 64 as in the below ? Does that get around the
> overflow issue ?
> 
> -- Hal
> 
> ibsim/sim.h: Fix NodeDescription size so can have maximum size
> NodeDescription per IBA spec rather than truncating them
>  
> Signed-off-by: Hal Rosenstock <hal at xsigo.com>
> 
> diff --git a/ibsim/sim.h b/ibsim/sim.h
> index bea136a..0bf14fd 100644
> --- a/ibsim/sim.h
> +++ b/ibsim/sim.h
> @@ -65,9 +65,8 @@
>  #define        DEFAULT_LINKWIDTH       LINKWIDTH_4x
>  #define DEFAULT_LINKSPEED      LINKSPEED_SDR
>  
> -#define NODEIDBASE     20
>  #define NODEPREFIX     20
> -#define NODEIDLEN      (NODEIDBASE+NODEPREFIX+1)
> +#define NODEIDLEN      64
>  #define ALIASLEN       40

It is likely will prevent overflow, but will potentially truncate last
NodeDesc character due to string NULL terminator. What about something
like below?

Sasha


diff --git a/ibsim/sim.h b/ibsim/sim.h
index 81bb47c..d3294f4 100644
--- a/ibsim/sim.h
+++ b/ibsim/sim.h
@@ -67,7 +67,7 @@
 
 #define NODEIDBASE	20
 #define NODEPREFIX	20
-#define NODEIDLEN	(NODEIDBASE+NODEPREFIX+1)
+#define NODEIDLEN	65
 #define ALIASLEN 	40
 
 #define MAXHOPS 16
@@ -237,7 +237,7 @@ struct Node {
 	uint64_t sysguid;
 	uint64_t nodeguid;	// also portguid
 	int portsbase;		// in port table
-	char nodeid[64];	// contain nodeid[NODEIDLEN]
+	char nodeid[NODEIDLEN];	// contain nodeid[NODEIDLEN]
 	uint8_t nodeinfo[64];
 	char nodedesc[64];
 	Switch *sw;
diff --git a/ibsim/sim_cmd.c b/ibsim/sim_cmd.c
index 5f64229..fe3e9be 100644
--- a/ibsim/sim_cmd.c
+++ b/ibsim/sim_cmd.c
@@ -56,7 +56,7 @@ extern Port *ports;
 extern Port **lids;
 extern int netnodes, netports, netswitches;
 
-#define NAMELEN	64
+#define NAMELEN	NODEIDLEN
 
 char *portstates[] = {
 	"-", "Down", "Init", "Armed", "Active",
diff --git a/ibsim/sim_net.c b/ibsim/sim_net.c
index bf7a06a..2a9c19b 100644
--- a/ibsim/sim_net.c
+++ b/ibsim/sim_net.c
@@ -267,17 +267,16 @@ static int new_hca(Node * nd)
 	return 0;
 }
 
-static int build_nodeid(char *nodeid, char *base)
+static int build_nodeid(char *nodeid, size_t len, char *base)
 {
 	if (strchr(base, '#') || strchr(base, '@')) {
 		IBWARN("bad nodeid \"%s\": '#' & '@' characters are reserved",
 		       base);
 		return -1;
 	}
-	if (netprefix[0] == 0)
-		strncpy(nodeid, base, NODEIDLEN);
-	else
-		snprintf(nodeid, NODEIDLEN, "%s#%s", netprefix, base);
+
+	snprintf(nodeid, len, "%s%s%s", netprefix, *netprefix ? "#" : "", base);
+
 	return 0;
 }
 
@@ -287,7 +286,7 @@ static Node *new_node(int type, char *nodename, char *nodedesc, int nodeports)
 	char nodeid[NODEIDLEN];
 	Node *nd;
 
-	if (build_nodeid(nodeid, nodename) < 0)
+	if (build_nodeid(nodeid, sizeof(nodeid), nodename) < 0)
 		return 0;
 
 	if (find_node(nodeid)) {
@@ -310,11 +309,9 @@ static Node *new_node(int type, char *nodename, char *nodedesc, int nodeports)
 
 	nd->type = type;
 	nd->numports = nodeports;
-	strncpy(nd->nodeid, nodeid, NODEIDLEN - 1);
-	if (nodedesc && nodedesc[0])
-		strncpy(nd->nodedesc, nodedesc, NODEIDLEN - 1);
-	else
-		strncpy(nd->nodedesc, nodeid, NODEIDLEN - 1);
+	strncpy(nd->nodeid, nodeid, sizeof(nd->nodeid) - 1);
+	strncpy(nd->nodedesc, nodedesc && *nodedesc ? nodedesc : nodeid,
+		sizeof(nd->nodedesc) - 1);
 	nd->sysguid = nd->nodeguid = guids[type];
 	if (type == SWITCH_NODE) {
 		nodeports++;	// port 0 is SMA
@@ -551,22 +548,20 @@ char *expand_name(char *base, char *name, char **portstr)
 		if (netprefix[0] != 0 && !strchr(base, '#'))
 			snprintf(name, NODEIDLEN, "%s#%s", netprefix, base);
 		else
-			strcpy(name, base);
+			strncpy(name, base, NODEIDLEN - 1);
 		if (portstr)
-			*portstr = 0;
+			*portstr = NULL;
 		PDEBUG("name %s port %s", name, portstr ? *portstr : 0);
 		return name;
 	}
-	if (base[0] == '@')
-		snprintf(name, ALIASLEN, "%s%s", netprefix, base);
-	else
-		strcpy(name, base);
+
+	snprintf(name, NODEIDLEN, "%s%s", base[0] == '@' ? netprefix : "", base);
 	PDEBUG("alias %s", name);
 
 	if (!(s = map_alias(name)))
 		return 0;
 
-	strcpy(name, s);
+	strncpy(name, s, NODEIDLEN - 1);
 
 	if (portstr) {
 		*portstr = name;
@@ -1075,12 +1070,12 @@ int link_ports(Port * lport, Port * rport)
 	lport->remotenode = rnode;
 	lport->remoteport = rport->portnum;
 	set_portinfo(lport, lnode->type == SWITCH_NODE ? swport : hcaport);
-	memcpy(lport->remotenodeid, rnode->nodeid, NODEIDLEN);
+	memcpy(lport->remotenodeid, rnode->nodeid, sizeof(lport->remotenodeid));
 
 	rport->remotenode = lnode;
 	rport->remoteport = lport->portnum;
 	set_portinfo(rport, rnode->type == SWITCH_NODE ? swport : hcaport);
-	memcpy(rport->remotenodeid, lnode->nodeid, NODEIDLEN);
+	memcpy(rport->remotenodeid, lnode->nodeid, sizeof(rport->remotenodeid));
 	lport->state = rport->state = 2;	// Initialilze
 	lport->physstate = rport->physstate = 5;	// LinkUP
 	if (lnode->sw)
@@ -1166,7 +1161,7 @@ int connect_ports(void)
 			}
 		} else if (remoteport->remoteport != port->portnum ||
 			   strncmp(remoteport->remotenodeid, port->node->nodeid,
-				   NODEIDLEN)) {
+				   sizeof(remoteport->remotenodeid))) {
 			IBWARN
 			    ("remote port %d in node \"%s\" is not connected to "
 			     "node \"%s\" port %d (\"%s\" %d)",