Bug #24498 Stack overflow in mysqltest
Submitted: 22 Nov 2006 10:28 Modified: 30 Jan 2007 3:44
Reporter: Vasil Dimov Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Tests Severity:S3 (Non-critical)
Version:5.1.14-beta BK OS:
Assigned to: Magnus Blåudd CPU Architecture:Any

[22 Nov 2006 10:28] Vasil Dimov
Description:
--- cut ---
$ ./mysql-test-run.pl --mysqld=--binlog-format=mixed --do-test=ndb_restore
...
ndb_restore                    [ fail ]

ERROR: mysqltest returned unexpected code 138, it has probably crashed

Aborting: ndb_restore failed in default mode. To continue, re-run with '--force'
--- cut ---

This is the backtrace from the core file:

--- mysqltest backtrace begins here ---
Core was generated by `mysqltest'.
Program terminated with signal 10, Bus error.
#0  0x0000000800e961e0 in __findenv () from /lib/libc.so.6
[New LWP 100227]
#0  0x0000000800e961e0 in __findenv () from /lib/libc.so.6
#1  0x0000000800e1cbd9 in setenv () from /lib/libc.so.6
#2  0x0000000800dfdcae in putenv () from /lib/libc.so.6
#3  0x000000000040596a in var_set (
    var_name=0x7fffffffc770 "the_backup_idРh4z\025", 'Р' <repeats 182 times>..., var_name_end=0x5e80a8 "а\200^", var_val=0x0, var_val_end=0x5e903f "")
    at mysqltest.c:1186
#4  0xd0d0d0d0d0d0d0d0 in ?? ()
#5  0xd0d0d0d0d0d0d0d0 in ?? ()
....
--- mysqltest backtrace ends here ---

So the stack is smashed, let's find out why - run mysqltest via gdb:

--- gdb session begins here ---
...
Breakpoint 1, var_set (var_name=0x7fffffffcba0 "(",
    var_name_end=0x5ee2ba "`select @the_backup_id`",
    var_val=0x5ee2b9 "=`select @the_backup_id`",
    var_val_end=0x41b82a "get_replace_columns") at mysqltest.c:1150
1150    {
(gdb) n
1151      int digit, env_var= 0;
(gdb)
1153      DBUG_ENTER("var_set");
(gdb)
1154      DBUG_PRINT("enter", ("var_name: '%.*s' = '%.*s' (length: %d)",
(gdb)
1159      if (*var_name != '$')
(gdb)
1160        env_var= 1;
(gdb)
1164      digit= *var_name - '0';
(gdb)
1165      if (!(digit < 10 && digit >= 0))
(gdb)
1167        v= var_obtain(var_name, (uint) (var_name_end - var_name));
(gdb)
1172      eval_expr(v, var_val, (const char**) &var_val_end);
(gdb)
1174      if (env_var)
(gdb)
1176        char buf[1024], *old_env_s= v->env_s;
(gdb)
1177        if (v->int_dirty)
(gdb)
1183        strxmov(buf, v->name, "=", v->str_val, NullS);
(gdb) ins v->name
$1 = 0x5ee360 "the_backup_idРh4z\025", 'Р' <repeats 182 times>...
//
// v->name is not '\0' terminated,
// let's see what other members of v contain:
//
(gdb) ins *v
$2 = {name = 0x5ee360 "the_backup_idРh4z\025", 'Р' <repeats 182 times>...,
  name_len = 13, str_val = 0x591668 "1", str_val_len = 1, int_val = 1,
  alloced_len = 16, int_dirty = 0, alloced = 1, env_s = 0x0}
//
// There is a name_len member which is correctly initialized,
// so maybe v->name is OK not to be '\0' terminated.
//
(gdb) bt
#0  var_set (var_name=0x5ee2ac "the_backup_id=`select @the_backup_id`",
    var_name_end=0x5ee328 "`г^", var_val=0x0, var_val_end=0x5f003f "")
    at mysqltest.c:1183
