List:Commits« Previous MessageNext Message »
From:Magne Mahre Date:February 17 2011 11:25am
Subject:bzr commit into mysql-5.1 branch (magne.mahre:3596) Bug#48053
View as plain text  
#At file:///export/home/tmp/x/mysql-5.1-48053/ based on revid:jonathan.perkin@strippeddlacxc

 3596 Magne Mahre	2011-02-17
      Bug#48053 String::c_ptr has a race and/or does an invalid 
                memory reference
      
      There are two issues present here.
        1) There is a possibility that we test a byte beyond the
           allocated buffer
      
        2) We compare a byte that might never have been
           initalized to see if it's 0.
      
      The first issue is not triggered by existing code, but an
      ASSERT has been added to safe-guard against introducing
      new code that triggers it.
      
      The second issue is what triggers the Valgrind warnings
      reported in the bug report. A buffer is allocated in
      class String to hold the value. This buffer is populated
      by the character data constituting the string, but is not
      zero-terminated in most cases.  Testing if it is indeed
      zero-terminated means that we check a byte that has never
      been explicitly set, thus causing Valgrind to trigger.
      
      Note that issue 2 is not a serious problem.  The variable
      is read, and if it's not zero, we will set it to zero.
      There are no further consequences.
      
      Note that this patch does not fix the underlying problems
      with issue 1, as it is deemed too risky to fix at this
      point (as noted in the bug report).  As discussed in
      the report, the c_ptr() method should probably be
      replaced, but this requires a thorough analysis of the
      ~200 calls to the method.
     @ sql/set_var.cc
        These two cases have been reported to fail
        with Valgrind.

    modified:
      mysql-test/r/ctype_cp1250_ch.result
      mysql-test/r/ctype_cp1251.result
      mysql-test/r/ctype_eucjpms.result*
      mysql-test/t/ctype_cp1250_ch.test
      mysql-test/t/ctype_cp1251.test
      mysql-test/t/ctype_eucjpms.test
      sql/set_var.cc
      sql/sql_string.h
=== modified file 'mysql-test/r/ctype_cp1250_ch.result'
--- a/mysql-test/r/ctype_cp1250_ch.result	2008-03-07 21:14:56 +0000
+++ b/mysql-test/r/ctype_cp1250_ch.result	2011-02-17 11:25:35 +0000
@@ -238,3 +238,6 @@ select a from t1 where a like "abcdefgh�
 a
 abcdefgh�
 drop table t1;
+set global LC_MESSAGES=convert((@@global.log_bin_trust_function_creators) 
+using cp1250);
+ERROR HY000: Unknown system variable 'LC_MESSAGES'

=== modified file 'mysql-test/r/ctype_cp1251.result'
--- a/mysql-test/r/ctype_cp1251.result	2010-11-26 13:58:54 +0000
+++ b/mysql-test/r/ctype_cp1251.result	2011-02-17 11:25:35 +0000
@@ -375,6 +375,10 @@ FD	FD	FD	D18D	FD
 FE	FE	FE	D18E	FE	
 FF	FF	FF	D18F	FF	
 DROP TABLE t1;
+set global LC_MESSAGES=convert((@@global.log_bin_trust_function_creators) using cp1250);
+ERROR HY000: Unknown system variable 'LC_MESSAGES'
+set global LC_TIME_NAMES=convert((-8388608) using cp1251);
+ERROR HY000: Unknown locale: '-8388608'
 #
 # End of 5.1 tests
 #

