Commit 46ca22b0 authored by Luis Soares's avatar Luis Soares

BUG#48357: SHOW BINLOG EVENTS: Wrong offset or I/O error

In function log_event.cc:Query_log_event::write, there was a cast that
was triggering undefined behavior. The offending cast is the
following:

  write_str_with_code_and_len((char **)(&start),
                              catalog, catalog_len, Q_CATALOG_NZ_CODE);

This results in calling write_str_with_code_and_len with first
argument pointing to a (char **) while "start" is itself a pointer to
uchar (uchar *). Inside write_str_with_..., the content of start is
then be updated:

  (*dst)+= len;

The instruction above would cause the (*dst) pointer (ie, the "start"
argument, from the caller point of view, and which actually points to
uchar instead of pointing to char) to be updated so that it would
increment catalog_len. However, this seems to break strict-aliasing
rules ultimately causing the increment and assignment to behave
unexpectedly.

We fix this by removing the cast and by making the types match.
parent a95f54c6
...@@ -2138,8 +2138,8 @@ void Query_log_event::pack_info(Protocol *protocol) ...@@ -2138,8 +2138,8 @@ void Query_log_event::pack_info(Protocol *protocol)
/** /**
Utility function for the next method (Query_log_event::write()) . Utility function for the next method (Query_log_event::write()) .
*/ */
static void write_str_with_code_and_len(char **dst, const char *src, static void write_str_with_code_and_len(uchar **dst, const char *src,
int len, uint code) uint len, uint code)
{ {
/* /*
only 1 byte to store the length of catalog, so it should not only 1 byte to store the length of catalog, so it should not
...@@ -2234,7 +2234,7 @@ bool Query_log_event::write(IO_CACHE* file) ...@@ -2234,7 +2234,7 @@ bool Query_log_event::write(IO_CACHE* file)
} }
if (catalog_len) // i.e. this var is inited (false for 4.0 events) if (catalog_len) // i.e. this var is inited (false for 4.0 events)
{ {
write_str_with_code_and_len((char **)(&start), write_str_with_code_and_len(&start,
catalog, catalog_len, Q_CATALOG_NZ_CODE); catalog, catalog_len, Q_CATALOG_NZ_CODE);
/* /*
In 5.0.x where x<4 masters we used to store the end zero here. This was In 5.0.x where x<4 masters we used to store the end zero here. This was
...@@ -2272,7 +2272,7 @@ bool Query_log_event::write(IO_CACHE* file) ...@@ -2272,7 +2272,7 @@ bool Query_log_event::write(IO_CACHE* file)
{ {
/* In the TZ sys table, column Name is of length 64 so this should be ok */ /* In the TZ sys table, column Name is of length 64 so this should be ok */
DBUG_ASSERT(time_zone_len <= MAX_TIME_ZONE_NAME_LENGTH); DBUG_ASSERT(time_zone_len <= MAX_TIME_ZONE_NAME_LENGTH);
write_str_with_code_and_len((char **)(&start), write_str_with_code_and_len(&start,
time_zone_str, time_zone_len, Q_TIME_ZONE_CODE); time_zone_str, time_zone_len, Q_TIME_ZONE_CODE);
} }
if (lc_time_names_number) if (lc_time_names_number)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment