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

Use constant sized arrays for some service strings

Browse Source 
The `user`, `password`, `version_string` and `weightby` values should be
allocated as a part of the service structure. This allows them to be
modified at runtime without having to worry about memory allocation
problems.

Although this removes the problem of reallocation, it still does not make
the updating of the strings thread-safe. This can cause invalid values to
be read from the service strings.
This commit is contained in:
Markus Mäkelä
2017-04-24 11:31:25 +03:00
parent b434c94563
commit e62be5034a
10 changed files with 67 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
include/maxscale/server.h
View File

@ -26,8 +26,8 @@
MXS_BEGIN_DECLS MXS_BEGIN_DECLS
#define MAX_SERVER_NAME_LEN 1024 #define MAX_SERVER_NAME_LEN 1024
#define MAX_SERVER_MONUSER_LEN 512 #define MAX_SERVER_MONUSER_LEN 1024
#define MAX_SERVER_MONPW_LEN 512 #define MAX_SERVER_MONPW_LEN 1024
#define MAX_NUM_SLAVES 128 /**< Maximum number of slaves under a single server*/ #define MAX_NUM_SLAVES 128 /**< Maximum number of slaves under a single server*/
/** /**
@ -302,7 +302,7 @@ extern void server_set_status_nolock(SERVER *server, unsigned bit);
extern void server_clear_status_nolock(SERVER *server, unsigned bit); extern void server_clear_status_nolock(SERVER *server, unsigned bit);
extern void server_transfer_status(SERVER *dest_server, const SERVER *source_server); extern void server_transfer_status(SERVER *dest_server, const SERVER *source_server);
extern void server_add_mon_user(SERVER *server, const char *user, const char *passwd); extern void server_add_mon_user(SERVER *server, const char *user, const char *passwd);
extern const char *server_get_parameter(const SERVER *server, char *name); extern const char *server_get_parameter(const SERVER *server, const char *name);
extern void server_update_credentials(SERVER *server, const char *user, const char *passwd); extern void server_update_credentials(SERVER *server, const char *user, const char *passwd);
extern DCB *server_get_persistent(SERVER *server, const char *user, const char *protocol, int id); extern DCB *server_get_persistent(SERVER *server, const char *user, const char *protocol, int id);
extern void server_update_address(SERVER *server, const char *address); extern void server_update_address(SERVER *server, const char *address);

22
include/maxscale/service.h
View File

@ -45,6 +45,11 @@ struct mxs_router;
struct mxs_router_object; struct mxs_router_object;
struct users; struct users;
#define MAX_SERVICE_USER_LEN 1024
#define MAX_SERVICE_PASSWORD_LEN 1024
#define MAX_SERVICE_WEIGHTBY_LEN 1024
#define MAX_SERVICE_VERSION_LEN 1024
/** /**
* The service statistics structure * The service statistics structure
*/ */
@ -64,8 +69,8 @@ typedef struct
*/ */
typedef struct typedef struct
{ {
char *name; /**< The user name to use to extract information */ char name[MAX_SERVICE_USER_LEN]; /**< The user name to use to extract information */
char *authdata; /**< The authentication data requied */ char authdata[MAX_SERVICE_PASSWORD_LEN]; /**< The authentication data requied */
} SERVICE_USER; } SERVICE_USER;
/** /**
@ -130,7 +135,7 @@ typedef struct service
char **routerOptions; /**< Router specific option strings */ char **routerOptions; /**< Router specific option strings */
struct mxs_router_object *router; /**< The router we are using */ struct mxs_router_object *router; /**< The router we are using */
struct mxs_router *router_instance;/**< The router instance for this service */ struct mxs_router *router_instance;/**< The router instance for this service */
char *version_string; /**< version string for this service listeners */ char version_string[MAX_SERVICE_VERSION_LEN]; /**< version string for this service listeners */
SERVER_REF *dbref; /**< server references */ SERVER_REF *dbref; /**< server references */
int n_dbref; /**< Number of server references */ int n_dbref; /**< Number of server references */
SERVICE_USER credentials; /**< The cedentials of the service user */ SERVICE_USER credentials; /**< The cedentials of the service user */
@ -149,7 +154,7 @@ typedef struct service
MXS_FILTER_DEF **filters; /**< Ordered list of filters */ MXS_FILTER_DEF **filters; /**< Ordered list of filters */
int n_filters; /**< Number of filters */ int n_filters; /**< Number of filters */
int64_t conn_idle_timeout; /**< Session timeout in seconds */ int64_t conn_idle_timeout; /**< Session timeout in seconds */
char *weightby; /**< Service weighting parameter name */ char weightby[MAX_SERVICE_WEIGHTBY_LEN]; /**< Service weighting parameter name */
struct service *next; /**< The next service in the linked list */ struct service *next; /**< The next service in the linked list */
bool retry_start; /**< If starting of the service should be retried later */ bool retry_start; /**< If starting of the service should be retried later */
bool log_auth_warnings; /**< Log authentication failures and warnings */ bool log_auth_warnings; /**< Log authentication failures and warnings */
@ -254,17 +259,18 @@ bool serviceHasListener(SERVICE *service, const char *protocol,
bool service_port_is_used(unsigned short port); bool service_port_is_used(unsigned short port);
int serviceGetUser(SERVICE *service, char **user, char **auth); int serviceGetUser(SERVICE *service, char **user, char **auth);
int serviceSetUser(SERVICE *service, char *user, char *auth); int serviceSetUser(SERVICE *service, const char *user, const char *auth);
bool serviceSetFilters(SERVICE *service, char *filters); bool serviceSetFilters(SERVICE *service, char *filters);
int serviceEnableRootUser(SERVICE *service, int action); int serviceEnableRootUser(SERVICE *service, int action);
int serviceSetTimeout(SERVICE *service, int val); int serviceSetTimeout(SERVICE *service, int val);
int serviceSetConnectionLimits(SERVICE *service, int max, int queued, int timeout); int serviceSetConnectionLimits(SERVICE *service, int max, int queued, int timeout);
void serviceSetRetryOnFailure(SERVICE *service, char* value); void serviceSetRetryOnFailure(SERVICE *service, const char* value);
void serviceWeightBy(SERVICE *service, char *weightby); void serviceWeightBy(SERVICE *service, const char *weightby);
char* serviceGetWeightingParameter(SERVICE *service); const char* serviceGetWeightingParameter(SERVICE *service);
int serviceEnableLocalhostMatchWildcardHost(SERVICE *service, int action); int serviceEnableLocalhostMatchWildcardHost(SERVICE *service, int action);
int serviceStripDbEsc(SERVICE* service, int action); int serviceStripDbEsc(SERVICE* service, int action);
int serviceAuthAllServers(SERVICE *service, int action); int serviceAuthAllServers(SERVICE *service, int action);
void serviceSetVersionString(SERVICE *service, const char* value);
int service_refresh_users(SERVICE *service); int service_refresh_users(SERVICE *service);
/** /**

22
server/core/config.cc
View File

@ -1870,11 +1870,7 @@ process_config_update(CONFIG_CONTEXT *context)
if (version_string) if (version_string)
{ {
if (service->version_string) serviceSetVersionString(service, version_string);
{
MXS_FREE(service->version_string);
}
service->version_string = MXS_STRDUP_A(version_string);
} }
if (user && auth) if (user && auth)
@ -2816,22 +2812,18 @@ int create_new_service(CONFIG_CONTEXT *obj)
if (version_string[0] != '5') if (version_string[0] != '5')
{ {
size_t len = strlen(version_string) + strlen("5.5.5-") + 1; size_t len = strlen(version_string) + strlen("5.5.5-") + 1;
service->version_string = (char*)MXS_MALLOC(len); char ver[len];
MXS_ABORT_IF_NULL(service->version_string); snprintf(ver, sizeof(ver), "5.5.5-%s", version_string);
strcpy(service->version_string, "5.5.5-"); serviceSetVersionString(service, ver);
strcat(service->version_string, version_string);
} }
else else
{ {
service->version_string = MXS_STRDUP_A(version_string); serviceSetVersionString(service, version_string);
} }
} }
else else if (gateway.version_string)
{ {
if (gateway.version_string) serviceSetVersionString(service, gateway.version_string);
{
service->version_string = MXS_STRDUP_A(gateway.version_string);
}
} }

2
server/core/server.cc
View File

@ -911,7 +911,7 @@ static void server_parameter_free(SERVER_PARAM *tofree)
* @return The parameter value or NULL if not found * @return The parameter value or NULL if not found
*/ */
const char * const char *
server_get_parameter(const SERVER *server, char *name) server_get_parameter(const SERVER *server, const char *name)
{ {
SERVER_PARAM *param = server->parameters; SERVER_PARAM *param = server->parameters;

73
server/core/service.cc
View File

@ -142,10 +142,6 @@ SERVICE* service_alloc(const char *name, const char *router)
service->localhost_match_wildcard_host = SERVICE_PARAM_UNINIT; service->localhost_match_wildcard_host = SERVICE_PARAM_UNINIT;
service->retry_start = true; service->retry_start = true;
service->conn_idle_timeout = SERVICE_NO_SESSION_TIMEOUT; service->conn_idle_timeout = SERVICE_NO_SESSION_TIMEOUT;
service->weightby = NULL;
service->credentials.authdata = NULL;
service->credentials.name = NULL;
service->version_string = NULL;
service->svc_config_param = NULL; service->svc_config_param = NULL;
service->routerOptions = NULL; service->routerOptions = NULL;
service->log_auth_warnings = true; service->log_auth_warnings = true;
@ -720,10 +716,6 @@ void service_free(SERVICE *service)
MXS_FREE(service->name); MXS_FREE(service->name);
MXS_FREE(service->routerModule); MXS_FREE(service->routerModule);
MXS_FREE(service->weightby);
MXS_FREE(service->version_string);
MXS_FREE(service->credentials.name);
MXS_FREE(service->credentials.authdata);
config_parameter_free(service->svc_config_param); config_parameter_free(service->svc_config_param);
serviceClearRouterOptions(service); serviceClearRouterOptions(service);
@ -992,23 +984,19 @@ serviceClearRouterOptions(SERVICE *service)
* @return 0 on failure * @return 0 on failure
*/ */
int int
serviceSetUser(SERVICE *service, char *user, char *auth) serviceSetUser(SERVICE *service, const char *user, const char *auth)
{ {
user = MXS_STRDUP(user); if (service->credentials.name != user)
auth = MXS_STRDUP(auth);
if (!user || !auth)
{ {
MXS_FREE(user); snprintf(service->credentials.name,
MXS_FREE(auth); sizeof(service->credentials.name), "%s", user);
return 0;
} }
MXS_FREE(service->credentials.name); if (service->credentials.authdata != auth)
MXS_FREE(service->credentials.authdata); {
snprintf(service->credentials.authdata,
service->credentials.name = user; sizeof(service->credentials.authdata), "%s", auth);
service->credentials.authdata = auth; }
return 1; return 1;
} }
@ -1123,6 +1111,14 @@ serviceSetTimeout(SERVICE *service, int val)
return 1; return 1;
} }
void serviceSetVersionString(SERVICE *service, const char* value)
{
if (service->version_string != value)
{
snprintf(service->version_string, sizeof(service->version_string), "%s", value);
}
}
/** /**
* Sets the connection limits, if any, for the service. * Sets the connection limits, if any, for the service.
* @param service Service to configure * @param service Service to configure
@ -1185,7 +1181,7 @@ service_queue_check(void *data)
* @param service Service to configure * @param service Service to configure
* @param value A string representation of a boolean value * @param value A string representation of a boolean value
*/ */
void serviceSetRetryOnFailure(SERVICE *service, char* value) void serviceSetRetryOnFailure(SERVICE *service, const char* value)
{ {
if (value) if (value)
{ {
@ -1444,7 +1440,7 @@ void dprintService(DCB *dcb, SERVICE *service)
} }
server = server->next; server = server->next;
} }
if (service->weightby) if (*service->weightby)
{ {
dcb_printf(dcb, "\tRouting weight parameter: %s\n", dcb_printf(dcb, "\tRouting weight parameter: %s\n",
service->weightby); service->weightby);
@ -1793,14 +1789,12 @@ service_get_name(SERVICE *svc)
* @param service The service pointer * @param service The service pointer
* @param weightby The parameter name to weight the routing by * @param weightby The parameter name to weight the routing by
*/ */
void void serviceWeightBy(SERVICE *service, const char *weightby)
serviceWeightBy(SERVICE *service, char *weightby)
{ {
if (service->weightby) if (service->weightby != weightby)
{ {
MXS_FREE(service->weightby); snprintf(service->weightby, sizeof(service->weightby), "%s", weightby);
} }
service->weightby = MXS_STRDUP_A(weightby);
} }
/** /**
@ -1808,8 +1802,7 @@ serviceWeightBy(SERVICE *service, char *weightby)
* by * by
* @param service The Service pointer * @param service The Service pointer
*/ */
char * const char* serviceGetWeightingParameter(SERVICE *service)
serviceGetWeightingParameter(SERVICE *service)
{ {
return service->weightby; return service->weightby;
} }
@ -2101,8 +2094,9 @@ bool service_all_services_have_listeners()
static void service_calculate_weights(SERVICE *service) static void service_calculate_weights(SERVICE *service)
{ {
char *weightby = serviceGetWeightingParameter(service); const char *weightby = serviceGetWeightingParameter(service);
if (weightby && service->dbref)
if (*weightby && service->dbref)
{ {
/** Service has a weighting parameter and at least one server */ /** Service has a weighting parameter and at least one server */
int total = 0; int total = 0;
@ -2367,17 +2361,8 @@ json_t* service_parameters_to_json(const SERVICE* service)
} }
json_object_set_new(rval, CN_ROUTER_OPTIONS, json_string(options.c_str())); json_object_set_new(rval, CN_ROUTER_OPTIONS, json_string(options.c_str()));
json_object_set_new(rval, CN_USER, json_string(service->credentials.name));
/** Can these be NULL? */ json_object_set_new(rval, CN_PASSWORD, json_string(service->credentials.authdata));
if (service->credentials.name)
{
json_object_set_new(rval, CN_USER, json_string(service->credentials.name));
}
if (service->credentials.authdata)
{
json_object_set_new(rval, CN_PASSWORD, json_string(service->credentials.authdata));
}
json_object_set_new(rval, CN_ENABLE_ROOT_USER, json_boolean(service->enable_root)); json_object_set_new(rval, CN_ENABLE_ROOT_USER, json_boolean(service->enable_root));
json_object_set_new(rval, CN_MAX_RETRY_INTERVAL, json_integer(service->max_retry_interval)); json_object_set_new(rval, CN_MAX_RETRY_INTERVAL, json_integer(service->max_retry_interval));
@ -2390,7 +2375,7 @@ json_t* service_parameters_to_json(const SERVICE* service)
json_boolean(service->localhost_match_wildcard_host)); json_boolean(service->localhost_match_wildcard_host));
json_object_set_new(rval, CN_VERSION_STRING, json_string(service->version_string)); json_object_set_new(rval, CN_VERSION_STRING, json_string(service->version_string));
if (service->weightby) if (*service->weightby)
{ {
json_object_set_new(rval, CN_WEIGHTBY, json_string(service->weightby)); json_object_set_new(rval, CN_WEIGHTBY, json_string(service->weightby));
} }

2
server/modules/protocol/MySQL/MySQLClient/mysql_client.c
View File

@ -248,7 +248,7 @@ int MySQLSendHandshake(DCB* dcb)
GWBUF *buf; GWBUF *buf;
/* get the version string from service property if available*/ /* get the version string from service property if available*/
if (dcb->service->version_string != NULL) if (dcb->service->version_string[0])
{ {
version_string = dcb->service->version_string; version_string = dcb->service->version_string;
len_version_string = strlen(version_string); len_version_string = strlen(version_string);

4
server/modules/routing/binlogrouter/blr.c
View File

@ -243,8 +243,8 @@ createInstance(SERVICE *service, char **options)
int rc = 0; int rc = 0;
char task_name[BLRM_TASK_NAME_LEN + 1] = ""; char task_name[BLRM_TASK_NAME_LEN + 1] = "";
if (service->credentials.name == NULL || if (!service->credentials.name[0] ||
service->credentials.authdata == NULL) !service->credentials.authdata[0])
{ {
MXS_ERROR("%s: Error: Service is missing user credentials." MXS_ERROR("%s: Error: Service is missing user credentials."
" Add the missing username or passwd parameter to the service.", " Add the missing username or passwd parameter to the service.",

4
server/modules/routing/binlogrouter/test/testbinlog.c
View File

@ -97,8 +97,8 @@ int main(int argc, char **argv)
set_libdir(MXS_STRDUP_A("..")); set_libdir(MXS_STRDUP_A(".."));
service = service_alloc("test_service", "binlogrouter"); service = service_alloc("test_service", "binlogrouter");
service->credentials.name = MXS_STRDUP_A("foo"); strcpy(service->credentials.name, "foo");
service->credentials.authdata = MXS_STRDUP_A("bar"); strcpy(service->credentials.authdata, "bar");
{ {
char *lasts; char *lasts;

9
server/modules/routing/readconnroute/readconnroute.c
View File

@ -621,7 +621,7 @@ static void
diagnostics(MXS_ROUTER *router, DCB *dcb) diagnostics(MXS_ROUTER *router, DCB *dcb)
{ {
ROUTER_INSTANCE *router_inst = (ROUTER_INSTANCE *) router; ROUTER_INSTANCE *router_inst = (ROUTER_INSTANCE *) router;
char *weightby; const char *weightby = serviceGetWeightingParameter(router_inst->service);
dcb_printf(dcb, "\tNumber of router sessions: %d\n", dcb_printf(dcb, "\tNumber of router sessions: %d\n",
router_inst->stats.n_sessions); router_inst->stats.n_sessions);
@ -629,8 +629,7 @@ diagnostics(MXS_ROUTER *router, DCB *dcb)
router_inst->service->stats.n_current); router_inst->service->stats.n_current);
dcb_printf(dcb, "\tNumber of queries forwarded: %d\n", dcb_printf(dcb, "\tNumber of queries forwarded: %d\n",
router_inst->stats.n_queries); router_inst->stats.n_queries);
if ((weightby = serviceGetWeightingParameter(router_inst->service)) if (*weightby)
!= NULL)
{ {
dcb_printf(dcb, "\tConnection distribution based on %s " dcb_printf(dcb, "\tConnection distribution based on %s "
"server parameter.\n", "server parameter.\n",
@ -662,9 +661,9 @@ static json_t* diagnostics_json(const MXS_ROUTER *router)
json_object_set_new(rval, "current_connections", json_integer(router_inst->service->stats.n_current)); json_object_set_new(rval, "current_connections", json_integer(router_inst->service->stats.n_current));
json_object_set_new(rval, "queries", json_integer(router_inst->stats.n_queries)); json_object_set_new(rval, "queries", json_integer(router_inst->stats.n_queries));
char *weightby = serviceGetWeightingParameter(router_inst->service); const char *weightby = serviceGetWeightingParameter(router_inst->service);
if (weightby) if (*weightby)
{ {
json_object_set_new(rval, "weightby", json_string(weightby)); json_object_set_new(rval, "weightby", json_string(weightby));
} }

8
server/modules/routing/readwritesplit/readwritesplit.c
View File

@ -651,7 +651,7 @@ static int routeQuery(MXS_ROUTER *instance, MXS_ROUTER_SESSION *router_session,
static void diagnostics(MXS_ROUTER *instance, DCB *dcb) static void diagnostics(MXS_ROUTER *instance, DCB *dcb)
{ {
ROUTER_INSTANCE *router = (ROUTER_INSTANCE *)instance; ROUTER_INSTANCE *router = (ROUTER_INSTANCE *)instance;
char *weightby; const char *weightby = serviceGetWeightingParameter(router->service);
double master_pct = 0.0, slave_pct = 0.0, all_pct = 0.0; double master_pct = 0.0, slave_pct = 0.0, all_pct = 0.0;
dcb_printf(dcb, "\n"); dcb_printf(dcb, "\n");
@ -695,7 +695,7 @@ static void diagnostics(MXS_ROUTER *instance, DCB *dcb)
dcb_printf(dcb, "\tNumber of queries forwarded to all: %" PRIu64 " (%.2f%%)\n", dcb_printf(dcb, "\tNumber of queries forwarded to all: %" PRIu64 " (%.2f%%)\n",
router->stats.n_all, all_pct); router->stats.n_all, all_pct);
if ((weightby = serviceGetWeightingParameter(router->service)) != NULL) if (*weightby)
{ {
dcb_printf(dcb, "\tConnection distribution based on %s " dcb_printf(dcb, "\tConnection distribution based on %s "
"server parameter.\n", "server parameter.\n",
@ -754,9 +754,9 @@ static json_t* diagnostics_json(const MXS_ROUTER *instance)
json_object_set_new(rval, "route_slave", json_integer(router->stats.n_slave)); json_object_set_new(rval, "route_slave", json_integer(router->stats.n_slave));
json_object_set_new(rval, "route_all", json_integer(router->stats.n_all)); json_object_set_new(rval, "route_all", json_integer(router->stats.n_all));
char *weightby = serviceGetWeightingParameter(router->service); const char *weightby = serviceGetWeightingParameter(router->service);
if (weightby) if (*weightby)
{ {
json_object_set_new(rval, "weightby", json_string(weightby)); json_object_set_new(rval, "weightby", json_string(weightby));
} }