The branch, v3-5-test has been updated via b9d3f82 Fix the loop unrolling inside resolve_ads(). via 6d5aae1 Protect all of the name resolution methods from returning null addrs. Ensure all returns go through remove_duplicate_addrs2(). via 3226be5 Fix convert_ss2service() to filter out zero addresses. via 8e9db61 Fix remove_duplicate_addrs2 to do exactly what it says. Previously it could leave zero addresses in the list. from 991f83f Fix bug #8957 - Typo in pam_winbindd code MUST fix. (cherry picked from commit ee4ef9a535a2d9db11bd94987fb96ae8f8771e79)
http://gitweb.samba.org/?p=samba.git;a=shortlog;h=v3-5-test - Log ----------------------------------------------------------------- commit b9d3f8258396873d6ec8b6ea9ad066e2f1f8e973 Author: Jeremy Allison <j...@samba.org> Date: Mon Apr 30 16:32:51 2012 -0700 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. The last 4 patches address bug #8910 (resolve_ads() code can return zero addresses and miss valid DC IP addresses). commit 6d5aae1d9680657c7021af2974db9b0dc2336f13 Author: Jeremy Allison <j...@samba.org> Date: Mon Apr 30 16:29:19 2012 -0700 Protect all of the name resolution methods from returning null addrs. Ensure all returns go through remove_duplicate_addrs2(). commit 3226be5b5ab771c8cdf98588c40713d36eae4702 Author: Jeremy Allison <j...@samba.org> Date: Mon Apr 30 16:24:27 2012 -0700 Fix convert_ss2service() to filter out zero addresses. commit 8e9db61b447d22bad84a8c9ae450a71d9c3e6d58 Author: Jeremy Allison <j...@samba.org> Date: Mon Apr 30 16:16:39 2012 -0700 Fix remove_duplicate_addrs2 to do exactly what it says. Previously it could leave zero addresses in the list. ----------------------------------------------------------------------- Summary of changes: source3/libsmb/namequery.c | 189 +++++++++++++++++++++++++++++--------------- 1 files changed, 126 insertions(+), 63 deletions(-) Changeset truncated at 500 lines: diff --git a/source3/libsmb/namequery.c b/source3/libsmb/namequery.c index 858330d..af76f3f 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; i<count; i++ ) { if ( is_zero_addr((struct sockaddr *)&iplist[i].ss)) { continue; @@ -585,18 +585,17 @@ static int remove_duplicate_addrs2(struct ip_service *iplist, int count ) } } - /* one loop to clean up any holes we left */ - /* first ip should never be a zero_ip() */ - for (i = 0; i<count; ) { - if (is_zero_addr((struct sockaddr *)&iplist[i].ss) ) { - if (i != count-1) { - memmove(&iplist[i], &iplist[i+1], - (count - i - 1)*sizeof(iplist[i])); + /* Now remove any addresses set to zero above. */ + for (i = 0; i < count; i++) { + while (i < count && + is_zero_addr((struct sockaddr *)&iplist[i].ss)) { + if (count-i-1>0) { + memmove(&iplist[i], + &iplist[i+1], + (count-i-1)*sizeof(struct ip_service)); } count--; - continue; } - i++; } return count; @@ -849,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<orig_count; i++ ) { + if (is_zero_addr((struct sockaddr *)&ss_list[i])) { + continue; + } + real_count++; + } + if (real_count==0) { + return false; + } + /* copy the ip address; port will be PORT_NONE */ - if ((*return_iplist = SMB_MALLOC_ARRAY(struct ip_service, count)) == + if ((*return_iplist = SMB_MALLOC_ARRAY(struct ip_service, real_count)) == NULL) { DEBUG(0,("convert_ip2service: malloc failed " - "for %d enetries!\n", count )); + "for %d enetries!\n", real_count )); return False; } - for ( i=0; i<count; i++ ) { - (*return_iplist)[i].ss = ss_list[i]; - (*return_iplist)[i].port = PORT_NONE; + for ( i=0, real_count = 0; i<orig_count; i++ ) { + if (is_zero_addr((struct sockaddr *)&ss_list[i])) { + continue; + } + (*return_iplist)[real_count].ss = ss_list[i]; + (*return_iplist)[real_count].port = PORT_NONE; + real_count++; } + *pcount = real_count; return true; } @@ -947,7 +967,7 @@ NTSTATUS name_resolve_bcast(const char *name, success: status = NT_STATUS_OK; - if (!convert_ss2service(return_iplist, ss_list, *return_count) ) + if (!convert_ss2service(return_iplist, ss_list, return_count) ) status = NT_STATUS_INVALID_PARAMETER; SAFE_FREE(ss_list); @@ -1082,7 +1102,7 @@ NTSTATUS resolve_wins(const char *name, success: status = NT_STATUS_OK; - if (!convert_ss2service(return_iplist, ss_list, *return_count)) + if (!convert_ss2service(return_iplist, ss_list, return_count)) status = NT_STATUS_INVALID_PARAMETER; SAFE_FREE(ss_list); @@ -1230,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, @@ -1263,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; @@ -1312,7 +1336,11 @@ static NTSTATUS resolve_ads(const char *name, } for (i=0;i<numdcs;i++) { - numaddrs += MAX(dcs[i].num_ips,1); + if (!dcs[i].ss_s) { + numaddrs += 1; + } else { + numaddrs += dcs[i].num_ips; + } } if ((*return_iplist = SMB_MALLOC_ARRAY(struct ip_service, numaddrs)) == @@ -1326,42 +1354,75 @@ static NTSTATUS resolve_ads(const char *name, /* now unroll the list of IP addresses */ *return_count = 0; - i = 0; - j = 0; - while ( i < numdcs && (*return_count<numaddrs) ) { - struct ip_service *r = &(*return_iplist)[*return_count]; - - r->port = 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; + } + } } } @@ -1418,6 +1479,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; } @@ -1425,6 +1490,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; @@ -1519,10 +1586,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 ) { @@ -1538,7 +1602,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 */ @@ -2001,10 +2067,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. */ -- Samba Shared Repository