avoid some memory leaks / free memory access when reloading conf and have inherited config. patch from Roman Hoog Antink <rha@open.ch> trunk
authorvanhu <vanhu@NetBSD.org>
Mon, 14 Mar 2011 15:50:36 +0000
branchtrunk
changeset 199019 87b132e256e9
parent 199018 4284e191f7b3
child 199020 bec2e06fd7ce
avoid some memory leaks / free memory access when reloading conf and have inherited config. patch from Roman Hoog Antink <rha@open.ch>
crypto/dist/ipsec-tools/src/racoon/cfparse.y
crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.c
crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.h
crypto/dist/ipsec-tools/src/racoon/remoteconf.c
crypto/dist/ipsec-tools/src/racoon/remoteconf.h
crypto/dist/ipsec-tools/src/racoon/rsalist.c
crypto/dist/ipsec-tools/src/racoon/rsalist.h
--- a/crypto/dist/ipsec-tools/src/racoon/cfparse.y	Mon Mar 14 15:21:22 2011 +0000
+++ b/crypto/dist/ipsec-tools/src/racoon/cfparse.y	Mon Mar 14 15:50:36 2011 +0000
@@ -1,4 +1,4 @@
-/*	$NetBSD: cfparse.y,v 1.41 2011/03/02 14:58:27 vanhu Exp $	*/
+/*	$NetBSD: cfparse.y,v 1.42 2011/03/14 15:50:36 vanhu Exp $	*/
 
 /* Id: cfparse.y,v 1.66 2006/08/22 18:17:17 manubsd Exp */
 
@@ -145,6 +145,7 @@
 
 static struct secprotospec *newspspec __P((void));
 static void insspspec __P((struct remoteconf *, struct secprotospec *));
+void dupspspec_list __P((struct remoteconf *dst, struct remoteconf *src));
 void flushspspec __P((struct remoteconf *));
 static void adminsock_conf __P((vchar_t *, vchar_t *, vchar_t *, int));
 
@@ -1629,7 +1630,7 @@
 				return -1;
 			}
 
-			new = duprmconf(from);
+			new = duprmconf_shallow(from);
 			if (new == NULL) {
 				yyerror("failed to duplicate remoteconf from \"%s\".",
 					$4->v);
@@ -1674,13 +1675,14 @@
 				return -1;
 			}
 
-			new = duprmconf(from);
+			new = duprmconf_shallow(from);
 			if (new == NULL) {
 				yyerror("failed to duplicate remoteconf from %s.",
 					saddr2str($4));
 				return -1;
 			}
 
+			racoon_free($4);
 			new->remote = $2;
 			cur_rmconf = new;
 		}
@@ -1727,11 +1729,19 @@
 					return -1;
 				}
 			}
-			
+
+			if (duprmconf_finish(cur_rmconf))
+				return -1;
+
+#if 0
+			/* this pointer copy will never happen, because duprmconf_shallow
+			 * already copied all pointers.
+			 */
 			if (cur_rmconf->spspec == NULL &&
 			    cur_rmconf->inherited_from != NULL) {
 				cur_rmconf->spspec = cur_rmconf->inherited_from->spspec;
 			}
+#endif
 			if (set_isakmp_proposal(cur_rmconf) != 0)
 				return -1;
 
@@ -2415,6 +2425,62 @@
 	rmconf->spspec = spspec;
 }
 
+static struct secprotospec *
+dupspspec(spspec)
+	struct secprotospec *spspec;
+{
+	struct secprotospec *new;
+
+	new = newspspec();
+	if (new == NULL) {
+		plog(LLV_ERROR, LOCATION, NULL, 
+		    "dupspspec: malloc failed\n");
+		return NULL;
+	}
+	memcpy(new, spspec, sizeof(*new));
+
+	if (spspec->gssid) {
+		new->gssid = racoon_strdup(spspec->gssid);
+		STRDUP_FATAL(new->gssid);
+	}
+	if (spspec->remote) {
+		new->remote = racoon_malloc(sizeof(*new->remote));
+		if (new->remote == NULL) {
+			plog(LLV_ERROR, LOCATION, NULL, 
+			    "dupspspec: malloc failed (remote)\n");
+			return NULL;
+		}
+		memcpy(new->remote, spspec->remote, sizeof(*new->remote));
+	}
+
+	return new;
+}
+
+/*
+ * copy the whole list
+ */
+void
+dupspspec_list(dst, src)
+	struct remoteconf *dst, *src;
+{
+	struct secprotospec *p, *new, *last;
+
+	for(p = src->spspec, last = NULL; p; p = p->next, last = new) {
+		new = dupspspec(p);
+		if (new == NULL)
+			exit(1);
+
+		new->prev = last;
+		new->next = NULL; /* not necessary but clean */
+
+		if (last)
+			last->next = new;
+		else /* first element */
+			dst->spspec = new;
+
+	}
+}
+
 /*
  * delete the whole list
  */
