mysql_client.c:gw_client_close didn't close client session in cases where session->state == SESSION_STATE_STOPPING. That is a bug and lead to situation where session wasn't closed at all.

Also changed 'authorization failed' to 'access denied'
mysql_common.c: fixed memory leak in gw_receive_backend_auth, and replaced error code '2800' with '28000'.
readconnroute.c:handleError didn't set *succp pointer so uninitialized value was used in caller's context.
makefile.inc: added -lm to linker flags
mysql_backend.c: added a few comments
This commit is contained in:
VilhoRaatikka
2014-10-06 11:46:12 +03:00
parent bb11f6236f
commit aca8596efa
6 changed files with 43 additions and 44 deletions

View File

@ -25,7 +25,7 @@ endif
# -O2 -g -pipe -Wformat -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector --param=ssp-buffer-size=4 -fPIC # -O2 -g -pipe -Wformat -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -fstack-protector --param=ssp-buffer-size=4 -fPIC
CFLAGS := $(CFLAGS) -Wall CFLAGS := $(CFLAGS) -Wall
LDLIBS := $(LDLIBS) -pthread LDLIBS := $(LDLIBS) -pthread -lm
LDMYSQL := -lmysqld LDMYSQL := -lmysqld
CPP_LDLIBS := -lstdc++ CPP_LDLIBS := -lstdc++

View File

