From 6813f760a5cda874dd78c940771a1e0f4e769501 Mon Sep 17 00:00:00 2001
From: vraatikka <vilho.raatikka@skysql.com>
Date: Mon, 9 Sep 2013 15:00:37 +0300
Subject: [PATCH] Fix for bug #205 - http://bugs.skysql.com/show_bug.cgi?id=205
 . In gw_read_backend_event, read client_protocol by using
 dcb->session->client pointer but only after it is sure that there is
 something to write to client. This doesn't ensure that client pointer in
 session is valid, but it should be.

Return value of dcb_read is checked and buffer pointer is not used if nothing was read.
---
 server/modules/protocol/mysql_backend.c | 43 +++++++++++++++++--------
 1 file changed, 30 insertions(+), 13 deletions(-)

diff --git a/server/modules/protocol/mysql_backend.c b/server/modules/protocol/mysql_backend.c
index 01ac1f772..e739b7b2f 100644
--- a/server/modules/protocol/mysql_backend.c
+++ b/server/modules/protocol/mysql_backend.c
@@ -153,7 +153,6 @@ static int gw_read_backend_event(DCB *dcb) {
         
         ss_info_dassert(dcb->session->client != NULL,
                         "Session's client dcb pointer is NULL");
-
         client_protocol = SESSION_PROTOCOL(dcb->session, MySQLProtocol);
         backend_protocol = (MySQLProtocol *) dcb->protocol;
 
@@ -196,7 +195,6 @@ static int gw_read_backend_event(DCB *dcb) {
 	
                 router = session->service->router;
                 router_instance = session->service->router_instance;
-                rsession = session->router_session;
 		
 		/* read backed auth reply */
 		rv = gw_receive_backend_auth(backend_protocol);
@@ -215,6 +213,7 @@ static int gw_read_backend_event(DCB *dcb) {
 
 				backend_protocol->state = MYSQL_AUTH_FAILED;
 #if 0
+                                /** vraa : this traps easily. Why? */
                                 ss_dassert(backend_protocol->state !=
                                            MYSQL_AUTH_FAILED);
 #endif
@@ -286,9 +285,6 @@ static int gw_read_backend_event(DCB *dcb) {
 	} /**< if (backend_protocol->state == MYSQL_AUTH_RECV) */
 
 	/* reading MySQL command output from backend and writing to the client */
-
-	if ((client_protocol->state == MYSQL_WAITING_RESULT) ||
-            (client_protocol->state == MYSQL_IDLE))
         {
 		GWBUF		*head = NULL;
 		ROUTER_OBJECT	*router = NULL;
@@ -299,8 +295,8 @@ static int gw_read_backend_event(DCB *dcb) {
                 CHK_SESSION(session);
 		/* read available backend data */
 		rc = dcb_read(dcb, &head);
-
-		if (rc == -1) {
+                
+                if (rc <= 0) {
                         rc = 1;
                         goto return_rc;
                 }
@@ -308,6 +304,7 @@ static int gw_read_backend_event(DCB *dcb) {
                 router = session->service->router;
                 router_instance = session->service->router_instance;
                 rsession = session->router_session;
+
 		/* Note the gwbuf doesn't have here a valid queue->command
                  * descriptions as it is a fresh new one!
                  * We only have the copied value in dcb->command from
@@ -315,8 +312,23 @@ static int gw_read_backend_event(DCB *dcb) {
                  * router->clientReply
                  * and pass now the  gwbuf to the router
                  */
-		router->clientReply(router_instance, rsession, head, dcb);
-                rc = 1;
+
+                /**
+                 * If dcb->session->client is freed already it may be NULL, and
+                 * protocol can't be read. However, then it wouldn't be possible
+                 * that there was anything to write to client in that case.
+                 * Should this be protected somehow, anyway?
+                 */
+                client_protocol = SESSION_PROTOCOL(dcb->session, MySQLProtocol);
+                CHK_PROTOCOL(client_protocol);
+
+                if (client_protocol != NULL &&
+                    (client_protocol->state == MYSQL_WAITING_RESULT ||
+                     client_protocol->state == MYSQL_IDLE))
+                {
+                        router->clientReply(router_instance, rsession, head, dcb);
+                        rc = 1;
+                }
 		goto return_rc;
 	}
         rc = 0;
@@ -418,7 +430,8 @@ static int gw_create_backend_connection(
         int           fd = -1;
 
         protocol = mysql_protocol_init(backend_dcb);
-
+        ss_dassert(protocol != NULL);
+        
         if (protocol == NULL) {
                 skygw_log_write_flush(
                         LOGFILE_ERROR,
@@ -593,16 +606,20 @@ static int gw_change_user(DCB *backend, SERVER *server, SESSION *in_session, GWB
 
 	// get the auth token len
 	memcpy(&auth_token_len, client_auth_packet, 1);
+        ss_dassert(auth_token_len >= 0);
+        
 	client_auth_packet++;
 
         // allocate memory for token only if auth_token_len > 0
-        if (auth_token_len) {
-                if ((auth_token = (uint8_t *)malloc(auth_token_len)) == NULL)
+        if (auth_token_len > 0) {
+                auth_token = (uint8_t *)malloc(auth_token_len);
+                ss_dassert(auth_token != NULL);
+                
+                if (auth_token == NULL) 
 			return rv;
                 memcpy(auth_token, client_auth_packet, auth_token_len);
 		client_auth_packet += auth_token_len;
         }
-
         // decode the token and check the password
         // Note: if auth_token_len == 0 && auth_token == NULL, user is without password
         auth_ret = gw_check_mysql_scramble_data(backend->session->client, auth_token, auth_token_len, client_protocol->scramble, sizeof(client_protocol->scramble), username, client_sha1);