Subject: [PATCH v2] daemon.c: fix segfault on OS X

From: Benjamin Kramer <[email-masked]>
Date: Mon, 27 Apr 2009 15:59:49 +0200

On OS X (and maybe other unices), getaddrinfo(3) returns NULL
in the ai_canonname field if it's called with an IP address for
the hostname. We'll now use the IP address for the hostname if
ai_canonname was NULL, this also matches the behaviour on Linux.

steps to reproduce:
$ git daemon --export-all
$ git clone git://127.0.0.1/frotz
=> git daemon's fork (silently) segfaults.

Remove the pointless loop while at it. There is only one iteration
because of the break; on the last line and there are no continues.

Signed-off-by: Benjamin Kramer <[email-masked]>
---

Junio C Hamano wrote:
> But the part your patch touches is about supporting virtual hosting via
> pattern interpolation, and the daemon will still segfault even with your
> patch when somebody uses %CH expansion, because canon_hostname is left as
> NULL, won't it?  I suspect in such a case it might be safer to use a copy
> of the ip_address or something.

It doesn't segfault but it just assumes an empty hostname:

  $ git daemon --verbose --export-all --interpolated-path=%CH/%D
    [3251] Connection from 127.0.0.1:49423
    [3251] Extended attributes (16 bytes) exist <host=127.0.0.1>
    [3251] Request upload-pack for '/frotz'
    [3251] Interpolated dir '//frotz'

This upated patch uses a copy of ip_address. I did a quick test on my
Linux box and it looks like Linux' getaddrinfo(3) always returns the IP
address in ai_canonname instead of NULL when it is called with an IP
address hostname.

  $ git daemon --verbose --export-all --interpolated-path=%CH/%D
    [3871] Connection from 127.0.0.1:49427
    [3871] Extended attributes (16 bytes) exist <host=127.0.0.1>
    [3871] Request upload-pack for '/frotz'
    [3871] Interpolated dir '127.0.0.1//frotz'

Btw, if I connect to an IPv6 host with interpolated-path=%IP the IP
address gets converted to 0.0.0.0. Is this desired behaviour or yet
another bug?

(I hope thunderbird didn't f*ck up the formatting this time)

 daemon.c |   28 ++++++++++++++--------------
 1 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/daemon.c b/daemon.c
index 13401f1..daa4c8e 100644
--- a/daemon.c
+++ b/daemon.c
@@ -444,27 +444,27 @@ static void parse_extra_args(char *extra_args, int buflen)
 	if (hostname) {
 #ifndef NO_IPV6
 		struct addrinfo hints;
-		struct addrinfo *ai, *ai0;
+		struct addrinfo *ai;
 		int gai;
 		static char addrbuf[HOST_NAME_MAX + 1];
 
 		memset(&hints, 0, sizeof(hints));
 		hints.ai_flags = AI_CANONNAME;
 
-		gai = getaddrinfo(hostname, 0, &hints, &ai0);
+		gai = getaddrinfo(hostname, 0, &hints, &ai);
 		if (!gai) {
-			for (ai = ai0; ai; ai = ai->ai_next) {
-				struct sockaddr_in *sin_addr = (void *)ai->ai_addr;
-
-				inet_ntop(AF_INET, &sin_addr->sin_addr,
-					  addrbuf, sizeof(addrbuf));
-				free(canon_hostname);
-				canon_hostname = xstrdup(ai->ai_canonname);
-				free(ip_address);
-				ip_address = xstrdup(addrbuf);
-				break;
-			}
-			freeaddrinfo(ai0);
+			struct sockaddr_in *sin_addr = (void *)ai->ai_addr;
+
+			inet_ntop(AF_INET, &sin_addr->sin_addr,
+				  addrbuf, sizeof(addrbuf));
+			free(ip_address);
+			ip_address = xstrdup(addrbuf);
+
+			free(canon_hostname);
+			canon_hostname = xstrdup(ai->ai_canonname ?
+						 ai->ai_canonname : ip_address);
+
+			freeaddrinfo(ai);
 		}
 #else
 		struct hostent *hent;
-- 
1.6.3.rc3.1.g35108