Name lookup failures and CIDR regression under cygwin ( patch attached )

Steven Hartland killing at multiplay.co.uk
Fri Oct 24 00:37:41 GMT 2008


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 )

Unified diffs in the zip attached ( if it gets through ).

    Regards
    Steve

================================================
This e.mail is private and confidential between Multiplay (UK) Ltd. and the person or entity to whom it is addressed. In the event of misdirection, the recipient is prohibited from using, copying, printing or otherwise disseminating it or any information contained in it. 

In the event of misdirection, illegible or incomplete transmission please telephone +44 845 868 1337
or return the E.mail to postmaster at multiplay.co.uk.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: patch.zip
Type: application/x-zip-compressed
Size: 852 bytes
Desc: not available
Url : http://lists.samba.org/archive/rsync/attachments/20081024/922e1a22/patch.bin


More information about the rsync mailing list