Fix bug 5686 - libsmbclient segfaults with more than one SMBCCTX.
authorJeremy Allison <jra@samba.org>
Tue, 12 Aug 2008 20:35:15 +0000 (13:35 -0700)
committerJeremy Allison <jra@samba.org>
Tue, 12 Aug 2008 20:35:15 +0000 (13:35 -0700)
Here is a patch to allow many subsystems to be re-initialized. The only
functional change I made was to remove the null context tracking, as the memory
allocated here is designed to be left for the complete lifetime of the program.
Freeing this early (when all smb contexts are destroyed) could crash other
users of talloc.
Jeremy.

examples/libsmbclient/Makefile
examples/libsmbclient/testctx.c [new file with mode: 0644]
source/lib/charcnv.c
source/lib/debug.c
source/lib/util.c
source/lib/util_unistr.c
source/libsmb/libsmb_context.c
source/param/loadparm.c

index 7415f4f07ec1d753fd706dadd12238757eb470c9..047addc8f7ad856e9c865d117cba7b0bd00aebd1 100644 (file)
@@ -94,6 +94,10 @@ testwrite: testwrite.o
        @echo Linking testwrite
        $(CC) $(CFLAGS) $(LDFLAGS) -o $@ $< $(LIBSMBCLIENT) -lpopt
 
+testctx: testctx.o
+       @echo Linking testctx
+       $(CC) $(CFLAGS) $(LDFLAGS) -o $@ $< $(LIBSMBCLIENT) -lpopt
+
 smbsh:
        make -C smbwrapper
 
diff --git a/examples/libsmbclient/testctx.c b/examples/libsmbclient/testctx.c
new file mode 100644 (file)
index 0000000..8820bc8
--- /dev/null
@@ -0,0 +1,17 @@
+#include <libsmbclient.h>
+
+void create_and_destroy_context (void)
+{
+  SMBCCTX *ctx;
+  ctx = smbc_new_context ();
+  smbc_init_context (ctx);
+
+  smbc_free_context (ctx, 1);
+}
+
+int main (int argc, char **argv)
+{
+  create_and_destroy_context ();
+  create_and_destroy_context ();
+  return 0;
+}
index b1a53934613be463a148655b5fb3c308c64f5b9c..485212b100f510523e7701aa074a6468e33da95d 100644 (file)
@@ -47,6 +47,7 @@ char lp_failed_convert_char(void)
 
 static smb_iconv_t conv_handles[NUM_CHARSETS][NUM_CHARSETS];
 static bool conv_silent; /* Should we do a debug if the conversion fails ? */
+static bool initialized;
 
 /**
  * Return the name of a charset to give to iconv().
@@ -92,12 +93,10 @@ static const char *charset_name(charset_t ch)
 
 void lazy_initialize_conv(void)
 {
-       static int initialized = False;
-
        if (!initialized) {
-               initialized = True;
                load_case_tables();
                init_iconv();
+               initialized = true;
        }
 }
 
@@ -116,6 +115,7 @@ void gfree_charcnv(void)
                        }
                }
        }
+       initialized = false;
 }
 
 /**
index 2ded6bdc333f111fcf11cc457d22b8b8b07e3dd4..d835ea7c176e5a824b387a587f21ba94991bf6e5 100644 (file)
@@ -94,7 +94,7 @@ static TALLOC_CTX *tmp_debug_ctx;
 
 /*
  * This is to allow assignment to DEBUGLEVEL before the debug
- * system has been initialised.
+ * system has been initialized.
  */
 static int debug_all_class_hack = 1;
 static bool debug_all_class_isset_hack = True;
@@ -183,6 +183,8 @@ static char **classname_table = NULL;
  Free memory pointed to by global pointers.
 ****************************************************************************/
 
+static bool initialized;
+
 void gfree_debugsyms(void)
 {
        int i;
@@ -194,13 +196,23 @@ void gfree_debugsyms(void)
                SAFE_FREE( classname_table );
        }
 
-       if ( DEBUGLEVEL_CLASS != &debug_all_class_hack )
+       if ( DEBUGLEVEL_CLASS != &debug_all_class_hack ) {
                SAFE_FREE( DEBUGLEVEL_CLASS );
+               DEBUGLEVEL_CLASS = &debug_all_class_hack;
+       }
 
-       if ( DEBUGLEVEL_CLASS_ISSET != &debug_all_class_isset_hack )
+       if ( DEBUGLEVEL_CLASS_ISSET != &debug_all_class_isset_hack ) {
                SAFE_FREE( DEBUGLEVEL_CLASS_ISSET );
+               DEBUGLEVEL_CLASS_ISSET = &debug_all_class_isset_hack;
+       }
 
        SAFE_FREE(format_bufr);
