hcq/MaxScale
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MXS-548: Added missing locks to shared session data

Browse Source 
The shared session data was accessed and modified without a lock.
This commit is contained in:
Markus Makela
2016-01-21 10:02:59 +02:00
parent 8ba14ee4ee
commit 99f39cb213
1 changed files with 91 additions and 80 deletions
Download Patch File Download Diff File Expand all files Collapse all files

171
server/modules/protocol/mysql_backend.c
View File

@ -77,7 +77,7 @@ static bool sescmd_response_complete(DCB* dcb);
#if defined(NOT_USED) #if defined(NOT_USED)
static int gw_session(DCB *backend_dcb, void *data); static int gw_session(DCB *backend_dcb, void *data);
#endif #endif
static MYSQL_session* gw_get_shared_session_auth_info(DCB* dcb); static bool gw_get_shared_session_auth_info(DCB* dcb, MYSQL_session* session);
static GWPROTOCOL MyObject = { static GWPROTOCOL MyObject = {
gw_read_backend_event, /* Read - EPOLLIN handler */ gw_read_backend_event, /* Read - EPOLLIN handler */
@ -128,28 +128,35 @@ GetModuleObject()
} }
static MYSQL_session* gw_get_shared_session_auth_info( /**
DCB* dcb) * Copy shared session authentication info
*
* @param dcb A backend DCB
* @param session Destination where authentication data is copied
*/
static bool gw_get_shared_session_auth_info(DCB* dcb, MYSQL_session* session)
{ {
MYSQL_session* auth_info = NULL; bool rval = true;
CHK_DCB(dcb); CHK_DCB(dcb);
CHK_SESSION(dcb->session); CHK_SESSION(dcb->session);
spinlock_acquire(&dcb->session->ses_lock); spinlock_acquire(&dcb->session->ses_lock);
if (dcb->session->state != SESSION_STATE_ALLOC && dcb->session->state != SESSION_STATE_DUMMY) { if (dcb->session->state != SESSION_STATE_ALLOC &&
auth_info = dcb->session->data; dcb->session->state != SESSION_STATE_DUMMY)
} else { {
MXS_ERROR("%lu [gw_get_shared_session_auth_info] Couldn't get " memcpy(session, dcb->session->data, sizeof(MYSQL_session));
"session authentication info. Session in a wrong state %d.", }
pthread_self(), else
dcb->session->state); {
} MXS_ERROR("%lu [gw_get_shared_session_auth_info] Couldn't get "
spinlock_release(&dcb->session->ses_lock); "session authentication info. Session in a wrong state %d.",
pthread_self(), dcb->session->state);
return auth_info; rval = false;
}
spinlock_release(&dcb->session->ses_lock);
return rval;
} }
/** /**
* Backend Read Event for EPOLLIN on the MySQL backend protocol module * Backend Read Event for EPOLLIN on the MySQL backend protocol module
* @param dcb The backend Descriptor Control Block * @param dcb The backend Descriptor Control Block
@ -158,7 +165,7 @@ static MYSQL_session* gw_get_shared_session_auth_info(
static int gw_read_backend_event(DCB *dcb) { static int gw_read_backend_event(DCB *dcb) {
MySQLProtocol *client_protocol = NULL; MySQLProtocol *client_protocol = NULL;
MySQLProtocol *backend_protocol = NULL; MySQLProtocol *backend_protocol = NULL;
MYSQL_session *current_session = NULL; MYSQL_session local_session;
int rc = 0; int rc = 0;
CHK_DCB(dcb); CHK_DCB(dcb);
@ -176,8 +183,10 @@ static int gw_read_backend_event(DCB *dcb) {
CHK_SESSION(dcb->session); CHK_SESSION(dcb->session);
/*< return only with complete session */ /*< return only with complete session */
current_session = gw_get_shared_session_auth_info(dcb); if (!gw_get_shared_session_auth_info(dcb, &local_session))
ss_dassert(current_session != NULL); {
goto return_rc;
}
backend_protocol = (MySQLProtocol *) dcb->protocol; backend_protocol = (MySQLProtocol *) dcb->protocol;
CHK_PROTOCOL(backend_protocol); CHK_PROTOCOL(backend_protocol);
@ -229,9 +238,9 @@ static int gw_read_backend_event(DCB *dcb) {
* to backend. * to backend.
*/ */
if (gw_send_authentication_to_backend( if (gw_send_authentication_to_backend(
current_session->db, local_session.db,
current_session->user, local_session.user,
current_session->client_sha1, local_session.client_sha1,
backend_protocol) != 0) backend_protocol) != 0)
{ {
backend_protocol->protocol_auth_state = MYSQL_AUTH_FAILED; backend_protocol->protocol_auth_state = MYSQL_AUTH_FAILED;
@ -304,7 +313,7 @@ static int gw_read_backend_event(DCB *dcb) {
MXS_ERROR("Backend server didn't " MXS_ERROR("Backend server didn't "
"accept authentication for user " "accept authentication for user "
"%s.", "%s.",
current_session->user); local_session.user);
break; break;
case 1: case 1:
backend_protocol->protocol_auth_state = MYSQL_IDLE; backend_protocol->protocol_auth_state = MYSQL_IDLE;
@ -315,7 +324,7 @@ static int gw_read_backend_event(DCB *dcb) {
pthread_self(), pthread_self(),
dcb, dcb,
dcb->fd, dcb->fd,
current_session->user); local_session.user);
break; break;
default: default:
ss_dassert(receive_rc == 0); ss_dassert(receive_rc == 0);
@ -326,7 +335,7 @@ static int gw_read_backend_event(DCB *dcb) {
pthread_self(), pthread_self(),
dcb, dcb,
dcb->fd, dcb->fd,
current_session->user); local_session.user);
rc = 0; rc = 0;
goto return_with_lock; goto return_with_lock;
break; break;
@ -416,7 +425,7 @@ static int gw_read_backend_event(DCB *dcb) {
"user %s.", "user %s.",
pthread_self(), pthread_self(),
dcb->fd, dcb->fd,
current_session->user); local_session.user);
/* check the delay queue and flush the data */ /* check the delay queue and flush the data */
if (dcb->delayq) if (dcb->delayq)
@ -540,7 +549,7 @@ static int gw_read_backend_event(DCB *dcb) {
"Read buffer unexpectedly null, even though response " "Read buffer unexpectedly null, even though response "
"not marked as complete. User: %s", "not marked as complete. User: %s",
pthread_self(), pthread_self(),
current_session->user); local_session.user);
rc = 0; rc = 0;
goto return_rc; goto return_rc;
} }
@ -1236,23 +1245,21 @@ static int backend_write_delayqueue(DCB *dcb)
dcb->delayq = NULL; dcb->delayq = NULL;
spinlock_release(&dcb->delayqlock); spinlock_release(&dcb->delayqlock);
if (MYSQL_IS_CHANGE_USER(((uint8_t *)GWBUF_DATA(localq)))) if (MYSQL_IS_CHANGE_USER(((uint8_t *)GWBUF_DATA(localq))))
{ {
MYSQL_session* mses; MYSQL_session mses;
GWBUF* new_packet; GWBUF* new_packet;
mses = (MYSQL_session *)dcb->session->client->data; gw_get_shared_session_auth_info(dcb, &mses);
new_packet = gw_create_change_user_packet( new_packet = gw_create_change_user_packet( &mses, dcb->protocol);
mses, /**
(MySQLProtocol *)dcb->protocol); * Remove previous packet which lacks scramble
/** * and append the new.
* Remove previous packet which lacks scramble */
* and append the new. localq = gwbuf_consume(localq, GWBUF_LENGTH(localq));
*/ localq = gwbuf_append(localq, new_packet);
localq = gwbuf_consume(localq, GWBUF_LENGTH(localq)); }
localq = gwbuf_append(localq, new_packet); rc = dcb_write(dcb, localq);
}
rc = dcb_write(dcb, localq);
} }
if (rc == 0) if (rc == 0)
@ -1366,43 +1373,47 @@ static int gw_change_user(
if (client_auth_packet && *client_auth_packet) if (client_auth_packet && *client_auth_packet)
memcpy(&backend_protocol->charset, client_auth_packet, sizeof(int)); memcpy(&backend_protocol->charset, client_auth_packet, sizeof(int));
/* save current_database name */ spinlock_acquire(&in_session->ses_lock);
strncpy(current_database, current_session->db,MYSQL_DATABASE_MAXLEN);
/* /* save current_database name */
* Now clear database name in dcb as we don't do local authentication on db name for change user. strncpy(current_database, current_session->db, MYSQL_DATABASE_MAXLEN);
* Local authentication only for user@host and if successful the database name change is sent to backend.
*/
strcpy(current_session->db, "");
/* /*
* decode the token and check the password. * Now clear database name in dcb as we don't do local authentication on db name for change user.
* Note: if auth_token_len == 0 && auth_token == NULL, user is without password * Local authentication only for user@host and if successful the database name change is sent to backend.
*/ */
auth_ret = gw_check_mysql_scramble_data(backend->session->client, strncpy(current_session->db, "", MYSQL_DATABASE_MAXLEN);
auth_token,
auth_token_len,
client_protocol->scramble,
sizeof(client_protocol->scramble),
username,
client_sha1);
if (auth_ret != 0) { /*
if (!service_refresh_users(backend->session->client->service)) { * Decode the token and check the password.
/* Try authentication again with new repository data */ * Note: if auth_token_len == 0 && auth_token == NULL, user is without password
/* Note: if no auth client authentication will fail */ */
auth_ret = gw_check_mysql_scramble_data( auth_ret = gw_check_mysql_scramble_data(backend->session->client,
backend->session->client, auth_token, auth_token_len,
auth_token, auth_token_len, client_protocol->scramble,
client_protocol->scramble, sizeof (client_protocol->scramble),
sizeof(client_protocol->scramble), username, client_sha1);
username, strncpy(current_session->db, current_database, MYSQL_DATABASE_MAXLEN);
client_sha1); spinlock_release(&in_session->ses_lock);
}
}
/* copy back current datbase to client session */ if (auth_ret != 0)
strcpy(current_session->db, current_database); {
if (service_refresh_users(backend->session->client->service) == 0)
{
/* Try authentication again with new repository data */
/* Note: if no auth client authentication will fail */
spinlock_acquire(&in_session->ses_lock);
strncpy(current_session->db, "", MYSQL_DATABASE_MAXLEN);
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);
strncpy(current_session->db, current_database, MYSQL_DATABASE_MAXLEN);
spinlock_release(&in_session->ses_lock);
}
}
/* let's free the auth_token now */ /* let's free the auth_token now */
if (auth_token) if (auth_token)