MDEV-11902 mi_open race condition

TOCTOU bug. The path is checked to be valid, symlinks are resolved.
Then the resolved path is opened. Between the check and the open,
there's a window when one can replace some path component with a
symlink, bypassing validity checks.

Fix: after we resolved all symlinks in the path, don't allow open()
to resolve symlinks, there should be none.

Compared to the old MyISAM/Aria code:
* fastpath. Opening of not-symlinked files is just one open(),
  no fn_format() and lstat() anymore.
* opening of symlinked tables doesn't do fn_format() and lstat() either.
  it also doesn't to realpath() (which was lstat-ing every path
  component), instead if opens every path component with O_PATH.
* share->data_file_name stores realpath(path) not readlink(path). So,
  SHOW CREATE TABLE needs to do lstat/readlink() now (see ::info()),
  and certain error messages (cannot open file "XXX") show the real
  file path with all symlinks resolved.

include/my_sys.h

@@ -63,9 +63,9 @@ typedef struct my_aio_result {
 #define MY_FAE		8	/* Fatal if any error */
 #define MY_WME		16	/* Write message on error */
 #define MY_WAIT_IF_FULL 32	/* Wait and try again if disk full error */
-#define MY_IGNORE_BADFD 32      /* my_sync: ignore 'bad descriptor' errors */
-#define MY_UNUSED       64      /* Unused (was support for RAID) */
-#define MY_FULL_IO     512      /* For my_read - loop intil I/O is complete */
+#define MY_IGNORE_BADFD 32      /* my_sync(): ignore 'bad descriptor' errors */
+#define MY_NOSYMLINKS  512      /* my_open(): don't follow symlinks */
+#define MY_FULL_IO     512      /* my_read(): loop intil I/O is complete */
 #define MY_DONT_CHECK_FILESIZE 128 /* Option to init_io_cache() */
 #define MY_LINK_WARNING 32	/* my_redel() gives warning if links */
 #define MY_COPYTIME	64	/* my_redel() copys time */

mysql-test/r/symlink-aria-11902.result

+set default_storage_engine=Aria;
+call mtr.add_suppression("File.*t1.* not found");
+create table mysql.t1 (a int, b char(16), index(a));
+insert mysql.t1 values (100, 'test'),(101,'test');
+create table t1 (a int, b char(16), index(a))
+data directory="MYSQLTEST_VARDIR/tmp/foo";
+insert t1 values (200, 'some'),(201,'some');
+select * from t1;
+a	b
+200	some
+201	some
+flush tables;
+set debug_sync='mi_open_datafile SIGNAL ok WAIT_FOR go';
+select * from t1;
+set debug_sync='now WAIT_FOR ok';
+set debug_sync='now SIGNAL go';
+ERROR HY000: File 'MYSQLTEST_VARDIR/tmp/foo/t1.MAD' not found (Errcode: 20)
+flush tables;
+drop table if exists t1;
+create table t1 (a int, b char(16), index (a))
+index directory="MYSQLTEST_VARDIR/tmp/foo";
+insert t1 values (200, 'some'),(201,'some');
+explain select a from t1;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	index	NULL	a	5	NULL	2	Using index
+select a from t1;
+a
+200
+201
+flush tables;
+set debug_sync='mi_open_kfile SIGNAL waiting WAIT_FOR run';
+select a from t1;
+set debug_sync='now WAIT_FOR waiting';
+set debug_sync='now SIGNAL run';
+ERROR HY000: Can't find file: 't1' (errno: 20)
+flush tables;
+drop table if exists t1;
+drop table mysql.t1;
+set debug_sync='RESET';

mysql-test/r/symlink-myisam-11902.result

+call mtr.add_suppression("File.*t1.* not found");
+create table mysql.t1 (a int, b char(16), index(a));
+insert mysql.t1 values (100, 'test'),(101,'test');
+create table t1 (a int, b char(16), index(a))
+data directory="MYSQLTEST_VARDIR/tmp/foo";
+insert t1 values (200, 'some'),(201,'some');
+select * from t1;
+a	b
+200	some
+201	some
+flush tables;
+set debug_sync='mi_open_datafile SIGNAL ok WAIT_FOR go';
+select * from t1;
+set debug_sync='now WAIT_FOR ok';
+set debug_sync='now SIGNAL go';
+ERROR HY000: File 'MYSQLTEST_VARDIR/tmp/foo/t1.MYD' not found (Errcode: 20)
+flush tables;
+drop table if exists t1;
+create table t1 (a int, b char(16), index (a))
+index directory="MYSQLTEST_VARDIR/tmp/foo";
+insert t1 values (200, 'some'),(201,'some');
+explain select a from t1;
+id	select_type	table	type	possible_keys	key	key_len	ref	rows	Extra
+1	SIMPLE	t1	index	NULL	a	5	NULL	2	Using index
+select a from t1;
+a
+200
+201
+flush tables;
+set debug_sync='mi_open_kfile SIGNAL waiting WAIT_FOR run';
+select a from t1;
+set debug_sync='now WAIT_FOR waiting';
+set debug_sync='now SIGNAL run';
+ERROR HY000: Can't find file: 't1' (errno: 20)
+flush tables;
+drop table if exists t1;
+drop table mysql.t1;
+set debug_sync='RESET';

mysql-test/suite/federated/federated_bug_35333.result

@@ -27,9 +27,6 @@ TABLE_SCHEMA	TABLE_NAME	TABLE_TYPE	ENGINE	ROW_FORMAT	TABLE_ROWS	DATA_LENGTH	TABL
 test	t1	BASE TABLE	NULL	NULL	NULL	NULL	Can't find file: 't1' (errno: 2)
 Warnings:
 Warning	1017	Can't find file: 't1' (errno: 2)
-SHOW WARNINGS;
-Level	Code	Message
-Warning	1017	Can't find file: 't1' (errno: 2)
 DROP TABLE t1;
 ERROR 42S02: Unknown table 't1'
 #

mysql-test/suite/federated/federated_bug_35333.test

@@ -61,10 +61,10 @@ let $MYSQLD_DATADIR= `SELECT @@datadir`;
 --echo #
 --echo # Trigger a MyISAM system error during an INFORMATION_SCHEMA.TABLES query
 --echo #
+--replace_result 20 2
 SELECT TABLE_SCHEMA, TABLE_NAME, TABLE_TYPE, ENGINE, ROW_FORMAT, TABLE_ROWS, DATA_LENGTH, TABLE_COMMENT
        FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = 't1';
 
-SHOW WARNINGS;
 --disable_warnings
 --error 1051
 DROP TABLE t1;

mysql-test/t/repair_symlink-5543.test

@@ -9,7 +9,7 @@
 eval create table t1 (a int) engine=myisam data directory='$MYSQL_TMP_DIR';
 insert t1 values (1);
 --system ln -s $MYSQL_TMP_DIR/foobar5543 $MYSQL_TMP_DIR/t1.TMD
---replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
+--replace_regex / '.*\/t1/ 'MYSQL_TMP_DIR\/t1/
 repair table t1;
 drop table t1;
 
@@ -17,7 +17,7 @@ drop table t1;
 eval create table t2 (a int) engine=aria data directory='$MYSQL_TMP_DIR';
 insert t2 values (1);
 --system ln -s $MYSQL_TMP_DIR/foobar5543 $MYSQL_TMP_DIR/t2.TMD
---replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
+--replace_regex / '.*\/t2/ 'MYSQL_TMP_DIR\/t2/
 repair table t2;
 drop table t2;
 

mysql-test/t/symlink-aria-11902.test

+#
+# MDEV-11902 mi_open race condition
+#
+source include/have_maria.inc;
+set default_storage_engine=Aria;
+source symlink-myisam-11902.test;

mysql-test/t/symlink-myisam-11902.test

+#
+# MDEV-11902 mi_open race condition
+#
+source include/have_debug_sync.inc;
+source include/have_symlink.inc;
+source include/not_windows.inc;
+call mtr.add_suppression("File.*t1.* not found");
+
+create table mysql.t1 (a int, b char(16), index(a));
+insert mysql.t1 values (100, 'test'),(101,'test');
+let $datadir=`select @@datadir`;
+
+exec mkdir $MYSQLTEST_VARDIR/tmp/foo;
+replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR;
+eval create table t1 (a int, b char(16), index(a))
+                      data directory="$MYSQLTEST_VARDIR/tmp/foo";
+insert t1 values (200, 'some'),(201,'some');
+select * from t1;
+flush tables;
+set debug_sync='mi_open_datafile SIGNAL ok WAIT_FOR go';
+send select * from t1;
+connect con1, localhost, root;
+set debug_sync='now WAIT_FOR ok';
+exec rm -r $MYSQLTEST_VARDIR/tmp/foo;
+exec ln -s $datadir/mysql $MYSQLTEST_VARDIR/tmp/foo;
+set debug_sync='now SIGNAL go';
+connection default;
+replace_regex / '.*\/tmp\// 'MYSQLTEST_VARDIR\/tmp\// /31/20/;
+error 29;
+reap;
+flush tables;
+drop table if exists t1;
+exec rm -r $MYSQLTEST_VARDIR/tmp/foo;
+
+# same with INDEX DIRECTORY
+exec mkdir $MYSQLTEST_VARDIR/tmp/foo;
+replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR;
+eval create table t1 (a int, b char(16), index (a))
+                      index directory="$MYSQLTEST_VARDIR/tmp/foo";
+insert t1 values (200, 'some'),(201,'some');
+explain select a from t1;
+select a from t1;
+flush tables;
+set debug_sync='mi_open_kfile SIGNAL waiting WAIT_FOR run';
+send select a from t1;
+connection con1;
+set debug_sync='now WAIT_FOR waiting';
+exec rm -r $MYSQLTEST_VARDIR/tmp/foo;
+exec ln -s $datadir/mysql $MYSQLTEST_VARDIR/tmp/foo;
+set debug_sync='now SIGNAL run';
+connection default;
+replace_regex / '.*\/tmp\// 'MYSQLTEST_VARDIR\/tmp\// /31/20/;
+error ER_FILE_NOT_FOUND;
+reap;
+flush tables;
+drop table if exists t1;
+exec rm -r $MYSQLTEST_VARDIR/tmp/foo;
+
+drop table mysql.t1;
+set debug_sync='RESET';

mysys/my_open.c

@@ -15,9 +15,14 @@
 
 #include "mysys_priv.h"
 #include "mysys_err.h"
-#include <my_dir.h>
+#include <m_string.h>
 #include <errno.h>
 
+#if !defined(O_PATH) && defined(O_EXEC) /* FreeBSD */
+#define O_PATH O_EXEC
+#endif
+
+static int open_nosymlinks(const char *pathname, int flags, int mode);
 
 /*
   Open a file
@@ -46,7 +51,10 @@ File my_open(const char *FileName, int Flags, myf MyFlags)
 #if defined(_WIN32)
   fd= my_win_open(FileName, Flags);
 #else
-  fd = open(FileName, Flags, my_umask);
+  if (MyFlags & MY_NOSYMLINKS)
+    fd = open_nosymlinks(FileName, Flags, my_umask);
+  else
+    fd = open(FileName, Flags, my_umask);
 #endif
 
   fd= my_register_filename(fd, FileName, FILE_BY_OPEN,
@@ -174,3 +182,81 @@ void my_print_open_files(void)
 }
 
 #endif
+
+/**
+  like open(), but with symlinks are not accepted anywhere in the path
+
+  This is used for opening symlinked tables for DATA/INDEX DIRECTORY.
+  The paths there have been realpath()-ed. So, we can assume here that
+
+  * `pathname` is an absolute path
+  * no '.', '..', and '//' in the path
+  * file exists
+*/
+static int open_nosymlinks(const char *pathname, int flags, int mode)
+{
+#ifndef O_PATH
+#ifdef HAVE_REALPATH
+  char buf[PATH_MAX+1];
+  if (realpath(pathname, buf) == NULL)
+    return -1;
+  if (strcmp(pathname, buf))
+  {
+    errno= ENOTDIR;
+    return -1;
+  }
+#endif
+  return open(pathname, flags, mode | O_NOFOLLOW);
+#else
+
+  char buf[PATH_MAX+1];
+  char *s= buf, *e= buf+1, *end= strnmov(buf, pathname, sizeof(buf));
+  int fd, dfd= -1;
+
+  if (*end)
+  {
+    errno= ENAMETOOLONG;
+    return -1;
+  }
+
+  if (*s != '/') /* not an absolute path */
+  {
+    errno= ENOENT;
+    return -1;
+  }
+
+  for (;;)
+  {
+    if (*e == '/') /* '//' in the path */
+    {
+      errno= ENOENT;
+      goto err;
+    }
+    while (*e && *e != '/')
+      e++;
+    *e= 0;
+    if (!memcmp(s, ".", 2) || !memcmp(s, "..", 3))
+    {
+      errno= ENOENT;
+      goto err;
+    }
+
+    fd = openat(dfd, s, O_NOFOLLOW | (e < end ? O_PATH : flags), mode);
+    if (fd < 0)
+      goto err;
+
+    if (dfd >= 0)
+      close(dfd);
+
+    dfd= fd;
+    s= ++e;
+
+    if (e >= end)
+      return fd;
+  }
+err:
+  if (dfd >= 0)
+    close(dfd);
+  return -1;
+#endif
+}

sql/handler.cc

@@ -2908,6 +2908,7 @@ void handler::print_error(int error, myf errflag)
     textno=ER_FILE_USED;
     break;
   case ENOENT:
+  case ENOTDIR:
     textno=ER_FILE_NOT_FOUND;
     break;
   case ENOSPC:

storage/maria/ma_open.c

@@ -312,13 +312,16 @@ MARIA_HA *maria_open(const char *name, int mode, uint open_flags)
                       my_errno= HA_ERR_CRASHED;
                       goto err;
                     });