+
+       debug_num_classes = 0;
+
+       debug_level = DEBUGLEVEL_CLASS;
+
+       initialized = false;
 }
 
 /****************************************************************************
@@ -530,13 +542,12 @@ Init debugging (one time stuff)
 
 void debug_init(void)
 {
-       static bool initialised = False;
        const char **p;
 
-       if (initialised)
+       if (initialized)
                return;
 
-       initialised = True;
+       initialized = true;
 
        for(p = default_classname_table; *p; p++) {
                debug_add_class(*p);
index 27a1487663dbd1f76e76b1ff56e8f7ae0aee39f4..0fdc9956f1273eb8ffc8e8ac50e9c9c27fb53320 100644 (file)
@@ -189,12 +189,9 @@ void gfree_all( void )
        gfree_names();
        gfree_loadparm();
        gfree_case_tables();
-       gfree_debugsyms();
        gfree_charcnv();
        gfree_interfaces();
-
-       /* release the talloc null_context memory last */
-       talloc_disable_null_tracking();
+       gfree_debugsyms();
 }
 
 const char *my_netbios_names(int i)
index 76235ad041738e27c2385095f965e3a0ff940927..4e78d1b0642e55134d4f79eba57021b59f04ba6a 100644 (file)
@@ -33,6 +33,7 @@ static uint8 *valid_table;
 static bool upcase_table_use_unmap;
 static bool lowcase_table_use_unmap;
 static bool valid_table_use_unmap;
+static bool initialized;
 
 /**
  * Destroy global objects allocated by load_case_tables()
@@ -59,6 +60,7 @@ void gfree_case_tables(void)
                else
                        SAFE_FREE(valid_table);
        }
+       initialized = false;
 }
 
 /**
@@ -70,15 +72,14 @@ void gfree_case_tables(void)
 
 void load_case_tables(void)
 {
-       static int initialised;
        char *old_locale = NULL, *saved_locale = NULL;
        int i;
        TALLOC_CTX *frame = NULL;
 
-       if (initialised) {
+       if (initialized) {
                return;
        }
-       initialised = 1;
+       initialized = true;
 
        frame = talloc_stackframe();
 
index a9f0dd16b3b4cf677b8cb587eb93b13703af13ae..19843383de2bc864a130e57d1384b7e3e41c5117 100644 (file)
@@ -30,9 +30,8 @@
 /*
  * Is the logging working / configfile read ? 
  */
-static int SMBC_initialized = 0;
-
-
+static bool SMBC_initialized;
+static unsigned int initialized_ctx_count;
 
 /*
  * Get a new empty handle to fill in with your own info
@@ -201,22 +200,19 @@ smbc_free_context(SMBCCTX *context,
         
         DEBUG(3, ("Context %p successfully freed\n", context));
 
-       gfree_names();
-       gfree_loadparm();
-       gfree_case_tables();
-       gfree_charcnv();
-       gfree_interfaces();
-
-       gencache_shutdown();
-       secrets_shutdown();
-
-       /* release the talloc null_context memory last */
-       talloc_disable_null_tracking();
+       SAFE_FREE(context->internal);
+        SAFE_FREE(context);
 
-       gfree_debugsyms();
+       if (initialized_ctx_count) {
+               initialized_ctx_count--;
+       }
 
-        SAFE_FREE(context->internal);
-        SAFE_FREE(context);
+       if (initialized_ctx_count == 0 && SMBC_initialized) {
+               gencache_shutdown();
+               secrets_shutdown();
+               gfree_all();
+               SMBC_initialized = false;
+       }
         return 0;
 }
 
@@ -427,9 +423,6 @@ smbc_init_context(SMBCCTX *context)
         char *user = NULL;
         char *home = NULL;
         
-        /* track talloc null_context memory */
-        talloc_enable_null_tracking();
-
         if (!context) {
                 errno = EBADF;
                 return NULL;
@@ -527,7 +520,7 @@ smbc_init_context(SMBCCTX *context)
                 BlockSignals(True, SIGPIPE);
                 
                 /* Done with one-time initialisation */
-                SMBC_initialized = 1;
+                SMBC_initialized = true;
                 
                 TALLOC_FREE(frame);
         }
@@ -616,7 +609,8 @@ smbc_init_context(SMBCCTX *context)
          */
         
         context->internal->initialized = True;
-        
+       initialized_ctx_count++;
+
         return context;
 }
 
index 7fd7bb2bf4e2bea080e84d791f55330b34b78fdc..bc111df4e1f701fe4aac2630a6c921ee8997a90f 100644 (file)
@@ -8693,6 +8693,7 @@ void gfree_loadparm(void)
                SAFE_FREE( f );
                f = next;
        }
+       file_lists = NULL;
 
        /* Free resources allocated to services */