Bug #40097 mysql_register_view initializes query.str to a stack allocated buffer
Submitted: 17 Oct 2008 2:36 Modified: 29 Oct 2008 11:59
Reporter: Mark Callaghan Email Updates:
Status: Duplicate Impact on me:
None 
Category:MySQL Server: Views Severity:S2 (Serious)
Version:5.0.67, 5.0 bzr OS:Any
Assigned to: Gleb Shchepa CPU Architecture:Any
Tags: Contribution, invalid, mysql_register_view, read, valgrind

[17 Oct 2008 2:36] Mark Callaghan
Description:
mysql_register_view() initializes view->query.str to reference buff -- a stack allocated buffer. On function return, view->query.str references unallocated memory.

static int mysql_register_view(THD *thd, TABLE_LIST *view,
                               enum_view_create_mode mode)
{
  LEX *lex= thd->lex;
  char buff[4096];
  String str(buff,(uint32) sizeof(buff), system_charset_info);
...
  view->query.str= str.c_ptr_safe();
  view->query.length= str.length();

How to repeat:
1) apply the patch below
2) mysql-test-run.pl --valgrind check
3) read var/log/master.err

This patch makes the error obvious:
--- orig/sql/sql_view.cc  2008-08-04 05:20:12.000000000 -0700
+++ new/sql/sql_view.cc       2008-10-16 19:27:05.000000000 -0700
@@ -650,6 +650,8 @@
     }
     buff.append(STRING_WITH_LEN(" AS "));
     buff.append(views->query.str, views->query.length);
+    fprintf(stderr, "Read view->query.str addr(%x) len(%d) val(%s)\n",
+            views->query.str, views->query.length, views->query.str);
     if (views->with_check == VIEW_CHECK_LOCAL)
       buff.append(STRING_WITH_LEN(" WITH LOCAL CHECK OPTION"));
     else if (views->with_check == VIEW_CHECK_CASCADED)
@@ -776,6 +778,8 @@
   /* fill structure */
   view->query.str= str.c_ptr_safe();
   view->query.length= str.length();
+  fprintf(stderr, "Set view->query.str addr(%x) len(%d) val(%s) -- buff stack var addr(%x)\n",
+          view->query.str, view->query.length, view->query.str, buff);
   view->source.str= thd->query + thd->lex->create_view_select_start;
   view->source.length= (char *)skip_rear_comments(thd->charset(),
                                                   (char *)view->source.str,
@@ -928,10 +932,12 @@
     error= thd->net.report_error? -1 : 1;
     goto err;
   }
+  memset(buff, 0, sizeof(buff));
   DBUG_RETURN(0);
 err:
   view->md5.str= NULL;
   view->md5.length= 0;
+  memset(buff, 0, sizeof(buff));
   DBUG_RETURN(error);
 }
[17 Oct 2008 8:14] Sveta Smirnova
Thank you for the report.

Verified as described.
[17 Oct 2008 17:48] Mark Callaghan
This fixes the valgrind warning. I have used statement duration memory by allocating from THD::mem_root. Given that TABLE_LIST instances are also allocated from there, I think that is OK for now.

--- orig/sql/sql_view.cc  2008-08-04 05:20:12.000000000 -0700
+++ new/sql/sql_view.cc       2008-10-17 10:46:38.000000000 -0700
@@ -774,8 +774,16 @@
   DBUG_PRINT("info", ("View: %s", str.ptr()));

   /* fill structure */
-  view->query.str= str.c_ptr_safe();
+  view->query.str= thd->alloc(str.length() + 1);
+  if (!view->query.str)
+  {
+    my_error(ER_OUTOFMEMORY, MYF(0), str.length()+1);
+    error= -1;
+    goto err;
+  }
+  memcpy(view->query.str, str.ptr(), str.length());
   view->query.length= str.length();
+  view->query.str[view->query.length]= '\0';
   view->source.str= thd->query + thd->lex->create_view_select_start;
   view->source.length= (char *)skip_rear_comments(thd->charset(),
                                                   (char *)view->source.str,
[29 Oct 2008 11:59] Sergei Glukhov
bug#39040