bzr commit into mysql-5.1-bugteam branch (svoj:3404) Bug#51877

[Sergey Vojtovich]

#At file:///home/svoj/devel/bzr-mysql/mysql-5.1-bugteam-bug51877/ based on revid:svoj@stripped

 3404 Sergey Vojtovich	2010-03-25
      BUG#51877 - HANDLER interface causes invalid memory read
      
      Invalid memory read if HANDLER ... READ NEXT is executed
      after failed (e.g. empty table) HANDLER ... READ FIRST.
      
      The problem was that we attempted to perform READ NEXT,
      whereas there is no pivot available from failed READ FIRST.
      
      With this fix READ NEXT after failed READ FIRST equals
      to READ FIRST.
      
      This bug affects MyISAM tables only.
     @ mysql-test/r/gis-rtree.result
        Restore a test case for BUG51357.
     @ mysql-test/r/handler_myisam.result
        A test case for BUG#51877.
     @ mysql-test/t/gis-rtree.test
        Restore a test case for BUG51357.
     @ mysql-test/t/handler_myisam.test
        A test case for BUG#51877.
     @ storage/myisam/mi_rnext.c
        "search first" failed. This means we have no pivot for
        "search next", or in other words MI_INFO::lastkey is
        likely uninitialized.
        
        Normally SQL layer would never request "search next" if
        "search first" failed. But HANDLER may do anything.
        
        As mi_rnext() without preceeding mi_rkey()/mi_rfirst()
        equals to mi_rfirst(), we must restore original state
        as if failing mi_rfirst() was not called.

    modified:
      mysql-test/r/gis-rtree.result
      mysql-test/r/handler_myisam.result
      mysql-test/t/gis-rtree.test
      mysql-test/t/handler_myisam.test
      storage/myisam/mi_rnext.c
=== modified file 'mysql-test/r/gis-rtree.result'
--- a/mysql-test/r/gis-rtree.result	2010-03-10 10:22:08 +0000
+++ b/mysql-test/r/gis-rtree.result	2010-03-25 11:49:01 +0000
@@ -1540,5 +1540,12 @@ a
 HANDLER t1 READ a LAST;
 a
 HANDLER t1 CLOSE;
+HANDLER t1 OPEN;
+HANDLER t1 READ a FIRST;
+a
+INSERT INTO t1 VALUES (GeomFromText('Polygon((40 40,60 40,60 60,40 60,40 40))'));
+# should not crash
+HANDLER t1 READ a NEXT;
+HANDLER t1 CLOSE;
 DROP TABLE t1;
 End of 5.0 tests.

=== modified file 'mysql-test/r/handler_myisam.result'
--- a/mysql-test/r/handler_myisam.result	2009-10-13 18:21:42 +0000
+++ b/mysql-test/r/handler_myisam.result	2010-03-25 11:49:01 +0000
@@ -756,4 +756,17 @@ TRUNCATE t1;
 HANDLER t1 READ FIRST;
 ERROR 42S02: Unknown table 't1' in HANDLER
 DROP TABLE t1;
+#
+# BUG#51877 - HANDLER interface causes invalid memory read
+#
+CREATE TABLE t1(a INT, KEY(a));
+HANDLER t1 OPEN;
+HANDLER t1 READ a FIRST;
+a
+INSERT INTO t1 VALUES(1);
+HANDLER t1 READ a NEXT;
+a
+1
+HANDLER t1 CLOSE;
+DROP TABLE t1;
 End of 5.1 tests

=== modified file 'mysql-test/t/gis-rtree.test'
--- a/mysql-test/t/gis-rtree.test	2010-03-10 10:22:08 +0000
+++ b/mysql-test/t/gis-rtree.test	2010-03-25 11:49:01 +0000
@@ -914,14 +914,15 @@ HANDLER t1 READ a PREV;
 HANDLER t1 READ a LAST;
 HANDLER t1 CLOSE;
 
-#TODO: re-enable this test please when bug #51877 is solved
 # second crash fixed when the tree has changed since the last search.
-#HANDLER t1 OPEN;
-#HANDLER t1 READ a FIRST;
-#INSERT INTO t1 VALUES (GeomFromText('Polygon((40 40,60 40,60 60,40 60,40 40))'));
-#HANDLER t1 READ a NEXT;
-#HANDLER t1 CLOSE;
-#TODO: end of the 51877 dependent section
+HANDLER t1 OPEN;
+HANDLER t1 READ a FIRST;
+INSERT INTO t1 VALUES (GeomFromText('Polygon((40 40,60 40,60 60,40 60,40 40))'));
+--echo # should not crash
+--disable_result_log
+HANDLER t1 READ a NEXT;
+--enable_result_log
+HANDLER t1 CLOSE;
 
 DROP TABLE t1;
 

=== modified file 'mysql-test/t/handler_myisam.test'
--- a/mysql-test/t/handler_myisam.test	2009-10-13 18:21:42 +0000
+++ b/mysql-test/t/handler_myisam.test	2010-03-25 11:49:01 +0000
@@ -37,4 +37,15 @@ TRUNCATE t1;
 HANDLER t1 READ FIRST;
 DROP TABLE t1;
 
+--echo #
+--echo # BUG#51877 - HANDLER interface causes invalid memory read
+--echo #
+CREATE TABLE t1(a INT, KEY(a));
+HANDLER t1 OPEN;
+HANDLER t1 READ a FIRST;
+INSERT INTO t1 VALUES(1);
+HANDLER t1 READ a NEXT;
+HANDLER t1 CLOSE;
+DROP TABLE t1;
+
 --echo End of 5.1 tests

=== modified file 'storage/myisam/mi_rnext.c'
--- a/storage/myisam/mi_rnext.c	2007-05-10 09:59:39 +0000
+++ b/storage/myisam/mi_rnext.c	2010-03-25 11:49:01 +0000
@@ -28,6 +28,7 @@ int mi_rnext(MI_INFO *info, uchar *buf, 
 {
   int error,changed;
   uint flag;
+  uint update_mask= HA_STATE_NEXT_FOUND;
   DBUG_ENTER("mi_rnext");
 
   if ((inx = _mi_check_index(info,inx)) < 0)
@@ -55,6 +56,20 @@ int mi_rnext(MI_INFO *info, uchar *buf, 
 			   info->s->state.key_root[inx]);
       break;
     }
+    /*
+      "search first" failed. This means we have no pivot for
+      "search next", or in other words MI_INFO::lastkey is
+      likely uninitialized.
+
+      Normally SQL layer would never request "search next" if
+      "search first" failed. But HANDLER may do anything.
+
+      As mi_rnext() without preceeding mi_rkey()/mi_rfirst()
+      equals to mi_rfirst(), we must restore original state
+      as if failing mi_rfirst() was not called.
+    */
+    if (error)
+      update_mask|= HA_STATE_PREV_FOUND;
   }
   else
   {
@@ -100,7 +115,7 @@ int mi_rnext(MI_INFO *info, uchar *buf, 
   }
 	/* Don't clear if database-changed */
   info->update&= (HA_STATE_CHANGED | HA_STATE_ROW_CHANGED);
-  info->update|= HA_STATE_NEXT_FOUND;
+  info->update|= update_mask;
 
   if (error)
   {


Attachment: [text/bzr-bundle] bzr/[e-mail]