Go back to using data_len in struct ngpppoe_init_data after discussions

with Julian and Archie.

Implement a new ``sizedstring'' parse type for dealing with field pairs
consisting of a uint16_t followed by a data field of that size, and use
this to deal with the data_len and data fields.

Written by:		Archie with some input by me
Agreed in principle by:	julian

5 files changed, 146 insertions, 89 deletions

diff --git a/sys/netgraph/ng_ksocket.c b/sys/netgraph/ng_ksocket.c
index 5cf8b4c..56bbba0 100644
--- a/sys/netgraph/ng_ksocket.c
+++ b/sys/netgraph/ng_ksocket.c
@@ -220,7 +220,7 @@ ng_ksocket_sockaddr_parse(const struct ng_parse_type *type,
 		int toklen, pathlen;
 		char *path;
 
-		if ((path = ng_get_string_token(s, off, &toklen)) == NULL)
+		if ((path = ng_get_string_token(s, off, &toklen, NULL)) == NULL)
 			return (EINVAL);
 		pathlen = strlen(path);
 		if (pathlen > SOCK_MAXADDRLEN) {
@@ -306,8 +306,7 @@ ng_ksocket_sockaddr_unparse(const struct ng_parse_type *type,
 		char *pathtoken;
 
 		bcopy(sun->sun_path, pathbuf, pathlen);
-		pathbuf[pathlen] = '\0';
-		if ((pathtoken = ng_encode_string(pathbuf)) == NULL)
+		if ((pathtoken = ng_encode_string(pathbuf, pathlen)) == NULL)
 			return (ENOMEM);
 		slen += snprintf(cbuf, cbuflen, "local/%s", pathtoken);
 		FREE(pathtoken, M_NETGRAPH);
diff --git a/sys/netgraph/ng_parse.c b/sys/netgraph/ng_parse.c
index 94367cb..83bcb11 100644
--- a/sys/netgraph/ng_parse.c
+++ b/sys/netgraph/ng_parse.c
@@ -708,14 +708,14 @@ ng_string_parse(const struct ng_parse_type *type,
 {
 	char *sval;
 	int len;
+	int slen;
 
-	if ((sval = ng_get_string_token(s, off, &len)) == NULL)
+	if ((sval = ng_get_string_token(s, off, &len, &slen)) == NULL)
 		return (EINVAL);
 	*off += len;
-	len = strlen(sval) + 1;
-	bcopy(sval, buf, len);
+	bcopy(sval, buf, slen + 1);
 	FREE(sval, M_NETGRAPH);
-	*buflen = len;
+	*buflen = slen + 1;
 	return (0);
 }
 
@@ -724,7 +724,7 @@ ng_string_unparse(const struct ng_parse_type *type,
 	const u_char *data, int *off, char *cbuf, int cbuflen)
 {
 	const char *const raw = (const char *)data + *off;
-	char *const s = ng_encode_string(raw);
+	char *const s = ng_encode_string(raw, strlen(raw));
 
 	if (s == NULL)
 		return (ENOMEM);
@@ -768,16 +768,16 @@ ng_fixedstring_parse(const struct ng_parse_type *type,
 	const struct ng_parse_fixedstring_info *const fi = type->info;
 	char *sval;
 	int len;
+	int slen;
 
-	if ((sval = ng_get_string_token(s, off, &len)) == NULL)
+	if ((sval = ng_get_string_token(s, off, &len, &slen)) == NULL)
 		return (EINVAL);
-	if (strlen(sval) + 1 > fi->bufSize)
+	if (slen + 1 > fi->bufSize)
 		return (E2BIG);
 	*off += len;
-	len = strlen(sval) + 1;
-	bcopy(sval, buf, len);
+	bcopy(sval, buf, slen);
 	FREE(sval, M_NETGRAPH);
-	bzero(buf + len, fi->bufSize - len);
+	bzero(buf + slen, fi->bufSize - slen);
 	*buflen = fi->bufSize;
 	return (0);
 }
@@ -859,6 +859,68 @@ const struct ng_parse_type ng_parse_cmdbuf_type = {
 };
 
 /************************************************************************
+			EXPLICITLY SIZED STRING TYPE
+ ************************************************************************/
+
+static int
+ng_sizedstring_parse(const struct ng_parse_type *type,
+	const char *s, int *off, const u_char *const start,
+	u_char *const buf, int *buflen)
+{
+	char *sval;
+	int len;
+	int slen;
+
+	if ((sval = ng_get_string_token(s, off, &len, &slen)) == NULL)
+		return (EINVAL);
+	if (slen > 0xffff)
+		return (EINVAL);
+	*off += len;
+	*((u_int16_t *)buf) = (u_int16_t)slen;
+	bcopy(sval, buf + 2, slen);
+	FREE(sval, M_NETGRAPH);
+	*buflen = 2 + slen;
+	return (0);
+}
+
+static int
+ng_sizedstring_unparse(const struct ng_parse_type *type,
+	const u_char *data, int *off, char *cbuf, int cbuflen)
+{
+	const char *const raw = (const char *)data + *off + 2;
+	const int slen = *((const u_int16_t *)(data + *off));
+	char *const s = ng_encode_string(raw, slen);
+
+	if (s == NULL)
+		return (ENOMEM);
+	NG_PARSE_APPEND("%s", s);
+	FREE(s, M_NETGRAPH);
+	*off += slen + 2;
+	return (0);
+}
+
+static int
+ng_sizedstring_getDefault(const struct ng_parse_type *type,
+	const u_char *const start, u_char *buf, int *buflen)
+{
+	if (*buflen < 2)
+		return (ERANGE);
+	bzero(buf, 2);
+	*buflen = 2;
+	return (0);
+}
+
+const struct ng_parse_type ng_parse_sizedstring_type = {
+	NULL,
+	NULL,
+	NULL,
+	ng_sizedstring_parse,
+	ng_sizedstring_unparse,
+	ng_sizedstring_getDefault,
+	NULL
+};
+
+/************************************************************************
 			IP ADDRESS TYPE
  ************************************************************************/
 
@@ -949,18 +1011,18 @@ ng_bytearray_parse(const struct ng_parse_type *type,
 {
 	char *str;
 	int toklen;
+	int slen;
 
 	/* We accept either an array of bytes or a string constant */
-	if ((str = ng_get_string_token(s, off, &toklen)) != NULL) {
+	if ((str = ng_get_string_token(s, off, &toklen, &slen)) != NULL) {
 		ng_parse_array_getLength_t *const getLength = type->info;
-		int arraylen, slen;
+		int arraylen;
 
 		arraylen = (*getLength)(type, start, buf);
 		if (arraylen > *buflen) {
 			FREE(str, M_NETGRAPH);
 			return (ERANGE);
 		}
-		slen = strlen(str) + 1;
 		if (slen > arraylen) {
 			FREE(str, M_NETGRAPH);
 			return (E2BIG);
@@ -1518,7 +1580,7 @@ ng_parse_get_token(const char *s, int *startp, int *lenp)
 		*lenp = 1;
 		return T_EQUALS;
 	case '"':
-		if ((t = ng_get_string_token(s, startp, lenp)) == NULL)
+		if ((t = ng_get_string_token(s, startp, lenp, NULL)) == NULL)
 			return T_ERROR;
 		FREE(t, M_NETGRAPH);
 		return T_STRING;
@@ -1537,10 +1599,11 @@ ng_parse_get_token(const char *s, int *startp, int *lenp)
  * The normal C backslash escapes are recognized.
  */
 char *
-ng_get_string_token(const char *s, int *startp, int *lenp)
+ng_get_string_token(const char *s, int *startp, int *lenp, int *slenp)
 {
 	char *cbuf, *p;
 	int start, off;
+	int slen;
 
 	while (isspace(s[*startp]))
 		(*startp)++;
@@ -1551,10 +1614,12 @@ ng_get_string_token(const char *s, int *startp, int *lenp)
 	if (cbuf == NULL)
 		return (NULL);
 	strcpy(cbuf, s + start + 1);
-	for (off = 1, p = cbuf; *p != '\0'; off++, p++) {
+	for (slen = 0, off = 1, p = cbuf; *p != '\0'; slen++, off++, p++) {
 		if (*p == '"') {
 			*p = '\0';
 			*lenp = off + 1;
+			if (slenp != NULL)
+				*slenp = slen;
 			return (cbuf);
 		} else if (p[0] == '\\' && p[1] != '\0') {
 			int x, k;
@@ -1617,19 +1682,21 @@ ng_get_string_token(const char *s, int *startp, int *lenp)
 
 /*
  * Encode a string so it can be safely put in double quotes.
- * Caller must free the result.
+ * Caller must free the result. Exactly "slen" characters
+ * are encoded.
  */
 char *
-ng_encode_string(const char *raw)
+ng_encode_string(const char *raw, int slen)
 {
 	char *cbuf;
 	int off = 0;
+	int i;
 
 	MALLOC(cbuf, char *, strlen(raw) * 4 + 3, M_NETGRAPH, M_NOWAIT);
 	if (cbuf == NULL)
 		return (NULL);
 	cbuf[off++] = '"';
-	for ( ; *raw != '\0'; raw++) {
+	for (i = 0; i < slen; i++, raw++) {
 		switch (*raw) {
 		case '\t':
 			cbuf[off++] = '\\';
diff --git a/sys/netgraph/ng_parse.h b/sys/netgraph/ng_parse.h
index 027605d..13aa294 100644
--- a/sys/netgraph/ng_parse.h
+++ b/sys/netgraph/ng_parse.h
@@ -396,6 +396,17 @@ struct ng_parse_fixedstring_info {
 };
 
 /*
+ * EXPLICITLY SIZED STRING TYPE
+ *
+ * These are strings that have a two byte length field preceding them.
+ * Parsed strings are NOT NUL-terminated.
+ *
+ *   Default value:		Empty string
+ *   Additional info:		None
+ */
+extern const struct ng_parse_type ng_parse_sizedstring_type;
+
+/*
  * COMMONLY USED BOUNDED LENGTH STRING TYPES
  */
 extern const struct ng_parse_type ng_parse_nodebuf_type;  /* NG_NODELEN + 1 */
@@ -506,16 +517,20 @@ extern enum	ng_parse_token ng_parse_get_token(const char *s,
  * the string value.  The string token must be enclosed in double quotes
  * and the normal C backslash escapes are recognized.  The caller must
  * eventually free() the returned result.  Returns NULL if token is
- * not a string token, or parse or other error.
+ * not a string token, or parse or other error. Otherwise, *lenp contains
+ * the number of characters parsed, and *slenp (if not NULL) contains
+ * the actual number of characters in the parsed string.
  */
-extern char	*ng_get_string_token(const char *s, int *startp, int *lenp);
+extern char	*ng_get_string_token(const char *s, int *startp,
+			int *lenp, int *slenp);
 
 /*
  * Convert a raw string into a doubly-quoted string including any
  * necessary backslash escapes.  Caller must free the result.
- * Returns NULL if ENOMEM.
+ * Returns NULL if ENOMEM. Normally "slen" should equal strlen(s)
+ * unless you want to encode NUL bytes.
  */
-extern char	*ng_encode_string(const char *s);
+extern char	*ng_encode_string(const char *s, int slen);
 
 #endif /* _NETGRAPH_PARSE_H_ */
 
diff --git a/sys/netgraph/ng_pppoe.c b/sys/netgraph/ng_pppoe.c
index 3497152..331572f 100644
--- a/sys/netgraph/ng_pppoe.c
+++ b/sys/netgraph/ng_pppoe.c
@@ -60,14 +60,6 @@
 #include <netgraph/ng_parse.h>
 #include <netgraph/ng_pppoe.h>
 
-#define NGM_PPPOE_OLDCOOKIE		939032003
-
-struct ngpppoe_old_init_data {
-	char	hook[NG_HOOKLEN + 1];	/* hook to monitor on */
-	u_int16_t	data_len;	/* Length of the service name */
-	char	data[0];		/* init data goes here */
-};
-
 #define SIGNOFF "session closed"
 #define OFFSETOF(s, e) ((char *)&((s *)0)->e - (char *)((s *)0))
 
@@ -85,12 +77,11 @@ static ng_rcvdata_t	ng_pppoe_rcvdata;
 static ng_disconnect_t	ng_pppoe_disconnect;
 
 /* Parse type for struct ngpppoe_init_data */
-
-static const struct ng_parse_struct_info ng_pppoe_init_data_type_info
+static const struct ng_parse_struct_info ngpppoe_init_data_type_info
 	= NG_PPPOE_INIT_DATA_TYPE_INFO;
-static const struct ng_parse_type ng_pppoe_init_data_state_type = {
+static const struct ng_parse_type ngpppoe_init_data_state_type = {
 	&ng_parse_struct_type,
-	&ng_pppoe_init_data_type_info
+	&ngpppoe_init_data_type_info
 };
 
 /* Parse type for struct ngpppoe_sts */
@@ -107,21 +98,21 @@ static const struct ng_cmdlist ng_pppoe_cmds[] = {
 	  NGM_PPPOE_COOKIE,
 	  NGM_PPPOE_CONNECT,
 	  "pppoe_connect",
-	  &ng_pppoe_init_data_state_type,
+	  &ngpppoe_init_data_state_type,
 	  NULL
 	},
 	{
 	  NGM_PPPOE_COOKIE,
 	  NGM_PPPOE_LISTEN,
 	  "pppoe_listen",
-	  &ng_pppoe_init_data_state_type,
+	  &ngpppoe_init_data_state_type,
 	  NULL
 	},
 	{
 	  NGM_PPPOE_COOKIE,
 	  NGM_PPPOE_OFFER,
 	  "pppoe_offer",
-	  &ng_pppoe_init_data_state_type,
+	  &ngpppoe_init_data_state_type,
 	  NULL
 	},
 	{
@@ -209,7 +200,6 @@ typedef struct sess_neg *negp;
  * Session information that is needed after connection.
  */
 struct sess_con {
-	u_int32_t		typecookie;	/* cookie used by sender */
 	hook_p  		hook;
 	u_int16_t		Session_ID;
 	enum state		state;
@@ -598,7 +588,6 @@ AAA
 		bzero(sp, sizeof(*sp));
 
 		hook->private = sp;
-		sp->typecookie = NGM_PPPOE_COOKIE;
 		sp->hook = hook;
 	}
 	return(0);
@@ -616,53 +605,37 @@ ng_pppoe_rcvmsg(node_p node, struct ng_mesg *msg, const char *retaddr,
 {
 	priv_p privp = node->private;
 	struct ngpppoe_init_data *ourmsg = NULL;
-	struct ngpppoe_old_init_data *oldmsg;
 	struct ng_mesg *resp = NULL;
 	int error = 0;
 	hook_p hook = NULL;
 	sessp sp = NULL;
 	negp neg = NULL;
-	uint16_t datalen = 0;
 
 AAA
 	/* Deal with message according to cookie and command */
 	switch (msg->header.typecookie) {
-	case NGM_PPPOE_OLDCOOKIE: 
 	case NGM_PPPOE_COOKIE: 
 		switch (msg->header.cmd) {
 		case NGM_PPPOE_CONNECT:
 		case NGM_PPPOE_LISTEN: 
 		case NGM_PPPOE_OFFER: 
-			if (msg->header.typecookie == NGM_PPPOE_OLDCOOKIE) {
-				/* Make the old data look like new data */
-
-				oldmsg = (struct ngpppoe_old_init_data *)
-				    msg->data;
-				if (sizeof(*oldmsg) + oldmsg->data_len
-				    > msg->header.arglen) {
-					printf("pppoe_rcvmsg: bad arg size");
-					LEAVE(EMSGSIZE);
-				}
-				datalen = oldmsg->data_len;
-				if (datalen != 0)
-					bcopy(oldmsg->data, &oldmsg->data_len,
-					    datalen);
-				((char *)&oldmsg->data_len)[datalen] = '\0';
-				msg->header.arglen = datalen + sizeof(*ourmsg);
-			}
-
 			ourmsg = (struct ngpppoe_init_data *)msg->data;
+			if (msg->header.arglen < sizeof(*ourmsg)) {
+				printf("pppoe: init data too small\n");
+				LEAVE(EMSGSIZE);
+			}
 			if (msg->header.arglen - sizeof(*ourmsg) > 
 			    PPPOE_SERVICE_NAME_SIZE) {
 				printf("pppoe_rcvmsg: service name too big");
 				LEAVE(EMSGSIZE);
 			}
-			if (ourmsg->data[msg->header.arglen - sizeof(*ourmsg)]
-			    != '\0') {
-				printf("pppoe_rcvmsg: svc name unterminated");
+			if (msg->header.arglen - sizeof(*ourmsg) <
+			    ourmsg->data_len) {
+				printf("pppoe: init data has bad length,"
+				    " %d should be %d\n", ourmsg->data_len,
+				    msg->header.arglen - sizeof (*ourmsg));
 				LEAVE(EMSGSIZE);
 			}
-			datalen = strlen(ourmsg->data);
 
 			/* make sure strcmp will terminate safely */
 			ourmsg->hook[sizeof(ourmsg->hook) - 1] = '\0';
@@ -681,7 +654,6 @@ AAA
 				LEAVE(EINVAL);
 			}
 			sp = hook->private;
-			sp->typecookie = msg->header.typecookie;
 			if (sp->state |= PPPOE_SNONE) {
 				printf("pppoe: Session already active\n");
 				LEAVE(EISCONN);
@@ -749,11 +721,12 @@ AAA
 			 * start it.
 			 */
 			neg->service.hdr.tag_type = PTT_SRV_NAME;
-			neg->service.hdr.tag_len = htons(datalen);
-			if (datalen) {
-				bcopy(ourmsg->data, neg->service.data, datalen);
-			}
-			neg->service_len = datalen;
+			neg->service.hdr.tag_len =
+			    htons((u_int16_t)ourmsg->data_len);
+			if (ourmsg->data_len)
+				bcopy(ourmsg->data, neg->service.data,
+				    ourmsg->data_len);
+			neg->service_len = ourmsg->data_len;
 			pppoe_start(sp);
 			break;
 		case NGM_PPPOE_LISTEN:
@@ -766,12 +739,13 @@ AAA
 
 			 */
 			neg->service.hdr.tag_type = PTT_SRV_NAME;
-			neg->service.hdr.tag_len = htons(datalen);
+			neg->service.hdr.tag_len =
+			    htons((u_int16_t)ourmsg->data_len);
 
-			if (datalen) {
-				bcopy(ourmsg->data, neg->service.data, datalen);
-			}
-			neg->service_len = datalen;
+			if (ourmsg->data_len)
+				bcopy(ourmsg->data, neg->service.data,
+				    ourmsg->data_len);
+			neg->service_len = ourmsg->data_len;
 			neg->pkt->pkt_header.ph.code = PADT_CODE;
 			/*
 			 * wait for PADI packet coming from ethernet
@@ -786,11 +760,12 @@ AAA
 			 * Store the AC-Name given and go to PRIMED.
 			 */
 			neg->ac_name.hdr.tag_type = PTT_AC_NAME;
-			neg->ac_name.hdr.tag_len = htons(datalen);
-			if (datalen) {
-				bcopy(ourmsg->data, neg->ac_name.data, datalen);
-			}
-			neg->ac_name_len = datalen;
+			neg->ac_name.hdr.tag_len =
+			    htons((u_int16_t)ourmsg->data_len);
+			if (ourmsg->data_len)
+				bcopy(ourmsg->data, neg->ac_name.data,
+				    ourmsg->data_len);
+			neg->ac_name_len = ourmsg->data_len;
 			neg->pkt->pkt_header.ph.code = PADO_CODE;
 			/*
 			 * Wait for PADI packet coming from hook
@@ -1607,7 +1582,7 @@ pppoe_send_event(sessp sp, enum cmd cmdid)
 	struct ngpppoe_sts *sts;
 
 AAA
-	NG_MKMESSAGE(msg, sp->typecookie, cmdid,
+	NG_MKMESSAGE(msg, NGM_PPPOE_COOKIE, cmdid,
 			sizeof(struct ngpppoe_sts), M_NOWAIT);
 	sts = (struct ngpppoe_sts *)msg->data;
 	strncpy(sts->hook, sp->hook->name, NG_HOOKLEN + 1);
diff --git a/sys/netgraph/ng_pppoe.h b/sys/netgraph/ng_pppoe.h
index c357ecd..94cb95b 100644
--- a/sys/netgraph/ng_pppoe.h
+++ b/sys/netgraph/ng_pppoe.h
@@ -49,7 +49,7 @@
 /* Node type name. This should be unique among all netgraph node types */
 #define NG_PPPOE_NODE_TYPE	"pppoe"
 
-#define NGM_PPPOE_COOKIE		939032004
+#define NGM_PPPOE_COOKIE		939032003
 
 /* Number of active sessions we can handle */
 #define	PPPOE_NUM_SESSIONS		16 /* for now */
@@ -114,15 +114,16 @@ struct ngpppoestat {
  * and begin negotiation.
  */
 struct ngpppoe_init_data {
-	char	hook[NG_HOOKLEN + 1];	/* hook to monitor on */
-	char	data[1];		/* init data goes here */
+	char		hook[NG_HOOKLEN + 1];	/* hook to monitor on */
+	u_int16_t	data_len;		/* Length of the service name */
+	char		data[0];		/* init data goes here */
 };
 
 /* Keep this in sync with the above structure definition */
 #define NG_PPPOE_INIT_DATA_TYPE_INFO	{		\
 	{						\
 	  { "hook",	&ng_parse_hookbuf_type	},	\
-	  { "data",	&ng_parse_string_type	},	\
+	  { "data",	&ng_parse_sizedstring_type },	\
 	  { NULL }					\
 	}						\
 }