+    DEBUG_SYNC_C("mi_open_kfile");
     if ((kfile=mysql_file_open(key_file_kfile, name_buff,
-                               (open_mode=O_RDWR) | O_SHARE,MYF(0))) < 0)
+                               (open_mode=O_RDWR) | O_SHARE | O_NOFOLLOW,
+                               MYF(MY_NOSYMLINKS))) < 0)
     {
       if ((errno != EROFS && errno != EACCES) ||
 	  mode != O_RDONLY ||
 	  (kfile=mysql_file_open(key_file_kfile, name_buff,
-                                 (open_mode=O_RDONLY) | O_SHARE,MYF(0))) < 0)
+                                 (open_mode=O_RDONLY) | O_SHARE | O_NOFOLLOW,
+                                 MYF(MY_NOSYMLINKS))) < 0)
 	goto err;
     }
     share->mode=open_mode;
@@ -363,7 +366,18 @@ MARIA_HA *maria_open(const char *name, int mode, uint open_flags)
       (void) strmov(index_name, org_name);
     *strrchr(org_name, FN_EXTCHAR)= '\0';
     (void) fn_format(data_name,org_name,"",MARIA_NAME_DEXT,
-                     MY_APPEND_EXT|MY_UNPACK_FILENAME|MY_RESOLVE_SYMLINKS);
+                     MY_APPEND_EXT|MY_UNPACK_FILENAME);
+    if (my_is_symlink(data_name))
+    {
+      if (my_realpath(data_name, data_name, MYF(0)))
+        goto err;
+      if (mysys_test_invalid_symlink(data_name))
+      {
+        my_errno= HA_WRONG_CREATE_OPTION;
+        goto err;
+      }
+      share->mode|= O_NOFOLLOW; /* all symlinks are resolved by realpath() */
+    }
 
     info_length=mi_uint2korr(share->state.header.header_length);
     base_pos= mi_uint2korr(share->state.header.base_pos);
