summaryrefslogtreecommitdiff
path: root/mysys/typelib.c
diff options
context:
space:
mode:
authorGuilhem Bichot <guilhem.bichot@oracle.com>2011-02-11 15:00:09 +0100
committerGuilhem Bichot <guilhem.bichot@oracle.com>2011-02-11 15:00:09 +0100
commit1756d087cd93dd7e4b4963b9b235daaf5279c8dc (patch)
treed9d6129bd645413a78561d42f1fdc9c9b94cb638 /mysys/typelib.c
parent654dd03f2a5e1b532169e435602c92c1da2062b8 (diff)
downloadmariadb-git-1756d087cd93dd7e4b4963b9b235daaf5279c8dc.tar.gz
Fix for BUG#59894
"set optimizer_switch to e or d causes invalid memory writes/valgrind warnings": due to prefix support, the argument "e" was overwritten with its full value "engine_condition_pushdown", which caused a buffer overrun. This was wrong usage of find_type(); other wrong usages are fixed here too. Please start reading with the comment of typelib.c. client/mysqldump.c: A bug: find_type() expects a bitmap as 3rd argument (each bit is a flag controlling a behaviour of the function); here it was instead passed the length of the string to search! That could give random behaviour of find_type() depending on the string. We rather need to pass a correct flag to find_type(). The correct flag is FIND_TYPE_BASIC (0). Flag 8 is not needed as buff cannot have a comma (see how buff is filled). Flag 1 looks like a superfluous restriction. Flag 4 is not user-friendly (why use --compatible=2 rather than --compatible=mysql40 ?, and we probably not commit to "2" always meaning "mysql40" until the end of times). include/mysql.h.pp: This isn't a problematic API change as we go from char* to const char*: existing code will run unchanged. include/typelib.h: named constants. Not an enum to not significantly change the declaration of find_type() which would be an API change (typelib.h is included in mysql.h). mysql-test/r/mysqldump.result: correct result (see the two requested modes in SQL_MODE) mysql-test/suite/sys_vars/t/optimizer_switch_basic.test: test for BUG#59894. The second SET used to crash. mysql-test/t/mysqldump.test: we had no test for multiple modes in --compatible, which is supported according to --help mysys/typelib.c: Fix for BUG#59894. parse_name() is asked to match "e" with a row of the TYPELIB (the TYPELIB lists permitted flags of optimizer_switch; and comes from optimizer_switch_names[] of sys_vars.cc). find_type() is capable of supporting prefixes, but if it is not passed flag 2 in third argument, it will overwrite its first argument (the string to search for) with the complete name, here overwriting "e" with "engine_condition_pushdown". But as this "e" was a buffer allocated in an Item, it was not big enough to host the longer name, thus the crash. We don't need to know the complete flag's name; the output used from find_type() is just the flag's number (== function's return code). So we can pass flag 2 to find_type() in parse_name(). After doing this fix and the other fixes in this patch, all usages of find_type() were using flag 2; in most usages the string to search for, is not guaranteed to be long enough to host the complete name (it is either directly from argv, or from alloc_root/my_malloc done in an earlier call). Thus, flag 2 is here made implicit: callers need not pass it anymore, it is always automatically turned on. This allows to eliminate an oddity: parse_name() took a const char**, and then removed "const" before calling find_type(), which could theoretically modify the pointed data, thus lying on constness. Last, constants for find_type() are now named. sql-common/client.c: Two bugs: 1) The enum was not in sync with the array (due to a bad porting of WL 1054; the extra OPT_ values are about options present in 5.1 and deleted in 5.5); added a compile_time_assert() to make sure this doesn't happen again 2) find_type() was writing past the end of opt_arg; as opt_arg was allocated with alloc_root() with no extra space, this was an overrun; it could be seen when ** building with -DWITH_VALGRIND -DHAVE_purify -DEXTRA_DEBUG ** making execution go through the faulty code; this faulty code is executed only if the client asks to read a configuration file like this: mysql_options(mysql, MYSQL_READ_DEFAULT_FILE, "/tmp/cnf.cnf"); so by adding such line to the start of mysql_client_test.c::client_connect(), we could see the valgrind warning: ==30548== Invalid write of size 1 ==30548== at 0x4C2624C: strcpy (mc_replace_strmem.c:303) ==30548== by 0x48DC29: find_type (typelib.c:120) ==30548== by 0x465686: mysql_read_default_options (client.c:1344) ==30548== by 0x46830F: mysql_real_connect (client.c:2971) ==30548== by 0x409339: client_connect (mysql_client_test.c:331) ==30548== by 0x463A7F: main (mysql_client_test.c:19902) ==30548== Address 0x61875ad is 0 bytes after a block of size 29 alloc'd ==30548== at 0x4C25153: malloc (vg_replace_malloc.c:195) ==30548== by 0x49BFF1: my_malloc (my_malloc.c:38) ==30548== by 0x49C65C: alloc_root (my_alloc.c:166) ==30548== by 0x48EF97: handle_default_option (default.c:381) ==30548== by 0x49068C: search_default_file_with_ext (default.c:992) ==30548== by 0x48F929: search_default_file (default.c:670) ==30548== by 0x48EDC4: my_search_option_files (default.c:312) ==30548== by 0x48F4B1: my_load_defaults (default.c:576) ==30548== by 0x46517A: mysql_read_default_options (client.c:1207) ==30548== by 0x46830F: mysql_real_connect (client.c:2971) ==30548== by 0x409339: client_connect (mysql_client_test.c:331) ==30548== by 0x463A7F: main (mysql_client_test.c:19902) This is fixed by having find_type() not overwrite anymore. sql/sql_help.cc: cast not needed anymore. sql/table.cc: cast not needed anymore.
Diffstat (limited to 'mysys/typelib.c')
-rw-r--r--mysys/typelib.c49
1 files changed, 24 insertions, 25 deletions
diff --git a/mysys/typelib.c b/mysys/typelib.c
index 7681ff581ac..c0d37e26ecf 100644
--- a/mysys/typelib.c
+++ b/mysys/typelib.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2000 MySQL AB
+/* Copyright (c) 2000, 2011, Oracle and/or its affiliates. All rights reserved.
This program is free software; you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
@@ -27,7 +27,7 @@ int find_type_or_exit(const char *x, TYPELIB *typelib, const char *option)
int res;
const char **ptr;
- if ((res= find_type((char *) x, typelib, 2)) <= 0)
+ if ((res= find_type((char *) x, typelib, FIND_TYPE_BASIC)) <= 0)
{
ptr= typelib->type_names;
if (!*x)
@@ -48,16 +48,13 @@ int find_type_or_exit(const char *x, TYPELIB *typelib, const char *option)
Search after a string in a list of strings. Endspace in x is not compared.
@param x String to find
- @param lib TYPELIB (struct of pointer to values + count)
- @param full_name bitmap of what to do
- If & 1 accept only whole names
- If & 2 don't expand if half field
- If & 4 allow #number# as type
- If & 8 use ',' as string terminator
-
- @note
- If part, uniq field is found and full_name == 0 then x is expanded
- to full field.
+ @param typelib TYPELIB (struct of pointer to values + count)
+ @param flags flags to tune behaviour: a combination of
+ FIND_TYPE_NO_PREFIX
+ FIND_TYPE_ALLOW_NUMBER
+ FIND_TYPE_COMMA_TERM.
+ FIND_TYPE_NO_OVERWRITE can be passed but is
+ superfluous (is always implicitely on).
@retval
-1 Too many matching values
@@ -68,15 +65,17 @@ int find_type_or_exit(const char *x, TYPELIB *typelib, const char *option)
*/
-int find_type(char *x, const TYPELIB *typelib, uint full_name)
+int find_type(const char *x, const TYPELIB *typelib, uint flags)
{
int find,pos;
int UNINIT_VAR(findpos); /* guarded by find */
- reg1 char * i;
- reg2 const char *j;
+ const char *i;
+ const char *j;
DBUG_ENTER("find_type");
DBUG_PRINT("enter",("x: '%s' lib: 0x%lx", x, (long) typelib));
+ DBUG_ASSERT(!(flags & ~(FIND_TYPE_NO_PREFIX | FIND_TYPE_ALLOW_NUMBER |
+ FIND_TYPE_NO_OVERWRITE | FIND_TYPE_COMMA_TERM)));
if (!typelib->count)
{
DBUG_PRINT("exit",("no count"));
@@ -86,24 +85,26 @@ int find_type(char *x, const TYPELIB *typelib, uint full_name)
for (pos=0 ; (j=typelib->type_names[pos]) ; pos++)
{
for (i=x ;
- *i && (!(full_name & 8) || !is_field_separator(*i)) &&
+ *i && (!(flags & FIND_TYPE_COMMA_TERM) || !is_field_separator(*i)) &&
my_toupper(&my_charset_latin1,*i) ==
my_toupper(&my_charset_latin1,*j) ; i++, j++) ;
if (! *j)
{
while (*i == ' ')
i++; /* skip_end_space */
- if (! *i || ((full_name & 8) && is_field_separator(*i)))
+ if (! *i || ((flags & FIND_TYPE_COMMA_TERM) && is_field_separator(*i)))
DBUG_RETURN(pos+1);
}
- if ((!*i && (!(full_name & 8) || !is_field_separator(*i))) &&
- (!*j || !(full_name & 1)))
+ if ((!*i &&
+ (!(flags & FIND_TYPE_COMMA_TERM) || !is_field_separator(*i))) &&
+ (!*j || !(flags & FIND_TYPE_NO_PREFIX)))
{
find++;
findpos=pos;
}
}
- if (find == 0 && (full_name & 4) && x[0] == '#' && strend(x)[-1] == '#' &&
+ if (find == 0 && (flags & FIND_TYPE_ALLOW_NUMBER) && x[0] == '#' &&
+ strend(x)[-1] == '#' &&
(findpos=atoi(x+1)-1) >= 0 && (uint) findpos < typelib->count)
find=1;
else if (find == 0 || ! x[0])
@@ -111,13 +112,11 @@ int find_type(char *x, const TYPELIB *typelib, uint full_name)
DBUG_PRINT("exit",("Couldn't find type"));
DBUG_RETURN(0);
}
- else if (find != 1 || (full_name & 1))
+ else if (find != 1 || (flags & FIND_TYPE_NO_PREFIX))
{
DBUG_PRINT("exit",("Too many possybilities"));
DBUG_RETURN(-1);
}
- if (!(full_name & 2))
- (void) strmov(x,typelib->type_names[findpos]);
DBUG_RETURN(findpos+1);
} /* find_type */
@@ -192,7 +191,7 @@ my_ulonglong find_typeset(char *x, TYPELIB *lib, int *err)
x++;
if (x[0] && x[1]) /* skip separator if found */
x++;
- if ((find= find_type(i, lib, 2 | 8) - 1) < 0)
+ if ((find= find_type(i, lib, FIND_TYPE_COMMA_TERM) - 1) < 0)
DBUG_RETURN(0);
result|= (ULL(1) << find);
}
@@ -276,7 +275,7 @@ static TYPELIB on_off_default_typelib= {array_elements(on_off_default_names)-1,
static uint parse_name(const TYPELIB *lib, const char **strpos, const char *end)
{
const char *pos= *strpos;
- uint find= find_type((char*)pos, lib, 8);
+ uint find= find_type(pos, lib, FIND_TYPE_COMMA_TERM);
for (; pos != end && *pos != '=' && *pos !=',' ; pos++);
*strpos= pos;
return find;