[PATCH 1/4] WIP: gss_buffer_desc memory management fixes.

Sam Hartman hartmans at painless-security.com
Wed Sep 28 15:50:56 EDT 2011


From: Kevin Wasserman <kevin.wasserman at painless-security.com>

Introduce gss_malloc_buffer/gss_free_buffer for use with gss_buffer_desc.
Usually these will just evaluate to malloc/free, but on windows they
instead use the default process heap to avoid c runtime incompatibilities
when a buffer is allocated in one dll but freed in another, which is
inevitable given the current gssapi mechglue.

This commit does introduce additional malloc/copy overhead in order to
introduce a boundary between k5buf memory management and
gss_buffer_desc which is not actually needed except on windows.
Should rewrite with a platform-dependent inline function to do
the more efficient simple pointer swap on unix.

Signed-off-by: Kevin Wasserman <kevin.wasserman at painless-security.com>
---
 src/lib/gssapi/generic/gssapiP_generic.h |    1 +
 src/lib/gssapi/generic/oid_ops.c         |    4 +++-
 src/lib/gssapi/generic/rel_buffer.c      |    2 +-
 src/lib/gssapi/generic/util_buffer.c     |    3 ++-
 src/lib/gssapi/krb5/init_sec_context.c   |    7 +++----
 src/lib/gssapi/krb5/k5sealv3.c           |    6 +++---
 src/lib/gssapi/mechglue/g_glue.c         |    2 +-
 src/lib/gssapi/mechglue/g_rel_buffer.c   |    2 +-
 src/lib/gssapi/mechglue/g_rel_name.c     |    2 +-
 9 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/src/lib/gssapi/generic/gssapiP_generic.h b/src/lib/gssapi/generic/gssapiP_generic.h
index e084b81..4b2ce6e 100644
--- a/src/lib/gssapi/generic/gssapiP_generic.h
+++ b/src/lib/gssapi/generic/gssapiP_generic.h
@@ -41,6 +41,7 @@
 
 #include "gssapi_generic.h"
 #include "gssapi_ext.h"
+#include "gssapi_bufferext.h"
 #include "gssapi_err_generic.h"
 #include <errno.h>
 
diff --git a/src/lib/gssapi/generic/oid_ops.c b/src/lib/gssapi/generic/oid_ops.c
index c423542..d2aa778 100644
--- a/src/lib/gssapi/generic/oid_ops.c
+++ b/src/lib/gssapi/generic/oid_ops.c
@@ -271,7 +271,9 @@ generic_gss_oid_to_str(OM_uint32 *minor_status,
         return(GSS_S_FAILURE);
     }
     oid_str->length = krb5int_buf_len(&buf)+1;
-    oid_str->value = (void *) bp;
+    oid_str->value = gss_malloc_buffer(oid_str->length);
+    memcpy(oid_str->value, bp, oid_str->length);
+    krb5int_free_buf(&buf);
     return(GSS_S_COMPLETE);
 }
 
