From c2e85df66f674713f2e287421a77d34ef395e942 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Sat, 24 May 2014 00:04:15 +0300 Subject: [PATCH 1/9] Added logging to place where authentication with backend fails and the reason is unknown. --- server/modules/protocol/mysql_common.c | 47 +++++++++++++++++++++----- 1 file changed, 38 insertions(+), 9 deletions(-) diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 7a017d041..6f2d1cd06 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -315,12 +315,17 @@ int gw_receive_backend_auth( /*< * 5th byte is 0x0 if successful. */ - if (ptr[4] == '\x00') { + if (ptr[4] == 0x00) + { rc = 1; - } else { - uint8_t* tmpbuf = - (uint8_t *)calloc(1, GWBUF_LENGTH(head)+1); - memcpy(tmpbuf, ptr, GWBUF_LENGTH(head)); + } + else if (ptr[4] == 0xff) + { + size_t packetlen = MYSQL_GET_PACKET_LEN(ptr)+4; + char* bufstr = (char *)calloc(1, packetlen-3); + + snprintf(bufstr, packetlen-6, "%s", &ptr[7]); + LOGIF(LD, (skygw_log_write( LOGFILE_DEBUG, "%lu [gw_receive_backend_auth] Invalid " @@ -329,11 +334,35 @@ int gw_receive_backend_auth( pthread_self(), dcb, dcb->fd, - tmpbuf[4], - tmpbuf))); + ptr[4], + bufstr))); - free(tmpbuf); - rc = -1; + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Invalid authentication message " + "from backend. Msg : %s", + bufstr))); + + free(bufstr); + rc = -1; + } + else + { + LOGIF(LD, (skygw_log_write( + LOGFILE_DEBUG, + "%lu [gw_receive_backend_auth] Invalid " + "authentication message from backend dcb %p " + "fd %d, ptr[4] = %p", + pthread_self(), + dcb, + dcb->fd, + ptr[4]))); + + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Invalid authentication message " + "from backend. Packet type : %p", + ptr[4]))); } /*< * Remove data from buffer. From 2c306409b6f9f4c0c2f689d1ce12503d93a07f6f Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Mon, 26 May 2014 17:30:36 +0200 Subject: [PATCH 2/9] newline removed newline removed --- server/modules/protocol/mysql_common.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 6f2d1cd06..46009d457 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -1323,9 +1323,8 @@ GWBUF* gw_MySQL_get_next_packet( size_t packetlen; size_t totalbuflen; uint8_t* data; - readbuf = *p_readbuf; - + if (readbuf == NULL) { packetbuf = NULL; From 7319859a48dc12a88ecb49f430220c3f546f79f8 Mon Sep 17 00:00:00 2001 From: MassimilianoPinto Date: Mon, 26 May 2014 17:34:55 +0200 Subject: [PATCH 3/9] Log message update Log message update --- server/modules/protocol/mysql_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/protocol/mysql_common.c b/server/modules/protocol/mysql_common.c index 46009d457..1d0932d7b 100644 --- a/server/modules/protocol/mysql_common.c +++ b/server/modules/protocol/mysql_common.c @@ -634,7 +634,7 @@ int gw_do_connect_to_backend( LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, "Error: Establishing connection to backend server " - "%s:%d failed. Socket creation failed due " + "%s:%d failed.\n\t\t Socket creation failed due " "%d, %s.", host, port, From f9db3bc129239294e730c85e74372b84a0fad7cc Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Tue, 27 May 2014 10:08:17 +0300 Subject: [PATCH 4/9] Fix to bug #438 http://bugs.skysql.com/show_bug.cgi?id=438 Fixed some compiler warnings, added header includes, return values etc. --- server/core/Makefile | 4 ++-- server/core/dbusers.c | 1 + server/core/dcb.c | 1 + server/core/poll.c | 1 + server/core/secrets.c | 1 + server/core/service.c | 2 +- server/modules/monitor/mysql_mon.c | 4 ++-- server/modules/protocol/mysql_backend.c | 7 ++++++- 8 files changed, 15 insertions(+), 6 deletions(-) diff --git a/server/core/Makefile b/server/core/Makefile index 3f862a475..bd29fbae8 100644 --- a/server/core/Makefile +++ b/server/core/Makefile @@ -41,7 +41,7 @@ UTILSPATH := $(ROOT_PATH)/utils CC=cc -CFLAGS=-c -I/usr/include -I../include -I../inih \ +CFLAGS=-c -I/usr/include -I../include -I../modules/include -I../inih \ $(MYSQL_HEADERS) \ -I$(LOGPATH) -I$(UTILSPATH) \ -Wall -g @@ -59,7 +59,7 @@ SRCS= atomic.c buffer.c spinlock.c gateway.c \ monitor.c adminusers.c secrets.c HDRS= ../include/atomic.h ../include/buffer.h ../include/dcb.h \ - ../include/gw.h ../include/mysql_protocol.h \ + ../include/gw.h ../modules/include/mysql_client_server_protocol.h \ ../include/session.h ../include/spinlock.h ../include/thread.h \ ../include/modules.h ../include/poll.h ../include/config.h \ ../include/users.h ../include/hashtable.h ../include/gwbitmask.h \ diff --git a/server/core/dbusers.c b/server/core/dbusers.c index e5289a3a0..6bd314c79 100644 --- a/server/core/dbusers.c +++ b/server/core/dbusers.c @@ -42,6 +42,7 @@ #include #include #include +#include #define USERS_QUERY_NO_ROOT " AND user NOT IN ('root')" #define LOAD_MYSQL_USERS_QUERY "SELECT user, host, password, concat(user,host,password) AS userdata FROM mysql.user WHERE user IS NOT NULL AND user <> ''" diff --git a/server/core/dcb.c b/server/core/dcb.c index bfc815dfe..8a3186aec 100644 --- a/server/core/dcb.c +++ b/server/core/dcb.c @@ -67,6 +67,7 @@ #include #include #include +#include extern int lm_enabled_logfiles_bitmask; diff --git a/server/core/poll.c b/server/core/poll.c index f1c65ebea..0b23a6b05 100644 --- a/server/core/poll.c +++ b/server/core/poll.c @@ -27,6 +27,7 @@ #include #include #include +#include extern int lm_enabled_logfiles_bitmask; diff --git a/server/core/secrets.c b/server/core/secrets.c index a11b0276d..c4d1822f5 100644 --- a/server/core/secrets.c +++ b/server/core/secrets.c @@ -21,6 +21,7 @@ #include #include #include +#include extern int lm_enabled_logfiles_bitmask; /** diff --git a/server/core/service.c b/server/core/service.c index e44b9c697..2e7632bb8 100644 --- a/server/core/service.c +++ b/server/core/service.c @@ -667,7 +667,7 @@ SERVICE *ptr; * @param dcb DCB to print data to * @param service The service to print */ -dprintService(DCB *dcb, SERVICE *service) +void dprintService(DCB *dcb, SERVICE *service) { SERVER *server = service->databases; diff --git a/server/modules/monitor/mysql_mon.c b/server/modules/monitor/mysql_mon.c index fbb537a9c..a13cd7d73 100644 --- a/server/modules/monitor/mysql_mon.c +++ b/server/modules/monitor/mysql_mon.c @@ -126,7 +126,7 @@ MYSQL_MONITOR *handle; handle->defaultPasswd = NULL; spinlock_init(&handle->lock); } - handle->tid = thread_start(monitorMain, handle); + handle->tid = (THREAD)thread_start(monitorMain, handle); return handle; } @@ -141,7 +141,7 @@ stopMonitor(void *arg) MYSQL_MONITOR *handle = (MYSQL_MONITOR *)arg; handle->shutdown = 1; - thread_wait(handle->tid); + thread_wait((void *)handle->tid); } /** diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index acc2dacdd..902305c6b 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -283,6 +283,11 @@ static int gw_read_backend_event(DCB *dcb) { } if (backend_protocol->state == MYSQL_AUTH_FAILED) { + /** + * protocol state won't change anymore, + * lock can be freed + */ + spinlock_release(&dcb->authlock); spinlock_acquire(&dcb->delayqlock); /*< * vraa : errorHandle @@ -340,7 +345,7 @@ static int gw_read_backend_event(DCB *dcb) { /* close router_session */ router->closeSession(router_instance, rsession); rc = 1; - goto return_with_lock; + goto return_rc; } else { From a42e7b5702c3e51c1ba8cc739860df3fe529966d Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Wed, 28 May 2014 14:03:59 +0300 Subject: [PATCH 5/9] Bug #438, http://bugs.skysql.com/show_bug.cgi?id=438 try to complete the fix. dcb->authlock was double-freed. --- server/modules/protocol/mysql_backend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index e4b461dad..fff9a2604 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -326,7 +326,7 @@ static int gw_read_backend_event(DCB *dcb) { if (session->client->session == NULL) { rc = 1; - goto return_with_lock; + goto return_rc; } usleep(1); } From 6890129bf2aaccbf454dfcf5e8061cdf08993599 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Wed, 28 May 2014 14:03:59 +0300 Subject: [PATCH 6/9] Bug #438, http://bugs.skysql.com/show_bug.cgi?id=438 try to complete the fix. dcb->authlock was double-freed. --- server/modules/protocol/mysql_backend.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index e4b461dad..fff9a2604 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -326,7 +326,7 @@ static int gw_read_backend_event(DCB *dcb) { if (session->client->session == NULL) { rc = 1; - goto return_with_lock; + goto return_rc; } usleep(1); } From 89c51cff208fe3888e57a485a57b3dc3c1d52987 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Wed, 28 May 2014 14:36:53 +0300 Subject: [PATCH 7/9] Completing fix to #438 --- server/modules/protocol/mysql_backend.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index fff9a2604..9fe9f72ae 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -282,7 +282,8 @@ static int gw_read_backend_event(DCB *dcb) { } /* switch */ } - if (backend_protocol->state == MYSQL_AUTH_FAILED) { + if (backend_protocol->state == MYSQL_AUTH_FAILED) + { /** * protocol state won't change anymore, * lock can be freed @@ -333,7 +334,7 @@ static int gw_read_backend_event(DCB *dcb) { if (session->state == SESSION_STATE_STOPPING) { - goto return_with_lock; + goto return_rc; } spinlock_acquire(&session->ses_lock); session->state = SESSION_STATE_STOPPING; From a73c9c8076e1a5bb9f32047d12480105782aef95 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Wed, 28 May 2014 14:36:53 +0300 Subject: [PATCH 8/9] Completing fix to #438 --- server/modules/protocol/mysql_backend.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index fff9a2604..9fe9f72ae 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -282,7 +282,8 @@ static int gw_read_backend_event(DCB *dcb) { } /* switch */ } - if (backend_protocol->state == MYSQL_AUTH_FAILED) { + if (backend_protocol->state == MYSQL_AUTH_FAILED) + { /** * protocol state won't change anymore, * lock can be freed @@ -333,7 +334,7 @@ static int gw_read_backend_event(DCB *dcb) { if (session->state == SESSION_STATE_STOPPING) { - goto return_with_lock; + goto return_rc; } spinlock_acquire(&session->ses_lock); session->state = SESSION_STATE_STOPPING; From 1a2b8e5475ec254279ce24071f8c3cbd92b65bc8 Mon Sep 17 00:00:00 2001 From: VilhoRaatikka Date: Wed, 28 May 2014 18:29:02 +0300 Subject: [PATCH 9/9] Router spinlock wasn't used to protect routing. As a consequence router could have been closed in thread #1 while thread #2 was in a middle of execution of router code. Solved by holding router lock so that it covered whole routing operation. --- server/modules/protocol/mysql_backend.c | 3 +- .../routing/readwritesplit/readwritesplit.c | 41 +++++++++++-------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c index 9fe9f72ae..efa92a71d 100644 --- a/server/modules/protocol/mysql_backend.c +++ b/server/modules/protocol/mysql_backend.c @@ -585,7 +585,8 @@ gw_MySQLWrite_backend(DCB *dcb, GWBUF *queue) snprintf(str, len+1, "%s", startpoint); LOGIF(LE, (skygw_log_write_flush( LOGFILE_ERROR, - "Error : Authentication to backend failed."))); + "Error : Unable to write to backend due to " + "authentication failure."))); /** Consume query buffer */ while ((queue = gwbuf_consume( queue, diff --git a/server/modules/routing/readwritesplit/readwritesplit.c b/server/modules/routing/readwritesplit/readwritesplit.c index a203a9865..5b855afe4 100644 --- a/server/modules/routing/readwritesplit/readwritesplit.c +++ b/server/modules/routing/readwritesplit/readwritesplit.c @@ -1015,16 +1015,16 @@ static int routeQuery( router_cli_ses->rses_id))); ss_dassert(QUERY_IS_TYPE(qtype, QUERY_TYPE_READ)); + /** Lock router session */ + if (!rses_begin_locked_router_action(router_cli_ses)) + { + goto return_ret; + } + succp = get_dcb(&slave_dcb, router_cli_ses, BE_SLAVE); if (succp) - { - /** Lock router session */ - if (!rses_begin_locked_router_action(router_cli_ses)) - { - goto return_ret; - } - + { if ((ret = slave_dcb->func.write(slave_dcb, querybuf)) == 1) { atomic_add(&inst->stats.n_slave, 1); @@ -1036,8 +1036,9 @@ static int routeQuery( "Error : Routing query \"%s\" failed.", querystr))); } - rses_end_locked_router_action(router_cli_ses); } + rses_end_locked_router_action(router_cli_ses); + ss_dassert(succp); goto return_ret; } @@ -1061,6 +1062,11 @@ static int routeQuery( "routing to Master."))); } } + /** Lock router session */ + if (!rses_begin_locked_router_action(router_cli_ses)) + { + goto return_ret; + } if (master_dcb == NULL) { @@ -1068,21 +1074,22 @@ static int routeQuery( } if (succp) { - /** Lock router session */ - if (!rses_begin_locked_router_action(router_cli_ses)) - { - goto return_ret; - } if ((ret = master_dcb->func.write(master_dcb, querybuf)) == 1) { atomic_add(&inst->stats.n_master, 1); } - rses_end_locked_router_action(router_cli_ses); - } + } + rses_end_locked_router_action(router_cli_ses); + ss_dassert(succp); - ss_dassert(ret == 1); - goto return_ret; + + if (ret == 0) + { + LOGIF(LE, (skygw_log_write_flush( + LOGFILE_ERROR, + "Error : Routing to master failed."))); + } } return_ret: if (plainsqlbuf != NULL)