From de4ea067cffbce9c93bf4ba84d2f78af64821be3 Mon Sep 17 00:00:00 2001 From: ekorh475 Date: Fri, 18 Nov 2016 11:30:02 +0200 Subject: [PATCH] Fix for MXS-968 This commit adds a free() to null_auth_free_client_data, which plugs the memory leak in maxinfo. Also, this commit fixes some segfaults when multiple threads are running status_row() or variable_row(). The functions use statically allocated index variables, which often go out-of-bounds in concurrent use. This fix changes the indexes to thread-specific variables, with allocating and deallocating. This does seem to slow the functions down somewhat. --- server/modules/authenticator/null_auth.c | 6 ++- server/modules/routing/maxinfo/maxinfo_exec.c | 49 ++++++++++++------- 2 files changed, 37 insertions(+), 18 deletions(-) diff --git a/server/modules/authenticator/null_auth.c b/server/modules/authenticator/null_auth.c index 6819e1d06..c4ca31f08 100644 --- a/server/modules/authenticator/null_auth.c +++ b/server/modules/authenticator/null_auth.c @@ -150,4 +150,8 @@ null_auth_is_client_ssl_capable(DCB *dcb) * @param dcb Request handler DCB connected to the client */ static void -null_auth_free_client_data(DCB *dcb) {} +null_auth_free_client_data(DCB *dcb) +{ + free(dcb->data); + dcb->data = NULL; +} diff --git a/server/modules/routing/maxinfo/maxinfo_exec.c b/server/modules/routing/maxinfo/maxinfo_exec.c index bbe8a782e..884d65e42 100644 --- a/server/modules/routing/maxinfo/maxinfo_exec.c +++ b/server/modules/routing/maxinfo/maxinfo_exec.c @@ -825,7 +825,6 @@ typedef struct int index; char *like; } VARCONTEXT; - /** * Callback function to populate rows of the show variable * command @@ -836,9 +835,9 @@ typedef struct static RESULT_ROW * variable_row(RESULTSET *result, void *data) { - VARCONTEXT *context = (VARCONTEXT *)data; - RESULT_ROW *row; - char buf[80]; + VARCONTEXT *context = (VARCONTEXT *) data; + RESULT_ROW *row; + char buf[80]; if (variables[context->index].name) { @@ -862,10 +861,14 @@ variable_row(RESULTSET *result, void *data) (long)(*variables[context->index].func)()); resultset_row_set(row, 1, buf); break; + default: + ss_dassert(!true); } context->index++; return row; } + // We only get to this point once all variables have been printed + free(data); return NULL; } @@ -910,16 +913,20 @@ exec_show_variables(DCB *dcb, MAXINFO_TREE *filter) RESULTSET * maxinfo_variables() { - RESULTSET *result; - static VARCONTEXT context; - - context.like = NULL; - context.index = 0; - - if ((result = resultset_create(variable_row, &context)) == NULL) + RESULTSET *result; + VARCONTEXT *context; + if ((context = malloc(sizeof(VARCONTEXT))) == NULL) { return NULL; } + context->like = NULL; + context->index = 0; + + if ((result = resultset_create(variable_row, context)) == NULL) + { + free(context); + return NULL; + } resultset_add_column(result, "Variable_name", 40, COL_TYPE_VARCHAR); resultset_add_column(result, "Value", 40, COL_TYPE_VARCHAR); return result; @@ -1140,10 +1147,14 @@ status_row(RESULTSET *result, void *data) (long)(*status[context->index].func)()); resultset_row_set(row, 1, buf); break; + default: + ss_dassert(!true); } context->index++; return row; } + // We only get to this point once all status elements have been printed + free(data); return NULL; } @@ -1189,15 +1200,19 @@ RESULTSET * maxinfo_status() { RESULTSET *result; - static VARCONTEXT context; - - context.like = NULL; - context.index = 0; - - if ((result = resultset_create(status_row, &context)) == NULL) + VARCONTEXT *context; + if ((context = malloc(sizeof(VARCONTEXT))) == NULL) { return NULL; } + context->like = NULL; + context->index = 0; + + if ((result = resultset_create(status_row, context)) == NULL) + { + free(context); + return NULL; + } resultset_add_column(result, "Variable_name", 40, COL_TYPE_VARCHAR); resultset_add_column(result, "Value", 40, COL_TYPE_VARCHAR); return result;