Commit 65032267 authored by Magne Mahre's avatar Magne Mahre

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.
parent aa8ebbee
...@@ -238,3 +238,6 @@ select a from t1 where a like "abcdefgh ...@@ -238,3 +238,6 @@ select a from t1 where a like "abcdefgh
a a
abcdefgh abcdefgh
drop table t1; drop table t1;
set global LC_MESSAGES=convert((@@global.log_bin_trust_function_creators)
using cp1250);
ERROR HY000: Unknown system variable 'LC_MESSAGES'
...@@ -375,6 +375,8 @@ FD FD FD D18D FD ...@@ -375,6 +375,8 @@ FD FD FD D18D FD
FE FE FE D18E FE FE FE FE D18E FE
FF FF FF D18F FF FF FF FF D18F FF
DROP TABLE t1; DROP TABLE t1;
set global LC_TIME_NAMES=convert((-8388608) using cp1251);
ERROR HY000: Unknown locale: '-8388608'
# #
# End of 5.1 tests # End of 5.1 tests
# #
...@@ -9859,3 +9859,5 @@ hex(convert(_eucjpms 0xA5FE41 using ucs2)) ...@@ -9859,3 +9859,5 @@ hex(convert(_eucjpms 0xA5FE41 using ucs2))
select hex(convert(_eucjpms 0x8FABF841 using ucs2)); select hex(convert(_eucjpms 0x8FABF841 using ucs2));
hex(convert(_eucjpms 0x8FABF841 using ucs2)) hex(convert(_eucjpms 0x8FABF841 using ucs2))
003F0041 003F0041
set global LC_TIME_NAMES=convert((convert((0x63) using eucjpms)) using utf8);
ERROR HY000: Unknown locale: 'c'
...@@ -72,3 +72,13 @@ select a from t1 where a like "abcdefgh ...@@ -72,3 +72,13 @@ select a from t1 where a like "abcdefgh
drop table t1; drop table t1;
# End of 4.1 tests # 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);
...@@ -55,6 +55,16 @@ drop table t1; ...@@ -55,6 +55,16 @@ drop table t1;
--source include/ctype_8bit.inc --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 1105
set global LC_TIME_NAMES=convert((-8388608) using cp1251);
--echo # --echo #
--echo # End of 5.1 tests --echo # End of 5.1 tests
--echo # --echo #
...@@ -381,3 +381,11 @@ select hex(convert(_eucjpms 0xA5FE41 using ucs2)); ...@@ -381,3 +381,11 @@ select hex(convert(_eucjpms 0xA5FE41 using ucs2));
# the next character, which is a single byte character 0x41. # the next character, which is a single byte character 0x41.
select hex(convert(_eucjpms 0x8FABF841 using ucs2)); 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);
...@@ -1828,7 +1828,7 @@ bool sys_var::check_set(THD *thd, set_var *var, TYPELIB *enum_names) ...@@ -1828,7 +1828,7 @@ bool sys_var::check_set(THD *thd, set_var *var, TYPELIB *enum_names)
} }
var->save_result.ulong_value= ((ulong) var->save_result.ulong_value= ((ulong)
find_set(enum_names, res->c_ptr(), find_set(enum_names, res->c_ptr_safe(),
res->length(), res->length(),
NULL, NULL,
&error, &error_len, &error, &error_len,
...@@ -2941,7 +2941,7 @@ bool sys_var_thd_lc_time_names::check(THD *thd, set_var *var) ...@@ -2941,7 +2941,7 @@ bool sys_var_thd_lc_time_names::check(THD *thd, set_var *var)
my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), name, "NULL"); my_error(ER_WRONG_VALUE_FOR_VAR, MYF(0), name, "NULL");
return 1; 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))) if (!(locale_match= my_locale_by_name(locale_str)))
{ {
my_printf_error(ER_UNKNOWN_ERROR, my_printf_error(ER_UNKNOWN_ERROR,
......
...@@ -106,6 +106,9 @@ class String ...@@ -106,6 +106,9 @@ class String
inline const char *ptr() const { return Ptr; } inline const char *ptr() const { return Ptr; }
inline char *c_ptr() inline char *c_ptr()
{ {
DBUG_ASSERT(!alloced || !Ptr || !Alloced_length ||
(Alloced_length >= (str_length + 1)));
if (!Ptr || Ptr[str_length]) /* Should be safe */ if (!Ptr || Ptr[str_length]) /* Should be safe */
(void) realloc(str_length); (void) realloc(str_length);
return Ptr; return Ptr;
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment