Skip to content

Commit e42be82

Browse files
author
Arun Kuruvila
committed
Bug#22322504 : MYSQL_REAL_CONNECT IS NOT THREAD SAFE
Description: When 'mysql_real_connect()' is invoked in parallel with either of the option MYSQL_READ_DEFAULT_FILE or MYSQL_READ_DEFAULT_GROUP enabled causes a noticeable percentage of thread connection failures. This is due to the inappropriate loading of options from the configuration files. Analysis: 'mysql_real_connect()' calls 'my_load_defaults()' which will toggle a static global variable 'is_login_file' before it calls 'my_search_option_files()'. Function 'my_search_option_files()' has conditional logic based on the static global variable, 'is_login_file', which is accessible across multiple threads. This makes 'mysql_real_connect()' thread unsafe causing noticeable percent of parallel thread connections to fail. Fix: A new argument, 'is_login_file', is passed to all the functions which deals with login file.
1 parent aa3fbfa commit e42be82

4 files changed

Lines changed: 55 additions & 45 deletions

File tree

‎extra/my_print_defaults.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,7 @@ int main(int argc, char **argv)
172172

173173
org_argv= argv;
174174
args_used= get_defaults_options(argc, argv, &defaults, &extra_defaults,
175-
&group_suffix, &login_path);
175+
&group_suffix, &login_path, FALSE);
176176

177177
/* Copy defaults-xxx arguments & program name */
178178
count=args_used+1;

‎include/my_default.h‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
/* Copyright (c) 2012, 2014, Oracle and/or its affiliates. All rights reserved.
1+
/* Copyright (c) 2012, 2016, Oracle and/or its affiliates. All rights reserved.
22
33
This program is free software; you can redistribute it and/or modify
44
it under the terms of the GNU General Public License as published by
@@ -33,16 +33,18 @@ typedef int (*Process_option_func)(void *ctx, const char *group_name,
3333
my_bool my_getopt_is_args_separator(const char* arg);
3434
int get_defaults_options(int argc, char **argv,
3535
char **defaults, char **extra_defaults,
36-
char **group_suffix, char **login_path);
36+
char **group_suffix, char **login_path,
37+
my_bool found_no_defaults);
3738
int my_load_defaults(const char *conf_file, const char **groups,
3839
int *argc, char ***argv, const char ***);
39-
int check_file_permissions(const char *file_name);
40+
int check_file_permissions(const char *file_name, my_bool is_login_file);
4041
int load_defaults(const char *conf_file, const char **groups,
4142
int *argc, char ***argv);
4243
int my_search_option_files(const char *conf_file, int *argc,
4344
char ***argv, uint *args_used,
4445
Process_option_func func, void *func_ctx,
45-
const char **default_directories);
46+
const char **default_directories,
47+
my_bool is_login_file, my_bool found_no_defaults);
4648
void free_defaults(char **argv);
4749
void my_print_default_files(const char *conf_file);
4850
void print_defaults(const char *conf_file, const char **groups);

‎mysys_ssl/my_default.cc‎

Lines changed: 47 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -106,16 +106,8 @@ static const char *my_login_path= 0;
106106
static char my_defaults_file_buffer[FN_REFLEN];
107107
static char my_defaults_extra_file_buffer[FN_REFLEN];
108108

109-
static char my_login_file[FN_REFLEN];
110-
111109
static my_bool defaults_already_read= FALSE;
112110

113-
/* Set to TRUE, if --no-defaults is found. */
114-
static my_bool found_no_defaults= FALSE;
115-
116-
/* Set to TRUE, when login file is being processed. */
117-
static my_bool is_login_file= FALSE;
118-
119111
/* Which directories are searched for options (and in which order) */
120112

121113
#define MAX_DEFAULT_DIRS 6
@@ -148,13 +140,16 @@ struct handle_option_ctx
148140
};
149141

150142
static int search_default_file(Process_option_func func, void *func_ctx,
151-
const char *dir, const char *config_file);
143+
const char *dir, const char *config_file,
144+
my_bool is_login_file);
152145
static int search_default_file_with_ext(Process_option_func func,
153146
void *func_ctx,
154147
const char *dir, const char *ext,
155-
const char *config_file, int recursion_level);
156-
static my_bool mysql_file_getline(char *str, int size, MYSQL_FILE *file);
157-
148+
const char *config_file,
149+
int recursion_level,
150+
my_bool is_login_file);
151+
static my_bool mysql_file_getline(char *str, int size, MYSQL_FILE *file,
152+
my_bool is_login_file);
158153