@@ -1867,27 +1881,11 @@ void _ma_set_index_pagecache_callbacks(PAGECACHE_FILE *file,
 
 int _ma_open_datafile(MARIA_HA *info, MARIA_SHARE *share, const char *org_name)
 {
-  char *data_name= share->data_file_name.str;
-  char real_data_name[FN_REFLEN];
-
-  if (org_name)
-  {
-    fn_format(real_data_name, org_name, "", MARIA_NAME_DEXT, 4);
-    if (my_is_symlink(real_data_name))
-    {
-      if (my_realpath(real_data_name, real_data_name, MYF(0)) ||
-          mysys_test_invalid_symlink(real_data_name))
-      {
-        my_errno= HA_WRONG_CREATE_OPTION;
-        return 1;
-      }
-      data_name= real_data_name;
-    }
-  }
-
+  myf flags= MY_WME | (share->mode & O_NOFOLLOW ? MY_NOSYMLINKS : 0);
+  DEBUG_SYNC_C("mi_open_datafile");
   info->dfile.file= share->bitmap.file.file=
-    mysql_file_open(key_file_dfile, data_name,
-                    share->mode | O_SHARE, MYF(MY_WME));
+    mysql_file_open(key_file_dfile, share->data_file_name.str,
+                    share->mode | O_SHARE, MYF(flags));
   return info->dfile.file >= 0 ? 0 : 1;
 }
 
@@ -1901,8 +1899,8 @@ int _ma_open_keyfile(MARIA_SHARE *share)
   mysql_mutex_lock(&share->intern_lock);
   share->kfile.file= mysql_file_open(key_file_kfile,
                                      share->unique_file_name.str,
-                                     share->mode | O_SHARE,
-                             MYF(MY_WME));
+                                     share->mode | O_SHARE | O_NOFOLLOW,
+                             MYF(MY_WME | MY_NOSYMLINKS));
   mysql_mutex_unlock(&share->intern_lock);
   return (share->kfile.file < 0);
 }

