dcb.b:dcb_final_free:replaced ((dcb->flags & DCBF_CLONE)==0) with macro !DCB_IS_CLONE(dcb)

readwritesplit.h:Removed invalid macros which assumed that ROUTER_CLIENT_SES->rses_backend_ref always pointed to valid and used backend reference and thus included potential risk of NULL-pointer refernce.
mysql_backend.c and mysql_client.c:avoid executing CHK_PROTOCOL(p) after original DCB has been released the memory.
readwritesplit.c:Replaced RSES_CLEINT_DCB macro with a function which returns client DCB for a given router client session.
This commit is contained in:
VilhoRaatikka 2014-12-23 16:10:27 +02:00
parent d2281d6391
commit ff5fe23ce6
7 changed files with 39 additions and 52 deletions

View File

@ -400,9 +400,9 @@ DCB_CALLBACK *cb;
}
}
if (dcb->protocol && ((dcb->flags & DCBF_CLONE) ==0))
if (dcb->protocol && (!DCB_IS_CLONE(dcb)))
free(dcb->protocol);
if (dcb->data && ((dcb->flags & DCBF_CLONE) ==0))
if (dcb->data && (!DCB_IS_CLONE(dcb)))
free(dcb->data);
if (dcb->remote)
free(dcb->remote);

View File

@ -303,11 +303,9 @@ typedef struct {
#endif /** _MYSQL_PROTOCOL_H */
void gw_mysql_close(MySQLProtocol **ptr);
MySQLProtocol* mysql_protocol_init(DCB* dcb, int fd);
void mysql_protocol_done (DCB* dcb);
MySQLProtocol *gw_mysql_init(MySQLProtocol *data);
void gw_mysql_close(MySQLProtocol **ptr);
int gw_receive_backend_auth(MySQLProtocol *protocol);
int gw_decode_mysql_server_handshake(MySQLProtocol *protocol, uint8_t *payload);
int gw_read_backend_handshake(MySQLProtocol *protocol);

View File

@ -323,8 +323,5 @@ typedef struct router_instance {
#define BACKEND_TYPE(b) (SERVER_IS_MASTER((b)->backend_server) ? BE_MASTER : \
(SERVER_IS_SLAVE((b)->backend_server) ? BE_SLAVE : BE_UNDEFINED));
#define RSES_SESSION(r) (r->rses_backend_ref->bref_dcb->session)
#define RSES_CLIENT_DCB(r) (RSES_SESSION(r)->client)
#endif /*< _RWSPLITROUTER_H */

View File

@ -1495,7 +1495,7 @@ static GWBUF* process_response_data (
/** Get command which was stored in gw_MySQLWrite_backend */
p = DCB_PROTOCOL(dcb, MySQLProtocol);
CHK_PROTOCOL(p);
if (!DCB_IS_CLONE(dcb)) CHK_PROTOCOL(p);
/** All buffers processed here are sescmd responses */
gwbuf_set_type(readbuf, GWBUF_TYPE_SESCMD_RESPONSE);
@ -1629,7 +1629,7 @@ static bool sescmd_response_complete(
bool succp;
p = DCB_PROTOCOL(dcb, MySQLProtocol);
CHK_PROTOCOL(p);
if (!DCB_IS_CLONE(dcb)) CHK_PROTOCOL(p);
protocol_get_response_status(p, &npackets_left, &nbytes_left);

View File

@ -1395,20 +1395,13 @@ gw_client_close(DCB *dcb)
dcb->state == DCB_STATE_NOPOLLING ||
dcb->state == DCB_STATE_ZOMBIE)
{
CHK_PROTOCOL(protocol);
if (!DCB_IS_CLONE(dcb)) CHK_PROTOCOL(protocol);
}
#endif
LOGIF(LD, (skygw_log_write(LOGFILE_DEBUG,
"%lu [gw_client_close]",
pthread_self())));
/**
* Since only protocol pointer is copied from original DCB to clone in
* dcb_clone, only dcb_close for the original DCB closes protocol.
*/
if (!DCB_IS_CLONE(dcb))
{
mysql_protocol_done(dcb);
}
mysql_protocol_done(dcb);
session = dcb->session;
/**
* session may be NULL if session_alloc failed.

View File

@ -151,38 +151,6 @@ retblock:
}
/**
* gw_mysql_close
*
* close a connection if opened
* free data scructure for MySQLProtocol
*
* @param ptr The MySQLProtocol ** to close/free
*
*/
void gw_mysql_close(MySQLProtocol **ptr) {
MySQLProtocol *conn = *ptr;
ss_dassert(*ptr != NULL);
if (*ptr == NULL)
return;
if (conn->fd > 0) {
/* COM_QUIT will not be sent here, but from the caller of this routine! */
close(conn->fd);
} else {
// no socket here
}
free(*ptr);
*ptr = NULL;
}
/**
* Read the backend server MySQL handshake
*

View File

@ -99,6 +99,7 @@ static int router_get_servercount(ROUTER_INSTANCE* router);
static int rses_get_max_slavecount(ROUTER_CLIENT_SES* rses, int router_nservers);
static int rses_get_max_replication_lag(ROUTER_CLIENT_SES* rses);
static backend_ref_t* get_bref_from_dcb(ROUTER_CLIENT_SES* rses, DCB* dcb);
static DCB* rses_get_client_dcb(ROUTER_CLIENT_SES* rses);
static route_target_t get_route_target (
skygw_query_type_t qtype,
@ -1770,6 +1771,33 @@ static void check_create_tmp_table(
}
}
/**
* Get client DCB pointer of the router client session.
* This routine must be protected by Router client session lock.
*
* @param rses Router client session pointer
*
* @return Pointer to client DCB
*/
static DCB* rses_get_client_dcb(
ROUTER_CLIENT_SES* rses)
{
DCB* dcb = NULL;
int i;
for (i=0; i<rses->rses_nbackends; i++)
{
if ((dcb = rses->rses_backend_ref[i].bref_dcb) != NULL &&
BREF_IS_IN_USE(&rses->rses_backend_ref[i]) &&
dcb->session != NULL &&
dcb->session->client != NULL)
{
return dcb->session->client;
}
}
return NULL;
}
/**
* The main routing entry, this is called with every packet that is
* received and has to be forwarded to the backend database.
@ -1806,6 +1834,9 @@ static int routeQuery(
CHK_CLIENT_RSES(router_cli_ses);
/**
* GWBUF is called "type undefined" when the incoming data isn't parsed
* and MySQL packets haven't been extracted to separate buffers.
* "Undefined" == "untyped".
* Untyped GWBUF means that it can consist of incomplete and/or multiple
* MySQL packets.
* Read and route found MySQL packets one by one and store potential
@ -1824,7 +1855,7 @@ static int routeQuery(
{
if (GWBUF_LENGTH(tmpbuf) > 0)
{
DCB* dcb = RSES_CLIENT_DCB(router_cli_ses);
DCB* dcb = rses_get_client_dcb(router_cli_ses);
dcb->dcb_readqueue = gwbuf_append(dcb->dcb_readqueue, tmpbuf);
}