159154
/**
160155
Create the list of default directories.
@@ -230,6 +225,7 @@ fn_expand(const char *filename, char *result_buf)
230225
func_ctx It's context. Usually it is the structure to
231226
store additional options.
232227
default_directories List of default directories.
228+
found_no_defaults TRUE, if --no-defaults is specified.
233229
234230
DESCRIPTION
235231
Process the default options from argc & argv
@@ -253,7 +249,8 @@ fn_expand(const char *filename, char *result_buf)
253249

254250
int my_search_option_files(const char *conf_file, int *argc, char ***argv,
255251
uint *args_used, Process_option_func func,
256-
void *func_ctx, const char **default_directories)
252+
void *func_ctx, const char **default_directories,
253+
my_bool is_login_file, my_bool found_no_defaults)
257254
{
258255
const char **dirs, *forced_default_file, *forced_extra_defaults;
259256
int error= 0;
@@ -267,7 +264,7 @@ int my_search_option_files(const char *conf_file, int *argc, char ***argv,
267264
(char **) &forced_default_file,
268265
(char **) &forced_extra_defaults,
269266
(char **) &my_defaults_group_suffix,
270-
(char **) &my_login_path);
267+
(char **) &my_login_path, found_no_defaults);
271268

272269
if (! my_defaults_group_suffix)
273270
my_defaults_group_suffix= getenv(STRINGIFY_ARG(DEFAULT_GROUP_SUFFIX_ENV));
@@ -382,14 +379,16 @@ int my_search_option_files(const char *conf_file, int *argc, char ***argv,
382379
// If conf_file is an absolute path, we only read it
383380
if (dirname_length(conf_file))
384381
{
385-
if ((error= search_default_file(func, func_ctx, NullS, conf_file)) < 0)
386-
goto err;
382+
if ((error= search_default_file(func, func_ctx, NullS, conf_file,
383+
is_login_file)) < 0)
384+
goto err;
387385
}
388386
// If my defaults file is set (from a previous run), we read it
389387
else if (my_defaults_file)
390388
{
391389
if ((error= search_default_file_with_ext(func, func_ctx, "", "",
392-
my_defaults_file, 0)) < 0)
390+
my_defaults_file, 0,
391+
is_login_file)) < 0)
393392
goto err;
394393
if (error > 0)
395394
{
@@ -404,14 +403,16 @@ int my_search_option_files(const char *conf_file, int *argc, char ***argv,
404403
{
405404
if (**dirs)
406405
{
407-
if (search_default_file(func, func_ctx, *dirs, conf_file) < 0)
408-
goto err;
406+
if (search_default_file(func, func_ctx, *dirs, conf_file,
407+
is_login_file) < 0)
408+
goto err;
409409
}
410410
else if (my_defaults_extra_file)
411411
{
412412
if ((error= search_default_file_with_ext(func, func_ctx, "", "",
413-
my_defaults_extra_file, 0)) < 0)
414-
goto err; /* Fatal error */
413+
my_defaults_extra_file, 0,
414+
is_login_file)) < 0)
415+
goto err; /* Fatal error */
415416
if (error > 0)
416417
{
417418
fprintf(stderr, "Could not open required defaults file: %s\n",
@@ -499,7 +500,8 @@ int get_defaults_options(int argc, char **argv,
499500
char **defaults,
500501
char **extra_defaults,
501502
char **group_suffix,
502-
char **login_path)
503+
char **login_path,
504+
my_bool found_no_defaults)
503505
{
504506
int org_argc= argc, prev_argc= 0, default_option_count= 0;
505507
*defaults= *extra_defaults= *group_suffix= *login_path= 0;
@@ -626,6 +628,8 @@ int my_load_defaults(const char *conf_file, const char **groups,
626628
char *ptr,**res;
627629
struct handle_option_ctx ctx;
628630
const char **dirs;
631+
char my_login_file[FN_REFLEN];
632+
my_bool found_no_defaults= false;
629633
uint args_sep= my_getopt_use_args_separator ? 1 : 0;
630634
DBUG_ENTER("load_defaults");
631635

@@ -655,23 +659,21 @@ int my_load_defaults(const char *conf_file, const char **groups,
655659

656660
if ((error= my_search_option_files(conf_file, argc, argv,
657661
&args_used, handle_default_option,
658-
(void *) &ctx, dirs)))
662+
(void *) &ctx, dirs, false, found_no_defaults)))
659663
{
660664
free_root(&alloc,MYF(0));
661665
DBUG_RETURN(error);
662666
}
663667

664668
/* Read options from login group. */
665-
is_login_file= TRUE;
666669
if (my_default_get_login_file(my_login_file, sizeof(my_login_file)) &&
667670
(error= my_search_option_files(my_login_file,argc, argv, &args_used,
668671
handle_default_option, (void *) &ctx,
669-
dirs)))
672+
dirs, true, found_no_defaults)))
670673
{
671674
free_root(&alloc,MYF(0));
672675
DBUG_RETURN(error);
673676
}
674-
is_login_file= FALSE;
675677