diff --git a/src/lib/gssapi/generic/rel_buffer.c b/src/lib/gssapi/generic/rel_buffer.c
index fb67123..2a57a25 100644
--- a/src/lib/gssapi/generic/rel_buffer.c
+++ b/src/lib/gssapi/generic/rel_buffer.c
@@ -48,7 +48,7 @@ generic_gss_release_buffer(
         return(GSS_S_COMPLETE);
 
     if (buffer->value) {
-        free(buffer->value);
+        gss_free_buffer(buffer->value);
         buffer->length = 0;
         buffer->value = NULL;
     }
diff --git a/src/lib/gssapi/generic/util_buffer.c b/src/lib/gssapi/generic/util_buffer.c
index 81d86fc..218d370 100644
--- a/src/lib/gssapi/generic/util_buffer.c
+++ b/src/lib/gssapi/generic/util_buffer.c
@@ -39,10 +39,11 @@ int g_make_string_buffer(const char *str, gss_buffer_t buffer)
 
     buffer->length = strlen(str);
 
-    if ((buffer->value = strdup(str)) == NULL) {
+    if ((buffer->value = gss_malloc_buffer(buffer->length+1)) == NULL) {
         buffer->length = 0;
         return(0);
     }
+    strcpy(buffer->value, str);
 
     return(1);
 }
diff --git a/src/lib/gssapi/krb5/init_sec_context.c b/src/lib/gssapi/krb5/init_sec_context.c
index d62822e..d15dcd9 100644
--- a/src/lib/gssapi/krb5/init_sec_context.c
+++ b/src/lib/gssapi/krb5/init_sec_context.c
@@ -475,14 +475,13 @@ make_ap_req_v1(context, ctx, cred, k_cred, ad_context,
          * typical GSS wrapping.
          */
         token->length = ap_req.length;
-        token->value = ap_req.data;
-
-        ap_req.data = NULL; /* don't double free */
+        token->value = gss_malloc_buffer(ap_req.length);
+        memcpy(token->value, ap_req.data, ap_req.length);
     } else {
         /* allocate space for the token */
         tlen = g_token_size((gss_OID) mech_type, ap_req.length);
 
-        if ((t = (unsigned char *) xmalloc(tlen)) == NULL) {
+        if ((t = (unsigned char *) gss_malloc_buffer(tlen)) == NULL) {
             code = ENOMEM;
             goto cleanup;
         }
diff --git a/src/lib/gssapi/krb5/k5sealv3.c b/src/lib/gssapi/krb5/k5sealv3.c
index f050f6d..b3124bc 100644
--- a/src/lib/gssapi/krb5/k5sealv3.c
+++ b/src/lib/gssapi/krb5/k5sealv3.c
@@ -136,7 +136,7 @@ gss_krb5int_make_seal_token_v3 (krb5_context context,
         /* Get size of ciphertext.  */
         bufsize = 16 + krb5_encrypt_size (plain.length, key->keyblock.enctype);
         /* Allocate space for header plus encrypted data.  */
-        outbuf = malloc(bufsize);
+        outbuf = gss_malloc_buffer(bufsize);
         if (outbuf == NULL) {
             free(plain.data);
             return ENOMEM;
@@ -204,7 +204,7 @@ gss_krb5int_make_seal_token_v3 (krb5_context context,
         assert(cksumsize <= 0xffff);
 
         bufsize = 16 + message2->length + cksumsize;
-        outbuf = malloc(bufsize);
+        outbuf = gss_malloc_buffer(bufsize);
         if (outbuf == NULL) {
             free(plain.data);
             plain.data = 0;
@@ -290,7 +290,7 @@ gss_krb5int_make_seal_token_v3 (krb5_context context,
     return 0;
 
 error:
-    free(outbuf);
+    gss_free_buffer(outbuf);
     token->value = NULL;
     token->length = 0;
     return err;
diff --git a/src/lib/gssapi/mechglue/g_glue.c b/src/lib/gssapi/mechglue/g_glue.c
index 90febd5..074437b 100644
--- a/src/lib/gssapi/mechglue/g_glue.c
+++ b/src/lib/gssapi/mechglue/g_glue.c
@@ -724,7 +724,7 @@ gssint_create_copy_buffer(srcBuf, destBuf, addNullChar)
     else
 	len = srcBuf->length;
 
-    if (!(aBuf->value = (void*)malloc(len))) {
+    if (!(aBuf->value = (void*)gss_malloc_buffer(len))) {
 	free(aBuf);
 	return (GSS_S_FAILURE);
     }
diff --git a/src/lib/gssapi/mechglue/g_rel_buffer.c b/src/lib/gssapi/mechglue/g_rel_buffer.c
index c1104fd..31518fe 100644
--- a/src/lib/gssapi/mechglue/g_rel_buffer.c
+++ b/src/lib/gssapi/mechglue/g_rel_buffer.c
@@ -49,7 +49,7 @@ gss_buffer_t		buffer;
 
     if ((buffer->length) &&
 	(buffer->value)) {
-	    free(buffer->value);
+	    gss_free_buffer(buffer->value);
 	    buffer->length = 0;
 	    buffer->value = NULL;
     }
diff --git a/src/lib/gssapi/mechglue/g_rel_name.c b/src/lib/gssapi/mechglue/g_rel_name.c
index e8ac6c3..762014a 100644
--- a/src/lib/gssapi/mechglue/g_rel_name.c
+++ b/src/lib/gssapi/mechglue/g_rel_name.c
@@ -70,7 +70,7 @@ gss_name_t *		input_name;
 
     if (union_name->external_name != GSS_C_NO_BUFFER) {
 	if (union_name->external_name->value != NULL)
-	    free(union_name->external_name->value);
+	    gss_free_buffer(union_name->external_name->value);
 	free(union_name->external_name);
     }
 
-- 
1.7.4.1




More information about the krbdev mailing list