daemon: fix network address handling bugs

PATCH In review Sebastien Tardif via GitGitGadget 2 participants 2 messages May 9 – May 11

lore

Share & tools

lore

Sebastien Tardif via GitGitGadget

author

2026-05-09 01:48 UTC #0 lore raw

PATCH 1/1 daemon: fix network address handling bugs

From: Sebastien Tardif <[email protected]>

Fix three related issues in daemon.c's network address handling:

lookup_hostname() calls getaddrinfo() with AF_UNSPEC hints, so it may return IPv6 results. However, the code unconditionally casts ai_addr to sockaddr_in and passes AF_INET to inet_ntop(). On IPv6-only hosts, this reads from the wrong struct offset, producing garbage IP addresses. Fix by checking ai_family and handling both AF_INET and AF_INET6.

ip2str() passes the sockaddr struct size (ai_addrlen) as the output buffer size argument to inet_ntop(). For IPv6, sizeof(sockaddr_in6) is 28 bytes but INET6_ADDRSTRLEN is 46, so long IPv6 addresses are silently truncated. Fix by passing sizeof(ip) instead, and drop the now-unused len parameter.

execute() logs "Connection from %s:%s" using REMOTE_ADDR and REMOTE_PORT environment variables, but only checks REMOTE_ADDR for NULL. If REMOTE_PORT is unset, NULL is passed to printf's %s, which is undefined behavior. Fix by using a fallback string.

Signed-off-by Sebastien Tardif <[email protected]>

---
daemon: fix network address handling bugs

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-git-2299%2FSebTardif%2Ffix%2Fdaemon-ipv6-and-null-port-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-git-2299/SebTardif/fix/daemon-ipv6-and-null-port-v1
Pull-Request: https://github.com/git/git/pull/2299

 daemon.c | 31 +++++++++++++++++++------------
 1 file changed, 19 insertions(+), 12 deletions(-)

daemon.c +19 ~~-12~~

