[MDEV-11248] uint expression is used for the value that is passed as my_off_t for DDL log Created: 2016-11-07  Updated: 2016-12-08  Resolved: 2016-11-10

Status: Closed
Project: MariaDB Server
Component/s: Data Definition - Alter Table
Affects Version/s: 10.1.18, 5.5.53, 10.0.28
Fix Version/s: 5.5.54

Type: Bug Priority: Major
Reporter: Geoff Montee (Inactive) Assignee: Vladislav Vaintroub
Resolution: Fixed Votes: 0
Labels: ddl, upstream


 Description   

valerii reported the following upstream bug that also seems to effect MariaDB:

http://bugs.mysql.com/bug.php?id=83708

We can see the same block of code in MariaDB 10.1:

https://github.com/MariaDB/server/blob/a98c85bb501e9021c0d8d509b8c040611d4c0c3a/sql/sql_table.cc#L663

and in MariaDB 10.0:

https://github.com/MariaDB/server/blob/5569ac00590ba139bbc575c20de4c682919721e5/sql/sql_table.cc#L664

and in MariaDB 5.5:

https://github.com/MariaDB/server/blob/19e3597e0c718a4cfdfe8789c7b4b11a4e0ba0c6/sql/sql_table.cc#L677

Bug description from upstream report:

Please, check the following code in sql/sql_table.cc:
 
static bool read_ddl_log_file_entry(uint entry_no)
{
  bool error= FALSE;
  File file_id= global_ddl_log.file_id;
  uchar *file_entry_buf= (uchar*)global_ddl_log.file_entry_buf;
  uint io_size= global_ddl_log.io_size;
  DBUG_ENTER("read_ddl_log_file_entry");
 
  mysql_mutex_assert_owner(&LOCK_gdl);
  if (mysql_file_pread(file_id, file_entry_buf, io_size, io_size * entry_no,
                       MYF(MY_WME)) != io_size)
    error= TRUE;
  DBUG_RETURN(error);
}
 
I see two potential problems with it:
 
1. We multiply two uint values, io_size (that is typically 4096, see:
 
include/my_global.h:332:#define IO_SIZE                 4096
 
etc)
 
and entry_no, without any type cast. The resulting expression is going to be of uint type (4 bytes long unsigned), but formal parameter of mysql_file_pread (and my_pread that it ends up defined as) is of type my_off_t. This type is 8 bytes long (on 64-bit systems) usually:
 
#if defined(_WIN32)
typedef unsigned long long my_off_t;
typedef unsigned long long os_off_t;
#else
typedef off_t os_off_t;
#if SIZEOF_OFF_T > 4
typedef ulonglong my_off_t;
#else
typedef unsigned long my_off_t;
#endif
#endif /*_WIN32*/
 
So, we may end up passing the value of 4294955008 all the time whenever entry_no > 1048573
 
Function to write DDL log (static bool write_ddl_log_file_entry(uint entry_no)) is affected in a similar way it seems.
 
2. We pass MY_WME flag, that is, we just write error to the error log in case of failure, but continue attempts to read in my_pread() indefinitely:
 
#define MY_FFNF         1       /* Fatal if file not found */
#define MY_FNABP        2       /* Fatal if not all bytes read/writen */
#define MY_NABP         4       /* Error if not all bytes read/writen */
#define MY_FAE          8       /* Fatal if any error */
#define MY_WME          16      /* Write message on error */
...
 
So, when offset is big enough we start to loop indefinitely, adding messages to error log and holding LOCK_gdl mutex. As a result, any concurrent DDL activity is essentially blocked forever until we get rid of ddl_log.dll file that is 4G in size.
 
How to repeat:
Read the code.
 
There are reasons to think that as soon as more than 1048573 entries (4096 bytes each) are written to the DDL Log (see https://dev.mysql.com/doc/refman/5.7/en/ddl-log.html) so that it becomes 4G in size, there is no way to read the entries from it successfully.
 
Suggested fix:
Make sure the value of offset passed to mysql_file_pread() and my_file_pwrite() is of type my_off_t in all cases and is not affected by artificial 4 bytes limits.
 
Additionally, consider other flags (MY_FNABP?) when I/O happens with some mutex hold.


Generated at Thu Feb 08 07:48:28 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.