Skip to content

Commit

Permalink
Fix bug 5686 - libsmbclient segfaults with more than one SMBCCTX.
Browse files Browse the repository at this point in the history
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.
(This used to be commit 8c630ef)
  • Loading branch information
jrasamba committed Aug 12, 2008
1 parent 9caabc4 commit 03991ab
Show file tree
Hide file tree
Showing 8 changed files with 63 additions and 38 deletions.
4 changes: 4 additions & 0 deletions examples/libsmbclient/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
17 changes: 17 additions & 0 deletions examples/libsmbclient/testctx.c
Original file line number Diff line number Diff line change
@@ -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;
}
6 changes: 3 additions & 3 deletions source3/lib/charcnv.c
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand Down Expand Up @@ -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;
}
}

Expand All @@ -116,6 +115,7 @@ void gfree_charcnv(void)
}
}
}
initialized = false;
}

/**
Expand Down
23 changes: 17 additions & 6 deletions source3/lib/debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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;
}

/****************************************************************************
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 1 addition & 4 deletions source3/lib/util.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
7 changes: 4 additions & 3 deletions source3/lib/util_unistr.c
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -59,6 +60,7 @@ void gfree_case_tables(void)
else
SAFE_FREE(valid_table);
}
initialized = false;
}

/**
Expand All @@ -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();

Expand Down
38 changes: 16 additions & 22 deletions source3/libsmb/libsmb_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -616,7 +609,8 @@ smbc_init_context(SMBCCTX *context)
*/

context->internal->initialized = True;

initialized_ctx_count++;

return context;
}

Expand Down
1 change: 1 addition & 0 deletions source3/param/loadparm.c
Original file line number Diff line number Diff line change
Expand Up @@ -8693,6 +8693,7 @@ void gfree_loadparm(void)
SAFE_FREE( f );
f = next;
}
file_lists = NULL;

/* Free resources allocated to services */

Expand Down

0 comments on commit 03991ab

Please sign in to comment.