From da96e91524acaf73334fe68ec3e83e896d23463e Mon Sep 17 00:00:00 2001 From: Alexander Nozdrin Date: Mon, 16 May 2011 12:50:42 +0400 Subject: A patch for Bug#12429877: QUERY_TABLES_LIST::SROUTINES IS NOT FREED IN FLUSH_READ_LOCK (VALGRIND WARNING). The problem was that under some circustances the memory allocated for Query_tables_list::sroutines was not freed properly. The cause of this problem was the absence of LEX::restore_backup_query_tables_list() call in one of the branches in mysql_table_grant() function. --- sql/sql_acl.cc | 1 + 1 file changed, 1 insertion(+) (limited to 'sql/sql_acl.cc') diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 18758130767..0eb05489015 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -3644,6 +3644,7 @@ int mysql_table_grant(THD *thd, TABLE_LIST *table_list, { // Should never happen /* Restore the state of binlog format */ DBUG_ASSERT(!thd->is_current_stmt_binlog_format_row()); + thd->lex->restore_backup_query_tables_list(&backup); if (save_binlog_row_based) thd->set_current_stmt_binlog_format_row(); DBUG_RETURN(TRUE); /* purecov: deadcode */ -- cgit v1.2.1 From 25221cccd2dde986c91c1ad3bdca3ee2869ed2ac Mon Sep 17 00:00:00 2001 From: Guilhem Bichot Date: Mon, 16 May 2011 22:04:01 +0200 Subject: Fix for BUG#11755168 '46895: test "outfile_loaddata" fails (reproducible)'. In sql_class.cc, 'row_count', of type 'ha_rows', was used as last argument for ER_TRUNCATED_WRONG_VALUE_FOR_FIELD which is "Incorrect %-.32s value: '%-.128s' for column '%.192s' at row %ld". So 'ha_rows' was used as 'long'. On SPARC32 Solaris builds, 'long' is 4 bytes and 'ha_rows' is 'longlong' i.e. 8 bytes. So the printf-like code was reading only the first 4 bytes. Because the CPU is big-endian, 1LL is 0x00 0x00 0x00 0x00 0x00 0x00 0x00 0x01 so the first four bytes yield 0. So the warning message had "row 0" instead of "row 1" in test outfile_loaddata.test: -Warning 1366 Incorrect string value: '\xE1\xE2\xF7' for column 'b' at row 1 +Warning 1366 Incorrect string value: '\xE1\xE2\xF7' for column 'b' at row 0 All error-messaging functions which internally invoke some printf-life function are potential candidate for such mistakes. One apparently easy way to catch such mistakes is to use ATTRIBUTE_FORMAT (from my_attribute.h). But this works only when call site has both: a) the format as a string literal b) the types of arguments. So: func(ER(ER_BLAH), 10); will silently not be checked, because ER(ER_BLAH) is not known at compile time (it is known at run-time, and depends on the chosen language). And func("%s", a va_list argument); has the same problem, as the *real* type of arguments is not known at this site at compile time (it's known in some caller). Moreover, func(ER(ER_BLAH)); though possibly correct (if ER(ER_BLAH) has no '%' markers), will not compile (gcc says "error: format not a string literal and no format arguments"). Consequences: 1) ATTRIBUTE_FORMAT is here added only to functions which in practice take "string literal" formats: "my_error_reporter" and "print_admin_msg". 2) it cannot be added to the other functions: my_error(), push_warning_printf(), Table_check_intact::report_error(), general_log_print(). To do a one-time check of functions listed in (2), the following "static code analysis" has been done: 1) replace my_error(ER_xxx, arguments for substitution in format) with the equivalent my_printf_error(ER_xxx,ER(ER_xxx), arguments for substitution in format), so that we have ER(ER_xxx) and the arguments *in the same call site* 2) add ATTRIBUTE_FORMAT to push_warning_printf(), Table_check_intact::report_error(), general_log_print() 3) replace ER(xxx) with the hard-coded English text found in errmsg.txt (like: ER(ER_UNKNOWN_ERROR) is replaced with "Unknown error"), so that a call site has the format as string literal 4) this way, ATTRIBUTE_FORMAT can effectively do its job 5) compile, fix errors detected by ATTRIBUTE_FORMAT 6) revert steps 1-2-3. The present patch has no compiler error when submitted again to the static code analysis above. It cannot catch all problems though: see Field::set_warning(), in which a call to push_warning_printf() has a variable error (thus, not replacable by a string literal); I checked set_warning() calls by hand though. See also WL 5883 for one proposal to avoid such bugs from appearing again in the future. The issues fixed in the patch are: a) mismatch in types (like 'int' passed to '%ld') b) more arguments passed than specified in the format. This patch resolves mismatches by changing the type/number of arguments, not by changing error messages of sql/share/errmsg.txt. The latter would be wrong, per the following old rule: errmsg.txt must be as stable as possible; no insertions or deletions of messages, no changes of type or number of printf-like format specifiers, are allowed, as long as the change impacts a message already released in a GA version. If this rule is not followed: - Connectors, which use error message numbers, will be confused (by insertions/deletions of messages) - using errmsg.sys of MySQL 5.1.n with mysqld of MySQL 5.1.(n+1) could produce wrong messages or crash; such usage can easily happen if installing 5.1.(n+1) while /etc/my.cnf still has --language=/path/to/5.1.n/xxx; or if copying mysqld from 5.1.(n+1) into a 5.1.n installation. When fixing b), I have verified that the superfluous arguments were not used in the format in the first 5.1 GA (5.1.30 'bteam@astra04-20081114162938-z8mctjp6st27uobm'). Had they been used, then passing them today, even if the message doesn't use them anymore, would have been necessary, as explained above. --- sql/sql_acl.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) (limited to 'sql/sql_acl.cc') diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 718da07bc86..6d5d34d0602 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -1970,13 +1970,12 @@ static int replace_user_table(THD *thd, TABLE *table, const LEX_USER &combo, */ else if (!password_len && no_auto_create) { - my_error(ER_PASSWORD_NO_MATCH, MYF(0), combo.user.str, combo.host.str); + my_error(ER_PASSWORD_NO_MATCH, MYF(0)); goto end; } else if (!can_create_user) { - my_error(ER_CANT_CREATE_USER_WITH_GRANT, MYF(0), - thd->security_ctx->user, thd->security_ctx->host_or_ip); + my_error(ER_CANT_CREATE_USER_WITH_GRANT, MYF(0)); goto end; } old_row_exists = 0; -- cgit v1.2.1 From 92b901d297e666b20343e00c4e0b39b5843299ce Mon Sep 17 00:00:00 2001 From: Tatjana Azundris Nuernberg Date: Thu, 19 May 2011 10:47:43 +0100 Subject: Bug#11745920/Bug#21287: "SSL connection error" is not helpful! (ssl-verify-server-cert=true vs localhos) SSL errors on client and now more specific to aid end-user with debugging. Also restructures error handling for compliance with SSL docs. --- sql/sql_acl.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sql/sql_acl.cc') diff --git a/sql/sql_acl.cc b/sql/sql_acl.cc index 0eb05489015..5056fb32bc7 100644 --- a/sql/sql_acl.cc +++ b/sql/sql_acl.cc @@ -8528,14 +8528,14 @@ static ulong parse_client_handshake_packet(MPVIO_EXT *mpvio, DBUG_PRINT("info", ("client capabilities: %lu", mpvio->client_capabilities)); if (mpvio->client_capabilities & CLIENT_SSL) { - char error_string[1024] __attribute__((unused)); + unsigned long errptr; /* Do the SSL layering. */ if (!ssl_acceptor_fd) return packet_error; DBUG_PRINT("info", ("IO layer change in progress...")); - if (sslaccept(ssl_acceptor_fd, net->vio, net->read_timeout)) + if (sslaccept(ssl_acceptor_fd, net->vio, net->read_timeout, &errptr)) { DBUG_PRINT("error", ("Failed to accept new SSL connection")); return packet_error; -- cgit v1.2.1