| Bug #57112 | Incorrect option parsing code in my_getopt.c | ||
|---|---|---|---|
| Submitted: | 29 Sep 2010 18:16 | Modified: | 30 Sep 2010 18:04 |
| Reporter: | Sasha Pachev | Email Updates: | |
| Status: | Verified | Impact on me: | |
| Category: | MySQL Server: Options | Severity: | S3 (Non-critical) |
| Version: | 5.1.49, 5.6.99 | OS: | Any |
| Assigned to: | CPU Architecture: | Any | |
| Tags: | Contribution | ||
[29 Sep 2010 20:42]
Sasha Pachev
Patch for the bug, tested with Percona Server 11.3 which is based off MySQL 5.1.49, should work with all versions that have the same code in my_getopt.c:
--- Percona-Server-11.3-orig/mysys/my_getopt.c 2010-07-09 05:34:53.000000000 -0700
+++ Percona-Server-11.3/mysys/my_getopt.c 2010-09-29 13:27:20.000000000 -0700
@@ -1187,17 +1187,28 @@
{
const char *comment= optp->comment, *end= strend(comment);
- while ((uint) (end - comment) > comment_space)
+ while (comment + comment_space < end)
{
- for (line_end= comment + comment_space; *line_end != ' '; line_end--);
- for (; comment != line_end; comment++)
+ for (line_end= comment + comment_space; *line_end != ' ' &&
+ line_end > comment; line_end--);
+ if (line_end == comment)
+ {
+ for (line_end = comment + comment_space;
+ *line_end != ' ' && line_end < end; line_end++);
+ }
+ for (; comment < line_end; comment++)
putchar(*comment);
- comment++; /* skip the space, as a newline will take it's place now */
- putchar('\n');
+ if (*comment == ' ')
+ {
+ comment++; /* skip the space, as a newline will take it's place now */
+ putchar('\n');
+ }
for (col= 0; col < name_space; col++)
putchar(' ');
}
- printf("%s", comment);
+
+ if (comment < end)
+ printf("%s", comment);
}
putchar('\n');
}
[30 Sep 2010 18:04]
Sveta Smirnova
Thank you for the report. Verified as described.
[30 Sep 2010 18:27]
Sveta Smirnova
Crash information: 100930 20:26:25 [Note] Plugin 'FEDERATED' is disabled. 100930 20:26:26 - mysqld got signal 11 ; This could be because you hit a bug. It is also possible that this binary or one of the libraries it was linked against is corrupt, improperly built, or misconfigured. This error can also be caused by malfunctioning hardware. We will try our best to scrape up some info that will hopefully help diagnose the problem, but since we have already crashed, something is definitely wrong and this may fail. key_buffer_size=8388608 read_buffer_size=131072 max_used_connections=0 max_threads=151 thread_count=0 connection_count=0 It is possible that mysqld could use up to key_buffer_size + (read_buffer_size + sort_buffer_size)*max_threads = 338565 K bytes of memory Hope that's ok; if not, decrease some variables in the equation. thd: 0x0 Attempting backtrace. You can use the following information to find out where mysqld died. If you see no messages after this, something went terribly wrong... stack_bottom = (nil) thread_stack 0x40000 /users/ssmirnova/blade12/src/mysql-next-mr/sql/mysqld(my_print_stacktrace+0x35)[0xa41d1c] /users/ssmirnova/blade12/src/mysql-next-mr/sql/mysqld(handle_segfault+0x2ae)[0x52a4c4] /lib64/libpthread.so.0[0x3429e0dd40] /users/ssmirnova/blade12/src/mysql-next-mr/sql/mysqld(my_print_help+0x2d7)[0xa38cfa] /users/ssmirnova/blade12/src/mysql-next-mr/sql/mysqld[0x52fc07] /users/ssmirnova/blade12/src/mysql-next-mr/sql/mysqld[0x52fd16] /users/ssmirnova/blade12/src/mysql-next-mr/sql/mysqld(unireg_abort+0x36)[0x528bca] /users/ssmirnova/blade12/src/mysql-next-mr/sql/mysqld[0x52cd97] /users/ssmirnova/blade12/src/mysql-next-mr/sql/mysqld(_Z11mysqld_mainiPPc+0x42f)[0x52d8b1] /users/ssmirnova/blade12/src/mysql-next-mr/sql/mysqld(main+0x20)[0x526b74] /lib64/libc.so.6(__libc_start_main+0xf4)[0x342921d8a4] /users/ssmirnova/blade12/src/mysql-next-mr/sql/mysqld[0x526a99] The manual page at http://dev.mysql.com/doc/mysql/en/crashing.html contains information that should help you find out what is causing the crash. mysql-test-run: WARNING: Could not parse variable list line : /^K/^O/^X/(/,/[/_/q/u/Ä/Ý/ü/ý/^M0^W0^[0!0'01070@0J0K0i0y0<89>0<9a>0<9b>0¦0§0²0³0´0¸0Æ0Ó0â0ã0ä0å0æ0ç0è0é0ê0ë0ì0í0î0ï0ð0ñ0ò0ó0ô0õ0ö0÷0ø0ù0ú0û0ü0ý0þ0ÿ0^@1^A1^B1^C1^D1^E1^F1^G1^H1 1 @
Description: The following option parsing code is present in my_getopt.c: if (optp->comment && *optp->comment) { const char *comment= optp->comment, *end= strend(comment); while ((uint) (end - comment) > comment_space) { for (line_end= comment + comment_space; *line_end != ' '; line_end--); for (; comment != line_end; comment++) putchar(*comment); comment++; /* skip the space, as a newline will take it's place now */ putchar('\n'); for (col= 0; col < name_space; col++) putchar(' '); } printf("%s", comment); } putchar('\n'); It will break when parsing options that have comments with words that are long, I believe if you make it more than 57 characters, there will be a crash. Currently no MySQL option comments have such words, but I tried to add the following option: {"replicate-rewrite-charset-collation-connection", OPT_REPLICATE_REWRITE_CHARSET_COLLATION_CONNECTION, "Changes the original client collation charset to the given one. " " Example: replicate-rewrite-charset-collation-connection=latin1->utf8. ", 0, 0, 0, GET_STR, REQUIRED_ARG, 0, 0, 0, 0, 0, 0}, in mysqld.cc and eventually discovered the bug in my_print_help() in my_getopt.c There are a couple of issues with my_print_help() code quoted above. First, while ((uint) (end - comment) > comment_space) is unsafe because if comment runs past the end, (uint)(end-comment) will be definitely bigger than comment_space which is set to 57, so this will be true forever and ever causing an infinite loop. for (line_end= comment + comment_space; *line_end != ' '; line_end--); is not safe either because line_end can run to a point prior to the comment pointer if no spaces are found, which it looks like it does in my example. for (; comment != line_end; comment++) putchar(*comment); would have contained the damage from the previous line if it had not wrongfully assumed that comment through some bad luck could end up past line_end to begin with. This line produces beautiful output in my example printing the contents of the heap to stdout until it hits a protected page and segfaults. How to repeat: Try adding a new option to mysqld with the comment pasted in the description. Suggested fix: Safety first! The code should be while (comment + comment_space < end) avoiding the uint cast problem ... for (line_end= comment + comment_space; *line_end != ' ' && comment > line_end; line_end--); avoiding the underrun of line_end to before the comment for (; comment < line_end; comment++) putchar(*comment); avoiding the assumption that comment starts out before line_end