Fix to bugs #665, and #664. Potentially to #649.

http://bugs.skysql.com/show_bug.cgi?id=665
http://bugs.skysql.com/show_bug.cgi?id=664
http://bugs.skysql.com/show_bug.cgi?id=649

dcb.c:dcb_final_free: (665):set dcb->session->client pointer to NULL so that it won't be read anymore and other threads won't try to close it.
	dcb_final_free:(664):don't free dcb->data, it is either freed in session_alloc if session creation fails or in session_free only.
session.c:if session creation fails, free dcb->data and remove links between client DCB and session.
mysql_backend.c:(665):gw_backend_close:check that session->client isn't NULL and that client DCB's state is still polling before calling dcb_close for it.
mysql_client.c:gw_mysql_do_authentication:if anything fails, and session_alloc won't be called, free dcb->data.
mysql_common.c:gw_send_authentication_to_backend:if session is already closing then return with error.
This commit is contained in:
VilhoRaatikka
2014-12-29 20:19:01 +02:00
parent beacd524da
commit 635fcf708f
8 changed files with 97 additions and 29 deletions

View File

@ -395,15 +395,24 @@ DCB_CALLBACK *cb;
if (local_session->client == dcb) { if (local_session->client == dcb) {
local_session->client = NULL; local_session->client = NULL;
} }
/**
* Set session's client pointer NULL so that other threads
* won't try to call dcb_close for client DCB
* after this call.
*/
if (dcb->session->client == dcb)
{
spinlock_acquire(&dcb->session->ses_lock);
dcb->session->client = NULL;
spinlock_release(&dcb->session->ses_lock);
}
dcb->session = NULL; dcb->session = NULL;
session_free(local_session); session_free(local_session);
} }
} }
if (dcb->protocol && (!DCB_IS_CLONE(dcb))) if (dcb->protocol && (!DCB_IS_CLONE(dcb)))
free(dcb->protocol); free(dcb->protocol);
if (dcb->data && (!DCB_IS_CLONE(dcb)))
free(dcb->data);
if (dcb->remote) if (dcb->remote)
free(dcb->remote); free(dcb->remote);
if (dcb->user) if (dcb->user)
@ -428,7 +437,6 @@ DCB_CALLBACK *cb;
} }
spinlock_release(&dcb->cb_lock); spinlock_release(&dcb->cb_lock);
bitmask_free(&dcb->memdata.bitmask); bitmask_free(&dcb->memdata.bitmask);
free(dcb); free(dcb);
} }
@ -903,7 +911,8 @@ int below_water;
dcb->state != DCB_STATE_POLLING && dcb->state != DCB_STATE_POLLING &&
dcb->state != DCB_STATE_LISTENING && dcb->state != DCB_STATE_LISTENING &&
dcb->state != DCB_STATE_NOPOLLING && dcb->state != DCB_STATE_NOPOLLING &&
dcb->session->state != SESSION_STATE_STOPPING)) (dcb->session == NULL ||
dcb->session->state != SESSION_STATE_STOPPING)))
{ {
LOGIF(LD, (skygw_log_write( LOGIF(LD, (skygw_log_write(
LOGFILE_DEBUG, LOGFILE_DEBUG,

View File

@ -85,6 +85,11 @@ session_alloc(SERVICE *service, DCB *client_dcb)
"session object due error %d, %s.", "session object due error %d, %s.",
errno, errno,
strerror(errno)))); strerror(errno))));
if (client_dcb->data)
{
free(client_dcb->data);
client_dcb->data = NULL;
}
goto return_session; goto return_session;
} }
#if defined(SS_DEBUG) #if defined(SS_DEBUG)
@ -149,6 +154,7 @@ session_alloc(SERVICE *service, DCB *client_dcb)
* Decrease refcount, set dcb's session pointer NULL * Decrease refcount, set dcb's session pointer NULL
* and set session pointer to NULL. * and set session pointer to NULL.
*/ */
session->client = NULL;
session_free(session); session_free(session);
client_dcb->session = NULL; client_dcb->session = NULL;
session = NULL; session = NULL;
@ -189,6 +195,7 @@ session_alloc(SERVICE *service, DCB *client_dcb)
* Decrease refcount, set dcb's session pointer NULL * Decrease refcount, set dcb's session pointer NULL
* and set session pointer to NULL. * and set session pointer to NULL.
*/ */
session->client = NULL;
session_free(session); session_free(session);
client_dcb->session = NULL; client_dcb->session = NULL;
session = NULL; session = NULL;
@ -207,6 +214,7 @@ session_alloc(SERVICE *service, DCB *client_dcb)
if (session->state != SESSION_STATE_READY) if (session->state != SESSION_STATE_READY)
{ {
spinlock_release(&session->ses_lock); spinlock_release(&session->ses_lock);
session->client = NULL;
session_free(session); session_free(session);
client_dcb->session = NULL; client_dcb->session = NULL;
session = NULL; session = NULL;
@ -336,7 +344,11 @@ int session_unlink_dcb(
if (dcb != NULL) if (dcb != NULL)
{ {
dcb->session = NULL; if (session->client == dcb)
{
session->client = NULL;
}
dcb->session = NULL;
} }
spinlock_release(&session->ses_lock); spinlock_release(&session->ses_lock);
@ -429,6 +441,11 @@ bool session_free(
if (!session->ses_is_child) if (!session->ses_is_child)
{ {
session->state = SESSION_STATE_FREE; session->state = SESSION_STATE_FREE;
if (session->data)
{
free(session->data);
}
free(session); free(session);
} }
succp = true; succp = true;

View File

@ -111,9 +111,15 @@ typedef enum {
* *
*/ */
typedef struct mysql_session { typedef struct mysql_session {
#if defined(SS_DEBUG)
skygw_chk_t myses_chk_top;
#endif
uint8_t client_sha1[MYSQL_SCRAMBLE_LEN]; /*< SHA1(passowrd) */ uint8_t client_sha1[MYSQL_SCRAMBLE_LEN]; /*< SHA1(passowrd) */
char user[MYSQL_USER_MAXLEN+1]; /*< username */ char user[MYSQL_USER_MAXLEN+1]; /*< username */
char db[MYSQL_DATABASE_MAXLEN+1]; /*< database */ char db[MYSQL_DATABASE_MAXLEN+1]; /*< database */
#if defined(SS_DEBUG)
skygw_chk_t myses_chk_tail;
#endif
} MYSQL_session; } MYSQL_session;

View File

@ -699,16 +699,6 @@ gw_MySQLWrite_backend(DCB *dcb, GWBUF *queue)
switch (backend_protocol->protocol_auth_state) { switch (backend_protocol->protocol_auth_state) {
case MYSQL_HANDSHAKE_FAILED: case MYSQL_HANDSHAKE_FAILED:
case MYSQL_AUTH_FAILED: case MYSQL_AUTH_FAILED:
{
size_t len;
char* str;
uint8_t* packet = (uint8_t *)queue->start;
uint8_t* startpoint;
len = (size_t)MYSQL_GET_PACKET_LEN(packet);
startpoint = &packet[5];
str = (char *)malloc(len+1);
snprintf(str, len+1, "%s", startpoint);
LOGIF(LE, (skygw_log_write_flush( LOGIF(LE, (skygw_log_write_flush(
LOGFILE_ERROR, LOGFILE_ERROR,
"Error : Unable to write to backend due to " "Error : Unable to write to backend due to "
@ -717,13 +707,11 @@ gw_MySQLWrite_backend(DCB *dcb, GWBUF *queue)
while ((queue = gwbuf_consume( while ((queue = gwbuf_consume(
queue, queue,
GWBUF_LENGTH(queue))) != NULL); GWBUF_LENGTH(queue))) != NULL);
free(str);
rc = 0; rc = 0;
spinlock_release(&dcb->authlock); spinlock_release(&dcb->authlock);
goto return_rc; goto return_rc;
break; break;
}
case MYSQL_IDLE: case MYSQL_IDLE:
{ {
uint8_t* ptr = GWBUF_DATA(queue); uint8_t* ptr = GWBUF_DATA(queue);
@ -1166,21 +1154,33 @@ gw_backend_close(DCB *dcb)
mysql_protocol_done(dcb); mysql_protocol_done(dcb);
spinlock_acquire(&session->ses_lock);
/** /**
* If session->state is STOPPING, start closing client session. * If session->state is STOPPING, start closing client session.
* Otherwise only this backend connection is closed. * Otherwise only this backend connection is closed.
*/ */
if (session != NULL && session->state == SESSION_STATE_STOPPING) if (session != NULL &&
session->state == SESSION_STATE_STOPPING &&
session->client != NULL)
{ {
client_dcb = session->client; client_dcb = session->client;
if (client_dcb != NULL && if (client_dcb->state == DCB_STATE_POLLING)
client_dcb->state == DCB_STATE_POLLING)
{ {
spinlock_release(&session->ses_lock);
/** Close client DCB */ /** Close client DCB */
dcb_close(client_dcb); dcb_close(client_dcb);
} }
else
{
spinlock_release(&session->ses_lock);
}
} }
else
{
spinlock_release(&session->ses_lock);
}
return 1; return 1;
} }

View File

@ -377,15 +377,18 @@ MySQLSendHandshake(DCB* dcb)
* *
* Performs the MySQL protocol 4.1 authentication, using data in GWBUF *queue * Performs the MySQL protocol 4.1 authentication, using data in GWBUF *queue
* *
* The useful data: user, db, client_sha1 are copied into the MYSQL_session * dcb->session->data * (MYSQL_session*)client_data including: user, db, client_sha1 are copied into
* the dcb->data and later to dcb->session->data.
*
* client_capabilitiesa are copied into the dcb->protocol * client_capabilitiesa are copied into the dcb->protocol
* *
* @param dcb Descriptor Control Block of the client * @param dcb Descriptor Control Block of the client
* @param queue The GWBUF with data from client * @param queue The GWBUF with data from client
* @return 0 If succeed, otherwise non-zero value * @return 0 If succeed, otherwise non-zero value
* *
* @note in case of failure, dcb->data is freed before returning. If succeed,
* dcb->data is freed in session.c:session_free.
*/ */
static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) { static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) {
MySQLProtocol *protocol = NULL; MySQLProtocol *protocol = NULL;
/* int compress = -1; */ /* int compress = -1; */
@ -405,6 +408,13 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) {
protocol = DCB_PROTOCOL(dcb, MySQLProtocol); protocol = DCB_PROTOCOL(dcb, MySQLProtocol);
CHK_PROTOCOL(protocol); CHK_PROTOCOL(protocol);
client_data = (MYSQL_session *)calloc(1, sizeof(MYSQL_session)); client_data = (MYSQL_session *)calloc(1, sizeof(MYSQL_session));
#if defined(SS_DEBUG)
client_data->myses_chk_top = CHK_NUM_MYSQLSES;
client_data->myses_chk_tail = CHK_NUM_MYSQLSES;
#endif
/**
* Assign authentication structure with client DCB.
*/
dcb->data = client_data; dcb->data = client_data;
stage1_hash = client_data->client_sha1; stage1_hash = client_data->client_sha1;
@ -425,7 +435,10 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) {
*/ */
/* Detect now if there are enough bytes to continue */ /* Detect now if there are enough bytes to continue */
if (client_auth_packet_size < (4 + 4 + 4 + 1 + 23)) { if (client_auth_packet_size < (4 + 4 + 4 + 1 + 23))
{
free(dcb->data);
dcb->data = NULL;
return 1; return 1;
} }
@ -444,6 +457,8 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) {
if (username == NULL) if (username == NULL)
{ {
free(dcb->data);
dcb->data = NULL;
return 1; return 1;
} }
@ -512,6 +527,11 @@ static int gw_mysql_do_authentication(DCB *dcb, GWBUF *queue) {
if (auth_ret == 0) { if (auth_ret == 0) {
dcb->user = strdup(client_data->user); dcb->user = strdup(client_data->user);
} }
else
{
free(dcb->data);
dcb->data = NULL;
}
/* let's free the auth_token now */ /* let's free the auth_token now */
if (auth_token) { if (auth_token) {
@ -618,7 +638,7 @@ int gw_read_client_event(
* Now there should be at least one complete mysql packet in read_buffer. * Now there should be at least one complete mysql packet in read_buffer.
*/ */
switch (protocol->protocol_auth_state) { switch (protocol->protocol_auth_state) {
case MYSQL_AUTH_SENT: case MYSQL_AUTH_SENT:
{ {
int auth_val; int auth_val;

View File

@ -541,6 +541,16 @@ int gw_send_authentication_to_backend(
uint8_t *curr_passwd = NULL; uint8_t *curr_passwd = NULL;
unsigned int charset; unsigned int charset;
/**
* If session is stopping return with error.
*/
if (conn->owner_dcb->session == NULL ||
(conn->owner_dcb->session->state != SESSION_STATE_READY &&
conn->owner_dcb->session->state != SESSION_STATE_ROUTER_READY))
{
return 1;
}
if (strlen(dbname)) if (strlen(dbname))
curr_db = dbname; curr_db = dbname;

View File

@ -804,7 +804,7 @@ clientReply(
GWBUF *queue, GWBUF *queue,
DCB *backend_dcb) DCB *backend_dcb)
{ {
DCB *client = NULL; DCB *client ;
client = backend_dcb->session->client; client = backend_dcb->session->client;

View File

@ -124,7 +124,8 @@ typedef enum skygw_chk_t {
CHK_NUM_BACKEND, CHK_NUM_BACKEND,
CHK_NUM_BACKEND_REF, CHK_NUM_BACKEND_REF,
CHK_NUM_PREP_STMT, CHK_NUM_PREP_STMT,
CHK_NUM_PINFO CHK_NUM_PINFO,
CHK_NUM_MYSQLSES
} skygw_chk_t; } skygw_chk_t;
# define STRBOOL(b) ((b) ? "true" : "false") # define STRBOOL(b) ((b) ? "true" : "false")
@ -542,6 +543,11 @@ typedef enum skygw_chk_t {
"Parsing info struct has invalid check fields"); \ "Parsing info struct has invalid check fields"); \
} }
#define CHK_MYSQL_SESSION(s) { \
ss_info_dassert((s)->myses_chk_top == CHK_NUM_MYSQLSES && \
(s)->myses_chk_tail == CHK_NUM_MYSQLSES, \
"MYSQL session struct has invalid check fields"); \
}
#if defined(FAKE_CODE) #if defined(FAKE_CODE)