diff options
author | Guilhem Bichot <guilhem.bichot@oracle.com> | 2011-02-11 15:00:09 +0100 |
---|---|---|
committer | Guilhem Bichot <guilhem.bichot@oracle.com> | 2011-02-11 15:00:09 +0100 |
commit | 1756d087cd93dd7e4b4963b9b235daaf5279c8dc (patch) | |
tree | d9d6129bd645413a78561d42f1fdc9c9b94cb638 /mysys | |
parent | 654dd03f2a5e1b532169e435602c92c1da2062b8 (diff) | |
download | mariadb-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')
-rw-r--r-- | mysys/default.c | 4 | ||||
-rw-r--r-- | mysys/my_getopt.c | 4 | ||||
-rw-r--r-- | mysys/typelib.c | 49 |
3 files changed, 28 insertions, 29 deletions
diff --git a/mysys/default.c b/mysys/default.c index 9a4b990f003..25cef2a3a7f 100644 --- a/mysys/default.c +++ b/mysys/default.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2000-2003 MySQL AB, 2008-2009 Sun Microsystems, Inc +/* 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 @@ -376,7 +376,7 @@ static int handle_default_option(void *in_ctx, const char *group_name, if (!option) return 0; - if (find_type((char *)group_name, ctx->group, 3)) + if (find_type((char *)group_name, ctx->group, FIND_TYPE_NO_PREFIX)) { if (!(tmp= alloc_root(ctx->alloc, strlen(option) + 1))) return 1; diff --git a/mysys/my_getopt.c b/mysys/my_getopt.c index 6f7ed070ccf..ac4ae46eab5 100644 --- a/mysys/my_getopt.c +++ b/mysys/my_getopt.c @@ -1,4 +1,4 @@ -/* Copyright (C) 2002-2006 MySQL AB, 2008-2009 Sun Microsystems, Inc +/* Copyright (c) 2002, 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 @@ -695,7 +695,7 @@ static int setval(const struct my_option *opts, void *value, char *argument, break; case GET_ENUM: { - int type= find_type(argument, opts->typelib, 2); + int type= find_type(argument, opts->typelib, FIND_TYPE_BASIC); if (type == 0) { /* 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; |