Hi Dmitry,
On 11/24/10 7:07 AM, Dmitry Shulga wrote:
> #At file:///Users/shulga/projects/mysql/5.1-bugteam-bug58026/ based on
> revid:davi.arnaut@stripped
>
> 3514 Dmitry Shulga 2010-11-24
> Fixed bug#58026 - massive recursion and crash in regular expression
> handling.
Looks good so far, a few comments below.
> The problem was that parsing of nested regular expression involved
> recursive calls. Such recursion was not take care of available memory,
"Such recursion was not take care of available memory," sounds strange.
I suggest rewriting to:
"Such recursion didn't take into account the amount of available stack
space, which ended up leading to stack overflow crashes."
> which may lead to stack overflow crashes.
> @ client/mysqltest.cc
> Passed NULL pointer to function as last actual argument in call
> to my_regcomp. In this case check for enough memory in stack
> doesn't occur. Such checking doesn't need in mysql clients.
"Such checking is not necessary in mysql clients"
> @ regex/main.c
> Passed NULL pointer to function as last actual argument in call
> to my_regcomp. In this case check for enough memory
> in stack doesn't occur. Such checking doesn't need in regex's
> regression tests.
> @ regex/my_regex.h
> added pointer to function as last argument of my_regcomp() for check
> enough memory in stack.
> @ regex/regcomp.c
> Added pointer to function as last argument of p_ere()/p_ere_exp()
> that called in order to check that there are enough memory in stack
> for next recursion call.
> @ regex/regcomp.ih
> added pointer to function as last argument of my_regcomp() for check
> enough memory in stack.
> @ sql/item_cmpfunc.cc
> Passed pointer to function check_enough_stack_size() in call to
> my_regcomp(). The function check_enough_stack_size() will be
> called in during recursive descendant for regular expression parsing.
[..]
>
> === modified file 'regex/my_regex.h'
> --- a/regex/my_regex.h 2005-09-29 00:08:24 +0000
> +++ b/regex/my_regex.h 2010-11-24 09:07:00 +0000
> @@ -28,7 +28,8 @@ typedef struct {
>
>
> /* === regcomp.c === */
> -extern int my_regcomp(my_regex_t *, const char *, int, CHARSET_INFO *charset);
> +extern int my_regcomp(my_regex_t *, const char *, int, CHARSET_INFO *charset,
> + int (*check_enough_mem_in_stack)());
Please introduce a typedef for the stack check function. Suggestion:
typedef int (*my_regex_stack_check_t)();
To be added to my_regex.h
> #define REG_BASIC 0000
> #define REG_EXTENDED 0001
> #define REG_ICASE 0002
>
[..]
>
> === modified file 'sql/item_cmpfunc.cc'
> --- a/sql/item_cmpfunc.cc 2010-09-09 12:48:06 +0000
> +++ b/sql/item_cmpfunc.cc 2010-11-24 09:07:00 +0000
> @@ -4791,6 +4791,18 @@ void Item_func_like::cleanup()
>
> #ifdef USE_REGEX
>
> +extern "C"
> +int
> +check_enough_stack_size()
Hum, since the hook is global, there isn't much reason to pass the stack
check function each time we need to compile a expression.
Just add the hook to regcomp.c with internal linkage:
static my_regex_stack_check_t stack_check;
and we let a hook be registered at my_regex_init time:
my_regex_init(CHARSET_INFO *cs, my_regex_stack_check_t stack_check);
In this case, my_regex_init sets the hook to the given one.
> +{
> +#ifndef EMBEDDED_LIBRARY // Avoid compiler warning
> + uchar stack_top;
> +#endif
> + return check_stack_overrun(current_thd, STACK_MIN_SIZE,
> +&stack_top);
This won't compile with embedded? stack_top won't have been declared.
Regards,
Davi
