The Samba-Bugzilla – Attachment 18531 Details for
Bug 15782
winbindd shows memleak in kerberos_decode_pac
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
patch from master for v4-21-test
v4-21-test.patch (text/plain), 9.32 KB, created by
Guenther Deschner
on 2025-01-17 15:05:47 UTC
(
hide
)
Description:
patch from master for v4-21-test
Filename:
MIME Type:
Creator:
Guenther Deschner
Created:
2025-01-17 15:05:47 UTC
Size:
9.32 KB
patch
obsolete
>From 03d58fad9074f0e8714bc3cc1f1169cba360b8a9 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Wed, 15 Jan 2025 10:21:19 -0800 >Subject: [PATCH 1/2] auth: Add missing talloc_free() in error code path. >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15782 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Guenther Deschner <gd@samba.org> > >Autobuild-User(master): Günther Deschner <gd@samba.org> >Autobuild-Date(master): Thu Jan 16 14:32:39 UTC 2025 on atb-devel-224 > >(cherry picked from commit c514ce8dcadcbbf0d86f3038d2be0f9253a76b75) >--- > auth/kerberos/kerberos_pac.c | 1 + > 1 file changed, 1 insertion(+) > >diff --git a/auth/kerberos/kerberos_pac.c b/auth/kerberos/kerberos_pac.c >index b6272ac15eb..1f7d3e7ef26 100644 >--- a/auth/kerberos/kerberos_pac.c >+++ b/auth/kerberos/kerberos_pac.c >@@ -360,6 +360,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > if (ret) { > DEBUG(5, ("PAC Decode: Failed to verify the service " > "signature: %s\n", error_message(ret))); >+ talloc_free(tmp_ctx); > return NT_STATUS_ACCESS_DENIED; > } > >-- >2.48.1 > > >From 3fa1f90e4e0dbbed85fed0a2475077fef3b55351 Mon Sep 17 00:00:00 2001 >From: Jeremy Allison <jra@samba.org> >Date: Thu, 16 Jan 2025 16:12:31 -0800 >Subject: [PATCH 2/2] auth: Cleanup exit code paths in kerberos_decode_pac(). >MIME-Version: 1.0 >Content-Type: text/plain; charset=UTF-8 >Content-Transfer-Encoding: 8bit > >One more memory leak missed and now fixed. tmp_ctx >must be freed once the pac data is talloc_move'd. > >BUG: https://bugzilla.samba.org/show_bug.cgi?id=15782 > >Signed-off-by: Jeremy Allison <jra@samba.org> >Reviewed-by: Jennifer Sutton <jennifersutton@catalyst.net.nz> >Reviewed-by: Christian Ambach <ambi@samba.org> >Reviewed-by: Guenther Deschner <gd@samba.org> > >Autobuild-User(master): Günther Deschner <gd@samba.org> >Autobuild-Date(master): Fri Jan 17 12:01:47 UTC 2025 on atb-devel-224 > >(cherry picked from commit f9eb0b248da0689c82656f3e482161c45749afb6) >--- > auth/kerberos/kerberos_pac.c | 88 ++++++++++++++++++------------------ > 1 file changed, 43 insertions(+), 45 deletions(-) > >diff --git a/auth/kerberos/kerberos_pac.c b/auth/kerberos/kerberos_pac.c >index 1f7d3e7ef26..4c61cfe838f 100644 >--- a/auth/kerberos/kerberos_pac.c >+++ b/auth/kerberos/kerberos_pac.c >@@ -137,7 +137,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > time_t tgs_authtime, > struct PAC_DATA **pac_data_out) > { >- NTSTATUS status; >+ NTSTATUS status = NT_STATUS_NO_MEMORY; > enum ndr_err_code ndr_err; > krb5_error_code ret; > DATA_BLOB modified_pac_blob; >@@ -173,8 +173,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > kdc_sig_wipe = talloc(tmp_ctx, struct PAC_SIGNATURE_DATA); > srv_sig_wipe = talloc(tmp_ctx, struct PAC_SIGNATURE_DATA); > if (!pac_data_raw || !pac_data || !kdc_sig_wipe || !srv_sig_wipe) { >- talloc_free(tmp_ctx); >- return NT_STATUS_NO_MEMORY; >+ status = NT_STATUS_NO_MEMORY; >+ goto out; > } > > ndr_err = ndr_pull_struct_blob(&pac_data_blob, pac_data, pac_data, >@@ -183,15 +183,14 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > status = ndr_map_error2ntstatus(ndr_err); > DEBUG(0,("can't parse the PAC: %s\n", > nt_errstr(status))); >- talloc_free(tmp_ctx); >- return status; >+ goto out; > } > > if (pac_data->num_buffers < 4) { > /* we need logon_info, service_key and kdc_key */ > DEBUG(0,("less than 4 PAC buffers\n")); >- talloc_free(tmp_ctx); >- return NT_STATUS_INVALID_PARAMETER; >+ status = NT_STATUS_INVALID_PARAMETER; >+ goto out; > } > > ndr_err = ndr_pull_struct_blob( >@@ -201,15 +200,14 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > status = ndr_map_error2ntstatus(ndr_err); > DEBUG(0,("can't parse the PAC: %s\n", > nt_errstr(status))); >- talloc_free(tmp_ctx); >- return status; >+ goto out; > } > > if (pac_data_raw->num_buffers < 4) { > /* we need logon_info, service_key and kdc_key */ > DEBUG(0,("less than 4 PAC buffers\n")); >- talloc_free(tmp_ctx); >- return NT_STATUS_INVALID_PARAMETER; >+ status = NT_STATUS_INVALID_PARAMETER; >+ goto out; > } > > if (pac_data->num_buffers != pac_data_raw->num_buffers) { >@@ -217,8 +215,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > DEBUG(0, ("misparse! PAC_DATA has %d buffers while " > "PAC_DATA_RAW has %d\n", pac_data->num_buffers, > pac_data_raw->num_buffers)); >- talloc_free(tmp_ctx); >- return NT_STATUS_INVALID_PARAMETER; >+ status = NT_STATUS_INVALID_PARAMETER; >+ goto out; > } > > for (i=0; i < pac_data->num_buffers; i++) { >@@ -229,8 +227,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > DEBUG(0, ("misparse! PAC_DATA buffer %d has type " > "%d while PAC_DATA_RAW has %d\n", i, > data_buf->type, raw_buf->type)); >- talloc_free(tmp_ctx); >- return NT_STATUS_INVALID_PARAMETER; >+ status = NT_STATUS_INVALID_PARAMETER; >+ goto out; > } > switch (data_buf->type) { > case PAC_TYPE_LOGON_INFO: >@@ -263,26 +261,26 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > > if (!logon_info) { > DEBUG(0,("PAC no logon_info\n")); >- talloc_free(tmp_ctx); >- return NT_STATUS_INVALID_PARAMETER; >+ status = NT_STATUS_INVALID_PARAMETER; >+ goto out; > } > > if (!logon_name) { > DEBUG(0,("PAC no logon_name\n")); >- talloc_free(tmp_ctx); >- return NT_STATUS_INVALID_PARAMETER; >+ status = NT_STATUS_INVALID_PARAMETER; >+ goto out; > } > > if (!srv_sig_ptr || !srv_sig_blob) { > DEBUG(0,("PAC no srv_key\n")); >- talloc_free(tmp_ctx); >- return NT_STATUS_INVALID_PARAMETER; >+ status = NT_STATUS_INVALID_PARAMETER; >+ goto out; > } > > if (!kdc_sig_ptr || !kdc_sig_blob) { > DEBUG(0,("PAC no kdc_key\n")); >- talloc_free(tmp_ctx); >- return NT_STATUS_INVALID_PARAMETER; >+ status = NT_STATUS_INVALID_PARAMETER; >+ goto out; > } > > /* Find and zero out the signatures, >@@ -297,8 +295,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > status = ndr_map_error2ntstatus(ndr_err); > DEBUG(0,("can't parse the KDC signature: %s\n", > nt_errstr(status))); >- talloc_free(tmp_ctx); >- return status; >+ goto out; > } > > ndr_err = ndr_pull_struct_blob( >@@ -308,8 +305,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > status = ndr_map_error2ntstatus(ndr_err); > DEBUG(0,("can't parse the SRV signature: %s\n", > nt_errstr(status))); >- talloc_free(tmp_ctx); >- return status; >+ goto out; > } > > /* Now zero the decoded structure */ >@@ -326,8 +322,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > status = ndr_map_error2ntstatus(ndr_err); > DEBUG(0,("can't repack the KDC signature: %s\n", > nt_errstr(status))); >- talloc_free(tmp_ctx); >- return status; >+ goto out; > } > ndr_err = ndr_push_struct_blob( > srv_sig_blob, pac_data_raw, srv_sig_wipe, >@@ -336,8 +331,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > status = ndr_map_error2ntstatus(ndr_err); > DEBUG(0,("can't repack the SRV signature: %s\n", > nt_errstr(status))); >- talloc_free(tmp_ctx); >- return status; >+ goto out; > } > > /* push out the whole structure, but now with zero'ed signatures */ >@@ -348,8 +342,7 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > status = ndr_map_error2ntstatus(ndr_err); > DEBUG(0,("can't repack the RAW PAC: %s\n", > nt_errstr(status))); >- talloc_free(tmp_ctx); >- return status; >+ goto out; > } > > if (service_keyblock) { >@@ -360,8 +353,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > if (ret) { > DEBUG(5, ("PAC Decode: Failed to verify the service " > "signature: %s\n", error_message(ret))); >- talloc_free(tmp_ctx); >- return NT_STATUS_ACCESS_DENIED; >+ status = NT_STATUS_ACCESS_DENIED; >+ goto out; > } > > if (krbtgt_keyblock) { >@@ -371,8 +364,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > if (ret) { > DEBUG(1, ("PAC Decode: Failed to verify the KDC signature: %s\n", > smb_get_krb5_error_message(context, ret, tmp_ctx))); >- talloc_free(tmp_ctx); >- return NT_STATUS_ACCESS_DENIED; >+ status = NT_STATUS_ACCESS_DENIED; >+ goto out; > } > } > } >@@ -388,8 +381,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > nt_time_string(tmp_ctx, logon_name->logon_time))); > DEBUG(2, ("PAC Decode: Ticket: %s\n", > nt_time_string(tmp_ctx, tgs_authtime_nttime))); >- talloc_free(tmp_ctx); >- return NT_STATUS_ACCESS_DENIED; >+ status = NT_STATUS_ACCESS_DENIED; >+ goto out; > } > } > >@@ -401,8 +394,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > if (ret) { > DEBUG(2, ("Could not unparse name from ticket to match with name from PAC: [%s]:%s\n", > logon_name->account_name, error_message(ret))); >- talloc_free(tmp_ctx); >- return NT_STATUS_INVALID_PARAMETER; >+ status = NT_STATUS_INVALID_PARAMETER; >+ goto out; > } > > bool_ret = strcmp(client_principal_string, logon_name->account_name) == 0; >@@ -413,8 +406,8 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > logon_name->account_name, > client_principal_string)); > SAFE_FREE(client_principal_string); >- talloc_free(tmp_ctx); >- return NT_STATUS_ACCESS_DENIED; >+ status = NT_STATUS_ACCESS_DENIED; >+ goto out; > } > SAFE_FREE(client_principal_string); > >@@ -435,10 +428,15 @@ NTSTATUS kerberos_decode_pac(TALLOC_CTX *mem_ctx, > } > > if (pac_data_out) { >- *pac_data_out = talloc_steal(mem_ctx, pac_data); >+ *pac_data_out = talloc_move(mem_ctx, &pac_data); > } > >- return NT_STATUS_OK; >+ status = NT_STATUS_OK; >+ >+ out: >+ >+ TALLOC_FREE(tmp_ctx); >+ return status; > } > > NTSTATUS kerberos_pac_logon_info(TALLOC_CTX *mem_ctx, >-- >2.48.1 >
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=18531">View the attachment on a separate page</a>.</b>
View Attachment As Raw
Flags:
gd
:
review?
(
jra
)
ambi
:
review+
gd
:
ci-passed+
Actions:
View
Attachments on
bug 15782
: 18531 |
18532