=== modified file 'mysql-test/r/ctype_eucjpms.result' (properties changed: +x to -x)
--- a/mysql-test/r/ctype_eucjpms.result	2008-02-04 07:10:40 +0000
+++ b/mysql-test/r/ctype_eucjpms.result	2011-02-17 11:25:35 +0000
@@ -9859,3 +9859,5 @@ hex(convert(_eucjpms 0xA5FE41 using ucs2
 select hex(convert(_eucjpms 0x8FABF841 using ucs2));
 hex(convert(_eucjpms 0x8FABF841 using ucs2))
 003F0041
+set global LC_TIME_NAMES=convert((convert((0x63) using eucjpms)) using utf8);
+ERROR HY000: Unknown locale: 'c'

=== modified file 'mysql-test/t/ctype_cp1250_ch.test'
--- a/mysql-test/t/ctype_cp1250_ch.test	2008-03-07 11:28:51 +0000
+++ b/mysql-test/t/ctype_cp1250_ch.test	2011-02-17 11:25:35 +0000
@@ -72,3 +72,13 @@ select a from t1 where a like "abcdefgh�
 drop table t1;
 
 # End of 4.1 tests
+
+#
+# Bug #48053 String::c_ptr has a race and/or does an invalid 
+#            memory reference
+#            (triggered by Valgrind tests)
+#  (see also ctype_eucjpms.test, ctype_cp1250.test, ctype_cp1251.test)
+#
+--error 1193
+set global LC_MESSAGES=convert((@@global.log_bin_trust_function_creators) 
+    using cp1250);

=== modified file 'mysql-test/t/ctype_cp1251.test'
--- a/mysql-test/t/ctype_cp1251.test	2010-11-26 13:58:54 +0000
+++ b/mysql-test/t/ctype_cp1251.test	2011-02-17 11:25:35 +0000
@@ -55,6 +55,18 @@ drop table t1;
 
 --source include/ctype_8bit.inc
 
+#
+# Bug #48053 String::c_ptr has a race and/or does an invalid 
+#            memory reference
+#            (triggered by Valgrind tests)
+#  (see also ctype_eucjpms.test, ctype_cp1250.test, ctype_cp1251.test)
+#
+--error 1193
+set global LC_MESSAGES=convert((@@global.log_bin_trust_function_creators) using cp1250);
+--error 1105
+set global LC_TIME_NAMES=convert((-8388608) using cp1251);
+
+
 --echo #
 --echo # End of 5.1 tests
 --echo #

=== modified file 'mysql-test/t/ctype_eucjpms.test'
--- a/mysql-test/t/ctype_eucjpms.test	2008-02-04 07:10:40 +0000
+++ b/mysql-test/t/ctype_eucjpms.test	2011-02-17 11:25:35 +0000
@@ -381,3 +381,11 @@ select hex(convert(_eucjpms 0xA5FE41 usi
 # the next character, which is a single byte character 0x41.
 select hex(convert(_eucjpms 0x8FABF841 using ucs2));
 
+#
+# Bug #48053 String::c_ptr has a race and/or does an invalid 
+#            memory reference
+#            (triggered by Valgrind tests)
+#  (see also ctype_eucjpms.test, ctype_cp1250.test, ctype_cp1251.test)
+#
+--error 1105
+set global LC_TIME_NAMES=convert((convert((0x63) using eucjpms)) using utf8);

=== modified file 'sql/set_var.cc'
--- a/sql/set_var.cc	2011-02-02 17:05:28 +0000
+++ b/sql/set_var.cc	2011-02-17 11:25:35 +0000
@@ -1828,7 +1828,7 @@ bool sys_var::check_set(THD *thd, set_va
     }
 
     var->save_result.ulong_value= ((ulong)
-				   find_set(enum_names, res->c_ptr(),
+				   find_set(enum_names, res->c_ptr_safe(),
 					    res->length(),
                                             NULL,
                                             &error, &error_len,
@@ -2941,7 +2941,7 @@ bool sys_var_thd_lc_time_names::check(TH
       my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), name, "NULL");
       return 1;
     }
-    const char *locale_str= res->c_ptr();
+    const char *locale_str= res->c_ptr_safe();
     if (!(locale_match= my_locale_by_name(locale_str)))
     {
       my_printf_error(ER_UNKNOWN_ERROR,

=== modified file 'sql/sql_string.h'
--- a/sql/sql_string.h	2011-01-13 07:57:15 +0000
+++ b/sql/sql_string.h	2011-02-17 11:25:35 +0000
@@ -106,6 +106,9 @@ public:
   inline const char *ptr() const { return Ptr; }
   inline char *c_ptr()
   {
+    DBUG_ASSERT(!alloced || !Ptr || !Alloced_length || 
+                (Alloced_length >= (str_length + 1)));
+
     if (!Ptr || Ptr[str_length])		/* Should be safe */
       (void) realloc(str_length);
     return Ptr;

Attachment: [text/bzr-bundle] bzr/magne.mahre@oracle.com-20110217112535-b7dms42mkjy38go2.bundle
Thread
bzr commit into mysql-5.1 branch (magne.mahre:3596) Bug#48053Magne Mahre17 Feb