@@ -2430,8 +2496,13 @@
 		if (p->next != NULL)
 			p->next->prev = NULL; /* not necessary but clean */
 
-		racoon_free(p);		  
+		if (p->gssid)
+			racoon_free(p->gssid);
+		if (p->remote)
+			racoon_free(p->remote);
+		racoon_free(p);
 	}
+	rmconf->spspec = NULL;
 }
 
 /* set final acceptable proposal */
--- a/crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.c	Mon Mar 14 15:21:22 2011 +0000
+++ b/crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.c	Mon Mar 14 15:50:36 2011 +0000
@@ -1,4 +1,4 @@
-/*	$NetBSD: isakmp_xauth.c,v 1.21 2010/09/27 11:57:59 vanhu Exp $	*/
+/*	$NetBSD: isakmp_xauth.c,v 1.22 2011/03/14 15:50:36 vanhu Exp $	*/
 
 /* Id: isakmp_xauth.c,v 1.38 2006/08/22 18:17:17 manubsd Exp */
 
@@ -1764,3 +1764,42 @@
 
 	return;
 }
+
+struct xauth_rmconf *
+xauth_rmconf_dup(xauth_rmconf)
+	struct xauth_rmconf *xauth_rmconf;
+{
+	struct xauth_rmconf *new;
+
+	if (xauth_rmconf != NULL) {
+		new = racoon_malloc(sizeof(*new));
+		if (new == NULL) {
+			plog(LLV_ERROR, LOCATION, NULL, 
+			    "xauth_rmconf_dup: malloc failed\n");
+			return NULL;
+		}
+
+		memcpy(new, xauth_rmconf, sizeof(*new));
+
+		if (xauth_rmconf->login != NULL) {
+			new->login = vdup(xauth_rmconf->login);
+			if (new->login == NULL) {
+				plog(LLV_ERROR, LOCATION, NULL, 
+				    "xauth_rmconf_dup: malloc failed (login)\n");
+				return NULL;
+			}
+		}
+		if (xauth_rmconf->pass != NULL) {
+			new->pass = vdup(xauth_rmconf->pass);
+			if (new->pass == NULL) {
+				plog(LLV_ERROR, LOCATION, NULL, 
+				    "xauth_rmconf_dup: malloc failed (password)\n");
+				return NULL;
+			}
+		}
+
+		return new;
+	}
+
+	return NULL;
+}
--- a/crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.h	Mon Mar 14 15:21:22 2011 +0000
+++ b/crypto/dist/ipsec-tools/src/racoon/isakmp_xauth.h	Mon Mar 14 15:50:36 2011 +0000
@@ -1,4 +1,4 @@
-/*	$NetBSD: isakmp_xauth.h,v 1.6 2008/09/19 11:01:08 tteras Exp $	*/
+/*	$NetBSD: isakmp_xauth.h,v 1.7 2011/03/14 15:50:36 vanhu Exp $	*/
 
 /*	$KAME$ */
 
@@ -114,6 +114,7 @@
 int xauth_reply(struct ph1handle *, int, int, int);
 int xauth_rmconf_used(struct xauth_rmconf **);
 void xauth_rmconf_delete(struct xauth_rmconf **);
+struct xauth_rmconf * xauth_rmconf_dup(struct xauth_rmconf *);
 
 #ifdef HAVE_LIBPAM
 int xauth_login_pam(int, struct sockaddr *, char *, char *);