storage/myisam/ha_myisam.cc

@@ -1874,15 +1874,22 @@ int ha_myisam::info(uint flag)
      Set data_file_name and index_file_name to point at the symlink value
      if table is symlinked (Ie;  Real name is not same as generated name)
    */
+    char buf[FN_REFLEN];
     data_file_name= index_file_name= 0;
     fn_format(name_buff, file->filename, "", MI_NAME_DEXT,
               MY_APPEND_EXT | MY_UNPACK_FILENAME);
-    if (strcmp(name_buff, misam_info.data_file_name))
-      data_file_name=misam_info.data_file_name;
+    if (my_is_symlink(name_buff))
+    {
+      my_readlink(buf, name_buff, MYF(0));
+      data_file_name= ha_thd()->strdup(buf);
+    }
     fn_format(name_buff, file->filename, "", MI_NAME_IEXT,
               MY_APPEND_EXT | MY_UNPACK_FILENAME);
-    if (strcmp(name_buff, misam_info.index_file_name))
-      index_file_name=misam_info.index_file_name;
+    if (my_is_symlink(name_buff))
+    {
+      my_readlink(buf, name_buff, MYF(0));
+      index_file_name= ha_thd()->strdup(buf);
+    }
   }
   if (flag & HA_STATUS_ERRKEY)
   {

storage/myisam/mi_open.c

@@ -114,15 +114,17 @@ MI_INFO *mi_open(const char *name, int mode, uint open_flags)
                       my_errno= HA_ERR_CRASHED;
                       goto err;
                     });
