Bug #38108 Race condition in MyISAM backup driver
Submitted: 14 Jul 2008 18:23 Modified: 21 Aug 2008 21:24
Reporter: Ingo Strüwing
Status: Closed
Category:Server: Backup Severity:S3 (Non-critical)
Version:6.0.6 OS:Any
Assigned to: Ingo Strüwing Target Version:6.0
Triage: D3 (Medium) / R2 (Low) / E2 (Low)

[14 Jul 2008 18: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 20: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 18:25] Ingo Strüwing 
Approved by Guilhem via email.
[21 Jul 2008 20:49] Chuck Bell 
Patch approved pending change of 'online restore' to 'restore' or 'mysql restore' or
similar -- just not online. ;)
[23 Jul 2008 14: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 14: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 14:48] Ingo Strüwing 
Queued to mysql-6.0-backup.
[31 Jul 2008 16: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 16:08] Chuck Bell 
Pushed to 6.0.0.
[21 Aug 2008 17:37] Chuck Bell 
6.0.6
[21 Aug 2008 21: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 6: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)