676678
/*
677679
Here error contains <> 0 only if we have a fully specified conf_file
@@ -754,7 +756,8 @@ void free_defaults(char **argv)
754756
static int search_default_file(Process_option_func opt_handler,
755757
void *handler_ctx,
756758
const char *dir,
757-
const char *config_file)
759+
const char *config_file,
760+
my_bool is_login_file)
758761
{
759762
char **ext;
760763
const char *empty_list[]= { "", 0 };
@@ -766,7 +769,7 @@ static int search_default_file(Process_option_func opt_handler,
766769
int error;
767770
if ((error= search_default_file_with_ext(opt_handler, handler_ctx,
768771
dir, *ext,
769-
config_file, 0)) < 0)
772+
config_file, 0, is_login_file)) < 0)
770773
return error;
771774
}
772775
return 0;
@@ -838,6 +841,7 @@ static char *get_argument(const char *keyword, size_t kwlen,
838841
group groups to read
839842
recursion_level the level of recursion, got while processing
840843
"!include" or "!includedir"
844+
is_login_file TRUE, when login file is being processed.
841845
842846
RETURN
843847
0 Success
@@ -850,7 +854,8 @@ static int search_default_file_with_ext(Process_option_func opt_handler,
850854
const char *dir,
851855
const char *ext,
852856
const char *config_file,
853-
int recursion_level)
857+
int recursion_level,
858+
my_bool is_login_file)
854859
{
855860
char name[FN_REFLEN + 10], buff[4096], curr_gr[4096], *ptr, *end, **tmp_ext;
856861
char *value, option[4096+2], tmp[FN_REFLEN];
@@ -879,7 +884,7 @@ static int search_default_file_with_ext(Process_option_func opt_handler,
879884
}
880885
fn_format(name,name,"","",4);
881886

882-
if ((rc= check_file_permissions(name)) < 2)
887+
if ((rc= check_file_permissions(name, is_login_file)) < 2)
883888
return (int) rc;
884889

885890
if (is_login_file)
@@ -894,7 +899,7 @@ static int search_default_file_with_ext(Process_option_func opt_handler,
894899
return 1; /* Ignore wrong files */
895900
}
896901

897-
while (mysql_file_getline(buff, sizeof(buff) - 1, fp))
902+
while (mysql_file_getline(buff, sizeof(buff) - 1, fp, is_login_file))
898903
{
899904
line++;
900905
/* Ignore comment and empty lines */
@@ -955,7 +960,7 @@ static int search_default_file_with_ext(Process_option_func opt_handler,
955960
MY_UNPACK_FILENAME | MY_SAFE_PATH);
956961

957962
search_default_file_with_ext(opt_handler, handler_ctx, "", "", tmp,
958-
recursion_level + 1);
963+
recursion_level + 1, is_login_file);
959964
}
960965
}
961966

@@ -970,7 +975,7 @@ static int search_default_file_with_ext(Process_option_func opt_handler,
970975
goto err;
971976

972977
search_default_file_with_ext(opt_handler, handler_ctx, "", "", ptr,
973-
recursion_level + 1);
978+
recursion_level + 1, is_login_file);
974979
}
975980

976981
continue;
@@ -1132,15 +1137,17 @@ static char *remove_end_comment(char *ptr)
11321137
of scrambled login file, the line read is first
11331138
decrypted and then returned.
11341139
1135-
@param str [out] Buffer to store the read text.
1136-
@param size [in] At max, size-1 bytes to be read.
1137-
@param file [in] Source file.
1140+
@param str [out] Buffer to store the read text.
1141+
@param size [in] At max, size-1 bytes to be read.
1142+
@param file [in] Source file.
1143+
@param is_login_file [in] TRUE, when login file is being processed.
11381144
11391145
@return 1 Success
11401146
0 Error
11411147
*/
11421148

1143-
static my_bool mysql_file_getline(char *str, int size, MYSQL_FILE *file)
1149+
static my_bool mysql_file_getline(char *str, int size, MYSQL_FILE *file,
1150+
my_bool is_login_file)
11441151
{
11451152
uchar cipher[4096], len_buf[MAX_CIPHER_STORE_LEN];
11461153
static unsigned char my_key[LOGIN_KEY_LEN];
@@ -1453,13 +1460,14 @@ int my_default_get_login_file(char *file_name, size_t file_name_size)
14531460
/**
14541461
Check file permissions of the option file.
14551462
1456-
@param file_name [in] Name of the option file.
1463+
@param file_name [in] Name of the option file.
1464+
@param is_login_file [in] TRUE, when login file is being processed.
14571465
14581466
@return 0 - Non-allowable file permissions.
14591467
1 - Failed to stat.
14601468
2 - Success.
14611469
*/
1462-
int check_file_permissions(const char *file_name)
1470+
int check_file_permissions(const char *file_name, my_bool is_login_file)
14631471
{
14641472
#if !defined(__WIN__)
14651473
MY_STAT stat_info;

‎sql/mysqld.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4524,7 +4524,7 @@ static int init_server_auto_options()
45244524
/* load_defaults require argv[0] is not null */
45254525
char **argv= &name;
45264526
int argc= 1;
4527-
if (!check_file_permissions(fname))
4527+
if (!check_file_permissions(fname, false))
45284528
{
45294529
/*
45304530
Found a world writable file hence removing it as it is dangerous to write

0 commit comments

Comments
 (0)