From 0eadbf2f9c3e77eb73e2149cd56c0298c6f31655 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Fri, 27 Apr 2012 16:02:15 -0700 Subject: [PATCH 1/4] Fix remove_duplicate_addrs2 to do exactly what it says. Previously it could leave zero addresses in the list. (cherry picked from commit 01e884675e3d1cffb0149108225fbd21c3a73f4e) --- 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 c36773b..018725e 100644 --- a/source3/libsmb/namequery.c +++ b/source3/libsmb/namequery.c @@ -1078,7 +1078,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 fff176e3f92a86a2dbbee7a8fb88ec1f4d5be560 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 30 Apr 2012 14:45:43 -0700 Subject: [PATCH 2/4] Fix convert_ss2service() to filter out zero addresses. --- source3/libsmb/namequery.c | 45 ++++++++++++++++++++++++++++++++----------- 1 files changed, 33 insertions(+), 12 deletions(-) diff --git a/source3/libsmb/namequery.c b/source3/libsmb/namequery.c index 018725e..15cb97c 100644 --- a/source3/libsmb/namequery.c +++ b/source3/libsmb/namequery.c @@ -1475,32 +1475,53 @@ NTSTATUS name_query(const char *name, int name_type, } /******************************************************** - 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: Fri, 27 Apr 2012 16:25:58 -0700 Subject: [PATCH 3/4] Protect all of the name resolution methods from returning null addrs. Ensure all returns go through remove_duplicate_addrs2(). (cherry picked from commit 11973608186926e5417ee81c8709ea5b0a7ef2e1) --- source3/libsmb/namequery.c | 28 +++++++++++++++++++--------- 1 files changed, 19 insertions(+), 9 deletions(-) diff --git a/source3/libsmb/namequery.c b/source3/libsmb/namequery.c index 15cb97c..9ba8aac 100644 --- a/source3/libsmb/namequery.c +++ b/source3/libsmb/namequery.c @@ -1338,6 +1338,10 @@ static bool name_query_validator(struct packet_struct *p, void *private_data) putip((char *)&ip,&nmb->answers->rdata[2+i*6]); in_addr_to_sockaddr_storage(&addr, ip); + if (is_zero_addr(&addr)) { + continue; + } + for (j=0; jnum_addrs; j++) { if (sockaddr_equal( (struct sockaddr *)&addr, @@ -1822,6 +1826,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(&ss)) { + continue; + } + *return_count += 1; *return_iplist = SMB_REALLOC_ARRAY(*return_iplist, @@ -2013,6 +2021,10 @@ NTSTATUS internal_resolve_name(const char *name, SAFE_FREE(*return_iplist); return NT_STATUS_INVALID_PARAMETER; } + if (is_zero_addr(&(*return_iplist)->ss)) { + SAFE_FREE(*return_iplist); + return NT_STATUS_UNSUCCESSFUL; + } *return_count = 1; return NT_STATUS_OK; } @@ -2020,6 +2032,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; @@ -2120,10 +2134,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 ) { @@ -2139,7 +2150,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 */ @@ -2602,10 +2615,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 1107a8b94c54256b13325520fb2c789fed28b749 Mon Sep 17 00:00:00 2001 From: Jeremy Allison Date: Mon, 30 Apr 2012 11:05: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. Autobuild-User: Jeremy Allison Autobuild-Date: Mon Apr 30 23:21:16 CEST 2012 on sn-devel-104 (cherry picked from commit 1270cfb45ffa0bbcacf7254b5b45f492a8dcde77) --- source3/libsmb/namequery.c | 107 ++++++++++++++++++++++++++++++-------------- 1 files changed, 73 insertions(+), 34 deletions(-) diff --git a/source3/libsmb/namequery.c b/source3/libsmb/namequery.c index 9ba8aac..d0ab62f 100644 --- a/source3/libsmb/namequery.c +++ b/source3/libsmb/namequery.c @@ -1866,7 +1866,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; @@ -1915,8 +1915,12 @@ static NTSTATUS resolve_ads(const char *name, } for (i=0;iport = dcs[i].port; + while ( i < numdcs && (*return_countss, 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(&r->ss)) { - (*return_count)++; + /* Add in every IP from the lookup. How + many is that ? */ + for (p = res; p; p = p->ai_next) { + struct sockaddr_storage ss; + memcpy(&ss, p->ai_addr, p->ai_addrlen); + if (is_zero_addr(&ss)) { + 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(&(*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(&(*return_iplist)[*return_count].ss)) { + continue; + } + (*return_count)++; + /* Should never happen, but still... */ + if (*return_count>=numaddrs) { + break; + } + } } } -- 1.7.7.3