From 3c1be0006917b60c65a2633f8c56b8b6c1da50fb Mon Sep 17 00:00:00 2001 From: "tnurnberg@sin.intern.azundris.com" <> Date: Thu, 21 Jun 2007 04:30:10 +0200 Subject: [PATCH 1/2] Bug#24924: shared-memory-base-name that is too long causes buffer overflow long shared-memory-base-names could overflow a static internal buffer and thus crash mysqld and various clients. change both to dynamic buffers, show everything but overflowing those buffers still works. The test case for this would pretty much amount to mysqld --shared-memory-base-name=HeyMrBaseNameXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX --shared-memory=1 & mysqladmin --no-defaults --shared-memory-base-name=HeyMrBaseNameXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX shutdown Unfortunately, we can't just use an .opt file for the server. The .opt file is used at start-up, before any include in the actual test can tell mysqltest to skip this one on non-Windows. As a result, such a test would break on unices. Fixing mysql-test-run.pl to export full path for master and slave would enable us to start a server from within the test which is ugly and, what's more, doesn't work as the server blocks (mysqltest offers no fire-and-forget fork-and-exec), and mysqladmin never gets run. Making the test rpl_windows_shm or some such so we can is beyond ugly. As is introducing another file-name based special case (run "win*.test" only when on Windows). As is (yuck) coding half the test into mtr (as in, having it hand out a customized environment conductive to the shm- thing on Win only). Situation is exacerbated by the fact that .sh is not necessary run as expected on Win. In short, it's just not worth it. No test-case until we have a new-and-improved test framework. --- sql-common/client.c | 10 +++++++++- sql/mysqld.cc | 11 ++++++++++- 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/sql-common/client.c b/sql-common/client.c index 3e5ceb1a738..f8d52e02196 100644 --- a/sql-common/client.c +++ b/sql-common/client.c @@ -402,13 +402,19 @@ HANDLE create_shared_memory(MYSQL *mysql,NET *net, uint connect_timeout) HANDLE handle_file_map = NULL; ulong connect_number; char connect_number_char[22], *p; - char tmp[64]; + char *tmp= NULL; char *suffix_pos; DWORD error_allow = 0; DWORD error_code = 0; DWORD event_access_rights= SYNCHRONIZE | EVENT_MODIFY_STATE; char *shared_memory_base_name = mysql->options.shared_memory_base_name; + /* + get enough space base-name + '_' + longest suffix we might ever send + */ + if (!(tmp= (char *)my_malloc(strlen(shared_memory_base_name) + 32L, MYF(MY_FAE)))) + goto err; + /* The name of event and file-mapping events create agree next rule: shared_memory_base_name+unique_part @@ -551,6 +557,8 @@ err2: CloseHandle(handle_file_map); } err: + if (tmp) + my_free(tmp, MYF(0)); if (error_allow) error_code = GetLastError(); if (event_connect_request) diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 49d997f391e..afeb3b82a01 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -4420,7 +4420,7 @@ pthread_handler_t handle_connections_shared_memory(void *arg) HANDLE event_connect_answer= 0; ulong smem_buffer_length= shared_memory_buffer_length + 4; ulong connect_number= 1; - char tmp[63]; + char *tmp= NULL; char *suffix_pos; char connect_number_char[22], *p; const char *errmsg= 0; @@ -4429,6 +4429,12 @@ pthread_handler_t handle_connections_shared_memory(void *arg) DBUG_ENTER("handle_connections_shared_memorys"); DBUG_PRINT("general",("Waiting for allocated shared memory.")); + /* + get enough space base-name + '_' + longest suffix we might ever send + */ + if (!(tmp= (char *)my_malloc(strlen(shared_memory_base_name) + 32L, MYF(MY_FAE)))) + goto error; + if (my_security_attr_create(&sa_event, &errmsg, GENERIC_ALL, SYNCHRONIZE | EVENT_MODIFY_STATE)) goto error; @@ -4616,6 +4622,9 @@ errorconn: /* End shared memory handling */ error: + if (tmp) + my_free(tmp, MYF(0)); + if (errmsg) { char buff[180]; From 840344589e4f11b1b23fb2c9dbc71fdaabff943c Mon Sep 17 00:00:00 2001 From: "msvensson@pilot.(none)" <> Date: Thu, 21 Jun 2007 16:37:13 +0200 Subject: [PATCH 2/2] Add name of test that generated the warning to "warnings" file --- mysql-test/lib/mtr_report.pl | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/mysql-test/lib/mtr_report.pl b/mysql-test/lib/mtr_report.pl index 4b1dd2f3d6b..1abfc6b6b57 100644 --- a/mysql-test/lib/mtr_report.pl +++ b/mysql-test/lib/mtr_report.pl @@ -272,6 +272,7 @@ sub mtr_report_stats ($) { { foreach my $errlog ( sort glob("$::opt_vardir/log/*.err") ) { + my $testname= ""; unless ( open(ERR, $errlog) ) { mtr_warning("can't read $errlog"); @@ -287,10 +288,14 @@ sub mtr_report_stats ($) { { next; # Skip these lines } + if ( /CURRENT_TEST: (.*)/ ) + { + $testname= $1; + } if ( /$pattern/ ) { $found_problems= 1; - print WARN basename($errlog) . ": $_"; + print WARN basename($errlog) . ": $testname: $_"; } } }