Skip to content

Commit b27fd90

Browse files
vuvovavuvova
committed
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.
1 parent d78d0d4 commit b27fd90

13 files changed

+298
-68
lines changed

‎include/my_sys.h

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ typedef struct my_aio_result {
6363
#define MY_FAE 8 /* Fatal if any error */
6464
#define MY_WME 16 /* Write message on error */
6565
#define MY_WAIT_IF_FULL 32 /* Wait and try again if disk full error */
66-
#define MY_IGNORE_BADFD 32 /* my_sync: ignore 'bad descriptor' errors */
67-
#define MY_UNUSED 64 /* Unused (was support for RAID) */
68-
#define MY_FULL_IO 512 /* For my_read - loop intil I/O is complete */
66+
#define MY_IGNORE_BADFD 32 /* my_sync(): ignore 'bad descriptor' errors */
67+
#define MY_NOSYMLINKS 512 /* my_open(): don't follow symlinks */
68+
#define MY_FULL_IO 512 /* my_read(): loop intil I/O is complete */
6969
#define MY_DONT_CHECK_FILESIZE 128 /* Option to init_io_cache() */
7070
#define MY_LINK_WARNING 32 /* my_redel() gives warning if links */
7171
#define MY_COPYTIME 64 /* my_redel() copys time */

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
set default_storage_engine=Aria;
2+
call mtr.add_suppression("File.*t1.* not found");
3+
create table mysql.t1 (a int, b char(16), index(a));
4+
insert mysql.t1 values (100, 'test'),(101,'test');
5+
create table t1 (a int, b char(16), index(a))
6+
data directory="MYSQLTEST_VARDIR/tmp/foo";
7+
insert t1 values (200, 'some'),(201,'some');
8+
select * from t1;
9+
a b
10+
200 some
11+
201 some
12+
flush tables;
13+
set debug_sync='mi_open_datafile SIGNAL ok WAIT_FOR go';
14+
select * from t1;
15+
set debug_sync='now WAIT_FOR ok';
16+
set debug_sync='now SIGNAL go';
17+
ERROR HY000: File 'MYSQLTEST_VARDIR/tmp/foo/t1.MAD' not found (Errcode: 20)
18+
flush tables;
19+
drop table if exists t1;
20+
create table t1 (a int, b char(16), index (a))
21+
index directory="MYSQLTEST_VARDIR/tmp/foo";
22+
insert t1 values (200, 'some'),(201,'some');
23+
explain select a from t1;
24+
id select_type table type possible_keys key key_len ref rows Extra
25+
1 SIMPLE t1 index NULL a 5 NULL 2 Using index
26+
select a from t1;
27+
a
28+
200
29+
201
30+
flush tables;
31+
set debug_sync='mi_open_kfile SIGNAL waiting WAIT_FOR run';
32+
select a from t1;
33+
set debug_sync='now WAIT_FOR waiting';
34+
set debug_sync='now SIGNAL run';
35+
ERROR HY000: Can't find file: 't1' (errno: 20)
36+
flush tables;
37+
drop table if exists t1;
38+
drop table mysql.t1;
39+
set debug_sync='RESET';

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
call mtr.add_suppression("File.*t1.* not found");
2+
create table mysql.t1 (a int, b char(16), index(a));
3+
insert mysql.t1 values (100, 'test'),(101,'test');
4+
create table t1 (a int, b char(16), index(a))
5+
data directory="MYSQLTEST_VARDIR/tmp/foo";
6+
insert t1 values (200, 'some'),(201,'some');
7+
select * from t1;
8+
a b
9+
200 some
10+
201 some
11+
flush tables;
12+
set debug_sync='mi_open_datafile SIGNAL ok WAIT_FOR go';
13+
select * from t1;
14+
set debug_sync='now WAIT_FOR ok';
15+
set debug_sync='now SIGNAL go';
16+
ERROR HY000: File 'MYSQLTEST_VARDIR/tmp/foo/t1.MYD' not found (Errcode: 20)
17+
flush tables;
18+
drop table if exists t1;
19+
create table t1 (a int, b char(16), index (a))
20+
index directory="MYSQLTEST_VARDIR/tmp/foo";
21+
insert t1 values (200, 'some'),(201,'some');
22+
explain select a from t1;
23+
id select_type table type possible_keys key key_len ref rows Extra
24+
1 SIMPLE t1 index NULL a 5 NULL 2 Using index
25+
select a from t1;
26+
a
27+
200
28+
201
29+
flush tables;
30+
set debug_sync='mi_open_kfile SIGNAL waiting WAIT_FOR run';
31+
select a from t1;
32+
set debug_sync='now WAIT_FOR waiting';
33+
set debug_sync='now SIGNAL run';
34+
ERROR HY000: Can't find file: 't1' (errno: 20)
35+
flush tables;
36+
drop table if exists t1;
37+
drop table mysql.t1;
38+
set debug_sync='RESET';

‎mysql-test/suite/federated/federated_bug_35333.result

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,6 @@ TABLE_SCHEMA TABLE_NAME TABLE_TYPE ENGINE ROW_FORMAT TABLE_ROWS DATA_LENGTH TABL
2727
test t1 BASE TABLE NULL NULL NULL NULL Can't find file: 't1' (errno: 2)
2828
Warnings:
2929
Warning 1017 Can't find file: 't1' (errno: 2)
30-
SHOW WARNINGS;
31-
Level Code Message
32-
Warning 1017 Can't find file: 't1' (errno: 2)
3330
DROP TABLE t1;
3431
ERROR 42S02: Unknown table 't1'
3532
#

‎mysql-test/suite/federated/federated_bug_35333.test

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,10 @@ let $MYSQLD_DATADIR= `SELECT @@datadir`;
6161
--echo #
6262
--echo # Trigger a MyISAM system error during an INFORMATION_SCHEMA.TABLES query
6363
--echo #
64+
--replace_result 20 2
6465
SELECT TABLE_SCHEMA, TABLE_NAME, TABLE_TYPE, ENGINE, ROW_FORMAT, TABLE_ROWS, DATA_LENGTH, TABLE_COMMENT
6566
FROM INFORMATION_SCHEMA.TABLES WHERE TABLE_NAME = 't1';
6667

67-
SHOW WARNINGS;
6868
--disable_warnings
6969
--error 1051
7070
DROP TABLE t1;

‎mysql-test/t/repair_symlink-5543.test

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,15 @@
99
eval create table t1 (a int) engine=myisam data directory='$MYSQL_TMP_DIR';
1010
insert t1 values (1);
1111
--system ln -s $MYSQL_TMP_DIR/foobar5543 $MYSQL_TMP_DIR/t1.TMD
12-
--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
12+
--replace_regex / '.*\/t1/ 'MYSQL_TMP_DIR\/t1/
1313
repair table t1;
1414
drop table t1;
1515

1616
--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
1717
eval create table t2 (a int) engine=aria data directory='$MYSQL_TMP_DIR';
1818
insert t2 values (1);
1919
--system ln -s $MYSQL_TMP_DIR/foobar5543 $MYSQL_TMP_DIR/t2.TMD
20-
--replace_result $MYSQL_TMP_DIR MYSQL_TMP_DIR
20+
--replace_regex / '.*\/t2/ 'MYSQL_TMP_DIR\/t2/
2121
repair table t2;
2222
drop table t2;
2323

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#
2+
# MDEV-11902 mi_open race condition
3+
#
4+
source include/have_maria.inc;
5+
set default_storage_engine=Aria;
6+
source symlink-myisam-11902.test;

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

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
#
2+
# MDEV-11902 mi_open race condition
3+
#
4+
source include/have_debug_sync.inc;
5+
source include/have_symlink.inc;
6+
source include/not_windows.inc;
7+
call mtr.add_suppression("File.*t1.* not found");
8+
9+
create table mysql.t1 (a int, b char(16), index(a));
10+
insert mysql.t1 values (100, 'test'),(101,'test');
11+
let $datadir=`select @@datadir`;
12+
13+
exec mkdir $MYSQLTEST_VARDIR/tmp/foo;
14+
replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR;
15+
eval create table t1 (a int, b char(16), index(a))
16+
data directory="$MYSQLTEST_VARDIR/tmp/foo";
17+
insert t1 values (200, 'some'),(201,'some');
18+
select * from t1;
19+
flush tables;
20+
set debug_sync='mi_open_datafile SIGNAL ok WAIT_FOR go';
21+
send select * from t1;
22+
connect con1, localhost, root;
23+
set debug_sync='now WAIT_FOR ok';
24+
exec rm -r $MYSQLTEST_VARDIR/tmp/foo;
25+
exec ln -s $datadir/mysql $MYSQLTEST_VARDIR/tmp/foo;
26+
set debug_sync='now SIGNAL go';
27+
connection default;
28+
replace_regex / '.*\/tmp\// 'MYSQLTEST_VARDIR\/tmp\// /31/20/;
29+
error 29;
30+
reap;
31+
flush tables;
32+
drop table if exists t1;
33+
exec rm -r $MYSQLTEST_VARDIR/tmp/foo;
34+
35+
# same with INDEX DIRECTORY
36+
exec mkdir $MYSQLTEST_VARDIR/tmp/foo;
37+
replace_result $MYSQLTEST_VARDIR MYSQLTEST_VARDIR;
38+
eval create table t1 (a int, b char(16), index (a))
39+
index directory="$MYSQLTEST_VARDIR/tmp/foo";
40+
insert t1 values (200, 'some'),(201,'some');
41+
explain select a from t1;
42+
select a from t1;
43+
flush tables;
44+
set debug_sync='mi_open_kfile SIGNAL waiting WAIT_FOR run';
45+
send select a from t1;
46+
connection con1;
47+
set debug_sync='now WAIT_FOR waiting';
48+
exec rm -r $MYSQLTEST_VARDIR/tmp/foo;
49+
exec ln -s $datadir/mysql $MYSQLTEST_VARDIR/tmp/foo;
50+
set debug_sync='now SIGNAL run';
51+
connection default;
52+
replace_regex / '.*\/tmp\// 'MYSQLTEST_VARDIR\/tmp\// /31/20/;
53+
error ER_FILE_NOT_FOUND;
54+
reap;
55+
flush tables;
56+
drop table if exists t1;
57+
exec rm -r $MYSQLTEST_VARDIR/tmp/foo;
58+
59+
drop table mysql.t1;
60+
set debug_sync='RESET';

‎mysys/my_open.c

Lines changed: 88 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,14 @@
1515

1616
#include "mysys_priv.h"
1717
#include "mysys_err.h"
18-
#include <my_dir.h>
18+
#include <m_string.h>
1919
#include <errno.h>
2020

21+
#if !defined(O_PATH) && defined(O_EXEC) /* FreeBSD */
22+
#define O_PATH O_EXEC
23+
#endif
24+
25+
static int open_nosymlinks(const char *pathname, int flags, int mode);
2126

2227
/*
2328
Open a file
@@ -46,7 +51,10 @@ File my_open(const char *FileName, int Flags, myf MyFlags)
4651
#if defined(_WIN32)
4752
fd= my_win_open(FileName, Flags);
4853
#else
49-
fd = open(FileName, Flags, my_umask);
54+
if (MyFlags & MY_NOSYMLINKS)
55+
fd = open_nosymlinks(FileName, Flags, my_umask);
56+
else
57+
fd = open(FileName, Flags, my_umask);
5058
#endif
5159

5260
fd= my_register_filename(fd, FileName, FILE_BY_OPEN,
@@ -174,3 +182,81 @@ void my_print_open_files(void)
174182
}
175183

176184
#endif
185+
186+
/**
187+
like open(), but with symlinks are not accepted anywhere in the path
188+
189+
This is used for opening symlinked tables for DATA/INDEX DIRECTORY.
190+
The paths there have been realpath()-ed. So, we can assume here that
191+
192+
* `pathname` is an absolute path
193+
* no '.', '..', and '//' in the path
194+
* file exists
195+
*/
196+
static int open_nosymlinks(const char *pathname, int flags, int mode)
197+
{
198+
#ifndef O_PATH
199+
#ifdef HAVE_REALPATH
200+
char buf[PATH_MAX+1];
201+
if (realpath(pathname, buf) == NULL)
202+
return -1;
203+
if (strcmp(pathname, buf))
204+
{
205+
errno= ENOTDIR;
206+
return -1;
207+
}
208+
#endif
209+
return open(pathname, flags, mode | O_NOFOLLOW);
210+
#else
211+
212+
char buf[PATH_MAX+1];
213+
char *s= buf, *e= buf+1, *end= strnmov(buf, pathname, sizeof(buf));
214+
int fd, dfd= -1;
215+
216+
if (*end)
217+
{
218+
errno= ENAMETOOLONG;
219+
return -1;
220+
}
221+
222+
if (*s != '/') /* not an absolute path */
223+
{
224+
errno= ENOENT;
225+
return -1;
226+
}
227+
228+
for (;;)
229+
{
230+
if (*e == '/') /* '//' in the path */
231+
{
232+
errno= ENOENT;
233+
goto err;
234+
}
235+
while (*e && *e != '/')
236+
e++;
237+
*e= 0;
238+
if (!memcmp(s, ".", 2) || !memcmp(s, "..", 3))
239+
{
240+
errno= ENOENT;
241+
goto err;
242+
}
243+
244+
fd = openat(dfd, s, O_NOFOLLOW | (e < end ? O_PATH : flags), mode);
245+
if (fd < 0)
246+
goto err;
247+
248+
if (dfd >= 0)
249+
close(dfd);
250+
251+
dfd= fd;
252+
s= ++e;
253+
254+
if (e >= end)
255+
return fd;
256+
}
257+
err:
258+
if (dfd >= 0)
259+
close(dfd);
260+
return -1;
261+
#endif
262+
}

‎sql/handler.cc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2908,6 +2908,7 @@ void handler::print_error(int error, myf errflag)
29082908
textno=ER_FILE_USED;
29092909
break;
29102910
case ENOENT:
2911+
case ENOTDIR:
29112912
textno=ER_FILE_NOT_FOUND;
29122913
break;
29132914
case ENOSPC:

0 commit comments

Comments
 (0)