From d343f5e372bbd306b770966940c162172987151a Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Fri, 13 May 2011 18:04:49 +0200 Subject: Bug#12552516 LF_HASH REQUIRES MY_THREAD_INIT() Before this fix, a thread instrumented for the performance schema, that would perform file io operations, could crash inside the LF_HASH implementation, in cases when my_thread_init is not called. The crash itself has not been reported in 5.5 but similar crashes have been found in 5.6-based development branches, using LF_HASH for more instrumentation. The possibility of a crash in 5.5 is confirmed by code analysis. The problem is that, when my_thread_init() is not called, which can happen for threads in storage engines or thirs party code, my_thread_var is NULL. Using my_thread_var->stacks_ends_here in mysys/lf_alloc-pin.c is unsafe. Given that my_thread_var is used: - only for stacks_ends_here - only on platform with HAVE_ALLOCA - only when there is enough room on the stack and given that the LF_HASH implementation has a fallback algorythm implemented already when using alloca is not possible, using my_thread_var->stacks_ends_here is in fact not a strict requirement, and can be relaxed. The fix is to: - test explicitly if my_thread_var is NULL, to account for cases when my_thread_init() is not used by the calling thread. - not use alloca in this case, and rely on the fall back code already in place. so that the LF_HASH can be supported even without my_thread_init(). The implementation of mysys/lf_alloc-pin.c has been fixed to support this new usage. The units tests in unittest/mysys/lf-t.c have been adjusted accordingly. --- mysys/lf_alloc-pin.c | 49 +++++++++++++++++++++++++++++-------------------- 1 file changed, 29 insertions(+), 20 deletions(-) (limited to 'mysys/lf_alloc-pin.c') diff --git a/mysys/lf_alloc-pin.c b/mysys/lf_alloc-pin.c index 4ed01ac8083..cef51c89076 100644 --- a/mysys/lf_alloc-pin.c +++ b/mysys/lf_alloc-pin.c @@ -146,6 +146,7 @@ void lf_pinbox_destroy(LF_PINBOX *pinbox) */ LF_PINS *_lf_pinbox_get_pins(LF_PINBOX *pinbox) { + struct st_my_thread_var *var; uint32 pins, next, top_ver; LF_PINS *el; /* @@ -188,7 +189,12 @@ LF_PINS *_lf_pinbox_get_pins(LF_PINBOX *pinbox) el->link= pins; el->purgatory_count= 0; el->pinbox= pinbox; - el->stack_ends_here= & my_thread_var->stack_ends_here; + var= my_thread_var; + /* + Threads that do not call my_thread_init() should still be + able to use the LF_HASH. + */ + el->stack_ends_here= (var ? & var->stack_ends_here : NULL); return el; } @@ -327,34 +333,37 @@ static int match_pins(LF_PINS *el, void *addr) */ static void _lf_pinbox_real_free(LF_PINS *pins) { - int npins, alloca_size; - void *list, **addr; + int npins; + void *list; + void **addr= NULL; void *first= NULL, *last= NULL; LF_PINBOX *pinbox= pins->pinbox; npins= pinbox->pins_in_array+1; #ifdef HAVE_ALLOCA - alloca_size= sizeof(void *)*LF_PINBOX_PINS*npins; - /* create a sorted list of pinned addresses, to speed up searches */ - if (available_stack_size(&pinbox, *pins->stack_ends_here) > alloca_size) + if (pins->stack_ends_here != NULL) { - struct st_harvester hv; - addr= (void **) alloca(alloca_size); - hv.granary= addr; - hv.npins= npins; - /* scan the dynarray and accumulate all pinned addresses */ - _lf_dynarray_iterate(&pinbox->pinarray, - (lf_dynarray_func)harvest_pins, &hv); - - npins= hv.granary-addr; - /* and sort them */ - if (npins) - qsort(addr, npins, sizeof(void *), (qsort_cmp)ptr_cmp); + int alloca_size; + alloca_size= sizeof(void *)*LF_PINBOX_PINS*npins; + /* create a sorted list of pinned addresses, to speed up searches */ + if (available_stack_size(&pinbox, *pins->stack_ends_here) > alloca_size) + { + struct st_harvester hv; + addr= (void **) alloca(alloca_size); + hv.granary= addr; + hv.npins= npins; + /* scan the dynarray and accumulate all pinned addresses */ + _lf_dynarray_iterate(&pinbox->pinarray, + (lf_dynarray_func)harvest_pins, &hv); + + npins= hv.granary-addr; + /* and sort them */ + if (npins) + qsort(addr, npins, sizeof(void *), (qsort_cmp)ptr_cmp); + } } - else #endif - addr= 0; list= pins->purgatory; pins->purgatory= 0; -- cgit v1.2.1 From b1307dc0e173ba1d0b32e6c5fdb06b8aaf9e69dc Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Mon, 16 May 2011 22:47:59 +0200 Subject: Fixed code review comments --- mysys/lf_alloc-pin.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'mysys/lf_alloc-pin.c') diff --git a/mysys/lf_alloc-pin.c b/mysys/lf_alloc-pin.c index cef51c89076..127d7807761 100644 --- a/mysys/lf_alloc-pin.c +++ b/mysys/lf_alloc-pin.c @@ -344,8 +344,7 @@ static void _lf_pinbox_real_free(LF_PINS *pins) #ifdef HAVE_ALLOCA if (pins->stack_ends_here != NULL) { - int alloca_size; - alloca_size= sizeof(void *)*LF_PINBOX_PINS*npins; + int alloca_size= sizeof(void *)*LF_PINBOX_PINS*npins; /* create a sorted list of pinned addresses, to speed up searches */ if (available_stack_size(&pinbox, *pins->stack_ends_here) > alloca_size) { -- cgit v1.2.1