From: Jack Burton <jack@saosce.com.au>
Subject: ping: iscsid patches
To: tech@openbsd.org
Date: Mon, 13 Jan 2025 16:36:54 +1030

Afternoon, all.

A couple of weeks ago I posted a set of 3 patches for iscid(8) to
tech@ (the first two to fix a couple of bugs; and the last one to add
support for using a wider variety of iscsi targets), but haven't heard
anything back yet.

The original posts are at:
https://marc.info/?l=openbsd-tech&m=173563430210954&w=2
https://marc.info/?l=openbsd-tech&m=173563427710933&w=2
https://marc.info/?l=openbsd-tech&m=173563431810957&w=2

Perhaps I was being a bit too ambitious trying to propose all three at
once, so I'll try again now with just the first one to begin with.  On
the other hand if there's no interest in any further work on iscsid,
then please let me know and I won't bother the list about it again.


Fix memory leaks in some members of struct kvp.

It looks like this is what claudio@ originally intended the otherwise
unused flags member (which has been in the tree since iscsid was first
added 14 years ago) to be used for.

This patch will make very little difference as things stand (as in a
typical run the leaks are tiny), but this approach is necessary for
the third patch in my set, which makes much heavier use of struct kvp
members allocated on the heap.

cvs server: Diffing .
Index: connection.c
===================================================================
RCS file: /cvs/src/usr.sbin/iscsid/connection.c,v
diff -u -p -r1.21 connection.c
--- connection.c	5 Dec 2015 06:38:18 -0000	1.21
+++ connection.c	30 Dec 2024 05:56:32 -0000
@@ -327,11 +327,13 @@ conn_gen_kvp(struct connection *c, struc
 			kvp[i].key = strdup("MaxConnections");
 			if (kvp[i].key == NULL)
 				return -1;
+			kvp[i].flags |= KVP_KEY_ALLOCED;
 			if (asprintf(&kvp[i].value, "%hu",
 			    s->mine.MaxConnections) == -1) {
 				kvp[i].value = NULL;
 				return -1;
 			}
+			kvp[i].flags |= KVP_VALUE_ALLOCED;
 		}
 		i++;
 	}
@@ -341,11 +343,13 @@ conn_gen_kvp(struct connection *c, struc
 			kvp[i].key =
strdup("MaxRecvDataSegmentLength"); if (kvp[i].key == NULL)
 				return -1;
+			kvp[i].flags |= KVP_KEY_ALLOCED;
 			if (asprintf(&kvp[i].value, "%u",
 			    c->mine.MaxRecvDataSegmentLength) == -1) {
 				kvp[i].value = NULL;
 				return -1;
 			}
+			kvp[i].flags |= KVP_VALUE_ALLOCED;
 		}
 		i++;
 	}
Index: initiator.c
===================================================================
RCS file: /cvs/src/usr.sbin/iscsid/initiator.c,v
diff -u -p -r1.15 initiator.c
--- initiator.c	16 Jan 2015 15:57:06 -0000	1.15
+++ initiator.c	30 Dec 2024 05:56:32 -0000
@@ -280,7 +280,7 @@ initiator_login_kvp(struct connection *c
 		if (!(kvp = calloc(nkvp, sizeof(*kvp))))
 			return NULL;
 		if (conn_gen_kvp(c, kvp, &nkvp) == -1) {
-			free(kvp);
+			kvp_free(kvp);
 			return NULL;
 		}
 		break;
@@ -327,10 +327,10 @@ initiator_login_build(struct connection 
 		return NULL;
 	}
 	if ((n = text_to_pdu(kvp, p)) == -1) {
-		free(kvp);
+		kvp_free(kvp);
 		return NULL;
 	}
-	free(kvp);
+	kvp_free(kvp);
 
 	if (n > 8192) {
 		log_warn("initiator_login_build: help, I'm too
verbose"); @@ -409,11 +409,11 @@ initiator_login_cb(struct connection
*c, }
 
 		if (conn_parse_kvp(c, kvp) == -1) {
-			free(kvp);
+			kvp_free(kvp);
 			conn_fail(c);
 			goto done;
 		}
-		free(kvp);
+		kvp_free(kvp);
 	}
 
 	/* advance FSM if possible */
@@ -480,7 +480,7 @@ initiator_discovery_cb(struct connection
 		for (k = kvp; k->key; k++) {
 			log_debug("%s\t=>\t%s", k->key, k->value);  
 		}
-		free(kvp);
+		kvp_free(kvp);
 		session_shutdown(c->session);
 		break;
 	default:
Index: iscsid.h
===================================================================
RCS file: /cvs/src/usr.sbin/iscsid/iscsid.h,v
diff -u -p -r1.19 iscsid.h
--- iscsid.h	21 May 2024 05:00:48 -0000	1.19
+++ iscsid.h	30 Dec 2024 05:56:33 -0000
@@ -385,6 +385,7 @@ void	*pdu_getbuf(struct pdu *, size_t *,
 void	pdu_free(struct pdu *);
 int	socket_setblockmode(int, int);
 const char *log_sockaddr(void *);
+void	kvp_free(struct kvp *);
 
 void	task_init(struct task *, struct session *, int, void *,
 	    void (*)(struct connection *, void *, struct pdu *),
Index: util.c
===================================================================
RCS file: /cvs/src/usr.sbin/iscsid/util.c,v
diff -u -p -r1.9 util.c
--- util.c	12 Apr 2021 10:03:33 -0000	1.9
+++ util.c	30 Dec 2024 05:56:33 -0000
@@ -189,3 +189,19 @@ fail:
 	pdu_free(pdu);
 	return -1;
 }
+
+void
+kvp_free(struct kvp *kvp)
+{
+	struct kvp *k;
+
+	if (kvp == NULL)
+		return;
+	for (k = kvp; k->key; k++) {
+		if (k->flags & KVP_KEY_ALLOCED)
+			free(k->key);
+		if (k->flags & KVP_VALUE_ALLOCED)
+			free(k->value);
+	}
+	free(kvp);
+}