INC_HOST_ERRORS() IS CALLED.
Issue : Sequence of calling inc_host_errors()
and reset_host_errors() required some
changes in order to maintain correct
connection error count.
Solution : Call to reset_host_errors() is shifted
to a location after which no calls to
inc_host_errors() are made.
INC_HOST_ERRORS() IS CALLED.
Issue : Sequence of calling inc_host_errors()
and reset_host_errors() required some
changes in order to maintain correct
connection error count.
Solution : Call to reset_host_errors() is shifted
to a location after which no calls to
inc_host_errors() are made.
Analysis:
-------------
If server is started with limit of MAX_CONNECTIONS and
MAX_USER_CONNECTIONS then only MAX_USER_CONNECTIONS of any particular
users can be connected to server and total MAX_CONNECTIONS of client can
be connected to server.
Server maintains a counter for total CONNECTIONS and total CONNECTIONS
from particular user.
Here, MAX_CONNECTIONS of connections are created to server. Out of this
MAX_CONNECTIONS, connections from particular user (say USER1) are
also created. The connections from USER1 is lesser than
MAX_USER_CONNECTIONS. After that there was one more connection request from
USER1. Since USER1 can still create connections as he havent reached
MAX_USER_CONNECTIONS, server increments counter of CONNECTIONS per user.
As server already has MAX_CONNECTIONS of connections, next check to total
CONNECTION count fails. In this case control is returned WITHOUT
decrementing the CONNECTIONS per user. So the counter per user CONNECTIONS goes
on incrementing for each attempt until current connections are closed.
And because of this counter per CONNECTIONS reached MAX_USER_CONNECTIONS.
So, next connections form USER1 user always returns with MAX_USER_CONNECTION
limit error, even when total connection to sever are less than MAX_CONNECTIONS.
Fix:
-------------
This issue is occurred because of not handling counters properly in the
server. Changed the code to handle per user connection counters properly.
If init_command was incorrect, we couldn't let users execute
queries, but we couldn't report the issue to the client either
as it does not expect error messages before even sending a
command. Thus, we simply disconnected them without throwing
a clear error.
We now go through the proper sequence once (without executing
any user statements) so we can report back what the problem
is. Only then do we disconnect the user.
As always, root remains unaffected by this as init_command is
(still) not executed for them.
The problem is that clients implementing the 4.0 version of the
protocol (that is, mysql-4.0) do not null terminate a string
at the end of the authentication packet. These clients denote
the end of the string with the end of the packet.
Although this goes against the documented (see MySQL Internals
ClientServer Protocol wiki) description of the protocol, these
old clients still need to be supported.
The solution is to support the documented and actual behavior
of the clients. If a client is using the pre-4.1 version of
the protocol, the end of a string in the authentication packet
can either be denoted with a null character or by the end of
the packet. This restores backwards compatibility with old
clients implementing either the documented or actual behavior.
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.
Certain fields in the protcol required a strict formatting. If
off bound values were sent to the server this could under some
circumstances lead to a crash on the Windows platform.
A separate fix for 5.1 (as 5.1 and 5.5 have seriously
differged in the related pieces of the code).
A patch for 5.5 was approved earlier.
Problem: ucs2 was correctly disallowed in "SET NAMES" only,
while mysql_real_connect() and mysql_change_user() still allowed
to use ucs2, which made server crash.
Fix: disallow ucs2 in mysql_real_connect() and mysql_change_user().
@ sql/sql_priv.h
- changing return type for thd_init_client_charset() to bool,
to return errors to the caller
@ sql/sql_var.cc
- using new function
@ sql/sql_connect.cc
- thd_client_charset_init:
in case of unsupported client character set send error and return true;
in case of success return false
- check_connection:
Return error if character set initialization failed
@ sql/sql_parse.cc
- check charset in the very beginnig of the CMD_CHANGE_USER handling code
@ tests/mysql_client_test.c
- adding tests
Problem: ucs2 was correctly disallowed in "SET NAMES" only,
while mysql_real_connect() and mysql_change_user() still allowed
to use ucs2, which made server crash.
Fix: disallow ucs2 in mysql_real_connect() and mysql_change_user().
@ sql/set_var.cc
Using new function.
@ sql/sql_acl.cc
- Return error if character set initialization failed
- Getting rid of pointer aliasing:
Initialize user_name to NULL, to avoid double free().
@ sql/sql_connect.cc
- in case of unsupported client character set send error and return true
- in case of success return false
@ sql/sql_connect.h
- changing return type for thd_init_client_charset() to bool,
to return errors to the caller
@ sql/sql_parse.h
- introducing a new function, to reuse in all places where we need
to check client character set.
@ tests/mysql_client_test.c
Adding test
The problem is a race between a session closing its vio
(i.e. after a COM_QUIT) at the same time it is being killed by
another thread. This could trigger a assertion in vio_close()
as the two threads could end up closing the same vio, at the
same time. This could happen due to the implementation of
SIGNAL_WITH_VIO_CLOSE, which closes the vio of the thread
being killed.
The solution is to serialize the close of the Vio under
LOCK_thd_data, which protects THD data.
No regression test is added as this is essentially a debug
issue and the test case would be quite convoluted as we would
need to synchronize a session that is being killed -- which
is a bit difficult since debug sync points code does not
synchronize killed sessions.
Fixed the references to security_ctx->priv_user
to be real char * pointers instead of a C array name reference.
This is somehow important for some 3d party
dtrace replacements
MySQL 5.1 server
Server used to clip overly long user-names. This was presumably lost
when code was made UTF8-clean.
Now we emulate the behaviour for backward compatibility, but UTF8-ly
correct.
Essentially, the problem is that safemalloc is excruciatingly
slow as it checks all allocated blocks for overrun at each
memory management primitive, yielding a almost exponential
slowdown for the memory management functions (malloc, realloc,
free). The overrun check basically consists of verifying some
bytes of a block for certain magic keys, which catches some
simple forms of overrun. Another minor problem is violation
of aliasing rules and that its own internal list of blocks
is prone to corruption.
Another issue with safemalloc is rather the maintenance cost
as the tool has a significant impact on the server code.
Given the magnitude of memory debuggers available nowadays,
especially those that are provided with the platform malloc
implementation, maintenance of a in-house and largely obsolete
memory debugger becomes a burden that is not worth the effort
due to its slowness and lack of support for detecting more
common forms of heap corruption.
Since there are third-party tools that can provide the same
functionality at a lower or comparable performance cost, the
solution is to simply remove safemalloc. Third-party tools
can provide the same functionality at a lower or comparable
performance cost.
The removal of safemalloc also allows a simplification of the
malloc wrappers, removing quite a bit of kludge: redefinition
of my_malloc, my_free and the removal of the unused second
argument of my_free. Since free() always check whether the
supplied pointer is null, redudant checks are also removed.
Also, this patch adds unit testing for my_malloc and moves
my_realloc implementation into the same file as the other
memory allocation primitives.
In order to allow thread schedulers to be dynamically loaded,
it is necessary to make the following changes to the server:
- Two new service interfaces
- Modifications to InnoDB to inform the thread scheduler of state changes.
- Changes to the VIO subsystem for checking if data is available on a socket.
- Elimination of remains of the old thread pool implementation.
The two new service interfaces introduces are:
my_thread_scheduler
A service interface to register a thread
scheduler.
thd_wait
A service interface to inform thread scheduler
that the thread is about to start waiting.
In addition, the patch adds code that:
- Add a call to thd_wait for table locks in mysys
thd_lock.c by introducing a set function that
can be used to set a callback to be used when
waiting on a lock and resuming from waiting.
- Calling the mysys set function from the server
to set the callbacks correctly.
Conflicts:
Text conflict in mysql-test/r/explain.result
Text conflict in mysql-test/t/explain.test
Text conflict in sql/net_serv.cc
Text conflict in sql/sp_head.cc
Text conflict in sql/sql_priv.h
ChangeSet@1.2703, 2007-12-07 09:35:28-05:00, cmiller@zippy.cornsilk.net +40 -0
Bug#13174: SHA2 function
Patch contributed from Bill Karwin, paper unnumbered CLA in Seattle
Implement SHA2 functions.
Chad added code to make it work with YaSSL. Also, he removed the
(probable) bug of embedded server never using SSL-dependent
functions. (libmysqld/Makefile.am didn't read ANY autoconf defs.)
Function specification:
SHA2( string cleartext, integer hash_length )
-> string hash, or NULL
where hash_length is one of 224, 256, 384, or 512. If either is
NULL or a length is unsupported, then the result is NULL. The
resulting string is always the length of the hash_length parameter
or is NULL.
Include the canonical hash examples from the NIST in the test
results.
---
Polish and address concerns of reviewers.
Adding my_global.h first in all files using
NO_EMBEDDED_ACCESS_CHECKS.
Correcting a merge problem resulting from a changed definition
of check_some_access compared to the original patches.