@ -199,6 +199,7 @@ static int gw_read_backend_event(DCB *dcb) {
if (backend_protocol->protocol_auth_state == MYSQL_CONNECTED) if (backend_protocol->protocol_auth_state == MYSQL_CONNECTED)
{ {
/** Read cached backend handshake */
if (gw_read_backend_handshake(backend_protocol) != 0) if (gw_read_backend_handshake(backend_protocol) != 0)
{ {
backend_protocol->protocol_auth_state = MYSQL_AUTH_FAILED; backend_protocol->protocol_auth_state = MYSQL_AUTH_FAILED;
@ -209,11 +210,13 @@ static int gw_read_backend_event(DCB *dcb) {
"state = MYSQL_AUTH_FAILED.", "state = MYSQL_AUTH_FAILED.",
pthread_self(), pthread_self(),
backend_protocol->owner_dcb->fd))); backend_protocol->owner_dcb->fd)));
} }
else else
{ {
/* handshake decoded, send the auth credentials */ /**
* Decode password and send the auth credentials
* to backend.
*/
if (gw_send_authentication_to_backend( if (gw_send_authentication_to_backend(
current_session->db, current_session->db,
current_session->user, current_session->user,
@ -236,7 +239,7 @@ static int gw_read_backend_event(DCB *dcb) {
} }
} }
spinlock_release(&dcb->authlock); spinlock_release(&dcb->authlock);
} } /*< backend_protocol->protocol_auth_state == MYSQL_CONNECTED */
/* /*
* Now: * Now:
* -- check the authentication reply from backend * -- check the authentication reply from backend
@ -266,9 +269,10 @@ static int gw_read_backend_event(DCB *dcb) {
router_instance = session->service->router_instance; router_instance = session->service->router_instance;
rsession = session->router_session; rsession = session->router_session;
if (backend_protocol->protocol_auth_state == MYSQL_AUTH_RECV) { if (backend_protocol->protocol_auth_state == MYSQL_AUTH_RECV)
/*< {
* Read backed auth reply /**
* Read backed's reply to authentication message
*/ */
receive_rc = receive_rc =
gw_receive_backend_auth(backend_protocol); gw_receive_backend_auth(backend_protocol);
@ -284,7 +288,6 @@ static int gw_read_backend_event(DCB *dcb) {
pthread_self(), pthread_self(),
backend_protocol->owner_dcb->fd))); backend_protocol->owner_dcb->fd)));
LOGIF(LE, (skygw_log_write_flush( LOGIF(LE, (skygw_log_write_flush(
LOGFILE_ERROR, LOGFILE_ERROR,
"Error : Backend server didn't " "Error : Backend server didn't "
@ -371,7 +374,6 @@ static int gw_read_backend_event(DCB *dcb) {
&succp); &succp);
ss_dassert(!succp); ss_dassert(!succp);
LOGIF(LD, (skygw_log_write( LOGIF(LD, (skygw_log_write(
LOGFILE_DEBUG, LOGFILE_DEBUG,
"%lu [gw_read_backend_event] " "%lu [gw_read_backend_event] "
@ -1048,6 +1050,10 @@ gw_backend_close(DCB *dcb)
mysql_protocol_done(dcb); mysql_protocol_done(dcb);
/**
* If session->state is set to STOPPING the client and the session must
* be closed too.
*/
if (session != NULL && session->state == SESSION_STATE_STOPPING) if (session != NULL && session->state == SESSION_STATE_STOPPING)
{ {
client_dcb = session->client; client_dcb = session->client;

View File

@ -659,7 +659,7 @@ int gw_read_client_event(
dcb, dcb,
2, 2,
0, 0,
"Authorization failed"); "Access denied");
dcb_close(dcb); dcb_close(dcb);
} }
@ -801,9 +801,12 @@ int gw_read_client_event(
} }
/** succeed */ /** succeed */
if (rc) { if (rc)
{
rc = 0; /**< here '0' means success */ rc = 0; /**< here '0' means success */
} else { }
else
{
GWBUF* errbuf; GWBUF* errbuf;
bool succp; bool succp;
@ -1360,20 +1363,12 @@ gw_client_close(DCB *dcb)
CHK_SESSION(session); CHK_SESSION(session);
spinlock_acquire(&session->ses_lock); spinlock_acquire(&session->ses_lock);
if (session->state == SESSION_STATE_STOPPING) if (session->state != SESSION_STATE_STOPPING)
{ {
/** session->state = SESSION_STATE_STOPPING;
* Session is already getting closed so avoid }
* redundant calls spinlock_release(&session->ses_lock);
*/
spinlock_release(&session->ses_lock);
return 1;
}
else
{
session->state = SESSION_STATE_STOPPING;
spinlock_release(&session->ses_lock);
}
router = session->service->router; router = session->service->router;
router_instance = session->service->router_instance; router_instance = session->service->router_instance;
rsession = session->router_session; rsession = session->router_session;

View File

@ -463,6 +463,7 @@ int gw_receive_backend_auth(
bufstr))); bufstr)));
free(bufstr); free(bufstr);
free(err);
rc = -1; rc = -1;
} }
else else
@ -1458,7 +1459,7 @@ mysql_send_auth_error (
} }
mysql_errno = 1045; mysql_errno = 1045;
mysql_error_msg = "Access denied!"; mysql_error_msg = "Access denied!";
mysql_state = "2800"; mysql_state = "28000";
field_count = 0xff; field_count = 0xff;
gw_mysql_set_byte2(mysql_err, mysql_errno); gw_mysql_set_byte2(mysql_err, mysql_errno);

View File

@ -825,7 +825,8 @@ handleError(
DCB *client = NULL; DCB *client = NULL;
SESSION *session = backend_dcb->session; SESSION *session = backend_dcb->session;
client = session->client; client = session->client;
/** false because connection is not available anymore */
*succp = false;
ss_dassert(client != NULL); ss_dassert(client != NULL);
} }

View File

@ -266,7 +266,7 @@ static bool handle_error_new_connection(
ROUTER_CLIENT_SES* rses, ROUTER_CLIENT_SES* rses,
DCB* backend_dcb, DCB* backend_dcb,
GWBUF* errmsg); GWBUF* errmsg);
static bool handle_error_reply_client(SESSION* ses, GWBUF* errmsg); static void handle_error_reply_client(SESSION* ses, GWBUF* errmsg);
static backend_ref_t* get_root_master_bref(ROUTER_CLIENT_SES* rses); static backend_ref_t* get_root_master_bref(ROUTER_CLIENT_SES* rses);
@ -1683,7 +1683,6 @@ static int routeQuery(
packet = GWBUF_DATA(querybuf); packet = GWBUF_DATA(querybuf);
packet_type = packet[4]; packet_type = packet[4];
if (rses_is_closed) if (rses_is_closed)
{ {
/** /**
@ -4015,7 +4014,8 @@ static void handleError (
case ERRACT_REPLY_CLIENT: case ERRACT_REPLY_CLIENT:
{ {
*succp = handle_error_reply_client(session, errmsgbuf); handle_error_reply_client(session, errmsgbuf);
*succp = false; /*< no new backend servers were made available */
break; break;
} }
@ -4026,13 +4026,12 @@ static void handleError (
} }
static bool handle_error_reply_client( static void handle_error_reply_client(
SESSION* ses, SESSION* ses,
GWBUF* errmsg) GWBUF* errmsg)
{ {
session_state_t sesstate; session_state_t sesstate;
DCB* client_dcb; DCB* client_dcb;
bool succp;
spinlock_acquire(&ses->ses_lock); spinlock_acquire(&ses->ses_lock);
sesstate = ses->state; sesstate = ses->state;
@ -4049,9 +4048,6 @@ static bool handle_error_reply_client(
while ((errmsg=gwbuf_consume(errmsg, GWBUF_LENGTH(errmsg))) != NULL) while ((errmsg=gwbuf_consume(errmsg, GWBUF_LENGTH(errmsg))) != NULL)
; ;
} }
succp = false; /** false because new servers aren's selected. */
return succp;
} }
/** /**