From 5d5d6cb98a42a09d02b8427bfc70ba1e9505eb50 Mon Sep 17 00:00:00 2001 From: Simo Sorce Date: Thu, 26 Feb 2015 15:49:59 -0500 Subject: Properly handle security contexts on error On error we need to make sure we do not return a pointer to a security context that may have been already freed. So make sure to always unconditionally return the context that we've been returned by our callees. Also reorganize the code so we do not accidently wipe the context and leak memoy on error. This fixed a double-free bug found by NFS folks @ Red Hat Signed-off-by: Simo Sorce --- proxy/src/client/gpm_accept_sec_context.c | 14 ++++++++------ proxy/src/client/gpm_init_sec_context.c | 13 +++++++------ 2 files changed, 15 insertions(+), 12 deletions(-) (limited to 'proxy/src/client') diff --git a/proxy/src/client/gpm_accept_sec_context.c b/proxy/src/client/gpm_accept_sec_context.c index 5f8fb06..dc03443 100644 --- a/proxy/src/client/gpm_accept_sec_context.c +++ b/proxy/src/client/gpm_accept_sec_context.c @@ -116,12 +116,6 @@ OM_uint32 gpm_accept_sec_context(OM_uint32 *minor_status, goto done; } - /* replace old ctx handle if any */ - if (*context_handle) { - xdr_free((xdrproc_t)xdr_gssx_ctx, (char *)*context_handle); - free(*context_handle); - } - *context_handle = ctx; if (mech_type) { *mech_type = mech; } @@ -157,6 +151,7 @@ done: arg->context_handle = NULL; arg->cred_handle = NULL; gpm_free_xdrs(GSSX_ACCEPT_SEC_CONTEXT, &uarg, &ures); + if (ret) { if (ctx) { xdr_free((xdrproc_t)xdr_gssx_ctx, (char *)ctx); @@ -178,6 +173,13 @@ done: return GSS_S_FAILURE; } + /* always replace old ctx handle and set new */ + if (*context_handle) { + xdr_free((xdrproc_t)xdr_gssx_ctx, (char *)*context_handle); + free(*context_handle); + } + *context_handle = ctx; + return ret_maj; } diff --git a/proxy/src/client/gpm_init_sec_context.c b/proxy/src/client/gpm_init_sec_context.c index bd88055..14c65e4 100644 --- a/proxy/src/client/gpm_init_sec_context.c +++ b/proxy/src/client/gpm_init_sec_context.c @@ -137,12 +137,6 @@ done: gpm_free_xdrs(GSSX_INIT_SEC_CONTEXT, &uarg, &ures); if (ret_maj == GSS_S_COMPLETE || ret_maj == GSS_S_CONTINUE_NEEDED) { - /* replace old ctx handle if any */ - if (*context_handle) { - xdr_free((xdrproc_t)xdr_gssx_ctx, (char *)*context_handle); - free(*context_handle); - } - *context_handle = ctx; if (actual_mech_type) { *actual_mech_type = mech; } @@ -171,6 +165,13 @@ done: } } + /* always replace old ctx handle and set new */ + if (*context_handle) { + xdr_free((xdrproc_t)xdr_gssx_ctx, (char *)*context_handle); + free(*context_handle); + } + *context_handle = ctx; + *minor_status = ret_min; return ret_maj; } -- cgit