The Samba-Bugzilla – Attachment 13907 Details for
Bug 13209
Small improvements in winbindd for the resource cleanup in error cases
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
patch for 4.8
winbind_fixes.patch1.txt (text/plain), 16.26 KB, created by
Andreas Schneider
on 2018-01-16 08:42:53 UTC
(
hide
)
Description:
patch for 4.8
Filename:
MIME Type:
Creator:
Andreas Schneider
Created:
2018-01-16 08:42:53 UTC
Size:
16.26 KB
patch
obsolete
>From 47926cc5769dd02ef017ec43030db98a5765754b Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Thu, 11 Jan 2018 09:23:05 +0100 >Subject: [PATCH 1/4] s3:winbindd: Improve logic so it is easier to understand > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13209 > >Signed-off-by: Andreas Schneider <asn@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 264249db0f5515d8333d16218f1553ae9f0e7193) >--- > source3/winbindd/winbindd_pam.c | 22 ++++++++++++---------- > 1 file changed, 12 insertions(+), 10 deletions(-) > >diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c >index de3e3f5cc81..f60a90ceb18 100644 >--- a/source3/winbindd/winbindd_pam.c >+++ b/source3/winbindd/winbindd_pam.c >@@ -1683,22 +1683,24 @@ static NTSTATUS winbindd_dual_pam_auth_samlogon( > true, /* interactive */ > &authoritative, > &info3); >- if (NT_STATUS_IS_OK(result)) { >- result = map_info3_to_validation(mem_ctx, >- info3, >- &validation_level, >- &validation); >- TALLOC_FREE(info3); >- if (!NT_STATUS_IS_OK(result)) { >- goto done; >- } >- } > > /* > * We need to try the remote NETLOGON server if this is > * not authoritative (for example on the RODC). > */ > if (authoritative != 0) { >+ if (NT_STATUS_IS_OK(result)) { >+ result = map_info3_to_validation( >+ mem_ctx, >+ info3, >+ &validation_level, >+ &validation); >+ TALLOC_FREE(info3); >+ if (!NT_STATUS_IS_OK(result)) { >+ goto done; >+ } >+ } >+ > goto done; > } > } >-- >2.15.1 > > >From ac546ddbac286cfd3d94a1b31acd4e4dcd747a63 Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Thu, 11 Jan 2018 09:27:50 +0100 >Subject: [PATCH 2/4] s3:winbind: Use a goto for cleaning up at the end > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13209 > >Signed-off-by: Andreas Schneider <asn@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 00d176c6c592af59cc14271de4af1614578090a3) >--- > source3/winbindd/winbindd_pam.c | 20 +++++++++----------- > 1 file changed, 9 insertions(+), 11 deletions(-) > >diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c >index f60a90ceb18..c7d2ca434fc 100644 >--- a/source3/winbindd/winbindd_pam.c >+++ b/source3/winbindd/winbindd_pam.c >@@ -889,14 +889,14 @@ NTSTATUS append_auth_data(TALLOC_CTX *mem_ctx, > const char *name_user) > { > struct netr_SamInfo3 *info3 = NULL; >- NTSTATUS result; >+ NTSTATUS result = NT_STATUS_UNSUCCESSFUL; > > result = map_validation_to_info3(talloc_tos(), > validation_level, > validation, > &info3); > if (!NT_STATUS_IS_OK(result)) { >- return result; >+ goto out; > } > > if (request_flags & WBFLAG_PAM_USER_SESSION_KEY) { >@@ -919,8 +919,7 @@ NTSTATUS append_auth_data(TALLOC_CTX *mem_ctx, > if (!NT_STATUS_IS_OK(result)) { > DEBUG(10,("Failed to append Unix Username: %s\n", > nt_errstr(result))); >- TALLOC_FREE(info3); >- return result; >+ goto out; > } > } > >@@ -931,8 +930,7 @@ NTSTATUS append_auth_data(TALLOC_CTX *mem_ctx, > if (!NT_STATUS_IS_OK(result)) { > DEBUG(10,("Failed to append INFO3 (NDR): %s\n", > nt_errstr(result))); >- TALLOC_FREE(info3); >- return result; >+ goto out; > } > } > >@@ -943,8 +941,7 @@ NTSTATUS append_auth_data(TALLOC_CTX *mem_ctx, > if (!NT_STATUS_IS_OK(result)) { > DEBUG(10,("Failed to append INFO3 (TXT): %s\n", > nt_errstr(result))); >- TALLOC_FREE(info3); >- return result; >+ goto out; > } > } > >@@ -954,13 +951,14 @@ NTSTATUS append_auth_data(TALLOC_CTX *mem_ctx, > if (!NT_STATUS_IS_OK(result)) { > DEBUG(10,("Failed to append AFS token: %s\n", > nt_errstr(result))); >- TALLOC_FREE(info3); >- return result; >+ goto out; > } > } > >+ result = NT_STATUS_OK; >+out: > TALLOC_FREE(info3); >- return NT_STATUS_OK; >+ return result; > } > > static NTSTATUS winbindd_dual_pam_auth_cached(struct winbindd_domain *domain, >-- >2.15.1 > > >From a3c4fd31a3218be82e0c6b312217dd9c04b4b719 Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Thu, 11 Jan 2018 09:37:22 +0100 >Subject: [PATCH 3/4] s3:winbind: Use a stackframe and cleanup when leaving > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13209 > >Signed-off-by: Andreas Schneider <asn@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit bfc727f0b2d837a97fc9eb94a8811f23a656c4e4) >--- > source3/winbindd/winbindd_pam.c | 39 ++++++++++++++++++++------------------- > 1 file changed, 20 insertions(+), 19 deletions(-) > >diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c >index c7d2ca434fc..01d08c1e2b3 100644 >--- a/source3/winbindd/winbindd_pam.c >+++ b/source3/winbindd/winbindd_pam.c >@@ -56,16 +56,17 @@ static NTSTATUS append_info3_as_txt(TALLOC_CTX *mem_ctx, > union netr_Validation *validation) > { > struct netr_SamInfo3 *info3 = NULL; >- char *ex; >+ char *ex = NULL; > uint32_t i; >- NTSTATUS status; >+ NTSTATUS status = NT_STATUS_UNSUCCESSFUL; >+ TALLOC_CTX *frame = talloc_stackframe(); > >- status = map_validation_to_info3(talloc_tos(), >+ status = map_validation_to_info3(frame, > validation_level, > validation, > &info3); > if (!NT_STATUS_IS_OK(status)) { >- return status; >+ goto out; > } > > resp->data.auth.info3.logon_time = >@@ -120,10 +121,10 @@ static NTSTATUS append_info3_as_txt(TALLOC_CTX *mem_ctx, > validation->sam6->principal_name.string); > } > >- ex = talloc_strdup(mem_ctx, ""); >+ ex = talloc_strdup(frame, ""); > if (ex == NULL) { >- TALLOC_FREE(info3); >- return NT_STATUS_NO_MEMORY; >+ status = NT_STATUS_NO_MEMORY; >+ goto out; > } > > for (i=0; i < info3->base.groups.count; i++) { >@@ -131,36 +132,36 @@ static NTSTATUS append_info3_as_txt(TALLOC_CTX *mem_ctx, > info3->base.groups.rids[i].rid, > info3->base.groups.rids[i].attributes); > if (ex == NULL) { >- TALLOC_FREE(info3); >- return NT_STATUS_NO_MEMORY; >+ status = NT_STATUS_NO_MEMORY; >+ goto out; > } > } > > for (i=0; i < info3->sidcount; i++) { > char *sid; > >- sid = dom_sid_string(mem_ctx, info3->sids[i].sid); >+ sid = dom_sid_string(frame, info3->sids[i].sid); > if (sid == NULL) { >- TALLOC_FREE(info3); >- return NT_STATUS_NO_MEMORY; >+ status = NT_STATUS_NO_MEMORY; >+ goto out; > } > > ex = talloc_asprintf_append_buffer(ex, "%s:0x%08X\n", > sid, > info3->sids[i].attributes); > if (ex == NULL) { >- TALLOC_FREE(info3); >- return NT_STATUS_NO_MEMORY; >+ status = NT_STATUS_NO_MEMORY; >+ goto out; > } >- >- talloc_free(sid); > } > >- resp->extra_data.data = ex; > resp->length += talloc_get_size(ex); >+ resp->extra_data.data = talloc_move(mem_ctx, &ex); > >- TALLOC_FREE(info3); >- return NT_STATUS_OK; >+ status = NT_STATUS_OK; >+out: >+ TALLOC_FREE(frame); >+ return status; > } > > static NTSTATUS append_info3_as_ndr(TALLOC_CTX *mem_ctx, >-- >2.15.1 > > >From 622f1e360d8840851fd955202f13994785ed1e0b Mon Sep 17 00:00:00 2001 >From: Andreas Schneider <asn@samba.org> >Date: Thu, 11 Jan 2018 09:06:31 +0100 >Subject: [PATCH 4/4] s3:rpc_client: Clenup copy_netr_SamInfo3() code > >This gets rid of some strange macro and makes sure we clenaup at the >end. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=13209 > >Signed-off-by: Andreas Schneider <asn@samba.org> >Reviewed-by: Ralph Boehme <slow@samba.org> >(cherry picked from commit 05ebafd91ee2dd511372ce63d656e9fc6735ee28) >--- > source3/auth/auth_util.c | 14 ++++--- > source3/auth/server_info.c | 45 +++++++++++++-------- > source3/rpc_client/util_netlogon.c | 80 ++++++++++++++++++++++---------------- > source3/rpc_client/util_netlogon.h | 5 ++- > source3/winbindd/winbindd_pam.c | 9 +++-- > 5 files changed, 92 insertions(+), 61 deletions(-) > >diff --git a/source3/auth/auth_util.c b/source3/auth/auth_util.c >index 5bb5a69dfa7..f543b33eead 100644 >--- a/source3/auth/auth_util.c >+++ b/source3/auth/auth_util.c >@@ -1008,6 +1008,7 @@ static struct auth_serversupplied_info *copy_session_info_serverinfo_guest(TALLO > struct auth_serversupplied_info *server_info) > { > struct auth_serversupplied_info *dst; >+ NTSTATUS status; > > dst = make_server_info(mem_ctx); > if (dst == NULL) { >@@ -1055,8 +1056,10 @@ static struct auth_serversupplied_info *copy_session_info_serverinfo_guest(TALLO > dst->lm_session_key = data_blob_talloc(dst, src->session_key.data, > src->session_key.length); > >- dst->info3 = copy_netr_SamInfo3(dst, server_info->info3); >- if (!dst->info3) { >+ status = copy_netr_SamInfo3(dst, >+ server_info->info3, >+ &dst->info3); >+ if (!NT_STATUS_IS_OK(status)) { > TALLOC_FREE(dst); > return NULL; > } >@@ -1433,9 +1436,10 @@ NTSTATUS make_server_info_info3(TALLOC_CTX *mem_ctx, > result->unix_name = talloc_strdup(result, found_username); > > /* copy in the info3 */ >- result->info3 = copy_netr_SamInfo3(result, info3); >- if (result->info3 == NULL) { >- nt_status = NT_STATUS_NO_MEMORY; >+ nt_status = copy_netr_SamInfo3(result, >+ info3, >+ &result->info3); >+ if (!NT_STATUS_IS_OK(nt_status)) { > goto out; > } > >diff --git a/source3/auth/server_info.c b/source3/auth/server_info.c >index 20d43d237fa..78981751286 100644 >--- a/source3/auth/server_info.c >+++ b/source3/auth/server_info.c >@@ -63,11 +63,14 @@ struct auth_serversupplied_info *make_server_info(TALLOC_CTX *mem_ctx) > NTSTATUS serverinfo_to_SamInfo2(struct auth_serversupplied_info *server_info, > struct netr_SamInfo2 *sam2) > { >- struct netr_SamInfo3 *info3; >+ struct netr_SamInfo3 *info3 = NULL; >+ NTSTATUS status; > >- info3 = copy_netr_SamInfo3(sam2, server_info->info3); >- if (!info3) { >- return NT_STATUS_NO_MEMORY; >+ status = copy_netr_SamInfo3(sam2, >+ server_info->info3, >+ &info3); >+ if (!NT_STATUS_IS_OK(status)) { >+ return status; > } > > if (server_info->session_key.length) { >@@ -96,11 +99,14 @@ NTSTATUS serverinfo_to_SamInfo2(struct auth_serversupplied_info *server_info, > NTSTATUS serverinfo_to_SamInfo3(const struct auth_serversupplied_info *server_info, > struct netr_SamInfo3 *sam3) > { >- struct netr_SamInfo3 *info3; >+ struct netr_SamInfo3 *info3 = NULL; >+ NTSTATUS status; > >- info3 = copy_netr_SamInfo3(sam3, server_info->info3); >- if (!info3) { >- return NT_STATUS_NO_MEMORY; >+ status = copy_netr_SamInfo3(sam3, >+ server_info->info3, >+ &info3); >+ if (!NT_STATUS_IS_OK(status)) { >+ return status; > } > > if (server_info->session_key.length) { >@@ -133,7 +139,8 @@ NTSTATUS serverinfo_to_SamInfo6(struct auth_serversupplied_info *server_info, > struct netr_SamInfo6 *sam6) > { > struct pdb_domain_info *dominfo; >- struct netr_SamInfo3 *info3; >+ struct netr_SamInfo3 *info3 = NULL; >+ NTSTATUS status; > > if ((pdb_capabilities() & PDB_CAP_ADS) == 0) { > DEBUG(10,("Not adding validation info level 6 " >@@ -146,9 +153,11 @@ NTSTATUS serverinfo_to_SamInfo6(struct auth_serversupplied_info *server_info, > return NT_STATUS_NO_MEMORY; > } > >- info3 = copy_netr_SamInfo3(sam6, server_info->info3); >- if (!info3) { >- return NT_STATUS_NO_MEMORY; >+ status = copy_netr_SamInfo3(sam6, >+ server_info->info3, >+ &info3); >+ if (!NT_STATUS_IS_OK(status)) { >+ return status; > } > > if (server_info->session_key.length) { >@@ -335,11 +344,15 @@ NTSTATUS create_info3_from_pac_logon_info(TALLOC_CTX *mem_ctx, > struct netr_SamInfo3 **pp_info3) > { > NTSTATUS status; >- struct netr_SamInfo3 *info3 = copy_netr_SamInfo3(mem_ctx, >- &logon_info->info3); >- if (info3 == NULL) { >- return NT_STATUS_NO_MEMORY; >+ struct netr_SamInfo3 *info3 = NULL; >+ >+ status = copy_netr_SamInfo3(mem_ctx, >+ &logon_info->info3, >+ &info3); >+ if (!NT_STATUS_IS_OK(status)) { >+ return status; > } >+ > status = merge_resource_sids(logon_info, info3); > if (!NT_STATUS_IS_OK(status)) { > TALLOC_FREE(info3); >diff --git a/source3/rpc_client/util_netlogon.c b/source3/rpc_client/util_netlogon.c >index ac804f84196..15c769ffe41 100644 >--- a/source3/rpc_client/util_netlogon.c >+++ b/source3/rpc_client/util_netlogon.c >@@ -62,45 +62,52 @@ NTSTATUS copy_netr_SamBaseInfo(TALLOC_CTX *mem_ctx, > return NT_STATUS_OK; > } > >-#undef RET_NOMEM >- >-#define RET_NOMEM(ptr) do { \ >- if (!ptr) { \ >- TALLOC_FREE(info3); \ >- return NULL; \ >- } } while(0) >- >-struct netr_SamInfo3 *copy_netr_SamInfo3(TALLOC_CTX *mem_ctx, >- const struct netr_SamInfo3 *orig) >+NTSTATUS copy_netr_SamInfo3(TALLOC_CTX *mem_ctx, >+ const struct netr_SamInfo3 *in, >+ struct netr_SamInfo3 **pout) > { >- struct netr_SamInfo3 *info3; >+ struct netr_SamInfo3 *info3 = NULL; > unsigned int i; >- NTSTATUS status; >+ NTSTATUS status = NT_STATUS_UNSUCCESSFUL; > > info3 = talloc_zero(mem_ctx, struct netr_SamInfo3); >- if (!info3) return NULL; >+ if (info3 == NULL) { >+ status = NT_STATUS_NO_MEMORY; >+ goto out; >+ } > >- status = copy_netr_SamBaseInfo(info3, &orig->base, &info3->base); >+ status = copy_netr_SamBaseInfo(info3, &in->base, &info3->base); > if (!NT_STATUS_IS_OK(status)) { >- TALLOC_FREE(info3); >- return NULL; >+ goto out; > } > >- if (orig->sidcount) { >- info3->sidcount = orig->sidcount; >+ if (in->sidcount) { >+ info3->sidcount = in->sidcount; > info3->sids = talloc_array(info3, struct netr_SidAttr, >- orig->sidcount); >- RET_NOMEM(info3->sids); >- for (i = 0; i < orig->sidcount; i++) { >+ in->sidcount); >+ if (info3->sids == NULL) { >+ status = NT_STATUS_NO_MEMORY; >+ goto out; >+ } >+ >+ for (i = 0; i < in->sidcount; i++) { > info3->sids[i].sid = dom_sid_dup(info3->sids, >- orig->sids[i].sid); >- RET_NOMEM(info3->sids[i].sid); >- info3->sids[i].attributes = >- orig->sids[i].attributes; >+ in->sids[i].sid); >+ if (info3->sids[i].sid == NULL) { >+ status = NT_STATUS_NO_MEMORY; >+ goto out; >+ } >+ info3->sids[i].attributes = in->sids[i].attributes; > } > } > >- return info3; >+ *pout = info3; >+ info3 = NULL; >+ >+ status = NT_STATUS_OK; >+out: >+ TALLOC_FREE(info3); >+ return status; > } > > NTSTATUS map_validation_to_info3(TALLOC_CTX *mem_ctx, >@@ -108,7 +115,7 @@ NTSTATUS map_validation_to_info3(TALLOC_CTX *mem_ctx, > union netr_Validation *validation, > struct netr_SamInfo3 **info3_p) > { >- struct netr_SamInfo3 *info3; >+ struct netr_SamInfo3 *info3 = NULL; > struct netr_SamInfo6 *info6 = NULL; > NTSTATUS status; > >@@ -122,10 +129,13 @@ NTSTATUS map_validation_to_info3(TALLOC_CTX *mem_ctx, > return NT_STATUS_INVALID_PARAMETER; > } > >- info3 = copy_netr_SamInfo3(mem_ctx, validation->sam3); >- if (info3 == NULL) { >- return NT_STATUS_NO_MEMORY; >+ status = copy_netr_SamInfo3(mem_ctx, >+ validation->sam3, >+ &info3); >+ if (!NT_STATUS_IS_OK(status)) { >+ return status; > } >+ > break; > case 6: > if (validation->sam6 == NULL) { >@@ -186,16 +196,18 @@ NTSTATUS map_info3_to_validation(TALLOC_CTX *mem_ctx, > union netr_Validation **_validation) > { > union netr_Validation *validation = NULL; >+ NTSTATUS status; > > validation = talloc_zero(mem_ctx, union netr_Validation); > if (validation == NULL) { > return NT_STATUS_NO_MEMORY; > } > >- validation->sam3 = copy_netr_SamInfo3(mem_ctx, info3); >- if (validation->sam3 == NULL) { >- TALLOC_FREE(validation); >- return NT_STATUS_NO_MEMORY; >+ status = copy_netr_SamInfo3(mem_ctx, >+ info3, >+ &validation->sam3); >+ if (!NT_STATUS_IS_OK(status)) { >+ return status; > } > > * _validation_level = 3; >diff --git a/source3/rpc_client/util_netlogon.h b/source3/rpc_client/util_netlogon.h >index 80c7bff99d1..8b3a372ee8e 100644 >--- a/source3/rpc_client/util_netlogon.h >+++ b/source3/rpc_client/util_netlogon.h >@@ -25,8 +25,9 @@ > NTSTATUS copy_netr_SamBaseInfo(TALLOC_CTX *mem_ctx, > const struct netr_SamBaseInfo *in, > struct netr_SamBaseInfo *out); >-struct netr_SamInfo3 *copy_netr_SamInfo3(TALLOC_CTX *mem_ctx, >- const struct netr_SamInfo3 *orig); >+NTSTATUS copy_netr_SamInfo3(TALLOC_CTX *mem_ctx, >+ const struct netr_SamInfo3 *in, >+ struct netr_SamInfo3 **pout); > NTSTATUS map_validation_to_info3(TALLOC_CTX *mem_ctx, > uint16_t validation_level, > union netr_Validation *validation, >diff --git a/source3/winbindd/winbindd_pam.c b/source3/winbindd/winbindd_pam.c >index 01d08c1e2b3..9a61cd3a578 100644 >--- a/source3/winbindd/winbindd_pam.c >+++ b/source3/winbindd/winbindd_pam.c >@@ -2916,10 +2916,11 @@ NTSTATUS winbindd_pam_auth_pac_send(struct winbindd_cli_state *state, > * returning a copy talloc'ed off > * the state->mem_ctx. > */ >- info3_copy = copy_netr_SamInfo3(state->mem_ctx, >- &logon_info->info3); >- if (info3_copy == NULL) { >- return NT_STATUS_NO_MEMORY; >+ result = copy_netr_SamInfo3(state->mem_ctx, >+ &logon_info->info3, >+ &info3_copy); >+ if (!NT_STATUS_IS_OK(result)) { >+ return result; > } > } > } >-- >2.15.1 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Raw
Flags:
slow
:
review+
Actions:
View
Attachments on
bug 13209
: 13907