--- a/crypto/dist/ipsec-tools/src/racoon/remoteconf.c	Mon Mar 14 15:21:22 2011 +0000
+++ b/crypto/dist/ipsec-tools/src/racoon/remoteconf.c	Mon Mar 14 15:50:36 2011 +0000
@@ -1,4 +1,4 @@
-/*	$NetBSD: remoteconf.c,v 1.25 2011/03/02 15:04:01 vanhu Exp $	*/
+/*	$NetBSD: remoteconf.c,v 1.26 2011/03/14 15:50:36 vanhu Exp $	*/
 
 /* Id: remoteconf.c,v 1.38 2006/05/06 15:52:44 manubsd Exp */
 
@@ -570,8 +570,25 @@
 	return NULL;
 }
 
+void *
+duprsa(entry, arg)
+	void *entry;
+	void *arg;
+{
+	struct rsa_key *new;
+
+	new = rsa_key_dup((struct rsa_key *)entry);
+	if (new == NULL)
+		return (void *) -1;
+	genlist_append(arg, new);
+
+	/* keep genlist_foreach going */
+	return NULL;
+}
+
+/* Creates shallow copy of a remote config. Used for "inherit" keyword. */
 struct remoteconf *
-duprmconf (rmconf)
+duprmconf_shallow (rmconf)
 	struct remoteconf *rmconf;
 {
 	struct remoteconf *new;
@@ -585,31 +602,113 @@
 	new->name = NULL;
 	new->inherited_from = rmconf;
 
-	/* duplicate dynamic structures */
-	if (new->etypes)
+	new->proposal = NULL; /* will be filled by set_isakmp_proposal() */
+
+	return new;
+}
+
+/* Copies pointer structures of an inherited remote config. 
+ * Used by "inherit" mechanism in a two step copy method, necessary to
+ * prevent both double free() and memory leak during config reload.
+ */
+int
+duprmconf_finish (new)
+	struct remoteconf *new;
+{
+	struct remoteconf *rmconf;
+	int i;
+
+	if (new->inherited_from == NULL)
+		return 0; /* nothing todo, no inheritance */
+
+	rmconf = new->inherited_from;
+
+	/* duplicate dynamic structures unless value overridden */
+	if (new->etypes != NULL && new->etypes == rmconf->etypes)
 		new->etypes = dupetypes(new->etypes);
-	new->idvl_p = genlist_init();
-	genlist_foreach(rmconf->idvl_p, dupidvl, new->idvl_p);
+	if (new->idvl_p == rmconf->idvl_p) {
+		new->idvl_p = genlist_init();
+		genlist_foreach(rmconf->idvl_p, dupidvl, new->idvl_p);
+	}
 
-        /* duplicate strings */ 
-	if (new->mycertfile != NULL) { 
+	if (new->rsa_private == rmconf->rsa_private) {
+		new->rsa_private = genlist_init();
+		genlist_foreach(rmconf->rsa_private, duprsa, new->rsa_private);
+	}
+	if (new->rsa_public == rmconf->rsa_public) {
+		new->rsa_public = genlist_init();
+		genlist_foreach(rmconf->rsa_public, duprsa, new->rsa_public);
+	}
+	if (new->remote != NULL && new->remote == rmconf->remote) {
+		new->remote = racoon_malloc(sizeof(*new->remote));
+		if (new->remote == NULL) {
+			plog(LLV_ERROR, LOCATION, NULL, 
+			    "duprmconf_finish: malloc failed (remote)\n");
+			exit(1);
+		}
+		memcpy(new->remote, rmconf->remote, sizeof(*new->remote));
+	}
+	if (new->spspec != NULL && new->spspec == rmconf->spspec) {
+		dupspspec_list(new, rmconf);
+	}
+
+	/* proposal has been deep copied already from spspec's, see
+	 * cfparse.y:set_isakmp_proposal, which in turn calls
+	 * cfparse.y:expand_isakmpspec where the copying happens.
+	 */
+
+#ifdef ENABLE_HYBRID
+	if (new->xauth != NULL && new->xauth == rmconf->xauth) {
+		new->xauth = xauth_rmconf_dup(new->xauth);
+		if (new->xauth == NULL)
+			exit(1);
+	}
+#endif
+
+        /* duplicate strings unless value overridden */ 
+	if (new->mycertfile != NULL && new->mycertfile == rmconf->mycertfile) { 
 		new->mycertfile = racoon_strdup(new->mycertfile); 
 		STRDUP_FATAL(new->mycertfile); 
 	} 
-	if (new->myprivfile != NULL) { 
+	if (new->myprivfile != NULL && new->myprivfile == rmconf->myprivfile) { 
 		new->myprivfile = racoon_strdup(new->myprivfile); 
 		STRDUP_FATAL(new->myprivfile); 
 	} 
-	if (new->peerscertfile != NULL) { 
+	if (new->peerscertfile != NULL && new->peerscertfile == rmconf->peerscertfile) { 
 		new->peerscertfile = racoon_strdup(new->peerscertfile); 
 		STRDUP_FATAL(new->peerscertfile); 
 	} 
-	if (new->cacertfile != NULL) { 
-                new->cacertfile = racoon_strdup(new->cacertfile); 
+	if (new->cacertfile != NULL && new->cacertfile == rmconf->cacertfile) { 
+		new->cacertfile = racoon_strdup(new->cacertfile); 
 		STRDUP_FATAL(new->cacertfile); 
 	} 
+	if (new->idv != NULL && new->idv == rmconf->idv) {
+		new->idv = vdup(new->idv); 
+		STRDUP_FATAL(new->idv); 
+	}
+	if (new->key != NULL && new->key == rmconf->key) {
+		new->key = vdup(new->key); 
+		STRDUP_FATAL(new->key); 
+	}
+	if (new->mycert != NULL && new->mycert == rmconf->mycert) {
+		new->mycert = vdup(new->mycert);
+		STRDUP_FATAL(new->mycert); 
+	}
+	if (new->peerscert != NULL && new->peerscert == rmconf->peerscert) {
+		new->peerscert = vdup(new->peerscert);
+		STRDUP_FATAL(new->peerscert); 
+	}
+	if (new->cacert != NULL && new->cacert == rmconf->cacert) {
+		new->cacert = vdup(new->cacert);
+		STRDUP_FATAL(new->cacert); 
+	}
+	for (i = 0; i <= SCRIPT_MAX; i++)
+		if (new->script[i] != NULL && new->script[i] == rmconf->script[i]) {
+			new->script[i] = vdup(new->script[i]);
+			STRDUP_FATAL(new->script[i]);
+		}
 
-	return new;
+	return 0;
 }
 
 static void
@@ -623,6 +722,8 @@
 delrmconf(rmconf)
 	struct remoteconf *rmconf;
 {
+	int i;
+
 #ifdef ENABLE_HYBRID
 	if (rmconf->xauth)
 		xauth_rmconf_delete(&rmconf->xauth);
@@ -631,12 +732,17 @@
 		deletypes(rmconf->etypes);
 		rmconf->etypes=NULL;
 	}
+	if (rmconf->idv)
+		vfree(rmconf->idv);
+	if (rmconf->key)
+		vfree(rmconf->key);
 	if (rmconf->idvl_p)
 		genlist_free(rmconf->idvl_p, idspec_free);
 	if (rmconf->dhgrp)
 		oakley_dhgrp_free(rmconf->dhgrp);
 	if (rmconf->proposal)
 		delisakmpsa(rmconf->proposal);
+	flushspspec(rmconf);
 	if (rmconf->mycert)
 		vfree(rmconf->mycert);
 	if (rmconf->mycertfile)
@@ -659,7 +765,10 @@
 		racoon_free(rmconf->name);
 	if (rmconf->remote)
 		racoon_free(rmconf->remote);
-	flushspspec(rmconf);
+	for (i = 0; i <= SCRIPT_MAX; i++)
+		if (rmconf->script[i])
+			vfree(rmconf->script[i]);
+
 	racoon_free(rmconf);
 }
 
--- a/crypto/dist/ipsec-tools/src/racoon/remoteconf.h	Mon Mar 14 15:21:22 2011 +0000
+++ b/crypto/dist/ipsec-tools/src/racoon/remoteconf.h	Mon Mar 14 15:50:36 2011 +0000
@@ -1,4 +1,4 @@
-/*	$NetBSD: remoteconf.h,v 1.15 2011/03/02 14:58:27 vanhu Exp $	*/
+/*	$NetBSD: remoteconf.h,v 1.16 2011/03/14 15:50:36 vanhu Exp $	*/
 
 /* Id: remoteconf.h,v 1.26 2006/05/06 15:52:44 manubsd Exp */
 
@@ -201,13 +201,15 @@
 extern struct remoteconf *getrmconf_by_name __P((const char *name));
 
 extern struct remoteconf *newrmconf __P((void));
-extern struct remoteconf *duprmconf __P((struct remoteconf *));
+extern struct remoteconf *duprmconf_shallow __P((struct remoteconf *));
+extern int duprmconf_finish __P((struct remoteconf *));
 extern void delrmconf __P((struct remoteconf *));
 extern void deletypes __P((struct etypes *));
 extern struct etypes * dupetypes __P((struct etypes *));
 extern void insrmconf __P((struct remoteconf *));
 extern void remrmconf __P((struct remoteconf *));
 extern void flushrmconf __P((void));
+extern void dupspspec_list __P((struct remoteconf *, struct remoteconf *));
 extern void flushspspec __P((struct remoteconf *));
 extern void initrmconf __P((void));
 extern void rmconf_start_reload __P((void));
--- a/crypto/dist/ipsec-tools/src/racoon/rsalist.c	Mon Mar 14 15:21:22 2011 +0000
+++ b/crypto/dist/ipsec-tools/src/racoon/rsalist.c	Mon Mar 14 15:50:36 2011 +0000
@@ -1,4 +1,4 @@
-/*	$NetBSD: rsalist.c,v 1.5 2011/03/02 15:04:01 vanhu Exp $	*/
+/*	$NetBSD: rsalist.c,v 1.6 2011/03/14 15:50:36 vanhu Exp $	*/
 
 /* Id: rsalist.c,v 1.3 2004/11/08 12:04:23 ludvigm Exp */
 
@@ -88,6 +88,48 @@
 	return 0;
 }
 
+struct rsa_key *
+rsa_key_dup(struct rsa_key *key)
+{
+	struct rsa_key *new;
+
+	new = calloc(sizeof(struct rsa_key), 1);
+	if (new == NULL)
+		return NULL;
+
+	if (key->rsa) {
+		new->rsa = key->rsa->d != NULL ? RSAPrivateKey_dup(key->rsa) : RSAPublicKey_dup(key->rsa);
+		if (new->rsa == NULL)
+			goto dup_error;
+	}
+
+	if (key->src) {
+		new->src = malloc(sizeof(*new->src));
+		if (new->src == NULL)
+			goto dup_error;
+		memcpy(new->src, key->src, sizeof(*new->src));
+	}	
+	if (key->dst) {
+		new->dst = malloc(sizeof(*new->dst));
+		if (new->dst == NULL)
+			goto dup_error;
+		memcpy(new->dst, key->dst, sizeof(*new->dst));
+	}
+
+	return new;
+
+dup_error:
+	if (new->rsa != NULL)
+		RSA_free(new->rsa);
+	if (new->dst != NULL)
+		free(new->dst);
+	if (new->src != NULL)
+		free(new->src);
+
+	free(new);
+	return NULL;
+}
+
 void
 rsa_key_free(void *data)
 {
--- a/crypto/dist/ipsec-tools/src/racoon/rsalist.h	Mon Mar 14 15:21:22 2011 +0000
+++ b/crypto/dist/ipsec-tools/src/racoon/rsalist.h	Mon Mar 14 15:50:36 2011 +0000
@@ -1,4 +1,4 @@
-/*	$NetBSD: rsalist.h,v 1.5 2011/03/02 15:04:01 vanhu Exp $	*/
+/*	$NetBSD: rsalist.h,v 1.6 2011/03/14 15:50:36 vanhu Exp $	*/
 
 /* Id: rsalist.h,v 1.2 2004/07/12 20:43:51 ludvigm Exp */
 /*
@@ -53,6 +53,7 @@
 };
 
 int rsa_key_insert(struct genlist *list, struct netaddr *src, struct netaddr *dst, RSA *rsa);
+struct rsa_key *rsa_key_dup(struct rsa_key *key);
 void rsa_key_free(void *data);
 void rsa_key_dump(struct genlist *list);