index 0a7b1aae44..84a5e38f92 100644
@@ -674,10 +674,17 @@ static void lookup_hostname(struct hostinfo *hi)
 
 		gai = getaddrinfo(hi->hostname.buf, NULL, &hints, &ai);
 		if (!gai) {
-			struct sockaddr_in *sin_addr = (void *)ai->ai_addr;
-
-			inet_ntop(AF_INET, &sin_addr->sin_addr,
-				  addrbuf, sizeof(addrbuf));
+			if (ai->ai_family == AF_INET) {
+				struct sockaddr_in *sa =
+					(struct sockaddr_in *)ai->ai_addr;
+				inet_ntop(AF_INET, &sa->sin_addr,
+					  addrbuf, sizeof(addrbuf));
+			} else if (ai->ai_family == AF_INET6) {
+				struct sockaddr_in6 *sa6 =
+					(struct sockaddr_in6 *)ai->ai_addr;
+				inet_ntop(AF_INET6, &sa6->sin6_addr,
+					  addrbuf, sizeof(addrbuf));
+			}
 			strbuf_addstr(&hi->ip_address, addrbuf);
 
 			if (ai->ai_canonname)

@@ -742,7 +749,7 @@ static int execute(void)
 	struct strvec env = STRVEC_INIT;
 
 	if (addr)
-		loginfo("Connection from %s:%s", addr, port);
+		loginfo("Connection from %s:%s", addr, port ? port : "?");
 
 	set_keep_alive(0);
 	alarm(init_timeout ? init_timeout : timeout);

@@ -936,7 +943,7 @@ struct socketlist {
 	size_t alloc;
 };
 
-static const char *ip2str(int family, struct sockaddr *sin, socklen_t len)
+static const char *ip2str(int family, struct sockaddr *sin)
 {
 #ifdef NO_IPV6
 	static char ip[INET_ADDRSTRLEN];

@@ -947,11 +954,11 @@ static const char *ip2str(int family, struct sockaddr *sin, socklen_t len)
 	switch (family) {
 #ifndef NO_IPV6
 	case AF_INET6:
-		inet_ntop(family, &((struct sockaddr_in6*)sin)->sin6_addr, ip, len);
+		inet_ntop(family, &((struct sockaddr_in6*)sin)->sin6_addr, ip, sizeof(ip));
 		break;
 #endif
 	case AF_INET:
-		inet_ntop(family, &((struct sockaddr_in*)sin)->sin_addr, ip, len);
+		inet_ntop(family, &((struct sockaddr_in*)sin)->sin_addr, ip, sizeof(ip));
 		break;
 	default:
 		xsnprintf(ip, sizeof(ip), "<unknown>");

@@ -1008,14 +1015,14 @@ static int setup_named_sock(char *listen_addr, int listen_port, struct socketlis
 
 		if (bind(sockfd, ai->ai_addr, ai->ai_addrlen) < 0) {
 			logerror("Could not bind to %s: %s",
-				 ip2str(ai->ai_family, ai->ai_addr, ai->ai_addrlen),
+				 ip2str(ai->ai_family, ai->ai_addr),
 				 strerror(errno));
 			close(sockfd);
 			continue;	/* not fatal */
 		}
 		if (listen(sockfd, 5) < 0) {
 			logerror("Could not listen to %s: %s",
-				 ip2str(ai->ai_family, ai->ai_addr, ai->ai_addrlen),
+				 ip2str(ai->ai_family, ai->ai_addr),
 				 strerror(errno));
 			close(sockfd);
 			continue;	/* not fatal */

@@ -1069,7 +1076,7 @@ static int setup_named_sock(char *listen_addr, int listen_port, struct socketlis
 
 	if ( bind(sockfd, (struct sockaddr *)&sin, sizeof sin) < 0 ) {
 		logerror("Could not bind to %s: %s",
-			 ip2str(AF_INET, (struct sockaddr *)&sin, sizeof(sin)),
+			 ip2str(AF_INET, (struct sockaddr *)&sin),
 			 strerror(errno));
 		close(sockfd);
 		return 0;

@@ -1077,7 +1084,7 @@ static int setup_named_sock(char *listen_addr, int listen_port, struct socketlis
 
 	if (listen(sockfd, 5) < 0) {
 		logerror("Could not listen to %s: %s",
-			 ip2str(AF_INET, (struct sockaddr *)&sin, sizeof(sin)),
+			 ip2str(AF_INET, (struct sockaddr *)&sin),
 			 strerror(errno));
 		close(sockfd);
 		return 0;

base-commit: 94f057755b7941b321fd11fec1b2e3ca5313a4e0

Signature · 1 line

gitgitgadget

Patrick Steinhardt

2026-05-11 07:54 UTC #1 lore raw

Quoted 3 lines · Sebastien Tardif via GitGitGadget

From: Sebastien Tardif <[email protected]>
Fix three related issues in daemon.c's network address handling:

This is a good indicator that this patch should be split up into three patches, where each patch addresses one of the issues.

Quoted 16 lines

daemon.c +4 -4

index 0a7b1aae44..84a5e38f92 100644
@@ -674,10 +674,17 @@ static void lookup_hostname(struct hostinfo *hi)
 
 		gai = getaddrinfo(hi->hostname.buf, NULL, &hints, &ai);
 		if (!gai) {
-			struct sockaddr_in *sin_addr = (void *)ai->ai_addr;
-
-			inet_ntop(AF_INET, &sin_addr->sin_addr,
-				  addrbuf, sizeof(addrbuf));
+			if (ai->ai_family == AF_INET) {
+				struct sockaddr_in *sa =
+					(struct sockaddr_in *)ai->ai_addr;
+				inet_ntop(AF_INET, &sa->sin_addr,

daemon.c static void lookup_hostname(struct hostinfo *hi) · line 690

Quoted 10 lines

daemon.c +7 -0

+					  addrbuf, sizeof(addrbuf));
+			} else if (ai->ai_family == AF_INET6) {
+				struct sockaddr_in6 *sa6 =
+					(struct sockaddr_in6 *)ai->ai_addr;
+				inet_ntop(AF_INET6, &sa6->sin6_addr,
+					  addrbuf, sizeof(addrbuf));
+			}
 			strbuf_addstr(&hi->ip_address, addrbuf);
 
 			if (ai->ai_canonname)

We could deduplicate the logic by assigning the address pointer to a local field first:

    gai = getaddrinfo(hi->hostname.buf, NULL, &hints, &ai);
    if (!gai) {
        struct sockaddr_in *sin_addr = (void *)ai->ai_addr;
        void *addr;

        if (ai->ai_family == AF_INET) {
            struct sockaddr_in *sa = ai->ai_addr;
            addr = sa->sin_addr;
        } else if (ai->ai_family == AF_INET6) {
            struct sockaddr_in6 *sa6 = ai->ai_addr;
            addr = sa->sin6_addr;
        } else {
            die("unexpected address info family");
        }

        inet_ntop(ai->ai_family, addr, addrbuf, sizeof(addrbuf));
        strbuf_addstr(&hi->ip_address, addrbuf);

daemon.c static int execute(void) · line 752

Quoted 6 lines

daemon.c +1 -1

@@ -742,7 +749,7 @@ static int execute(void)
 	struct strvec env = STRVEC_INIT;
 
 	if (addr)
-		loginfo("Connection from %s:%s", addr, port);
+		loginfo("Connection from %s:%s", addr, port ? port : "?");

Hm. It shouldn't ever happen that either of these is unset as far as I know. But it's weird indeed that we check for one of them to exist, but not for the other.

Quoted 13 lines

daemon.c +1 -1

@@ -936,7 +943,7 @@ struct socketlist {
 	size_t alloc;
 };
 
-static const char *ip2str(int family, struct sockaddr *sin, socklen_t len)
+static const char *ip2str(int family, struct sockaddr *sin)
 {
 #ifdef NO_IPV6
 	static char ip[INET_ADDRSTRLEN];

@@ -947,11 +954,11 @@ static const char *ip2str(int family, struct sockaddr *sin, socklen_t len)
 	switch (family) {
 #ifndef NO_IPV6
 	case AF_INET6:

daemon.c static const char *ip2str(int family, struct sockaddr *sin, socklen_t len) · line 964

Quoted 10 lines

daemon.c +2 -2

-		inet_ntop(family, &((struct sockaddr_in6*)sin)->sin6_addr, ip, len);
+		inet_ntop(family, &((struct sockaddr_in6*)sin)->sin6_addr, ip, sizeof(ip));
 		break;
 #endif
 	case AF_INET:
-		inet_ntop(family, &((struct sockaddr_in*)sin)->sin_addr, ip, len);
+		inet_ntop(family, &((struct sockaddr_in*)sin)->sin_addr, ip, sizeof(ip));
 		break;
 	default:
 		xsnprintf(ip, sizeof(ip), "<unknown>");

Right, the last parameter of inet_ntop(3p) declares the size of the output buffer, not of the input address.

Thanks!

Patrick