From 5c210455fa2d568bf708d012e09895475ff70080 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 14 Jan 2015 18:20:59 +0200 Subject: [PATCH 1/6] Fixed variable declaration being inside a debug build only block. --- server/modules/filter/tee.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/server/modules/filter/tee.c b/server/modules/filter/tee.c index 0f3021942..639ad7a51 100644 --- a/server/modules/filter/tee.c +++ b/server/modules/filter/tee.c @@ -283,7 +283,9 @@ orphan_free(void* data) while(finished) { +#ifdef SS_DEBUG o_freed++; +#endif tmp = finished; finished = finished->next; From b635eb1493506c290cc8617c8dbd40eafb63e428 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Wed, 14 Jan 2015 21:13:52 +0200 Subject: [PATCH 2/6] Additional debugging info added to tee filter. --- server/core/modutil.c | 43 +++---- server/modules/filter/tee.c | 220 +++++++++++++++++++++++------------- 2 files changed, 166 insertions(+), 97 deletions(-) diff --git a/server/core/modutil.c b/server/core/modutil.c index fd7fb3b68..7bb57ef2c 100644 --- a/server/core/modutil.c +++ b/server/core/modutil.c @@ -505,27 +505,32 @@ return_packetbuf: int modutil_count_signal_packets(GWBUF *reply,int use_ok, int n_found) { - unsigned char* ptr = (unsigned char*) reply->start; - unsigned char* end = (unsigned char*) reply->end; - int pktlen,pkt = 0; - - while(ptr < end) + unsigned char* ptr = (unsigned char*) reply->start; + unsigned char* end = (unsigned char*) reply->end; + unsigned char* prev = ptr; + int pktlen,pkt = 0,found = n_found; + + while(ptr < end) { - pktlen = gw_mysql_get_byte3(ptr) + 4; + + pktlen = MYSQL_GET_PACKET_LEN(ptr) + 4; - if(PTR_IS_ERR(ptr) || (PTR_IS_EOF(ptr) && !use_ok) || (use_ok && PTR_IS_OK(ptr))) - { - if(n_found) - { - if(ptr + pktlen >= end) - pkt++; - } - else - { - pkt++; - } + if( !found &&(PTR_IS_ERR(ptr) || PTR_IS_EOF(ptr))) + { + pkt++; + found++; } - ptr += pktlen; + + ptr += pktlen; } - return pkt; + + if(found) + { + ptr -= pktlen; + if(PTR_IS_ERR(ptr) || PTR_IS_EOF(ptr)) + pkt++; + } + + return pkt; } +5D diff --git a/server/modules/filter/tee.c b/server/modules/filter/tee.c index 639ad7a51..bb89f99c8 100644 --- a/server/modules/filter/tee.c +++ b/server/modules/filter/tee.c @@ -173,6 +173,9 @@ typedef struct { int residual; /* Any outstanding SQL text */ GWBUF* tee_replybuf; /* Buffer for reply */ SPINLOCK tee_lock; +#ifdef SS_DEBUG + long d_id; +#endif } TEE_SESSION; typedef struct orphan_session_tt @@ -181,6 +184,11 @@ typedef struct orphan_session_tt struct orphan_session_tt* next; }orphan_session_t; +#ifdef SS_DEBUG +static SPINLOCK debug_lock; +static long debug_id = 0; +#endif + static orphan_session_t* allOrphans = NULL; static SPINLOCK orphanLock; @@ -283,9 +291,7 @@ orphan_free(void* data) while(finished) { -#ifdef SS_DEBUG o_freed++; -#endif tmp = finished; finished = finished->next; @@ -322,7 +328,10 @@ void ModuleInit() { spinlock_init(&orphanLock); - hktask_add("tee orphan cleanup",orphan_free,NULL,15); + //hktask_add("tee orphan cleanup",orphan_free,NULL,15); +#ifdef SS_DEBUG + spinlock_init(&debug_lock); +#endif } /** @@ -707,6 +716,8 @@ session_state_t state; gwbuf_free(my_session->tee_replybuf); free(session); + orphan_free(NULL); + return; } /** @@ -824,6 +835,14 @@ unsigned char command = *((unsigned char*)queue->start + 4); memset(my_session->eof,0,2*sizeof(int)); memset(my_session->waiting,1,2*sizeof(bool)); my_session->command = command; +#ifdef SS_DEBUG + spinlock_acquire(&debug_lock); + my_session->d_id = ++debug_id; + skygw_log_write_flush(LOGFILE_DEBUG,"tee.c [%d] query command [%x]", + my_session->d_id, + my_session->command); + spinlock_release(&debug_lock); +#endif rval = my_session->down.routeQuery(my_session->down.instance, my_session->down.session, queue); @@ -873,93 +892,138 @@ unsigned char command = *((unsigned char*)queue->start + 4); static int clientReply (FILTER* instance, void *session, GWBUF *reply) { - int rc, branch, eof; - TEE_SESSION *my_session = (TEE_SESSION *) session; + int rc, branch, eof; + TEE_SESSION *my_session = (TEE_SESSION *) session; + bool route = false; - spinlock_acquire(&my_session->tee_lock); + spinlock_acquire(&my_session->tee_lock); - ss_dassert(my_session->active); + ss_dassert(my_session->active); - branch = instance == NULL ? CHILD : PARENT; - unsigned char *ptr = (unsigned char*)reply->start; + branch = instance == NULL ? CHILD : PARENT; + unsigned char *ptr = (unsigned char*)reply->start; - if(my_session->replies[branch] == 0) - { - /* Reply is in a single packet if it is an OK, ERR or LOCAL_INFILE packet. - * Otherwise the reply is a result set and the amount of packets is unknown. - */ - if(PTR_IS_ERR(ptr) || PTR_IS_LOCAL_INFILE(ptr) || - PTR_IS_OK(ptr) || !my_session->multipacket ) - { - my_session->waiting[branch] = false; - } + if(my_session->replies[branch] == 0) + { + /* Reply is in a single packet if it is an OK, ERR or LOCAL_INFILE packet. + * Otherwise the reply is a result set and the amount of packets is unknown. + */ + if(PTR_IS_ERR(ptr) || PTR_IS_LOCAL_INFILE(ptr) || + PTR_IS_OK(ptr) || !my_session->multipacket ) + { + my_session->waiting[branch] = false; + } #ifdef SS_DEBUG - else - { - ss_dassert(PTR_IS_RESULTSET(ptr)); - skygw_log_write_flush(LOGFILE_DEBUG,"tee.c: Waiting for a result set from %s session.",branch == PARENT?"parent":"child"); - } - ss_dassert(PTR_IS_ERR(ptr) || PTR_IS_LOCAL_INFILE(ptr)|| - PTR_IS_OK(ptr) || my_session->waiting[branch] || - !my_session->multipacket); + else + { + ss_dassert(PTR_IS_RESULTSET(ptr)); + skygw_log_write_flush(LOGFILE_DEBUG,"tee.c: [%d] Waiting for a result set from %s session.", + my_session->d_id, + branch == PARENT?"parent":"child"); + } + ss_dassert(PTR_IS_ERR(ptr) || PTR_IS_LOCAL_INFILE(ptr)|| + PTR_IS_OK(ptr) || my_session->waiting[branch] || + !my_session->multipacket); #endif - } + } - if(my_session->waiting[branch]) - { - - eof = modutil_count_signal_packets(reply,my_session->use_ok,my_session->eof[branch] > 0); - my_session->eof[branch] += eof; - if(my_session->eof[branch] >= 2 || - (my_session->command == 0x04 && my_session->eof[branch] > 0)) - { - ss_dassert(my_session->eof[branch] < 3) - my_session->waiting[branch] = false; - } - } + if(my_session->waiting[branch]) + { + eof = modutil_count_signal_packets(reply,my_session->use_ok,my_session->eof[branch] > 0); + my_session->eof[branch] += eof; + if(my_session->eof[branch] >= 2 || + (my_session->command == 0x04 && my_session->eof[branch] > 0)) + { + skygw_log_write_flush(LOGFILE_DEBUG,"tee.c [%d] %s received last EOF packet", + my_session->d_id, + branch == PARENT?"parent":"child"); + ss_dassert(my_session->eof[branch] < 3) + my_session->waiting[branch] = false; + } + } - if(branch == PARENT) - { - ss_dassert(my_session->tee_replybuf == NULL) - my_session->tee_replybuf = reply; - } - else - { - gwbuf_free(reply); - } + if(branch == PARENT) + { + ss_dassert(my_session->tee_replybuf == NULL) + my_session->tee_replybuf = reply; + } + else + { + gwbuf_free(reply); + } - my_session->replies[branch]++; - - if(my_session->tee_replybuf != NULL && - (my_session->branch_session == NULL || - my_session->waiting[PARENT] || - (!my_session->waiting[CHILD] && !my_session->waiting[PARENT]))) + my_session->replies[branch]++; + rc = 1; + + int min_eof = my_session->command != 0x04 ? 2 : 1; + + if(my_session->tee_replybuf != NULL) + { + + if(my_session->branch_session == NULL) + { + rc = 0; + gwbuf_free(my_session->tee_replybuf); + my_session->tee_replybuf = NULL; + skygw_log_write_flush(LOGFILE_ERROR,"Error : Tee child session was closed."); + } + + if(my_session->multipacket) + { + + if(my_session->waiting[PARENT]) + { + route = true; +#ifdef SS_DEBUG + ss_dassert(my_session->replies[PARENT] < 2 || + modutil_count_signal_packets(my_session->tee_replybuf, + my_session->use_ok, + my_session->eof[PARENT]) == 0); + skygw_log_write_flush(LOGFILE_DEBUG,"tee.c:[%d] Routing partial response set.",my_session->d_id); +#endif + } + else if(my_session->eof[PARENT] == min_eof && + my_session->eof[CHILD] == min_eof) + { + route = true; +#ifdef SS_DEBUG + skygw_log_write_flush(LOGFILE_DEBUG,"tee.c:[%d] Routing final packet of response set.",my_session->d_id); +#endif + } + } + else if(!my_session->waiting[PARENT] && + !my_session->waiting[CHILD]) + { +#ifdef SS_DEBUG + skygw_log_write_flush(LOGFILE_DEBUG,"tee.c:[%d] Routing single packet response.",my_session->d_id); +#endif + route = true; + } + } + + if(route) { #ifdef SS_DEBUG - skygw_log_write_flush(LOGFILE_DEBUG, "tee.c: Routing buffer '%p' parent(waiting [%s] replies [%d] eof[%d])" - " child(waiting [%s] replies[%d] eof [%d])", - my_session->tee_replybuf, - my_session->waiting[PARENT] ? "true":"false", - my_session->replies[PARENT], - my_session->eof[PARENT], - my_session->waiting[CHILD]?"true":"false", - my_session->replies[CHILD], - my_session->eof[CHILD]); + skygw_log_write_flush(LOGFILE_DEBUG, "tee.c:[%d] Routing buffer '%p' parent(waiting [%s] replies [%d] eof[%d])" + " child(waiting [%s] replies[%d] eof [%d])", + my_session->d_id, + my_session->tee_replybuf, + my_session->waiting[PARENT] ? "true":"false", + my_session->replies[PARENT], + my_session->eof[PARENT], + my_session->waiting[CHILD]?"true":"false", + my_session->replies[CHILD], + my_session->eof[CHILD]); #endif - - rc = my_session->up.clientReply ( - my_session->up.instance, - my_session->up.session, - my_session->tee_replybuf); - my_session->tee_replybuf = NULL; - } - else - { - rc = 1; - } - - spinlock_release(&my_session->tee_lock); - return rc; + + rc = my_session->up.clientReply (my_session->up.instance, + my_session->up.session, + my_session->tee_replybuf); + my_session->tee_replybuf = NULL; + } + + spinlock_release(&my_session->tee_lock); + return rc; } /** * Diagnostics routine From 30d27422944515cfd134cc7ae32d815f30af27d2 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 15 Jan 2015 04:40:33 +0200 Subject: [PATCH 3/6] Fixed garbled data at end of modutil.c --- server/core/modutil.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/core/modutil.c b/server/core/modutil.c index 7bb57ef2c..7f6ef1e27 100644 --- a/server/core/modutil.c +++ b/server/core/modutil.c @@ -532,5 +532,4 @@ modutil_count_signal_packets(GWBUF *reply,int use_ok, int n_found) } return pkt; -} -5D +} \ No newline at end of file From 85c84c9e71eb25db94b48fed6baf457f0c3f9fc3 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 15 Jan 2015 05:11:08 +0200 Subject: [PATCH 4/6] Fixed debug variables being used out of debug blocks. --- server/modules/filter/tee.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/server/modules/filter/tee.c b/server/modules/filter/tee.c index bb89f99c8..87bc8c3c2 100644 --- a/server/modules/filter/tee.c +++ b/server/modules/filter/tee.c @@ -291,7 +291,9 @@ orphan_free(void* data) while(finished) { +#ifdef SS_DEBUG o_freed++; +#endif tmp = finished; finished = finished->next; @@ -934,9 +936,11 @@ clientReply (FILTER* instance, void *session, GWBUF *reply) if(my_session->eof[branch] >= 2 || (my_session->command == 0x04 && my_session->eof[branch] > 0)) { +#ifdef SS_DEBUG skygw_log_write_flush(LOGFILE_DEBUG,"tee.c [%d] %s received last EOF packet", my_session->d_id, branch == PARENT?"parent":"child"); +#endif ss_dassert(my_session->eof[branch] < 3) my_session->waiting[branch] = false; } From 88a26f03abd200e9a1f9dd1a87d9b2a19dc5c195 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 15 Jan 2015 13:13:09 +0200 Subject: [PATCH 5/6] Fix to bug 685: http://bugs.mariadb.com/show_bug.cgi?id=685 Added the missing detection of partial packets in the buffers. --- server/core/modutil.c | 98 ++++++++++++++++++++++++++++--------- server/include/modutil.h | 1 + server/modules/filter/tee.c | 65 +++++++++++++++++------- 3 files changed, 121 insertions(+), 43 deletions(-) diff --git a/server/core/modutil.c b/server/core/modutil.c index 7f6ef1e27..33c302e7f 100644 --- a/server/core/modutil.c +++ b/server/core/modutil.c @@ -494,42 +494,92 @@ return_packetbuf: return packetbuf; } +/** + * Parse the buffer and split complete packets into individual buffers. + * Any partial packets are left in the old buffer. + * @param p_readbuf Buffer to split + * @return Head of the chain of complete packets + */ +GWBUF* modutil_get_complete_packets(GWBUF** p_readbuf) +{ + GWBUF *buff = NULL, *packet = NULL; + + while((packet = modutil_get_next_MySQL_packet(p_readbuf)) != NULL) + { + buff = gwbuf_append(buff,packet); + } + + return buff; +} /** - * Count the number of EOF, OK or ERR packets in the buffer. + * Count the number of EOF, OK or ERR packets in the buffer. Only complete + * packets are inspected and the buffer is assumed to only contain whole packets. + * If partial packets are in the buffer, they are ingnored. The caller must handle the + * detection of partial packets in buffers. * @param reply Buffer to use * @param use_ok Whether the DEPRECATE_EOF flag is set * @param n_found If there were previous packets found * @return Number of EOF packets */ int -modutil_count_signal_packets(GWBUF *reply,int use_ok, int n_found) +modutil_count_signal_packets(GWBUF *reply, int use_ok, int n_found) { - unsigned char* ptr = (unsigned char*) reply->start; - unsigned char* end = (unsigned char*) reply->end; - unsigned char* prev = ptr; - int pktlen,pkt = 0,found = n_found; - - while(ptr < end) + unsigned char* ptr = (unsigned char*) reply->start; + unsigned char* end = (unsigned char*) reply->end; + unsigned char* prev = ptr; + int pktlen, eof = 0, err = 0, found = n_found; + int errlen = 0, eoflen = 0; + int iserr = 0, iseof = 0; + while(ptr < end) { - - pktlen = MYSQL_GET_PACKET_LEN(ptr) + 4; - - if( !found &&(PTR_IS_ERR(ptr) || PTR_IS_EOF(ptr))) - { - pkt++; - found++; + + pktlen = MYSQL_GET_PACKET_LEN(ptr) + 4; + + if((iserr = PTR_IS_ERR(ptr)) || (iseof = PTR_IS_EOF(ptr))) + { + if(iserr) + { + err++; + errlen = pktlen; + } + else if(iseof) + { + eof++; + eoflen = pktlen; + } } - - ptr += pktlen; + + if((ptr + pktlen) > end) + { + ptr = prev; + break; + } + + prev = ptr; + ptr += pktlen; } - if(found) + + /* + * If there were new EOF/ERR packets found, make sure that they are the last + * packet in the buffer. + */ + if((eof || err) && n_found) { - ptr -= pktlen; - if(PTR_IS_ERR(ptr) || PTR_IS_EOF(ptr)) - pkt++; + if(err) + { + ptr -= errlen; + if(!PTR_IS_ERR(ptr)) + err = 0; + } + else + { + ptr -= eoflen; + if(!PTR_IS_EOF(ptr)) + eof = 0; + } } - - return pkt; -} \ No newline at end of file + + return(eof + err); +} diff --git a/server/include/modutil.h b/server/include/modutil.h index 2ccab523c..e287b080e 100644 --- a/server/include/modutil.h +++ b/server/include/modutil.h @@ -48,6 +48,7 @@ extern GWBUF *modutil_replace_SQL(GWBUF *, char *); extern char *modutil_get_query(GWBUF* buf); extern int modutil_send_mysql_err_packet(DCB *, int, int, int, const char *, const char *); GWBUF* modutil_get_next_MySQL_packet(GWBUF** p_readbuf); +GWBUF* modutil_get_complete_packets(GWBUF** p_readbuf); int modutil_MySQL_query_len(GWBUF* buf, int* nbytes_missing); GWBUF *modutil_create_mysql_err_msg( diff --git a/server/modules/filter/tee.c b/server/modules/filter/tee.c index 87bc8c3c2..17331e21a 100644 --- a/server/modules/filter/tee.c +++ b/server/modules/filter/tee.c @@ -161,7 +161,7 @@ typedef struct { FILTER_DEF* dummy_filterdef; int active; /* filter is active? */ bool use_ok; - bool multipacket; + bool multipacket[2]; unsigned char command; bool waiting[2]; /* if the client is waiting for a reply */ int eof[2]; @@ -172,6 +172,7 @@ typedef struct { int n_rejected; /* Number of rejected queries */ int residual; /* Any outstanding SQL text */ GWBUF* tee_replybuf; /* Buffer for reply */ + GWBUF* tee_partials[2]; SPINLOCK tee_lock; #ifdef SS_DEBUG long d_id; @@ -826,10 +827,10 @@ unsigned char command = *((unsigned char*)queue->start + 4); case 0x17: case 0x04: case 0x0a: - my_session->multipacket = true; + memset(my_session->multipacket,(char)true,2*sizeof(bool)); break; default: - my_session->multipacket = false; + memset(my_session->multipacket,(char)false,2*sizeof(bool)); break; } @@ -840,9 +841,16 @@ unsigned char command = *((unsigned char*)queue->start + 4); #ifdef SS_DEBUG spinlock_acquire(&debug_lock); my_session->d_id = ++debug_id; - skygw_log_write_flush(LOGFILE_DEBUG,"tee.c [%d] query command [%x]", + skygw_log_write_flush(LOGFILE_DEBUG,"tee.c [%d] command [%x]", my_session->d_id, my_session->command); + if(command == 0x03) + { + char* tmpstr = modutil_get_SQL(queue); + skygw_log_write_flush(LOGFILE_DEBUG,"tee.c query: '%s'", + tmpstr); + free(tmpstr); + } spinlock_release(&debug_lock); #endif rval = my_session->down.routeQuery(my_session->down.instance, @@ -896,24 +904,41 @@ clientReply (FILTER* instance, void *session, GWBUF *reply) { int rc, branch, eof; TEE_SESSION *my_session = (TEE_SESSION *) session; - bool route = false; - + bool route = false,mpkt; + GWBUF *complete = NULL; + unsigned char *ptr; + int min_eof = my_session->command != 0x04 ? 2 : 1; + spinlock_acquire(&my_session->tee_lock); ss_dassert(my_session->active); branch = instance == NULL ? CHILD : PARENT; - unsigned char *ptr = (unsigned char*)reply->start; - if(my_session->replies[branch] == 0) + my_session->tee_partials[branch] = gwbuf_append(my_session->tee_partials[branch], reply); + my_session->tee_partials[branch] = gwbuf_make_contiguous(my_session->tee_partials[branch]); + complete = modutil_get_complete_packets(&my_session->tee_partials[branch]); + complete = gwbuf_make_contiguous(complete); + + if(my_session->tee_partials[branch] && + GWBUF_EMPTY(my_session->tee_partials[branch])) + { + gwbuf_free(my_session->tee_partials[branch]); + my_session->tee_partials[branch] = NULL; + } + + ptr = (unsigned char*) complete->start; + + if(my_session->replies[branch] == 0) { /* Reply is in a single packet if it is an OK, ERR or LOCAL_INFILE packet. * Otherwise the reply is a result set and the amount of packets is unknown. */ if(PTR_IS_ERR(ptr) || PTR_IS_LOCAL_INFILE(ptr) || - PTR_IS_OK(ptr) || !my_session->multipacket ) + PTR_IS_OK(ptr) || !my_session->multipacket[branch] ) { my_session->waiting[branch] = false; + my_session->multipacket[branch] = false; } #ifdef SS_DEBUG else @@ -931,35 +956,37 @@ clientReply (FILTER* instance, void *session, GWBUF *reply) if(my_session->waiting[branch]) { - eof = modutil_count_signal_packets(reply,my_session->use_ok,my_session->eof[branch] > 0); + + eof = modutil_count_signal_packets(complete,my_session->use_ok,my_session->eof[branch] > 0); my_session->eof[branch] += eof; - if(my_session->eof[branch] >= 2 || - (my_session->command == 0x04 && my_session->eof[branch] > 0)) + if(my_session->eof[branch] >= min_eof) { #ifdef SS_DEBUG skygw_log_write_flush(LOGFILE_DEBUG,"tee.c [%d] %s received last EOF packet", my_session->d_id, branch == PARENT?"parent":"child"); #endif - ss_dassert(my_session->eof[branch] < 3) + ss_dassert(my_session->eof[branch] < 3) my_session->waiting[branch] = false; } } if(branch == PARENT) { - ss_dassert(my_session->tee_replybuf == NULL) - my_session->tee_replybuf = reply; + ss_dassert(my_session->tee_replybuf == NULL); + my_session->tee_replybuf = complete; } else { - gwbuf_free(reply); + if(complete) + gwbuf_free(complete); } my_session->replies[branch]++; rc = 1; - - int min_eof = my_session->command != 0x04 ? 2 : 1; + mpkt = my_session->multipacket[PARENT] || my_session->multipacket[CHILD]; + + if(my_session->tee_replybuf != NULL) { @@ -972,7 +999,7 @@ clientReply (FILTER* instance, void *session, GWBUF *reply) skygw_log_write_flush(LOGFILE_ERROR,"Error : Tee child session was closed."); } - if(my_session->multipacket) + if(mpkt) { if(my_session->waiting[PARENT]) From 533042b61ad0e09f8d0855e252ce3c221f1e9d00 Mon Sep 17 00:00:00 2001 From: Markus Makela Date: Thu, 15 Jan 2015 14:06:41 +0200 Subject: [PATCH 6/6] Fixes to coverity defects from 85514 up to 85529. --- query_classifier/query_classifier.cc | 4 +- server/modules/filter/fwfilter.c | 83 ++++++++++++++++----- server/modules/filter/test/harness_common.c | 2 +- 3 files changed, 70 insertions(+), 19 deletions(-) diff --git a/query_classifier/query_classifier.cc b/query_classifier/query_classifier.cc index 9eb33ab82..f98a08d9b 100644 --- a/query_classifier/query_classifier.cc +++ b/query_classifier/query_classifier.cc @@ -1118,7 +1118,8 @@ char** skygw_get_table_names(GWBUF* querybuf, int* tblsize, bool fullnames) lex->current_select = lex->current_select->next_select_in_list(); } /*< while(lex->current_select) */ retblock: - *tblsize = i; + if(tblsize) + *tblsize = i; return tables; } @@ -1229,6 +1230,7 @@ inline void add_str(char** buf, int* buflen, int* bufsize, char* str) } if(*buflen > 0){ + if(*buf) strcat(*buf," "); } strcat(*buf,str); diff --git a/server/modules/filter/fwfilter.c b/server/modules/filter/fwfilter.c index 2a15d126f..2e2c5bd44 100644 --- a/server/modules/filter/fwfilter.c +++ b/server/modules/filter/fwfilter.c @@ -686,11 +686,13 @@ void link_rules(char* rule, FW_INSTANCE* instance) user = (USER*)calloc(1,sizeof(USER)); if(user == NULL){ + free(rulelist); return; } if((user->lock = (SPINLOCK*)malloc(sizeof(SPINLOCK))) == NULL){ free(user); + free(rulelist); return; } @@ -750,7 +752,21 @@ void parse_rule(char* rule, FW_INSTANCE* instance) RULELIST* rlist = NULL; ruledef = (RULE*)calloc(1,sizeof(RULE)); + + if(ruledef == NULL) + { + skygw_log_write(LOGFILE_ERROR,"Error : Memory allocation failed."); + goto retblock; + } + rlist = (RULELIST*)calloc(1,sizeof(RULELIST)); + + if(rlist == NULL) + { + free(ruledef); + skygw_log_write(LOGFILE_ERROR,"Error : Memory allocation failed."); + goto retblock; + } ruledef->name = strdup(tok); ruledef->type = RT_UNDEFINED; ruledef->on_queries = QUERY_OP_UNDEFINED; @@ -843,12 +859,17 @@ void parse_rule(char* rule, FW_INSTANCE* instance) } str = calloc(((tok - start) + 1),sizeof(char)); + if(str == NULL) + { + skygw_log_write_flush(LOGFILE_ERROR, "Fatal Error: malloc returned NULL."); + goto retblock; + } re = (regex_t*)malloc(sizeof(regex_t)); - if(re == NULL || str == NULL){ + if(re == NULL){ skygw_log_write_flush(LOGFILE_ERROR, "Fatal Error: malloc returned NULL."); - - return; + free(str); + goto retblock; } memcpy(str, start, (tok-start)); @@ -857,9 +878,11 @@ void parse_rule(char* rule, FW_INSTANCE* instance) skygw_log_write(LOGFILE_ERROR, "fwfilter: Invalid regular expression '%s'.", str); free(re); } - - ruledef->type = RT_REGEX; - ruledef->data = (void*) re; + else + { + ruledef->type = RT_REGEX; + ruledef->data = (void*) re; + } free(str); } @@ -929,6 +952,7 @@ createInstance(char **options, FILTER_PARAMETER **params) if ((my_instance = calloc(1, sizeof(FW_INSTANCE))) == NULL || (my_instance->lock = (SPINLOCK*)malloc(sizeof(SPINLOCK))) == NULL){ + skygw_log_write(LOGFILE_ERROR, "Memory allocation for firewall filter failed."); return NULL; } @@ -947,11 +971,23 @@ createInstance(char **options, FILTER_PARAMETER **params) for(i = 0;params[i];i++){ if(strcmp(params[i]->name, "rules") == 0){ + + if(filename) + free(filename); + filename = strdup(params[i]->value); } } - + + if(filename == NULL) + { + skygw_log_write(LOGFILE_ERROR, "Unable to find rule file for firewall filter."); + free(my_instance); + return NULL; + } + if((file = fopen(filename,"rb")) == NULL ){ + skygw_log_write(LOGFILE_ERROR, "Error while opening rule file for firewall filter."); free(my_instance); free(filename); return NULL; @@ -964,6 +1000,8 @@ createInstance(char **options, FILTER_PARAMETER **params) if(fgets(buffer,2048,file) == NULL){ if(ferror(file)){ + skygw_log_write(LOGFILE_ERROR, "Error while reading rule file for firewall filter."); + fclose(file); free(my_instance); return NULL; } @@ -971,13 +1009,13 @@ createInstance(char **options, FILTER_PARAMETER **params) if(feof(file)){ break; } - } - + } + if((nl = strchr(buffer,'\n')) != NULL && ((char*)nl - (char*)buffer) < 2048){ *nl = '\0'; } + parse_rule(buffer,my_instance); - } fclose(file); @@ -1074,16 +1112,26 @@ setDownstream(FILTER *instance, void *session, DOWNSTREAM *downstream) GWBUF* gen_dummy_error(FW_SESSION* session, char* msg) { GWBUF* buf; - char* errmsg; - DCB* dcb = session->session->client; - MYSQL_session* mysql_session = (MYSQL_session*)session->session->data; + char* errmsg; + DCB* dcb; + MYSQL_session* mysql_session; unsigned int errlen; + + if(session == NULL || session->session == NULL || + session->session->data == NULL || + session->session->client == NULL) + { + skygw_log_write_flush(LOGFILE_ERROR, "Error : Firewall filter session missing data."); + return NULL; + } + dcb = session->session->client; + mysql_session = (MYSQL_session*)session->session->data; errlen = msg != NULL ? strlen(msg) : 0; errmsg = (char*)malloc((512 + errlen)*sizeof(char)); if(errmsg == NULL){ - skygw_log_write_flush(LOGFILE_ERROR, "Fatal Error: malloc returned NULL."); + skygw_log_write_flush(LOGFILE_ERROR, "Fatal Error: Memory allocation failed."); return NULL; } @@ -1110,7 +1158,8 @@ GWBUF* gen_dummy_error(FW_SESSION* session, char* msg) } buf = modutil_create_mysql_err_msg(1,0,1141,"HY000", (const char*)errmsg); - + free(errmsg); + return buf; } @@ -1192,7 +1241,7 @@ bool rule_matches(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *queue char emsg[512]; int qlen; bool is_sql, is_real, matches; - skygw_query_op_t optype; + skygw_query_op_t optype = QUERY_OP_UNDEFINED; STRLINK* strln = NULL; QUERYSPEED* queryspeed = NULL; QUERYSPEED* rule_qs = NULL; @@ -1469,7 +1518,7 @@ bool check_match_any(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *qu */ bool check_match_all(FW_INSTANCE* my_instance, FW_SESSION* my_session, GWBUF *queue, USER* user) { - bool is_sql, rval; + bool is_sql, rval = 0; int qlen; char *fullquery = NULL,*ptr; diff --git a/server/modules/filter/test/harness_common.c b/server/modules/filter/test/harness_common.c index 026f46234..254add4b1 100644 --- a/server/modules/filter/test/harness_common.c +++ b/server/modules/filter/test/harness_common.c @@ -995,7 +995,7 @@ GWBUF* gen_packet(PACKET pkt) int process_opts(int argc, char** argv) { int fd, buffsize = 1024; - int rd,rdsz, rval = 0,error; + int rd,rdsz, rval = 0, error = 0; size_t fsize; char *buff = calloc(buffsize,sizeof(char)), *tok = NULL;