#1  0x0000000000407988 in do_let (command=0x5ef028) at mysqltest.c:2341
#2  0x000000000040d4aa in main (argc=1, argv=0x1) at mysqltest.c:5629
//
// The stack is still OK here,
// let's execute strxmov at line 1183...
//
(gdb) n
1184        if (!(v->env_s= my_strdup(buf, MYF(MY_WME))))
(gdb) bt
#0  var_set (
    var_name=0x7fffffffc740 "the_backup_idРh4z\025", 'Р' <repeats 182 times>...,
 var_name_end=0x5ee328 "`г^", var_val=0x0, var_val_end=0x5f003f "")
    at mysqltest.c:1184
#1  0xd0d0d0d0d0d0d0d0 in ?? ()
#2  0xd0d0d0d0d0d0d0d0 in ?? ()
...
//
// Obviously it smashes the stack at this point because it expects
// the strings to be '\0' terminated.
//
--- gdb session ends here ---

How to repeat:
Run the ndb_restore test.

It is highly possible that the overflow does not occur if there is an occasional '\0' in v->name or v->str_val.

Suggested fix:
The fix would be to create buf in such a way that terminating '\0's are not expected at the end of the strings and the corresponding _len members are used.

The proposed patch uses snprintf(3) thus also protecting buf overflow in case of very large (>1024) _len values.

--- mysqltest.c_overflow.diff begins here ---
--- client/mysqltest.c.orig	Wed Nov 22 11:41:13 2006
+++ client/mysqltest.c	Tue Nov 21 18:35:36 2006
@@ -1180,7 +1180,8 @@
       v->int_dirty= 0;
       v->str_val_len= strlen(v->str_val);
     }
-    strxmov(buf, v->name, "=", v->str_val, NullS);
+    snprintf(buf, sizeof(buf), "%.*s=%.*s", v->name_len, v->name,
+	     v->str_val_len, v->str_val);
     if (!(v->env_s= my_strdup(buf, MYF(MY_WME))))
       die("Out of memory");
     putenv(v->env_s);
--- mysqltest.c_overflow.diff ends here ---
[22 Nov 2006 10:32] Vasil Dimov
Change "Category" from "Server" to "Server: Tests".
I was unable to select the later in the initial submission.
[22 Nov 2006 19:19] Sveta Smirnova
Thank you for the report.

I can not repeat it on Linux.

Please provide exact version of your operation system and describe how you built MySQL.
[23 Nov 2006 6:05] Vasil Dimov
Hi Sveta,

I did not mention my OS because I believe it is irrelevant. Anyway here it is:
FreeBSD 6.2-PRERELEASE/amd64. I am using the stock compiler which is:
$ gcc -v
Using built-in specs.
Configured with: FreeBSD/amd64 system compiler
Thread model: posix
gcc version 3.4.6 [FreeBSD] 20060305
$

MySQL is compiled with the BUILD/compile-amd64-debug-max script.

Furthermore I have configured my malloc(3) to always initialize the returned memory with 0xd0. See the J option in http://www.freebsd.org/cgi/man.cgi?query=malloc.conf

It is highly possible that you cannot reproduce this crash because you have occasional '\0' bytes in the memory area after the strings which are actually not '\0' terminated. The buf variable is quite big - 1024 bytes, the crash will not happen if there is at least one '\0' byte in v->name between 13th and 1024th byte.

When looking at the contents of the stack after it got overwritten (0xd0d0d0d...) I think that this has something to do with my malloc configuration (see above).
[6 Dec 2006 21:31] Magnus Blåudd
my_malloc can init allocated memory with zeroes.
[8 Dec 2006 15:12] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/16658

ChangeSet@1.2583, 2006-12-08 16:08:54+01:00, msvensson@neptunus.(none) +1 -0
  Bug#24498 Stack overflow in mysqltest
   - Thanks to Vasil Dimov for the patch!
[30 Jan 2007 3:44] Paul DuBois
Noted in 4.1.23, 5.0.36, 5.1.15 changelogs.

mysqltest crashed with a stack overflow.