From 24d8bc707a3d3161229b1cfc94b34dc50473bc59 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Sat, 18 Feb 2017 10:08:49 +0100 Subject: cleanup: my_register_filename() Don't let my_register_filename() fail because strdup() failed. Better to have NULL for a filename, then to fail the already successful open(). Filenames are only used for error reporting and there was already code to ignore OOMs (my_fdopen()) and to cope with missing filenames (my_filename()). --- mysys/my_open.c | 27 +++++++++------------------ 1 file changed, 9 insertions(+), 18 deletions(-) (limited to 'mysys/my_open.c') diff --git a/mysys/my_open.c b/mysys/my_open.c index 645d6709358..d19db0aaf0c 100644 --- a/mysys/my_open.c +++ b/mysys/my_open.c @@ -131,25 +131,16 @@ File my_register_filename(File fd, const char *FileName, enum file_type thread_safe_increment(my_file_opened,&THR_LOCK_open); DBUG_RETURN(fd); /* safeguard */ } - else - { - mysql_mutex_lock(&THR_LOCK_open); - if ((my_file_info[fd].name = (char*) my_strdup(FileName,MyFlags))) - { - my_file_opened++; - my_file_total_opened++; - my_file_info[fd].type = type_of_file; - mysql_mutex_unlock(&THR_LOCK_open); - DBUG_PRINT("exit",("fd: %d",fd)); - DBUG_RETURN(fd); - } - mysql_mutex_unlock(&THR_LOCK_open); - my_errno= ENOMEM; - } - (void) my_close(fd, MyFlags); + mysql_mutex_lock(&THR_LOCK_open); + my_file_info[fd].name = (char*) my_strdup(FileName, MyFlags); + my_file_opened++; + my_file_total_opened++; + my_file_info[fd].type = type_of_file; + mysql_mutex_unlock(&THR_LOCK_open); + DBUG_PRINT("exit",("fd: %d",fd)); + DBUG_RETURN(fd); } - else - my_errno= errno; + my_errno= errno; DBUG_PRINT("error",("Got error %d on open", my_errno)); if (MyFlags & (MY_FFNF | MY_FAE | MY_WME)) -- cgit v1.2.1 From d78d0d459d10dd12069de82d6735f1acf183c631 Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Sat, 18 Feb 2017 10:38:14 +0100 Subject: cleanup: NO_OPEN_3 was never defined --- mysys/my_open.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) (limited to 'mysys/my_open.c') diff --git a/mysys/my_open.c b/mysys/my_open.c index d19db0aaf0c..24d5c881372 100644 --- a/mysys/my_open.c +++ b/mysys/my_open.c @@ -45,10 +45,8 @@ File my_open(const char *FileName, int Flags, myf MyFlags) MyFlags|= my_global_flags; #if defined(_WIN32) fd= my_win_open(FileName, Flags); -#elif !defined(NO_OPEN_3) - fd = open(FileName, Flags, my_umask); /* Normal unix */ #else - fd = open((char *) FileName, Flags); + fd = open(FileName, Flags, my_umask); #endif fd= my_register_filename(fd, FileName, FILE_BY_OPEN, -- cgit v1.2.1 From b27fd90ad36f4194665744cc1dcdd05f2d0b47ef Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Wed, 15 Feb 2017 18:45:19 +0100 Subject: 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. --- mysys/my_open.c | 90 +++++++++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 88 insertions(+), 2 deletions(-) (limited to 'mysys/my_open.c') diff --git a/mysys/my_open.c b/mysys/my_open.c index 24d5c881372..0effc4bedda 100644 --- a/mysys/my_open.c +++ b/mysys/my_open.c @@ -15,9 +15,14 @@ #include "mysys_priv.h" #include "mysys_err.h" -#include +#include #include +#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 +} -- cgit v1.2.1 From 6d50324558e79d6e90848059bf7cd918685b02ba Mon Sep 17 00:00:00 2001 From: Sergei Golubchik Date: Mon, 20 Feb 2017 22:41:17 +0100 Subject: support MY_NOSYMLINKS in my_delete() --- mysys/my_open.c | 88 ++++----------------------------------------------------- 1 file changed, 5 insertions(+), 83 deletions(-) (limited to 'mysys/my_open.c') diff --git a/mysys/my_open.c b/mysys/my_open.c index 0effc4bedda..cafb94ef558 100644 --- a/mysys/my_open.c +++ b/mysys/my_open.c @@ -18,11 +18,11 @@ #include #include -#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); +CREATE_NOSYMLINK_FUNCTION( + open_nosymlinks(const char *pathname, int flags, int mode), + openat(dfd, filename, O_NOFOLLOW | flags, mode), + open(pathname, O_NOFOLLOW | flags, mode) +); /* Open a file @@ -182,81 +182,3 @@ 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 -} -- cgit v1.2.1