Bug #83264 uint3korr should stop reading four instead of three bytes on x86
Submitted: 5 Oct 2016 8:29 Modified: 10 Mar 2017 3:13
Reporter: Laurynas Biveinis (OCA) Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Replication Severity:S3 (Non-critical)
Version:5.5,5.6 OS:Any
Assigned to: CPU Architecture:Any
Tags: asan, Contribution

[5 Oct 2016 8:29] Laurynas Biveinis
Description:
uint3korr reads four bytes instead of three on 32-bit x86, in 5.5 and 5.6, but not in 5.7.

How to repeat:
cmake -DWITH_ASAN=ON -DWITH_DEBUG=ON
...
$ ./mtr --debug-server binlog_mysqlbinlog_row
...
binlog.binlog_mysqlbinlog_row 'row'      [ fail ]
        Test ended at 2016-10-05 11:25:26

CURRENT_TEST: binlog.binlog_mysqlbinlog_row
=================================================================
==12924==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xb4501751 at pc 0x08058cb3 bp 0xbfed2a48 sp 0xbfed2a38
READ of size 4 at 0xb4501751 thread T0
    #0 0x8058cb2 in log_event_print_value /home/laurynas/mysql-server/sql/log_event.cc:1679
    #1 0x8071cf6 in Rows_log_event::print_verbose_one_row(st_io_cache*, table_def*, st_print_event_info*, st_bitmap*, unsigned char const*, unsigned char const*) /home/laurynas/mysql-server/sql/log_event.cc:1959
    #2 0x80736bc in Rows_log_event::print_verbose(st_io_cache*, st_print_event_info*) /home/laurynas/mysql-server/sql/log_event.cc:2056
    #3 0x8074260 in Log_event::print_base64(st_io_cache*, st_print_event_info*, bool) /home/laurynas/mysql-server/sql/log_event.cc:2145
    #4 0x8074b0b in Rows_log_event::print_helper(_IO_FILE*, st_print_event_info*, char const*) /home/laurynas/mysql-server/sql/log_event.cc:8454
    #5 0x8074bf5 in Write_rows_log_event::print(_IO_FILE*, st_print_event_info*) /home/laurynas/mysql-server/sql/log_event.cc:9537
    #6 0x8076b76 in process_event(st_print_event_info*, Log_event*, unsigned long long, char const*) /home/laurynas/mysql-server/client/mysqlbinlog.cc:1039
    #7 0x8079285 in dump_local_log_entries /home/laurynas/mysql-server/client/mysqlbinlog.cc:2077
    #8 0x8079285 in dump_log_entries /home/laurynas/mysql-server/client/mysqlbinlog.cc:1514
    #9 0x8079a3d in main /home/laurynas/mysql-server/client/mysqlbinlog.cc:2183
    #10 0xb6e9f7ad in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x187ad)
    #11 0x804d750  (/home/laurynas/obj-5.5-asan/client/mysqlbinlog+0x804d750)

0xb4501754 is located 0 bytes to the right of 4-byte region [0xb4501750,0xb4501754)
allocated by thread T0 here:
    #0 0xb72bff06 in malloc (/usr/lib/i386-linux-gnu/libasan.so.2+0x96f06)
    #1 0x80fd4b6 in my_malloc /home/laurynas/mysql-server/mysys/my_malloc.c:38
    #2 0x8069b90 in Rows_log_event::Rows_log_event(char const*, unsigned int, Log_event_type, Format_description_log_event const*) /home/laurynas/mysql-server/sql/log_event.cc:7701
    #3 0x806ae8c in Write_rows_log_event::Write_rows_log_event(char const*, unsigned int, Format_description_log_event const*) /home/laurynas/mysql-server/sql/log_event.cc:9085
    #4 0x80741da in Log_event::print_base64(st_io_cache*, st_print_event_info*, bool) /home/laurynas/mysql-server/sql/log_event.cc:2130
    #5 0x8074b0b in Rows_log_event::print_helper(_IO_FILE*, st_print_event_info*, char const*) /home/laurynas/mysql-server/sql/log_event.cc:8454
    #6 0x8074bf5 in Write_rows_log_event::print(_IO_FILE*, st_print_event_info*) /home/laurynas/mysql-server/sql/log_event.cc:9537
    #7 0x8076b76 in process_event(st_print_event_info*, Log_event*, unsigned long long, char const*) /home/laurynas/mysql-server/client/mysqlbinlog.cc:1039
    #8 0x8079285 in dump_local_log_entries /home/laurynas/mysql-server/client/mysqlbinlog.cc:2077
    #9 0x8079285 in dump_log_entries /home/laurynas/mysql-server/client/mysqlbinlog.cc:1514
    #10 0x8079a3d in main /home/laurynas/mysql-server/client/mysqlbinlog.cc:2183
    #11 0xb6e9f7ad in __libc_start_main (/lib/i386-linux-gnu/libc.so.6+0x187ad)

Suggested fix:
Remove the 4-byte version of uint3korr in 5.5 and 5.6, same as 5.7 has done
[5 Oct 2016 9:42] Laurynas Biveinis
Bug 83264 fix for 5.5

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: bug83264-5.5.patch (application/octet-stream, text), 927 bytes.

[5 Oct 2016 9:43] Laurynas Biveinis
Bug 83264 fix for 5.6

(*) I confirm the code being submitted is offered under the terms of the OCA, and that I am authorized to contribute it.

Contribution: bug83264-5.6.patch (application/octet-stream, text), 1.78 KiB.

[6 Oct 2016 9:53] MySQL Verification Team
Hello Laurynas,

Thank you for the report and contribution.

Thanks,
Umesh
[12 Oct 2016 10:26] Ståle Deraas
Posted by developer:
 
Laurynas, will this actually fix a concrete issue that you see, or just make ASAN more clean for 5.5 and 5.6?
[12 Oct 2016 10:36] Laurynas Biveinis
It's for ASan. 

We still support 32-bit platforms, and would like to have a clean testing baseline accordingly. 

The only risk in applying this I'd see is worse code generated with some compiler lacking in optimisation, but I wouldn't consider it's that big of an issue for 32 bits, even if it does happen.
[9 Mar 2017 18:21] Paul DuBois
Posted by developer:
 
Noted in 5.5.56, 5.6.37, 5.7.19, 8.0.1 changelogs.

On x86 machines, the uint3korr() macro read 4 bytes of data instead
of the intended 3 bytes.
[9 Mar 2017 18:25] Paul DuBois
Posted by developer:
 
8.0.2 changelog, not 8.0.1.
[10 Mar 2017 3:13] Laurynas Biveinis
Can you please confirm the 5.7 / 8.0 fix? AFAIK the bug was only present in 5.5 and 5.6.
[27 Apr 2017 13:11] Paul DuBois
Posted by developer:
 
Retagged for release notes from 5.5.56 to 5.5.57.