MySQL Bugs: #53363: Unit test pfs

Bug #53363	Unit test pfs_instr-t still crashes
Submitted:	3 May 2010 9:28	Modified:	4 Aug 2010 20:10
Reporter:	Tor Didriksen	Email Updates:
Status:	Closed	Impact on me:	None
Category:	MySQL Server: Performance Schema	Severity:	S3 (Non-critical)
Version:	5.6.99-m4	OS:	Any
Assigned to:	Tor Didriksen	CPU Architecture:	Any

Description:
The fix for
Bug #51738  	Unit test pfs_instr-t crashes 
didn't quite solve all problems.

Compiled with SunPro
mysqld  Ver 5.6.99-m4 for pc-solaris2.10 on i386 (Source distribution)
this unit test still segfaults:

ok 24 - lost 3
ok 25 - no file
ok 26 - lost 4
Signal 11 thrown, attempting backtrace.
/net/atum07/export/home/didrik/mysqldev-next-mr/next-mr-bf-khepri21-automake/storage/perfschema/unittest/pfs_instr-t:my_print_stacktrace+0x1b
/net/atum07/export/home/didrik/mysqldev-next-mr/next-mr-bf-khepri21-automake/storage/perfschema/unittest/pfs_instr-t:0xd2ca
/lib/libc.so.1:0xa75df
/lib/libc.so.1:0x9d2a0
/net/atum07/export/home/didrik/mysqldev-next-mr/next-mr-bf-khepri21-automake/storage/perfschema/unittest/pfs_instr-t:__1cTfind_or_create_file6FpnKPFS_thread_pnOPFS_file_class_pkcI_pnIPFS_file__+0x139
[ Signal 11 (SEGV)]
0x58585858
Error when traversing the stack, stack appears corrupt.
Segmentation Fault (core dumped)

Running with valgrind on linux:
valgrind --track-origins=yes storage/perfschema/unittest/pfs_instr-t 
ok 26 - lost 4
==8428== Conditional jump or move depends on uninitialised value(s)
==8428==    at 0x40CDEE: dirname_length (mf_dirname.c:33)
==8428==    by 0x40A250: find_or_create_file(PFS_thread*, PFS_file_class*, char const*,
unsigned int) (pfs_instr.cc:862)
==8428==    by 0x407019: test_no_instances() (pfs_instr-t.cc:142)
==8428==    by 0x408091: do_all_tests() (pfs_instr-t.cc:403)
==8428==    by 0x4080CB: main (pfs_instr-t.cc:412)
==8428==  Uninitialised value was created by a stack allocation
==8428==    at 0x40A14C: find_or_create_file(PFS_thread*, PFS_file_class*, char const*,
unsigned int) (pfs_instr.cc:792)
==8428== 
==8428== Conditional jump or move depends on uninitialised value(s)
==8428==    at 0x4A07227: stpcpy (mc_replace_strmem.c:573)
==8428==    by 0x40A31F: find_or_create_file(PFS_thread*, PFS_file_class*, char const*,
unsigned int) (pfs_instr.cc:884)
==8428==    by 0x407019: test_no_instances() (pfs_instr-t.cc:142)
==8428==    by 0x408091: do_all_tests() (pfs_instr-t.cc:403)
==8428==    by 0x4080CB: main (pfs_instr-t.cc:412)
==8428==  Uninitialised value was created by a stack allocation
==8428==    at 0x40A14C: find_or_create_file(PFS_thread*, PFS_file_class*, char const*,
unsigned int) (pfs_instr.cc:792)
==8428== 
==8428== Source and destination overlap in stpcpy(0x7feffcb81, 0x7feffcd40)
==8428==    at 0x4A072A8: stpcpy (mc_replace_strmem.c:573)
==8428==    by 0x40A31F: find_or_create_file(PFS_thread*, PFS_file_class*, char const*,
unsigned int) (pfs_instr.cc:884)
==8428==    by 0x407019: test_no_instances() (pfs_instr-t.cc:142)
==8428==    by 0x408091: do_all_tests() (pfs_instr-t.cc:403)
==8428==    by 0x4080CB: main (pfs_instr-t.cc:412)
==8428== 
ok 27 - no file

How to repeat:
run executable pfs_instr-t

Suggested fix:
The first two warnings can be fixed with:
<     memcpy(safe_buffer, filename, FN_REFLEN - 2);
---
>     memcpy(safe_buffer, filename, FN_REFLEN - 1);

The third warning is caused by writing too much data into buffer[]
First the resolved path, then we concatenate the 512 byte safe_filename.
strcncpy() is safer:
<   ptr= strmov(ptr, safe_filename + dirlen);
<   *ptr= '\0';
---
>   char *buf_end= &buffer[sizeof(buffer)-1];
>   strncpy(ptr, safe_filename + dirlen, buf_end - ptr);
>   *buf_end= '\0';

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/107175

3165 Tor Didriksen	2010-05-03
      Bug#53363 Unit test pfs_instr-t still crashes 
      
      Copy 511 rather than 510 bytes into safe_buffer[].
      
      Use strncpy() to ensure we don't overwrite buffer[].
      
      Declare filename_hash_get_key() to be a "C" function,
      to remove warning from SunPro compiler.

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/107177

3165 Tor Didriksen	2010-05-03
      Bug#53363 Unit test pfs_instr-t still crashes 
      
      Copy 511 rather than 510 bytes into safe_buffer[].
      
      Use strncpy() to ensure we don't overwrite buffer[].
      
      Declare filename_hash_get_key() to be a "C" function,
      to remove warning from SunPro compiler.

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/107179

3858 Tor Didriksen	2010-05-03
      Merge (actually patch) from next-mr-bugfixing.
      Bug#53363 Unit test pfs_instr-t still crashes

Pushed to
bzr+ssh://bk-internal.mysql.com/bzrroot/server/mysql-6.0-codebase-bugfixing/bzr+ssh://bk-internal.mysql.com/bzrroot/server/mysql-next-mr-bugfixing/

Pushed into 6.0.14-alpha (revid:alik@sun.com-20100507091908-vqyhpwf2km0aokno) (version source revid:alik@sun.com-20100507091737-12vceffs11elb25g) (merge vers: 6.0.14-alpha) (pib:16)

Pushed into mysql-next-mr (revid:alik@sun.com-20100507091823-nzao4h3qosau4tin) (version source revid:alik@sun.com-20100507091720-ib9r8uny2aeazvas) (pib:16)

Noted in 6.0.14 changelog.

Performance Schema code was subject to a buffer overflow.

Pushed into mysql-trunk 5.6.1-m4 (revid:alik@ibmvm-20100804080001-bny5271e65xo34ig) (version source revid:alik@sun.com-20100507093958-2y0wy6svnc3zfgqb) (merge vers: 5.6.99-m4) (pib:18)

Pushed into mysql-trunk 5.6.1-m4 (revid:alik@ibmvm-20100804081533-c1d3rbipo9e8rt1s) (version source revid:alik@sun.com-20100507093958-2y0wy6svnc3zfgqb) (merge vers: 5.6.99-m4) (pib:18)

Noted in 5.6.0 changelog.