-    if ((kfile= mysql_file_open(mi_key_file_kfile,
-                                name_buff,
-                                (open_mode= O_RDWR) | O_SHARE, MYF(0))) < 0)
+
+    DEBUG_SYNC_C("mi_open_kfile");
+    if ((kfile= mysql_file_open(mi_key_file_kfile, name_buff,
+                                (open_mode= O_RDWR) | O_SHARE | O_NOFOLLOW,
+                                MYF(MY_NOSYMLINKS))) < 0)
     {
       if ((errno != EROFS && errno != EACCES) ||
 	  mode != O_RDONLY ||
-          (kfile= mysql_file_open(mi_key_file_kfile,
-                                  name_buff,
-                                  (open_mode= O_RDONLY) | O_SHARE, MYF(0))) < 0)
+          (kfile= mysql_file_open(mi_key_file_kfile, name_buff,
+                                  (open_mode= O_RDONLY) | O_SHARE| O_NOFOLLOW,
+                                  MYF(MY_NOSYMLINKS))) < 0)
 	goto err;
     }
     share->mode=open_mode;
@@ -166,7 +168,18 @@ MI_INFO *mi_open(const char *name, int mode, uint open_flags)
       (void) strmov(index_name, org_name);
     *strrchr(org_name, '.')= '\0';
     (void) fn_format(data_name,org_name,"",MI_NAME_DEXT,
-                     MY_APPEND_EXT|MY_UNPACK_FILENAME|MY_RESOLVE_SYMLINKS);
+                     MY_APPEND_EXT|MY_UNPACK_FILENAME);
+    if (my_is_symlink(data_name))
+    {
+      if (my_realpath(data_name, data_name, MYF(0)))
+        goto err;
+      if (mysys_test_invalid_symlink(data_name))
+      {
+        my_errno= HA_WRONG_CREATE_OPTION;
+        goto err;
+      }
+      share->mode|= O_NOFOLLOW; /* all symlinks are resolved by realpath() */
+    }
 
     info_length=mi_uint2korr(share->state.header.header_length);
     base_pos=mi_uint2korr(share->state.header.base_pos);
