From fd5898572f7952aa1d29d5d740381e049f7bc3b3 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 30 Apr 2012 16:16:39 -0700 Subject: [PATCH 1/4] Fix remove_duplicate_addrs2 to do exactly what it says. Previously it could leave zero addresses in the list. --- source3/libsmb/namequery.c | 19 +++++++++---------- 1 files changed, 9 insertions(+), 10 deletions(-) diff --git a/source3/libsmb/namequery.c b/source3/libsmb/namequery.c index 858330d..ff43706 100644 --- a/source3/libsmb/namequery.c +++ b/source3/libsmb/namequery.c @@ -571,7 +571,7 @@ static int remove_duplicate_addrs2(struct ip_service *iplist, int count ) DEBUG(10,("remove_duplicate_addrs2: " "looking for duplicate address/port pairs\n")); - /* one loop to remove duplicates */ + /* One loop to set duplicates to a zero addr. */ for ( i=0; i0) { + memmove(&iplist[i], + &iplist[i+1], + (count-i-1)*sizeof(struct ip_service)); } count--; - continue; } - i++; } return count; -- 1.7.7.3 From 5c22063f5eb80d8889ee05f7cd290ec5cba81ac5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 30 Apr 2012 16:24:27 -0700 Subject: [PATCH 2/4] Fix convert_ss2service() to filter out zero addresses. --- source3/libsmb/namequery.c | 41 +++++++++++++++++++++++++++++++---------- 1 files changed, 31 insertions(+), 10 deletions(-) diff --git a/source3/libsmb/namequery.c b/source3/libsmb/namequery.c index ff43706..b68448e 100644 --- a/source3/libsmb/namequery.c +++ b/source3/libsmb/namequery.c @@ -848,32 +848,53 @@ struct sockaddr_storage *name_query(int fd, } /******************************************************** - convert an array if struct sockaddr_storage to struct ip_service + Convert an array if struct sockaddr_storage to struct ip_service return false on failure. Port is set to PORT_NONE; + pcount is [in/out] - it is the length of ss_list on input, + and the length of return_iplist on output as we remove any + zero addresses from ss_list. *********************************************************/ static bool convert_ss2service(struct ip_service **return_iplist, const struct sockaddr_storage *ss_list, - int count) + int *pcount) { int i; + int orig_count = *pcount; + int real_count = 0; - if ( count==0 || !ss_list ) + if (orig_count==0 || !ss_list ) return False; + /* Filter out zero addrs. */ + for ( i=0; i Date: Mon, 30 Apr 2012 16:29:19 -0700 Subject: [PATCH 3/4] Protect all of the name resolution methods from returning null addrs. Ensure all returns go through remove_duplicate_addrs2(). --- source3/libsmb/namequery.c | 24 +++++++++++++++--------- 1 files changed, 15 insertions(+), 9 deletions(-) diff --git a/source3/libsmb/namequery.c b/source3/libsmb/namequery.c index b68448e..671cb76 100644 --- a/source3/libsmb/namequery.c +++ b/source3/libsmb/namequery.c @@ -1250,6 +1250,10 @@ static NTSTATUS resolve_hosts(const char *name, int name_type, ZERO_STRUCT(ss); memcpy(&ss, res->ai_addr, res->ai_addrlen); + if (is_zero_addr((struct sockaddr *)&ss)) { + continue; + } + *return_count += 1; *return_iplist = SMB_REALLOC_ARRAY(*return_iplist, @@ -1438,6 +1442,10 @@ NTSTATUS internal_resolve_name(const char *name, SAFE_FREE(*return_iplist); return NT_STATUS_INVALID_PARAMETER; } + if (is_zero_addr((struct sockaddr *)&(*return_iplist)->ss)) { + SAFE_FREE(*return_iplist); + return NT_STATUS_UNSUCCESSFUL; + } *return_count = 1; return NT_STATUS_OK; } @@ -1445,6 +1453,8 @@ NTSTATUS internal_resolve_name(const char *name, /* Check name cache */ if (namecache_fetch(name, name_type, return_iplist, return_count)) { + *return_count = remove_duplicate_addrs2(*return_iplist, + *return_count ); /* This could be a negative response */ if (*return_count > 0) { return NT_STATUS_OK; @@ -1539,10 +1549,7 @@ NTSTATUS internal_resolve_name(const char *name, controllers including the PDC in iplist[1..n]. Iterating over the iplist when the PDC is down will cause two sets of timeouts. */ - if ( *return_count ) { - *return_count = remove_duplicate_addrs2(*return_iplist, - *return_count ); - } + *return_count = remove_duplicate_addrs2(*return_iplist, *return_count ); /* Save in name cache */ if ( DEBUGLEVEL >= 100 ) { @@ -1558,7 +1565,9 @@ NTSTATUS internal_resolve_name(const char *name, } } - namecache_store(name, name_type, *return_count, *return_iplist); + if (*return_count) { + namecache_store(name, name_type, *return_count, *return_iplist); + } /* Display some debugging info */ @@ -2021,10 +2030,7 @@ static NTSTATUS get_dc_list(const char *domain, /* need to remove duplicates in the list if we have any explicit password servers */ - if (local_count) { - local_count = remove_duplicate_addrs2(return_iplist, - local_count ); - } + local_count = remove_duplicate_addrs2(return_iplist, local_count ); /* For DC's we always prioritize IPv4 due to W2K3 not * supporting LDAP, KRB5 or CLDAP over IPv6. */ -- 1.7.7.3 From 9abc441584a2c07b0518dbe5a1a21030818c1cf5 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 30 Apr 2012 16:32:51 -0700 Subject: [PATCH 4/4] Fix the loop unrolling inside resolve_ads(). If we don't get an IP list don't use interpret_string_addr(), as this only returns one address, use interpret_string_addr_internal() instead. --- source3/libsmb/namequery.c | 105 +++++++++++++++++++++++++++++-------------- 1 files changed, 71 insertions(+), 34 deletions(-) diff --git a/source3/libsmb/namequery.c b/source3/libsmb/namequery.c index 671cb76..af76f3f 100644 --- a/source3/libsmb/namequery.c +++ b/source3/libsmb/namequery.c @@ -1287,7 +1287,7 @@ static NTSTATUS resolve_ads(const char *name, struct ip_service **return_iplist, int *return_count) { - int i, j; + int i; NTSTATUS status; TALLOC_CTX *ctx; struct dns_rr_srv *dcs = NULL; @@ -1336,7 +1336,11 @@ static NTSTATUS resolve_ads(const char *name, } for (i=0;iport = dcs[i].port; - - /* If we don't have an IP list for a name, lookup it up */ + for (i = 0; i < numdcs; i++) { + /* If we don't have an IP list for a name, look it up */ if (!dcs[i].ss_s) { - interpret_string_addr(&r->ss, dcs[i].hostname, 0); - i++; - j = 0; - } else { - /* use the IP addresses from the SRV sresponse */ - - if ( j >= dcs[i].num_ips ) { - i++; - j = 0; + /* We need to get all IP addresses here. */ + struct addrinfo *res = NULL; + struct addrinfo *p; + int extra_addrs = 0; + + if (!interpret_string_addr_internal(&res, + dcs[i].hostname, + 0)) { continue; } - - r->ss = dcs[i].ss_s[j]; - j++; - } - - /* make sure it is a valid IP. I considered checking the - * negative connection cache, but this is the wrong place - * for it. Maybe only as a hack. After think about it, if - * all of the IP addresses returned from DNS are dead, what - * hope does a netbios name lookup have ? The standard reason - * for falling back to netbios lookups is that our DNS server - * doesn't know anything about the DC's -- jerry */ - - if (!is_zero_addr((struct sockaddr *)&r->ss)) { - (*return_count)++; + /* Add in every IP from the lookup. How + many is that ? */ + for (p = res; p; p = p->ai_next) { + if (is_zero_addr((struct sockaddr *)p->ai_addr)) { + continue; + } + extra_addrs++; + } + if (extra_addrs > 1) { + /* We need to expand the return_iplist array + as we only budgeted for one address. */ + numaddrs += (extra_addrs-1); + *return_iplist = SMB_REALLOC_ARRAY(*return_iplist, + struct ip_service, + numaddrs); + if (*return_iplist == NULL) { + if (res) { + freeaddrinfo(res); + } + talloc_destroy(ctx); + return NT_STATUS_NO_MEMORY; + } + } + for (p = res; p; p = p->ai_next) { + (*return_iplist)[*return_count].port = dcs[i].port; + memcpy(&(*return_iplist)[*return_count].ss, + p->ai_addr, + p->ai_addrlen); + if (is_zero_addr((struct sockaddr *)&(*return_iplist)[*return_count].ss)) { + continue; + } + (*return_count)++; + /* Should never happen, but still... */ + if (*return_count>=numaddrs) { + break; + } + } + if (res) { + freeaddrinfo(res); + } + } else { + /* use all the IP addresses from the SRV sresponse */ + int j; + for (j = 0; j < dcs[i].num_ips; j++) { + (*return_iplist)[*return_count].port = dcs[i].port; + (*return_iplist)[*return_count].ss = dcs[i].ss_s[j]; + if (is_zero_addr((struct sockaddr *)&(*return_iplist)[*return_count].ss)) { + continue; + } + (*return_count)++; + /* Should never happen, but still... */ + if (*return_count>=numaddrs) { + break; + } + } } } -- 1.7.7.3