From ba33a3357dc347091f12ab212aadefb93a2f86b3 Mon Sep 17 00:00:00 2001
From: Sergey Vojtovich <svoj@sun.com>
Date: Thu, 25 Mar 2010 15:49:01 +0400
Subject: [PATCH] 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      |  7 +++++++
 mysql-test/r/handler_myisam.result | 13 +++++++++++++
 mysql-test/t/gis-rtree.test        | 15 ++++++++-------
 mysql-test/t/handler_myisam.test   | 11 +++++++++++
 storage/myisam/mi_rnext.c          | 17 ++++++++++++++++-
 5 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/mysql-test/r/gis-rtree.result b/mysql-test/r/gis-rtree.result
index 58c603673df..eb9c350f589 100644
--- a/mysql-test/r/gis-rtree.result
+++ b/mysql-test/r/gis-rtree.result
@@ -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.
diff --git a/mysql-test/r/handler_myisam.result b/mysql-test/r/handler_myisam.result
index 90a1bdfe6be..a970e20a2c0 100644
--- a/mysql-test/r/handler_myisam.result
+++ b/mysql-test/r/handler_myisam.result
@@ -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
diff --git a/mysql-test/t/gis-rtree.test b/mysql-test/t/gis-rtree.test
index c5d5bfee861..b006096528e 100644
--- a/mysql-test/t/gis-rtree.test
+++ b/mysql-test/t/gis-rtree.test
@@ -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;
 
diff --git a/mysql-test/t/handler_myisam.test b/mysql-test/t/handler_myisam.test
index da02a90af0f..868ba14480a 100644
--- a/mysql-test/t/handler_myisam.test
+++ b/mysql-test/t/handler_myisam.test
@@ -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
diff --git a/storage/myisam/mi_rnext.c b/storage/myisam/mi_rnext.c
index 7ce66d41e0f..b9bbda3cacb 100644
--- a/storage/myisam/mi_rnext.c
+++ b/storage/myisam/mi_rnext.c
@@ -28,6 +28,7 @@ int mi_rnext(MI_INFO *info, uchar *buf, int inx)
 {
   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, int inx)
 			   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, int inx)
   }
 	/* 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)
   {