| Bug #96391 | a mrec_buf_t[] index bug in row_merge_blocks(). | ||
|---|---|---|---|
| Submitted: | 1 Aug 2019 1:00 | Modified: | 2 Aug 2019 18:39 |
| Reporter: | SHENGLIANG SONG | Email Updates: | |
| Status: | Verified | Impact on me: | |
| Category: | MySQL Server: InnoDB storage engine | Severity: | S3 (Non-critical) |
| Version: | 5.6 5.7 | OS: | Any |
| Assigned to: | CPU Architecture: | Any | |
[1 Aug 2019 12:48]
MySQL Verification Team
HI Mr. SONG, Thank you for your bug report. However, this is not a bug. If you read our latest code in 5.7, you will see that this part of code is totally changed. Not a bug.
[1 Aug 2019 21:04]
SHENGLIANG SONG
This the patch 5.7.
5.7 has same issue if row_merge_blocks() is not dead code.
commit 965d5ef4dbacda69e89a1cd3ab87356655e32843 (HEAD -> 5.7, origin/5.7)
Author: Sheng-Liang Song <shenglia@amazon.com>
Date: Mon Jun 24 23:37:13 2019 +0000
Fixed a mrec_buf_t[] index bug in row_merge_blocks().
typedef byte mrec_buf_t[UNIV_PAGE_SIZE_MAX];
sizeof(mrec_buf_t) = sizeof_of(byte)* UNIV_PAGE_SIZE_MAX
buf is a mrec_buf_t[3] type, not a byte[] type.
The index of buf should be 1, not srv_sort_buf_size.
diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc
index 03091e64489..0861972d47d 100644
--- a/storage/innobase/row/row0merge.cc
+++ b/storage/innobase/row/row0merge.cc
@@ -2751,7 +2751,7 @@ corrupt:
file->fd, foffs0, &mrec0, offsets0);
b1 = row_merge_read_rec(
&block[srv_sort_buf_size],
- &buf[srv_sort_buf_size], b1, dup->index,
+ &buf[1], b1, dup->index,
file->fd, foffs1, &mrec1, offsets1);
if (UNIV_UNLIKELY(!b0 && mrec0)
|| UNIV_UNLIKELY(!b1 && mrec1)) {
[1 Aug 2019 21:10]
SHENGLIANG SONG
commit a9f9005f2ced9ca61543d904a6cd6f617b0fa2d3 (HEAD -> 8.0, origin/HEAD, origin/8.0)
Author: Sheng-Liang Song <shenglia@amazon.com>
Date: Mon Jun 24 23:16:42 2019 +0000
Fixed a mrec_buf_t[] index bug in row_merge_blocks().
typedef byte mrec_buf_t[UNIV_PAGE_SIZE_MAX];
sizeof(mrec_buf_t) = sizeof_of(byte)* UNIV_PAGE_SIZE_MAX
buf is a mrec_buf_t[3] type, not a byte[] type.
The index of buf should be 1, not srv_sort_buf_size.
diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc
index 4c78ab7772a..1b5f58f408c 100644
--- a/storage/innobase/row/row0merge.cc
+++ b/storage/innobase/row/row0merge.cc
@@ -2480,7 +2480,7 @@ static MY_ATTRIBUTE((warn_unused_result)) dberr_t
b0 = row_merge_read_rec(&block[0], &buf[0], b0, dup->index, file->fd, foffs0,
&mrec0, offsets0);
- b1 = row_merge_read_rec(&block[srv_sort_buf_size], &buf[srv_sort_buf_size],
+ b1 = row_merge_read_rec(&block[srv_sort_buf_size], &buf[1],
b1, dup->index, file->fd, foffs1, &mrec1, offsets1);
if (UNIV_UNLIKELY(!b0 && mrec0) || UNIV_UNLIKELY(!b1 && mrec1)) {
goto corrupt;
[1 Aug 2019 21:11]
SHENGLIANG SONG
commit 930d1ea6e3cf01b11e67c64d8e412b221dbff0ca (HEAD -> 5.6, origin/5.6)
Author: Sheng-Liang Song <shenglia@amazon.com>
Date: Mon Jun 24 23:37:13 2019 +0000
Fixed a mrec_buf_t[] index bug in row_merge_blocks().
typedef byte mrec_buf_t[UNIV_PAGE_SIZE_MAX];
sizeof(mrec_buf_t) = sizeof_of(byte)* UNIV_PAGE_SIZE_MAX
buf is a mrec_buf_t[3] type, not a byte[] type.
The index of buf should be 1, not srv_sort_buf_size.
diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc
index 002700a592c..e4f9d9da7f7 100644
--- a/storage/innobase/row/row0merge.cc
+++ b/storage/innobase/row/row0merge.cc
@@ -2089,7 +2089,7 @@ corrupt:
file->fd, foffs0, &mrec0, offsets0);
b1 = row_merge_read_rec(
&block[srv_sort_buf_size],
- &buf[srv_sort_buf_size], b1, dup->index,
+ &buf[1], b1, dup->index,
file->fd, foffs1, &mrec1, offsets1);
if (UNIV_UNLIKELY(!b0 && mrec0)
|| UNIV_UNLIKELY(!b1 && mrec1)) {
[2 Aug 2019 17:49]
MySQL Verification Team
Hi, Can you clarify something for us ??? Isn't `buf[1]` smaller then `buf[srv_sort_buf_size]` ??? If it is, then what is the harm ??
[2 Aug 2019 18:22]
SHENGLIANG SONG
// gcc array_out_of_bounds.c
#include<stdio.h>
#include<string.h>
#include<stdlib.h>
// ./mysql/storage/innobase/include/univ.i:
#define UNIV_PAGE_SIZE_SHIFT_MAX 14
#define UNIV_PAGE_SIZE_MAX (1 << UNIV_PAGE_SIZE_SHIFT_MAX)
typedef unsigned char byte;
typedef byte mrec_buf_t[UNIV_PAGE_SIZE_MAX];
mrec_buf_t buf[3];
ulong srv_sort_buf_size = 1048576;
#define HAS_BUG 1
int main(void) {
int i;
printf("sizeof(mrec_buf_t):%d = %d\n",
sizeof(mrec_buf_t), sizeof(byte)* UNIV_PAGE_SIZE_MAX);
for (i = 0; i < 3; i++) {
printf("&buf[%d] = %p\n", i, &buf[i]);
memset(buf[i], 0xFF, UNIV_PAGE_SIZE_MAX);
}
#if HAS_BUG
memset(buf[srv_sort_buf_size], 0xFF, UNIV_PAGE_SIZE_MAX);
#endif
return 0;
}
// Simple reproduce a coredump with the above small c code.
sizeof(mrec_buf_t):16384 = 16384
&buf[0] = 0x600a00
&buf[1] = 0x604a00
&buf[2] = 0x608a00
zsh: segmentation fault (core dumped) ./a.out
[2 Aug 2019 18:27]
MySQL Verification Team
Hi, Seems that you are correct. However, it does not seem to me that this will be fixed in 5.6. However, I am not the one to decide on that. Verified.
[2 Aug 2019 18:39]
SHENGLIANG SONG
- 5.5 is fine. - 5.6,5,7,8.0 branches all have the same issue.
Description: a mrec_buf_t index bug in row_merge_blocks(). typedef byte mrec_buf_t[UNIV_PAGE_SIZE_MAX]; buf is a mrec_buf_t[3] type, not a byte[] type. The index of buf should be 1, not srv_sort_buf_size. How to repeat: Found this bug by reading the code. Suggested fix: diff --git a/storage/innobase/row/row0merge.cc b/storage/innobase/row/row0merge.cc index 4c78ab7772a..1b5f58f408c 100644 --- a/storage/innobase/row/row0merge.cc +++ b/storage/innobase/row/row0merge.cc @@ -2480,7 +2480,7 @@ static MY_ATTRIBUTE((warn_unused_result)) dberr_t b0 = row_merge_read_rec(&block[0], &buf[0], b0, dup->index, file->fd, foffs0, &mrec0, offsets0); - b1 = row_merge_read_rec(&block[srv_sort_buf_size], &buf[srv_sort_buf_size], + b1 = row_merge_read_rec(&block[srv_sort_buf_size], &buf[1], b1, dup->index, file->fd, foffs1, &mrec1, offsets1); if (UNIV_UNLIKELY(!b0 && mrec0) || UNIV_UNLIKELY(!b1 && mrec1)) { goto corrupt;