List:Commits« Previous MessageNext Message »
From:Davi Arnaut Date:January 17 2011 9:18am
Subject:Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514)
Bug#58026
View as plain text  
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
Thread
bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514) Bug#58026Dmitry Shulga24 Nov
  • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514)Bug#58026Davi Arnaut17 Jan
    • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514) Bug#58026Dmitry Shulga18 Jan
      • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514)Bug#58026Davi Arnaut18 Jan
        • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514) Bug#58026Dmitry Shulga19 Jan
          • Re: bzr commit into mysql-5.1-bugteam branch (Dmitry.Shulga:3514)Bug#58026Davi Arnaut19 Jan