Re: [PATCH] daemon: fix network address handling bugs

[Patrick Steinhardt <ps@pks.im>]

On Sat, May 09, 2026 at 01:48:10AM +0000, Sebastien Tardif via GitGitGadget wrote:
> From: Sebastien Tardif <sebtardif@ncf.ca>
> 
> 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.

> diff --git a/daemon.c b/daemon.c
> index 0a7b1aae44..84a5e38f92 100644
> --- a/daemon.c
> +++ b/daemon.c
> @@ -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)

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);

> @@ -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.

> @@ -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>");

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

Thanks!

Patrick