[PATCH] Remove fstring from wb_acct_info
Samuel Cabrero
scabrero at suse.de
Wed Oct 31 16:45:33 UTC 2018
Hi,
the attached patch removes two fstrings from wb_acct_info struct. The
reason for this change is because the winbindd group enumeration
backend functions (ADS in particular) try to allocate an array of
wb_acct_info as long as the number of groups in the domain, which may
result in a huge chunk of memory for domains with a large number of
groups.
Branch:
https://gitlab.com/samuelcabrero/samba/commits/winbind_enum_grp_nomem
CI:
https://gitlab.com/samuelcabrero/samba/pipelines/34956873
Please review and push if you agree.
-------------- next part --------------
From 218f58f374d7d79a62b43b407c6785b91e6c7a16 Mon Sep 17 00:00:00 2001
From: Samuel Cabrero <scabrero at suse.de>
Date: Tue, 30 Oct 2018 18:47:16 +0100
Subject: [PATCH] s3: winbind: Remove fstring from wb_acct_info struct
The group enumeration backend functions try to allocate an array of
wb_acct_info structs with a number of elements equal to the number of
groups. In domains with a large number of groups this allocation may
fail due to the size of the chunk.
Found while trying to enumerate the groups in a domain with more than
700k groups.
Signed-off-by: Samuel Cabrero <scabrero at suse.de>
---
source3/winbindd/winbindd.h | 4 ++--
source3/winbindd/winbindd_ads.c | 8 ++++----
source3/winbindd/winbindd_cache.c | 8 ++++----
source3/winbindd/winbindd_rpc.c | 16 ++++++++++++----
4 files changed, 22 insertions(+), 14 deletions(-)
diff --git a/source3/winbindd/winbindd.h b/source3/winbindd/winbindd.h
index 57371765484..6d4b92f27cf 100644
--- a/source3/winbindd/winbindd.h
+++ b/source3/winbindd/winbindd.h
@@ -189,8 +189,8 @@ struct winbindd_domain {
};
struct wb_acct_info {
- fstring acct_name; /* account name */
- fstring acct_desc; /* account name */
+ const char *acct_name; /* account name */
+ const char *acct_desc; /* account name */
uint32_t rid; /* domain-relative RID */
};
diff --git a/source3/winbindd/winbindd_ads.c b/source3/winbindd/winbindd_ads.c
index 76d6a304366..abc044d54ac 100644
--- a/source3/winbindd/winbindd_ads.c
+++ b/source3/winbindd/winbindd_ads.c
@@ -500,8 +500,8 @@ static NTSTATUS enum_dom_groups(struct winbindd_domain *domain,
struct dom_sid sid;
uint32_t rid;
- name = ads_pull_username(ads, mem_ctx, msg);
- gecos = ads_pull_string(ads, mem_ctx, msg, "name");
+ name = ads_pull_username(ads, (*info), msg);
+ gecos = ads_pull_string(ads, (*info), msg, "name");
if (!ads_pull_sid(ads, msg, "objectSid", &sid)) {
DEBUG(1,("No sid for %s !?\n", name));
continue;
@@ -512,8 +512,8 @@ static NTSTATUS enum_dom_groups(struct winbindd_domain *domain,
continue;
}
- fstrcpy((*info)[i].acct_name, name);
- fstrcpy((*info)[i].acct_desc, gecos);
+ (*info)[i].acct_name = name;
+ (*info)[i].acct_desc = gecos;
(*info)[i].rid = rid;
i++;
}
diff --git a/source3/winbindd/winbindd_cache.c b/source3/winbindd/winbindd_cache.c
index 7d98d613ac4..2e2a04d09d8 100644
--- a/source3/winbindd/winbindd_cache.c
+++ b/source3/winbindd/winbindd_cache.c
@@ -1565,8 +1565,8 @@ do_fetch_cache:
smb_panic_fn("enum_dom_groups out of memory");
}
for (i=0; i<(*num_entries); i++) {
- fstrcpy((*info)[i].acct_name, centry_string(centry, mem_ctx));
- fstrcpy((*info)[i].acct_desc, centry_string(centry, mem_ctx));
+ (*info)[i].acct_name = centry_string(centry, (*info));
+ (*info)[i].acct_desc = centry_string(centry, (*info));
(*info)[i].rid = centry_uint32(centry);
}
@@ -1660,8 +1660,8 @@ do_fetch_cache:
smb_panic_fn("enum_dom_groups out of memory");
}
for (i=0; i<(*num_entries); i++) {
- fstrcpy((*info)[i].acct_name, centry_string(centry, mem_ctx));
- fstrcpy((*info)[i].acct_desc, centry_string(centry, mem_ctx));
+ (*info)[i].acct_name = centry_string(centry, (*info));
+ (*info)[i].acct_desc = centry_string(centry, (*info));
(*info)[i].rid = centry_uint32(centry);
}
diff --git a/source3/winbindd/winbindd_rpc.c b/source3/winbindd/winbindd_rpc.c
index f50fb8fa5db..6f7cb07f4e3 100644
--- a/source3/winbindd/winbindd_rpc.c
+++ b/source3/winbindd/winbindd_rpc.c
@@ -155,9 +155,13 @@ NTSTATUS rpc_enum_dom_groups(TALLOC_CTX *mem_ctx,
for (g = 0; g < count; g++) {
struct wb_acct_info *i = &info[num_info + g];
- fstrcpy(i->acct_name,
+ i->acct_name = talloc_strdup(info,
sam_array->entries[g].name.string);
- fstrcpy(i->acct_desc, "");
+ if (i->acct_name == NULL) {
+ TALLOC_FREE(info);
+ return NT_STATUS_NO_MEMORY;
+ }
+ i->acct_desc = NULL;
i->rid = sam_array->entries[g].idx;
}
@@ -217,9 +221,13 @@ NTSTATUS rpc_enum_local_groups(TALLOC_CTX *mem_ctx,
for (g = 0; g < count; g++) {
struct wb_acct_info *i = &info[num_info + g];
- fstrcpy(i->acct_name,
+ i->acct_name = talloc_strdup(info,
sam_array->entries[g].name.string);
- fstrcpy(i->acct_desc, "");
+ if (i->acct_name == NULL) {
+ TALLOC_FREE(info);
+ return NT_STATUS_NO_MEMORY;
+ }
+ i->acct_desc = NULL;
i->rid = sam_array->entries[g].idx;
}
--
2.19.1
More information about the samba-technical
mailing list