Commit e02c3d7f authored by Magne Mahre's avatar Magne Mahre

BUG#12589870 CRASHES WITH MULTIQUERY PACKET + USE<DB> + QUERY CACHE

 
A buffer large enough to hold the query _plus_ some additional
data is allocated before parsing is started.   The additional data 
is used by the query cache, and consists of the name of the current 
database and a set of flags.
 
When a packet containing multiple SQL statements is sent to the
server and one of the statements changes the current database
(a "USE <db>" statement), and the name of the new current database 
is longer than of the previous,  there is not enough space in the 
buffer for the new name, and we write out over the buffer boundary.

The fix adds an extra field to store the number of bytes
allocated to the database name in the buffer.  If the current
database name changes, and the new name is longer than the
previous one, we refuse to cache the query.
parent 41dc3049
...@@ -1029,12 +1029,22 @@ subst_spvars(THD *thd, sp_instr *instr, LEX_STRING *query_str) ...@@ -1029,12 +1029,22 @@ subst_spvars(THD *thd, sp_instr *instr, LEX_STRING *query_str)
/* /*
Allocate additional space at the end of the new query string for the Allocate additional space at the end of the new query string for the
query_cache_send_result_to_client function. query_cache_send_result_to_client function.
The query buffer layout is:
buffer :==
<statement> The input statement(s)
'\0' Terminating null char
<length> Length of following current database name (size_t)
<db_name> Name of current database
<flags> Flags struct
*/ */
buf_len= qbuf.length() + thd->db_length + 1 + QUERY_CACHE_FLAGS_SIZE + 1; buf_len= qbuf.length() + 1 + sizeof(size_t) + thd->db_length +
QUERY_CACHE_FLAGS_SIZE + 1;
if ((pbuf= (char *) alloc_root(thd->mem_root, buf_len))) if ((pbuf= (char *) alloc_root(thd->mem_root, buf_len)))
{ {
memcpy(pbuf, qbuf.ptr(), qbuf.length()); memcpy(pbuf, qbuf.ptr(), qbuf.length());
pbuf[qbuf.length()]= 0; pbuf[qbuf.length()]= 0;
*(size_t *)(pbuf+qbuf.length()+1)= thd->db_length;
} }
else else
DBUG_RETURN(TRUE); DBUG_RETURN(TRUE);
......
...@@ -1241,8 +1241,8 @@ def_week_frmt: %lu, in_trans: %d, autocommit: %d", ...@@ -1241,8 +1241,8 @@ def_week_frmt: %lu, in_trans: %d, autocommit: %d",
/* Key is query + database + flag */ /* Key is query + database + flag */
if (thd->db_length) if (thd->db_length)
{ {
memcpy(thd->query() + thd->query_length() + 1, thd->db, memcpy(thd->query() + thd->query_length() + 1 + sizeof(size_t),
thd->db_length); thd->db, thd->db_length);
DBUG_PRINT("qcache", ("database: %s length: %u", DBUG_PRINT("qcache", ("database: %s length: %u",
thd->db, (unsigned) thd->db_length)); thd->db, (unsigned) thd->db_length));
} }
...@@ -1251,7 +1251,7 @@ def_week_frmt: %lu, in_trans: %d, autocommit: %d", ...@@ -1251,7 +1251,7 @@ def_week_frmt: %lu, in_trans: %d, autocommit: %d",
DBUG_PRINT("qcache", ("No active database")); DBUG_PRINT("qcache", ("No active database"));
} }
tot_length= thd->query_length() + thd->db_length + 1 + tot_length= thd->query_length() + thd->db_length + 1 +
QUERY_CACHE_FLAGS_SIZE; sizeof(size_t) + QUERY_CACHE_FLAGS_SIZE;
/* /*
We should only copy structure (don't use it location directly) We should only copy structure (don't use it location directly)
because of alignment issue because of alignment issue
...@@ -1462,7 +1462,28 @@ Query_cache::send_result_to_client(THD *thd, char *sql, uint query_length) ...@@ -1462,7 +1462,28 @@ Query_cache::send_result_to_client(THD *thd, char *sql, uint query_length)
goto err; goto err;
} }
} }
{
/*
We have allocated buffer space (in alloc_query) to hold the
SQL statement(s) + the current database name + a flags struct.
If the database name has changed during execution, which might
happen if there are multiple statements, we need to make
sure the new current database has a name with the same length
as the previous one.
*/
size_t *db_len= (size_t *) (sql + query_length + 1);
if (thd->db_length != *db_len)
{
/*
We should probably reallocate the buffer in this case,
but for now we just leave it uncached
*/
DBUG_PRINT("qcache",
("Current database has changed since start of query"));
goto err;
}
}
/* /*
Try to obtain an exclusive lock on the query cache. If the cache is Try to obtain an exclusive lock on the query cache. If the cache is
disabled or if a full cache flush is in progress, the attempt to disabled or if a full cache flush is in progress, the attempt to
...@@ -1484,10 +1505,12 @@ Query_cache::send_result_to_client(THD *thd, char *sql, uint query_length) ...@@ -1484,10 +1505,12 @@ Query_cache::send_result_to_client(THD *thd, char *sql, uint query_length)
Query_cache_block *query_block; Query_cache_block *query_block;
tot_length= query_length + thd->db_length + 1 + QUERY_CACHE_FLAGS_SIZE; tot_length= query_length + 1 + sizeof(size_t) +
thd->db_length + QUERY_CACHE_FLAGS_SIZE;
if (thd->db_length) if (thd->db_length)
{ {
memcpy(sql+query_length+1, thd->db, thd->db_length); memcpy(sql + query_length + 1 + sizeof(size_t), thd->db, thd->db_length);
DBUG_PRINT("qcache", ("database: '%s' length: %u", DBUG_PRINT("qcache", ("database: '%s' length: %u",
thd->db, (unsigned)thd->db_length)); thd->db, (unsigned)thd->db_length));
} }
......
...@@ -1912,13 +1912,30 @@ bool alloc_query(THD *thd, const char *packet, uint packet_length) ...@@ -1912,13 +1912,30 @@ bool alloc_query(THD *thd, const char *packet, uint packet_length)
pos--; pos--;
packet_length--; packet_length--;
} }
/* We must allocate some extra memory for query cache */ /* We must allocate some extra memory for query cache
The query buffer layout is:
buffer :==
<statement> The input statement(s)
'\0' Terminating null char (1 byte)
<length> Length of following current database name (size_t)
<db_name> Name of current database
<flags> Flags struct
*/
if (! (query= (char*) thd->memdup_w_gap(packet, if (! (query= (char*) thd->memdup_w_gap(packet,
packet_length, packet_length,
1 + thd->db_length + 1 + sizeof(size_t) + thd->db_length +
QUERY_CACHE_FLAGS_SIZE))) QUERY_CACHE_FLAGS_SIZE)))
return TRUE; return TRUE;
query[packet_length]= '\0'; query[packet_length]= '\0';
/*
Space to hold the name of the current database is allocated. We
also store this length, in case current database is changed during
execution. We might need to reallocate the 'query' buffer
*/
size_t *len = (size_t *) (query + packet_length + 1);
*len= thd->db_length;
thd->set_query(query, packet_length); thd->set_query(query, packet_length);
/* Reclaim some memory */ /* Reclaim some memory */
......
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