From 4b2a73e464a8f6ec30a8a8db0514ee4520180f42 Mon Sep 17 00:00:00 2001
From: Eric Faurot <eric@cvs.openbsd.org>
Date: Wed, 23 Dec 2020 15:42:04 +0000
Subject: Fix a memory leak: use a single memory allocation for struct addrinfo
 and the sockaddr it contains, as expected by freeaddrinfo(). Move the
 allocation to a helper function for clarity.

comments from martijn@ millert@

ok millert@
---
 usr.sbin/smtpd/resolver.c | 69 ++++++++++++++++++++++++++---------------------
 1 file changed, 38 insertions(+), 31 deletions(-)

(limited to 'usr.sbin/smtpd')

diff --git a/usr.sbin/smtpd/resolver.c b/usr.sbin/smtpd/resolver.c
index ce047b24927..cc0d28efcc4 100644
--- a/usr.sbin/smtpd/resolver.c
+++ b/usr.sbin/smtpd/resolver.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: resolver.c,v 1.5 2019/06/13 11:45:35 eric Exp $	*/
+/*	$OpenBSD: resolver.c,v 1.6 2020/12/23 15:42:03 eric Exp $	*/
 
 /*
  * Copyright (c) 2017-2018 Eric Faurot <eric@openbsd.org>
@@ -277,12 +277,39 @@ resolver_dispatch_request(struct mproc *proc, struct imsg *imsg)
 	}
 }
 
+static struct addrinfo *
+_alloc_addrinfo(const struct addrinfo *ai0, const struct sockaddr *sa,
+    const char *cname)
+{
+	struct addrinfo *ai;
+
+	ai = calloc(1, sizeof(*ai) + sa->sa_len);
+	if (ai == NULL) {
+		log_warn("%s: calloc", __func__);
+		return NULL;
+	}
+	*ai = *ai0;
+	ai->ai_addr = (void *)(ai + 1);
+	memcpy(ai->ai_addr, sa, sa->sa_len);
+
+	if (cname) {
+		ai->ai_canonname = strdup(cname);
+		if (ai->ai_canonname == NULL) {
+			log_warn("%s: strdup", __func__);
+			free(ai);
+			return NULL;
+		}
+	}
+
+	return ai;
+}
+
 void
 resolver_dispatch_result(struct mproc *proc, struct imsg *imsg)
 {
 	struct request key, *req;
 	struct sockaddr_storage ss;
-	struct addrinfo *ai;
+	struct addrinfo *ai, tai;
 	struct msg m;
 	const char *cname, *host, *serv;
 	const void *data;
@@ -299,40 +326,20 @@ resolver_dispatch_result(struct mproc *proc, struct imsg *imsg)
 	switch (imsg->hdr.type) {
 
 	case IMSG_GETADDRINFO:
-		ai = calloc(1, sizeof(*ai));
-		if (ai == NULL) {
-			log_warn("%s: calloc", __func__);
-			break;
-		}
-		m_get_int(&m, &ai->ai_flags);
-		m_get_int(&m, &ai->ai_family);
-		m_get_int(&m, &ai->ai_socktype);
-		m_get_int(&m, &ai->ai_protocol);
+		memset(&tai, 0, sizeof(tai));
+		m_get_int(&m, &tai.ai_flags);
+		m_get_int(&m, &tai.ai_family);
+		m_get_int(&m, &tai.ai_socktype);
+		m_get_int(&m, &tai.ai_protocol);
 		m_get_sockaddr(&m, (struct sockaddr *)&ss);
 		m_get_string(&m, &cname);
 		m_end(&m);
 
-		ai->ai_addr = malloc(ss.ss_len);
-		if (ai->ai_addr == NULL) {
-			log_warn("%s: malloc", __func__);
-			free(ai);
-			break;
+		ai = _alloc_addrinfo(&tai, (struct sockaddr *)&ss, cname);
+		if (ai) {
+			ai->ai_next = req->ai;
+			req->ai = ai;
 		}
-
-		memmove(ai->ai_addr, &ss, ss.ss_len);
-
-		if (cname) {
-			ai->ai_canonname = strdup(cname);
-			if (ai->ai_canonname == NULL) {
-				log_warn("%s: strdup", __func__);
-				free(ai->ai_addr);
-				free(ai);
-				break;
-			}
-		}
-
-		ai->ai_next = req->ai;
-		req->ai = ai;
 		break;
 
 	case IMSG_GETADDRINFO_END:
-- 
cgit v1.2.3