Bug 5851 - Name lookup failures
Name lookup failures
Status: RESOLVED FIXED
Product: rsync
Classification: Unclassified
Component: core
3.0.4
All Windows XP
: P3 major
: ---
Assigned To: Wayne Davison
Rsync QA Contact
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-10-24 12:59 UTC by Steven Hartland
Modified: 2008-10-25 13:06 UTC (History)
0 users

See Also:


Attachments
Patch to fix the issue (852 bytes, application/zip)
2008-10-24 13:00 UTC, Steven Hartland
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Steven Hartland 2008-10-24 12:59:25 UTC
Just updated to 3.0.4 under cygwin and I'm now getting lookup
failures in the log.
2008/10/24 00:13:23 [1984] name lookup failed for X.X.X.X: Unknown server error

Looking at the code it seems the call will never succeed?
    /* reverse lookup */
    name_err = getnameinfo((struct sockaddr *) ss, ss_len,
                   name_buf, name_buf_size,
                   port_buf, port_buf_size,
                   NI_NAMEREQD | NI_NUMERICSERV);
    if (name_err != 0) {
        strlcpy(name_buf, default_name, name_buf_size);
        rprintf(FLOG, "name lookup failed for %s: %s\n",
            client_addr(fd), gai_strerror(name_err));
        return name_err;
    }

Then in the supplied getnameinfo:-
    /* We don't support those. */
    if ((node && !(flags & NI_NUMERICHOST))
        || (service && !(flags & NI_NUMERICSERV)))
        return EAI_FAIL;

    if (node) {
        return gethostnameinfo(sa, node, nodelen, flags);
    }

    if (service) {
        return getservicenameinfo(sa, service, servicelen, flags);
    }
    return 0;

There seems to be three issues here:-
1. Both name ( node ) and port ( service ) are passed in but NI_NUMERICHOST
is not supplied so the call will always return EAI_FAIL due to the "We don't
support those" check.
2. getnameinfo returns early without completing the service lookup,
if node information is requested in addition to service info.
3. The "don't support these" check is totally invalid as the code does actually
appear support both these types requests as far as I can see.

Given the above I believe the fix is to remove the following block totally:-
    /* We don't support those. */
    if ((node && !(flags & NI_NUMERICHOST))
        || (service && !(flags & NI_NUMERICSERV)))
        return EAI_FAIL;

And update the remaining block to:-
    if (node) {
        int ret = gethostnameinfo(sa, node, nodelen, flags);
        if (0 != ret) {
            return ret;
        }
    }

    if (service) {
        return getservicenameinfo(sa, service, servicelen, flags);
    }

Next up is allow hosts CIDR notation seems to have been broken. This looks
like a really old regression caused by the following commit:-
http://git.samba.org/?p=rsync.git;a=commitdiff;h=bc2b4963a009dd8194b2e9f996a63b9c634a6263

The fix is fairly trivial:-
        if ( ! strchr(p,'.')) {
            // CIDR notation
            int bits = atoi(p+1);
            if (bits == 0) return 1;
            if (bits <= 0 || bits > 32) {
                rprintf(FLOG,"malformed mask in %s\n", tok);
                return 0;
            }
        }
        else if (inet_pton(resa->ai_addr->sa_family, p, mask) <= 0) {
            // Dot notation ( netmask )
Comment 1 Steven Hartland 2008-10-24 13:00:36 UTC
Created attachment 3686 [details]
Patch to fix the issue
Comment 2 Wayne Davison 2008-10-25 11:23:20 UTC
I have applied your fix to lib/getaddrinfo.c -- much appreciated!

The change to access.c is not needed, though, as the code right after your insertion is the code that handles CIDR notation.  A value such as "16" passed to inet_pton() should return back a 0, since it does not have the required format for either an IPv4 or IPv6 address.  If cygwin is returning a positive value, you can use the version of inet_pton() provided by the lib/inet_pton.c file (and report the error to the cygwin folks).

Thanks for your report!
Comment 3 Steven Hartland 2008-10-25 13:06:28 UTC
Confirmed inet_pton when passed "24" for x.x.x.x/24 is returning 1 and hence causing the issue. Forcing the use of lib/inet_pton.c fixed this.