From 102ba9075b8df32627f38074a293d804e04139ba Mon Sep 17 00:00:00 2001 From: Dirk Engling Date: Mon, 19 Apr 2021 22:33:23 +0200 Subject: Make accesslist reload thread safe. The last commit actually would make a free possible while another thread was bsearching that memory --- ot_accesslist.c | 58 +++++++++++++++++++++++++++++++++++++++------------------ 1 file changed, 40 insertions(+), 18 deletions(-) (limited to 'ot_accesslist.c') diff --git a/ot_accesslist.c b/ot_accesslist.c index 12bd29e..2e3778f 100644 --- a/ot_accesslist.c +++ b/ot_accesslist.c @@ -25,19 +25,25 @@ /* GLOBAL VARIABLES */ #ifdef WANT_ACCESSLIST char *g_accesslist_filename; -static ot_hash *g_accesslist; -static size_t g_accesslist_size; static pthread_mutex_t g_accesslist_mutex; +typedef struct { + ot_hash *list; + size_t size; +} ot_accesslist; +ot_accesslist * g_accesslist = NULL; +ot_accesslist * g_accesslist_old = NULL; + static int vector_compare_hash(const void *hash1, const void *hash2 ) { return memcmp( hash1, hash2, OT_HASH_COMPARE_SIZE ); } /* Read initial access list */ static void accesslist_readfile( void ) { - ot_hash *info_hash, *accesslist_new = NULL, *accesslist_old; - char *map, *map_end, *read_offs; - size_t maplen; + ot_accesslist * accesslist_new = malloc(sizeof(ot_accesslist)); + ot_hash *info_hash; + const char *map, *map_end, *read_offs; + size_t maplen; if( ( map = mmap_read( g_accesslist_filename, &maplen ) ) == NULL ) { char *wd = getcwd( NULL, 0 ); @@ -48,9 +54,11 @@ static void accesslist_readfile( void ) { /* You need at least 41 bytes to pass an info_hash, make enough room for the maximum amount of them */ - info_hash = accesslist_new = malloc( ( maplen / 41 ) * 20 ); + accesslist_new->size = 0; + info_hash = accesslist_new->list = malloc( ( maplen / 41 ) * 20 ); if( !accesslist_new ) { fprintf( stderr, "Warning: Not enough memory to allocate %zd bytes for accesslist buffer. May succeed later.\n", ( maplen / 41 ) * 20 ); + free(accesslist_new); return; } @@ -81,28 +89,33 @@ static void accesslist_readfile( void ) { while( read_offs <= map_end && *(read_offs++) != '\n' ); } #ifdef _DEBUG - fprintf( stderr, "Added %zd info_hashes to accesslist\n", (size_t)(info_hash - accesslist_new) ); + fprintf( stderr, "Added %zd info_hashes to accesslist\n", (size_t)(info_hash - accesslist_new->list) ); #endif mmap_unmap( map, maplen); - qsort( accesslist_new, info_hash - accesslist_new, sizeof( *info_hash ), vector_compare_hash ); + qsort( accesslist_new->list, info_hash - accesslist_new->list, sizeof( *info_hash ), vector_compare_hash ); + accesslist_new->size = info_hash - accesslist_new->list; /* Now exchange the accesslist vector in the least race condition prone way */ pthread_mutex_lock(&g_accesslist_mutex); - accesslist_old = g_accesslist; /* Keep a copy for later free */ - g_accesslist_size = 0; /* Set size to 0 to prevent clients from searching through uninitialised memory */ - g_accesslist = accesslist_new; /* Only now set a new list */ - g_accesslist_size = info_hash - accesslist_new; /* And finally store it's size */ - free(g_accesslist_old); /* If new list is active, the old one can be destroyed */ + if (g_accesslist_old) { + free(g_accesslist_old->list); + free(g_accesslist_old); + } + + g_accesslist_old = g_accesslist; /* Keep a copy for later free */ + g_accesslist = accesslist_new; /* Only now set a new list */ + pthread_mutex_unlock(&g_accesslist_mutex); } int accesslist_hashisvalid( ot_hash hash ) { - void *exactmatch; + /* Get working copy of current access list */ + ot_accesslist * accesslist = g_accesslist; - exactmatch = bsearch( hash, g_accesslist, g_accesslist_size, OT_HASH_COMPARE_SIZE, vector_compare_hash ); + void * exactmatch = bsearch( hash, accesslist->list, accesslist->size, OT_HASH_COMPARE_SIZE, vector_compare_hash ); #ifdef WANT_ACCESSLIST_BLACK return exactmatch == NULL; @@ -140,9 +153,18 @@ void accesslist_init( ) { void accesslist_deinit( void ) { pthread_cancel( thread_id ); pthread_mutex_destroy(&g_accesslist_mutex); - free( g_accesslist ); - g_accesslist = 0; - g_accesslist_size = 0; + + if (g_accesslist_old) { + free(g_accesslist_old->list); + free(g_accesslist_old); + g_accesslist_old = 0; + } + + if (g_accesslist) { + free(g_accesslist->list); + free(g_accesslist); + g_accesslist = 0; + } } #endif -- cgit v1.2.3