summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorVolker Lendecke <vl@samba.org>2010-07-06 11:54:31 +0200
committerVolker Lendecke <vl@samba.org>2010-07-06 14:21:41 +0200
commit60a3cc850a27a14110541439c05387efb0312210 (patch)
treeb5d0b61b17c3070d4d671d794f4b5725b8ade4d7
parent1dcf0e917e99cadf7267566db2139c3dbfc0815a (diff)
downloadsamba-60a3cc850a27a14110541439c05387efb0312210.tar.gz
samba-60a3cc850a27a14110541439c05387efb0312210.tar.bz2
samba-60a3cc850a27a14110541439c05387efb0312210.zip
s3: Fix another winbind crash
This is similar to 09a9cc3, this re-arranges winbindd_ads.c:query_user_list() so that "ads" is not accessed anymore across a call to nss_get_info_cached() call which can destroy it behind the scenes.
-rw-r--r--source3/winbindd/winbindd_ads.c83
1 files changed, 48 insertions, 35 deletions
diff --git a/source3/winbindd/winbindd_ads.c b/source3/winbindd/winbindd_ads.c
index 366732ad4d..c73e1a0aeb 100644
--- a/source3/winbindd/winbindd_ads.c
+++ b/source3/winbindd/winbindd_ads.c
@@ -153,7 +153,7 @@ static ADS_STRUCT *ads_cached_connection(struct winbindd_domain *domain)
static NTSTATUS query_user_list(struct winbindd_domain *domain,
TALLOC_CTX *mem_ctx,
uint32 *num_entries,
- struct wbint_userinfo **info)
+ struct wbint_userinfo **pinfo)
{
ADS_STRUCT *ads = NULL;
const char *attrs[] = { "*", NULL };
@@ -192,23 +192,18 @@ static NTSTATUS query_user_list(struct winbindd_domain *domain,
goto done;
}
- (*info) = TALLOC_ZERO_ARRAY(mem_ctx, struct wbint_userinfo, count);
- if (!*info) {
+ (*pinfo) = TALLOC_ZERO_ARRAY(mem_ctx, struct wbint_userinfo, count);
+ if (!*pinfo) {
status = NT_STATUS_NO_MEMORY;
goto done;
}
- i = 0;
+ count = 0;
for (msg = ads_first_entry(ads, res); msg; msg = ads_next_entry(ads, msg)) {
- const char *name;
- const char *gecos = NULL;
- const char *homedir = NULL;
- const char *shell = NULL;
+ struct wbint_userinfo *info = &((*pinfo)[count]);
uint32 group;
uint32 atype;
- struct dom_sid user_sid;
- gid_t primary_gid = (gid_t)-1;
if (!ads_pull_uint32(ads, msg, "sAMAccountType", &atype) ||
ds_atype_map(atype) != SID_NAME_USER) {
@@ -216,46 +211,64 @@ static NTSTATUS query_user_list(struct winbindd_domain *domain,
continue;
}
- name = ads_pull_username(ads, mem_ctx, msg);
-
- if ( ads_pull_sid( ads, msg, "objectSid", &user_sid ) ) {
- status = nss_get_info_cached( domain, &user_sid, mem_ctx,
- ads, msg, &homedir, &shell, &gecos,
- &primary_gid );
- }
-
- if (gecos == NULL) {
- gecos = ads_pull_string(ads, mem_ctx, msg, "name");
- }
+ info->acct_name = ads_pull_username(ads, mem_ctx, msg);
+ info->full_name = ads_pull_string(ads, mem_ctx, msg, "name");
+ info->homedir = NULL;
+ info->shell = NULL;
+ info->primary_gid = (gid_t)-1;
if (!ads_pull_sid(ads, msg, "objectSid",
- &(*info)[i].user_sid)) {
- DEBUG(1,("No sid for %s !?\n", name));
+ &info->user_sid)) {
+ DEBUG(1, ("No sid for %s !?\n", info->acct_name));
continue;
}
+
if (!ads_pull_uint32(ads, msg, "primaryGroupID", &group)) {
- DEBUG(1,("No primary group for %s !?\n", name));
+ DEBUG(1, ("No primary group for %s !?\n",
+ info->acct_name));
continue;
}
+ sid_compose(&info->group_sid, &domain->sid, group);
- (*info)[i].acct_name = name;
- (*info)[i].full_name = gecos;
- (*info)[i].homedir = homedir;
- (*info)[i].shell = shell;
- (*info)[i].primary_gid = primary_gid;
- sid_compose(&(*info)[i].group_sid, &domain->sid, group);
- i++;
+ count += 1;
+ }
+
+ (*num_entries) = count;
+ ads_msgfree(ads, res);
+
+ for (i=0; i<count; i++) {
+ struct wbint_userinfo *info = &((*pinfo)[i]);
+ const char *gecos = NULL;
+ gid_t primary_gid = (gid_t)-1;
+
+ /*
+ * Don't use our variable "ads" in this call here, every call
+ * to nss_get_info_cached can destroy the connection inside
+ * the domain.
+ */
+ status = nss_get_info_cached(domain, &info->user_sid, mem_ctx,
+ ads_cached_connection(domain),
+ msg, &info->homedir, &info->shell,
+ &gecos, &primary_gid);
+ if (!NT_STATUS_IS_OK(status)) {
+ /*
+ * Deliberately ignore this error, there might be more
+ * users to fill
+ */
+ continue;
+ }
+
+ if (gecos != NULL) {
+ info->full_name = gecos;
+ }
+ info->primary_gid = primary_gid;
}
- (*num_entries) = i;
status = NT_STATUS_OK;
DEBUG(3,("ads query_user_list gave %d entries\n", (*num_entries)));
done:
- if (res)
- ads_msgfree(ads, res);
-
return status;
}