@@ -1233,25 +1246,10 @@ active seek-positions.
 
 int mi_open_datafile(MI_INFO *info, MYISAM_SHARE *share, const char *org_name)
 {
-  char *data_name= share->data_file_name;
-  char real_data_name[FN_REFLEN];
-
-  if (org_name)
-  {
-    fn_format(real_data_name,org_name,"",MI_NAME_DEXT,4);
-    if (my_is_symlink(real_data_name))
-    {
-      if (my_realpath(real_data_name, real_data_name, MYF(0)) ||
-          mysys_test_invalid_symlink(real_data_name))
-      {
-        my_errno= HA_WRONG_CREATE_OPTION;
-        return 1;
-      }
-      data_name= real_data_name;
-    }
-  }
-  info->dfile= mysql_file_open(mi_key_file_dfile,
-                               data_name, share->mode | O_SHARE, MYF(MY_WME));
+  myf flags= MY_WME | (share->mode & O_NOFOLLOW ? MY_NOSYMLINKS: 0);
+  DEBUG_SYNC_C("mi_open_datafile");
+  info->dfile= mysql_file_open(mi_key_file_dfile, share->data_file_name,
+                               share->mode | O_SHARE, MYF(flags));
   return info->dfile >= 0 ? 0 : 1;
 }
 
@@ -1260,8 +1258,8 @@ int mi_open_keyfile(MYISAM_SHARE *share)
 {
   if ((share->kfile= mysql_file_open(mi_key_file_kfile,
                                      share->unique_file_name,
-                                     share->mode | O_SHARE,
-                                     MYF(MY_WME))) < 0)
+                                     share->mode | O_SHARE | O_NOFOLLOW,
+                                     MYF(MY_NOSYMLINKS | MY_WME))) < 0)
     return 1;
   return 0;
 }