Bug #36624 Backup stream library works wrong on 64bit platform
Submitted: 9 May 2008 12:42 Modified: 11 Aug 2008 23:49
Reporter: Rafal Somla Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Backup Severity:S3 (Non-critical)
Version:6.0-backup OS:Any
Assigned to: Øystein Grøvlen
Triage: D2 (Serious) / R2 (Low) / E1 (None/Negligible)

[9 May 2008 12:42] Rafal Somla
Description:
Backup stream library fails to parse the backup stream correctly due to non-portable code.

DETAILED PROBLEM DESCRIPTION

Backup stream is divided into fixed size blocks. The size of the block is stored at the beginning of the stream. It must be correctly read before the stream can be processed. It happens in this place inside load_buffer() function in stream_v1_transport.c:

>     for (i= 0; i<4; ++i)
>     {
>       block_size >>= 8;
>       block_size |= (*(s->buf.begin++)) << 3*8;
>     }

Here block_size is of type "long unsigned int" and s->buf.begin is a pointer to "unsigned char". The problem is caused by implicit cast to "int" type which happens inside the expression "(*(s->buf.begin++)) << 3*8". If the highest bit becomes 1, compiler thinks that this is a negative integer. When assigning negative integer to block_size which is of unsigned type, if the size of block_size is bigger than the size of int type, the overflowing bits are set to 1, not to 0! This leads to wrong value of block_size. A fix is
to use an explicit cast to "long unsigned int" like this

>     for (i= 0; i<4; ++i)
>     {
>       block_size >>= 8;
>       block_size |= ((unsigned long int)*(s->buf.begin++)) << 3*8;
>     }

Interestingly, the problem manifests itself only on 64bit platform, because only there "long int" is bigger thant "int". Also, due to the logic of reading backup stream, the problem shows up only when more than one input block is read and that required restoring a big database.

There are two problems to be fixed:

- verify that no such incorrect implicit casts are done in the code,
- make backup stream library more robust, so that it terminates gracefully in 
  case stream ends, even if it is confused about block size or other parameters 
  of the stream.

Note that completely satisfactory solution of this problem is to introduce checksums in the stream which is planned to be done in the future.

Resolving this issue should close BUG#34701.

How to repeat:
Very hard to repeat - backup image must have enough data to fill at least 2 blocks in the input stream. Block size must be at least 0x8000 = 32K (this is default).

So, backup a table which has more than 32K of data and try to restore on 64bit platform.

Suggested fix:
Inspect the code and add explicit casts to usigned values where needed.
[12 May 2008 7:04] Rafal Somla
Here is a test case with which I managed to repeat the problem in a tree with a native MyISAM backup. Perhaps it can be used to repeat it also in other scenarios.

 create database mysqltest;
 use mysqltest;

 CREATE TABLE t1 (a longtext) engine=myisam;

 use mysqltest;
 insert into t1 values ("text");
 let $1=13;
 while ($1)
 {
   update t1 set a=concat(a,a);
   dec $1;
 }
 select length(a) from t1;

 BACKUP DATABASE mysqltest TO 'test.ba';

 DROP DATABASE mysqltest;
 RESTORE FROM 'test.ba';

 select length(a) from t1;
 checksum table t1;
 drop database mysqltest;

The point is to have more than 32K of data in the backed-up table.
[13 May 2008 5:42] Rafal Somla
Similar problem reported in BUG#36571.
[6 Jun 2008 9:57] Øystein Grøvlen
If I apply the fix outlined in BUG#36586, I no longer get a hang.  Instead, I get:
At line 19: query 'RESTORE FROM 'test.ba'' failed: 1698: Error when reading summary section of backup image

I get the same error message with the reproduction described in BUG#36571.
[6 Jun 2008 13:57] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/47537

2632 Oystein Grovlen	2008-06-06
      BUG#36586 Online backup stream library can miss end of a stream.
      Make sure end-of-stream is detected.
[9 Jun 2008 8:25] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/47588

2632 Oystein Grovlen	2008-06-09
      BUG#36586 Online backup stream library can miss end of a stream.
      Make sure end-of-stream is detected.
[25 Jun 2008 11:26] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/48463

2639 Oystein Grovlen	2008-06-25
      BUG#36624 Backup stream library works wrong on 64bit platform.
      I have verified that all shift-left operations in the backup
      directory are now based on unsigned ints. This is necessary to
      avoid problems with sign bit on 64-bit platforms.
[25 Jun 2008 12:30] Chuck Bell
Patch approved.
[25 Jun 2008 13:41] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/48490

2642 Oystein Grovlen	2008-06-25
      BUG#36624 Backup stream library works wrong on 64bit platform.
      Restore failed on multi-block backup images on 64-bit platforms
      with 'Error when reading summary section of backup image'.
      Fixed by casting int to unsigned before performing shift left
      operation.
      
      I have verified that all shift-left operations in the backup
      directory are now based on unsigned ints. This is necessary to
      avoid problems with sign bit on 64-bit platforms.
[25 Jun 2008 13:42] Bugs System
A patch for this bug has been committed. After review, it may
be pushed to the relevant source trees for release in the next
version. You can access the patch from:

  http://lists.mysql.com/commits/48491

2642 Oystein Grovlen	2008-06-25
      BUG#36624 Backup stream library works wrong on 64bit platform.
      Restore failed on multi-block backup images on 64-bit platforms
      with 'Error when reading summary section of backup image'.
      Fixed by casting int to unsigned before performing shift left
      operation.
      
      I have verified that all shift-left operations in the backup
      directory are now based on unsigned ints. This is necessary to
      avoid problems with sign bit on 64-bit platforms.
[25 Jun 2008 13:42] Øystein Grøvlen
Patch push to mysql-6.0-backup branch
[7 Aug 2008 12:26] Øystein Grøvlen
Patch have been pushed to 6.0-main
[8 Aug 2008 12:34] Øystein Grøvlen
This bug was fixed in 6.0.6.
[11 Aug 2008 23:49] Paul Dubois
Noted in 6.0.6 changelog.

The online backup stream library failed to parse the backup stream on
64-bit systems.
[14 Sep 2008 2:05] Bugs System
Pushed into 6.0.6-alpha  (revid:oystein.grovlen@sun.com-20080625134049-75j0wqxhdpgv5rwk) (version source revid:hakan@mysql.com-20080716160034-sdexuyp3qow7zlc6) (pib:3)