Bug #38108 Race condition in MyISAM backup driver
Submitted: 14 Jul 2008 16:23 Modified: 21 Aug 2008 19:24
Reporter: Ingo Strüwing Email Updates:
Status: Closed Impact on me:
None 
Category:MySQL Server: Backup Severity:S3 (Non-critical)
Version:6.0.6 OS:Any
Assigned to: Ingo Strüwing CPU Architecture:Any

[14 Jul 2008 16:23] Ingo Strüwing
Description:
Recent changes in the MyISAM backup driver introduced a race condition that can be triggered if two restore operations run in parallel. This is not limited to restoring of the same tables or databases. Restoring two different databases, and hence different tables, can trigger the race condition.

The MyISAM backup driver performs direct file accesses, bypassing the MyISAM interface. In restore it modifies the files. At end of restore it updates in-memory structures from the new file contens. To force such update, the global variable myisam_single_user is modified to trick MyISAM into the believe that external programs modified the table.

Modification of the global variable is not protected by a mutex. If two or more threads run through this code with arbitrary scheduling, the final state of the variable is undefined.

How to repeat:
Read the code in mi_close_care_state() and Table_restore::close().

Suggested fix:
Add a new parameter to mi_state_info_read_dsk() which forces it to read from file, even if myisam_single_user is true.
[14 Jul 2008 18:30] 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/49710

2664 Ingo Struewing	2008-07-14
      Bug#38108 - Race condition in MyISAM backup driver
      
      A race condition between two or more running restore operations could leave the
      global variable 'myisam_single_user' in an undefined state.
      
      To force reading of state info from the index file, we changed the value of the
      global variable 'myisam_single_user' temporarily. If multiple threads run this
      code with arbitrary scheduling, the variable value may be left back with
      undefined value. It could even happen that a thread undoes a value change
      before the changing thread used it. Then the reading from file would be
      suppressed and the table left crashed.
      
      Fixed by adding a 'force' parameter to mi_state_info_read_dsk(). This makes
      modification of the global variable unnecessary.
[19 Jul 2008 16:25] Ingo Strüwing
Approved by Guilhem via email.
[21 Jul 2008 18:49] Chuck Bell
Patch approved pending change of 'online restore' to 'restore' or 'mysql restore' or similar -- just not online. ;)
[23 Jul 2008 12:46] 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/50309

2672 Ingo Struewing	2008-07-23
      Bug#38108 - Race condition in MyISAM backup driver
      
      A race condition between two or more running restore operations could leave the
      global variable 'myisam_single_user' in an undefined state.
      
      To force reading of state info from the index file, we changed the value of the
      global variable 'myisam_single_user' temporarily. If multiple threads run this
      code with arbitrary scheduling, the variable value may be left back with
      undefined value. It could even happen that a thread undoes a value change
      before the changing thread used it. Then the reading from file would be
      suppressed and the table left crashed.
      
      Fixed by adding a 'force' parameter to mi_state_info_read_dsk(). This makes
      modification of the global variable unnecessary.
[23 Jul 2008 12:47] 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/50310

2672 Ingo Struewing	2008-07-23
      Bug#38108 - Race condition in MyISAM backup driver
      
      A race condition between two or more running restore operations could leave the
      global variable 'myisam_single_user' in an undefined state.
      
      To force reading of state info from the index file, we changed the value of the
      global variable 'myisam_single_user' temporarily. If multiple threads run this
      code with arbitrary scheduling, the variable value may be left back with
      undefined value. It could even happen that a thread undoes a value change
      before the changing thread used it. Then the reading from file would be
      suppressed and the table left crashed.
      
      Fixed by adding a 'force' parameter to mi_state_info_read_dsk(). This makes
      modification of the global variable unnecessary.
[23 Jul 2008 12:48] Ingo Strüwing
Queued to mysql-6.0-backup.
[31 Jul 2008 14:27] 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/50789

2673 Rafal Somla	2008-07-31
      WL#4384 (Online backup: Revise error reporting)
            
      This patch marks places in the code where errors reported by called functions are not detected
      or not logged. This is for further analysis and fixing of these issues.
           
      The markers are "// FIXME: detect errors..." and "// FIXME: error logging...".
[20 Aug 2008 14:08] Chuck Bell
Pushed to 6.0.0.
[21 Aug 2008 15:37] Chuck Bell
6.0.6
[21 Aug 2008 19:24] Paul DuBois
Noted in 6.0.6 changelog.

The MyISAM backup driver was subject to a race condition that allowed
multiple RESTORE operations to occur simultaneously. This could
result in locking conflicts, incorrect entries in the progress
tables, or other problems.
[14 Sep 2008 4:29] Bugs System
Pushed into 6.0.7-alpha  (revid:ingo.struewing@sun.com-20080723124633-foe6h3l10gg5hps8) (version source revid:hakan@mysql.com-20080721095625-h2pyxb88uwtjeavf) (pib:3)
[4 Jan 2010 14:02] 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/95889

3022 Ingo Struewing	2010-01-04
      WL#5101 - MySQL Backup back port - MS09
      Merged revid:ingo.struewing@sun.com-20080723124633-foe6h3l10gg5hps8
        Bug#38108 - Race condition in MyISAM backup driver
        
        A race condition between two or more running restore operations could leave the
        global variable 'myisam_single_user' in an undefined state.
        
        To force reading of state info from the index file, we changed the value of the
        global variable 'myisam_single_user' temporarily. If multiple threads run this
        code with arbitrary scheduling, the variable value may be left back with
        undefined value. It could even happen that a thread undoes a value change
        before the changing thread used it. Then the reading from file would be
        suppressed and the table left crashed.
        
        Fixed by adding a 'force' parameter to mi_state_info_read_dsk(). This makes
        modification of the global variable unnecessary.
      
      original changeset: 2599.29.1
     @ storage/myisam/mi_dynrec.c
        WL#5101 - MySQL Backup back port - MS09
            Bug#38108 - Race condition in MyISAM backup driver
            Added an argument for the 'force' parameter of mi_state_info_read_dsk().
     @ storage/myisam/mi_examine_log.c
        WL#5101 - MySQL Backup back port - MS09
            Bug#38108 - Race condition in MyISAM backup driver
            Added an argument for the 'force' parameter of mi_state_info_read_dsk().
            Removed tampering with the global variable 'myisam_single_user'.
     @ storage/myisam/mi_locking.c
        WL#5101 - MySQL Backup back port - MS09
            Bug#38108 - Race condition in MyISAM backup driver
            Added an argument for the 'force' parameter of mi_state_info_read_dsk().
     @ storage/myisam/mi_open.c
        WL#5101 - MySQL Backup back port - MS09
            Bug#38108 - Race condition in MyISAM backup driver
            Added parameter 'force' to mi_state_info_read_dsk().
     @ storage/myisam/myisam_backup_engine.cc
        WL#5101 - MySQL Backup back port - MS09
            Bug#38108 - Race condition in MyISAM backup driver
            Added an argument for the 'force' parameter of mi_state_info_read_dsk().
            Removed tampering with the global variable 'myisam_single_user'.
     @ storage/myisam/myisamdef.h
        WL#5101 - MySQL Backup back port - MS09
            Bug#38108 - Race condition in MyISAM backup driver
            Added 'force' parameter to mi_state_info_read_dsk().