[Commits] 0ae13b51d78: MDEV-12632: Source and destination overlap in memcpy, encryption.innodb-discard-import-change fails in buildbot with valgrind

jan jan.lindstrom at mariadb.com
Thu Apr 12 11:03:06 EEST 2018


revision-id: 0ae13b51d780d4c0e2ef6fc251a8e03140663d35 (mariadb-10.1.32-55-g0ae13b51d78)
parent(s): 9518ddd1fb4f811df369a0ac729d3fcacbf1aee9
author: Jan Lindström
committer: Jan Lindström
timestamp: 2018-04-12 11:00:48 +0300
message:

MDEV-12632: Source and destination overlap in memcpy, encryption.innodb-discard-import-change fails in buildbot with valgrind

Problem was that if tablespace was encrypted we try to copy
also page 0 from read buffer to write buffer that are in
that case the same memory area.

fil_iterate
	When tablespace is encrypted or compressed its
        first page (i.e. page 0) is not encrypted or
	compressed and there is no need to copy buffer.

---
 storage/innobase/row/row0import.cc | 14 ++++++++++----
 storage/xtradb/row/row0import.cc   | 14 ++++++++++----
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/storage/innobase/row/row0import.cc b/storage/innobase/row/row0import.cc
index c00eb57f91d..b7da200f10a 100644
--- a/storage/innobase/row/row0import.cc
+++ b/storage/innobase/row/row0import.cc
@@ -3434,13 +3434,13 @@ fil_iterate(
 		bool		updated = false;
 		os_offset_t	page_off = offset;
 		ulint		n_pages_read = (ulint) n_bytes / iter.page_size;
-		bool		decrypted = false;
 		const ulint 	size = iter.page_size;
 		block->page.offset = page_off / size;
 
 		for (ulint i = 0; i < n_pages_read;
 		     ++i, page_off += size, block->frame += size,
 		     block->page.offset++) {
+			bool decrypted = false;
 			dberr_t	err = DB_SUCCESS;
 			byte*	src = readptr + (i * size);
 			byte*	dst = io_buffer + (i * size);
@@ -3487,6 +3487,7 @@ fil_iterate(
 						block->frame = src;
 						frame_changed = true;
 					} else {
+						ut_ad(dst != src);
 						memcpy(dst, src, size);
 					}
 				}
@@ -3547,9 +3548,13 @@ fil_iterate(
 			ut_ad(encrypted ?
 			      src != dst && dst != writeptr + (i * size):1);
 
-			if (encrypted) {
-				memcpy(writeptr + (i * size),
-				       callback.get_frame(block), size);
+			/* When tablespace is encrypted or compressed its
+			first page (i.e. page 0) is not encrypted or
+			compressed and there is no need to copy frame. */
+			if (encrypted && i != 0) {
+				byte *local_frame = callback.get_frame(block);
+				ut_ad((writeptr + (i * size)) != local_frame);
+				memcpy((writeptr + (i * size)), local_frame, size);
 			}
 
 			if (frame_changed) {
@@ -3597,6 +3602,7 @@ fil_iterate(
 
 				if (tmp == src) {
 					/* TODO: remove unnecessary memcpy's */
+					ut_ad(dest != src);
 					memcpy(dest, src, size);
 				}
 
diff --git a/storage/xtradb/row/row0import.cc b/storage/xtradb/row/row0import.cc
index c00eb57f91d..b7da200f10a 100644
--- a/storage/xtradb/row/row0import.cc
+++ b/storage/xtradb/row/row0import.cc
@@ -3434,13 +3434,13 @@ fil_iterate(
 		bool		updated = false;
 		os_offset_t	page_off = offset;
 		ulint		n_pages_read = (ulint) n_bytes / iter.page_size;
-		bool		decrypted = false;
 		const ulint 	size = iter.page_size;
 		block->page.offset = page_off / size;
 
 		for (ulint i = 0; i < n_pages_read;
 		     ++i, page_off += size, block->frame += size,
 		     block->page.offset++) {
+			bool decrypted = false;
 			dberr_t	err = DB_SUCCESS;
 			byte*	src = readptr + (i * size);
 			byte*	dst = io_buffer + (i * size);
@@ -3487,6 +3487,7 @@ fil_iterate(
 						block->frame = src;
 						frame_changed = true;
 					} else {
+						ut_ad(dst != src);
 						memcpy(dst, src, size);
 					}
 				}
@@ -3547,9 +3548,13 @@ fil_iterate(
 			ut_ad(encrypted ?
 			      src != dst && dst != writeptr + (i * size):1);
 
-			if (encrypted) {
-				memcpy(writeptr + (i * size),
-				       callback.get_frame(block), size);
+			/* When tablespace is encrypted or compressed its
+			first page (i.e. page 0) is not encrypted or
+			compressed and there is no need to copy frame. */
+			if (encrypted && i != 0) {
+				byte *local_frame = callback.get_frame(block);
+				ut_ad((writeptr + (i * size)) != local_frame);
+				memcpy((writeptr + (i * size)), local_frame, size);
 			}
 
 			if (frame_changed) {
@@ -3597,6 +3602,7 @@ fil_iterate(
 
 				if (tmp == src) {
 					/* TODO: remove unnecessary memcpy's */
+					ut_ad(dest != src);
 					memcpy(dest, src, size);
 				}
 


More information about the commits mailing list