diff --git a/builtin.cpp b/builtin.cpp index bdbf270ff..d33b0864b 100644 --- a/builtin.cpp +++ b/builtin.cpp @@ -2724,6 +2724,7 @@ static int builtin_contains( parser_t &parser, wchar_t ** argv ) */ static int builtin_source( parser_t &parser, wchar_t ** argv ) { + ASSERT_IS_MAIN_THREAD(); int fd; int res = STATUS_BUILTIN_OK; struct stat buf; @@ -2759,7 +2760,7 @@ static int builtin_source( parser_t &parser, wchar_t ** argv ) return STATUS_BUILTIN_ERROR; } - if( ( fd = wopen( argv[1], O_RDONLY ) ) == -1 ) + if( ( fd = wopen_cloexec( argv[1], O_RDONLY ) ) == -1 ) { append_format(stderr_buffer, _(L"%ls: Error encountered while sourcing file '%ls':\n"), argv[0], argv[1] ); builtin_wperror( L"." ); diff --git a/common.cpp b/common.cpp index a0c3fe79e..012bf13cb 100644 --- a/common.cpp +++ b/common.cpp @@ -49,14 +49,6 @@ parts of fish. #include #endif -#ifndef HOST_NAME_MAX -/** - Maximum length of hostname return. It is ok if this is too short, - getting the actual hostname is not critical, so long as the string - is unique in the filesystem namespace. -*/ -#define HOST_NAME_MAX 255 -#endif #if HAVE_NCURSES_H #include @@ -88,11 +80,7 @@ parts of fish. #include "util.cpp" #include "fallback.cpp" -/** - The number of milliseconds to wait between polls when attempting to acquire - a lockfile -*/ -#define LOCKPOLLINTERVAL 10 + struct termios shell_modes; @@ -1693,231 +1681,7 @@ bool unescape_string(wcstring &str, int escape_special) return success; } -/** - Writes a pid_t in decimal representation to str. - str must contain sufficient space. - The conservatively approximate maximum number of characters a pid_t will - represent is given by: (int)(0.31 * sizeof(pid_t) + CHAR_BIT + 1) - Returns the length of the string -*/ -static int sprint_pid_t( pid_t pid, char *str ) -{ - int len, i = 0; - int dig; - /* Store digits in reverse order into string */ - while( pid != 0 ) - { - dig = pid % 10; - str[i] = '0' + dig; - pid = ( pid - dig ) / 10; - i++; - } - len = i; - /* Reverse digits */ - i /= 2; - while( i ) - { - i--; - dig = str[i]; - str[i] = str[len - 1 - i]; - str[len - 1 - i] = dig; - } - return len; -} - -/** - Writes a pseudo-random number (between one and maxlen) of pseudo-random - digits into str. - str must point to an allocated buffer of size of at least maxlen chars. - Returns the number of digits written. - Since the randomness in part depends on machine time it has _some_ extra - strength but still not enough for use in concurrent locking schemes on a - single machine because gettimeofday may not return a different value on - consecutive calls when: - a) the OS does not support fine enough resolution - b) the OS is running on an SMP machine. - Additionally, gettimeofday errors are ignored. - Excludes chars other than digits since ANSI C only guarantees that digits - are consecutive. -*/ -static int sprint_rand_digits( char *str, int maxlen ) -{ - int i, max; - struct timeval tv; - - /* - Seed the pseudo-random generator based on time - this assumes - that consecutive calls to gettimeofday will return different values - and ignores errors returned by gettimeofday. - Cast to unsigned so that wrapping occurs on overflow as per ANSI C. - */ - (void)gettimeofday( &tv, NULL ); - srand( (unsigned int)tv.tv_sec + (unsigned int)tv.tv_usec * 1000000UL ); - max = 1 + (maxlen - 1) * (rand() / (RAND_MAX + 1.0)); - for( i = 0; i < max; i++ ) - { - str[i] = '0' + 10 * (rand() / (RAND_MAX + 1.0)); - } - return i; -} - -/** - Generate a filename unique in an NFS namespace by creating a copy of str and - appending .{hostname}.{pid} to it. If gethostname() fails then a pseudo- - random string is substituted for {hostname} - the randomness of the string - should be strong enough across different machines. The main assumption - though is that gethostname will not fail and this is just a "safe enough" - fallback. - The memory returned should be freed using free(). -*/ -static char *gen_unique_nfs_filename( const char *filename ) -{ - int pidlen, hnlen, orglen = strlen( filename ); - char hostname[HOST_NAME_MAX + 1]; - char *newname; - - if ( gethostname( hostname, HOST_NAME_MAX + 1 ) == 0 ) - { - hnlen = strlen( hostname ); - } - else - { - hnlen = sprint_rand_digits( hostname, HOST_NAME_MAX ); - hostname[hnlen] = '\0'; - } - newname = (char *)malloc( orglen + 1 /* period */ + hnlen + 1 /* period */ + - /* max possible pid size: 0.31 ~= log(10)2 */ - (int)(0.31 * sizeof(pid_t) * CHAR_BIT + 1) - + 1 /* '\0' */ ); - - if ( newname == NULL ) - { - debug( 1, L"gen_unique_nfs_filename: %s", strerror( errno ) ); - return newname; - } - memcpy( newname, filename, orglen ); - newname[orglen] = '.'; - memcpy( newname + orglen + 1, hostname, hnlen ); - newname[orglen + 1 + hnlen] = '.'; - pidlen = sprint_pid_t( getpid(), newname + orglen + 1 + hnlen + 1 ); - newname[orglen + 1 + hnlen + 1 + pidlen] = '\0'; -/* debug( 1, L"gen_unique_nfs_filename returning with: newname = \"%s\"; " - L"HOST_NAME_MAX = %d; hnlen = %d; orglen = %d; " - L"sizeof(pid_t) = %d; maxpiddigits = %d; malloc'd size: %d", - newname, (int)HOST_NAME_MAX, hnlen, orglen, - (int)sizeof(pid_t), - (int)(0.31 * sizeof(pid_t) * CHAR_BIT + 1), - (int)(orglen + 1 + hnlen + 1 + - (int)(0.31 * sizeof(pid_t) * CHAR_BIT + 1) + 1) ); */ - return newname; -} - -int acquire_lock_file( const char *lockfile, const int timeout, int force ) -{ - int fd, timed_out = 0; - int ret = 0; /* early exit returns failure */ - struct timespec pollint; - struct timeval start, end; - double elapsed; - struct stat statbuf; - - /* - (Re)create a unique file and check that it has one only link. - */ - char *linkfile = gen_unique_nfs_filename( lockfile ); - if( linkfile == NULL ) - { - goto done; - } - (void)unlink( linkfile ); - if( ( fd = open( linkfile, O_CREAT|O_RDONLY, 0600 ) ) == -1 ) - { - debug( 1, L"acquire_lock_file: open: %s", strerror( errno ) ); - goto done; - } - /* - Don't need to check exit status of close on read-only file descriptors - */ - close( fd ); - if( stat( linkfile, &statbuf ) != 0 ) - { - debug( 1, L"acquire_lock_file: stat: %s", strerror( errno ) ); - goto done; - } - if ( statbuf.st_nlink != 1 ) - { - debug( 1, L"acquire_lock_file: number of hardlinks on unique " - L"tmpfile is %d instead of 1.", (int)statbuf.st_nlink ); - goto done; - } - if( gettimeofday( &start, NULL ) != 0 ) - { - debug( 1, L"acquire_lock_file: gettimeofday: %s", strerror( errno ) ); - goto done; - } - end = start; - pollint.tv_sec = 0; - pollint.tv_nsec = LOCKPOLLINTERVAL * 1000000; - do - { - /* - Try to create a hard link to the unique file from the - lockfile. This will only succeed if the lockfile does not - already exist. It is guaranteed to provide race-free - semantics over NFS which the alternative of calling - open(O_EXCL|O_CREAT) on the lockfile is not. The lock - succeeds if the call to link returns 0 or the link count on - the unique file increases to 2. - */ - if( link( linkfile, lockfile ) == 0 || - ( stat( linkfile, &statbuf ) == 0 && - statbuf.st_nlink == 2 ) ) - { - /* Successful lock */ - ret = 1; - break; - } - elapsed = end.tv_sec + end.tv_usec/1000000.0 - - ( start.tv_sec + start.tv_usec/1000000.0 ); - /* - The check for elapsed < 0 is to deal with the unlikely event - that after the loop is entered the system time is set forward - past the loop's end time. This would otherwise result in a - (practically) infinite loop. - */ - if( timed_out || elapsed >= timeout || elapsed < 0 ) - { - if ( timed_out == 0 && force ) - { - /* - Timed out and force was specified - attempt to - remove stale lock and try a final time - */ - (void)unlink( lockfile ); - timed_out = 1; - continue; - } - else - { - /* - Timed out and final try was unsuccessful or - force was not specified - */ - debug( 1, L"acquire_lock_file: timed out " - L"trying to obtain lockfile %s using " - L"linkfile %s", lockfile, linkfile ); - break; - } - } - nanosleep( &pollint, NULL ); - } while( gettimeofday( &end, NULL ) == 0 ); - done: - /* The linkfile is not needed once the lockfile has been created */ - (void)unlink( linkfile ); - free( linkfile ); - return ret; -} void common_handle_winch( int signal ) { diff --git a/common.h b/common.h index bb7455596..a2e61ce30 100644 --- a/common.h +++ b/common.h @@ -616,16 +616,6 @@ wchar_t *unescape( const wchar_t * in, bool unescape_string( wcstring &str, int escape_special ); -/** - Attempt to acquire a lock based on a lockfile, waiting LOCKPOLLINTERVAL - milliseconds between polls and timing out after timeout seconds, - thereafter forcibly attempting to obtain the lock if force is non-zero. - Returns 1 on success, 0 on failure. - To release the lock the lockfile must be unlinked. - A unique temporary file named by appending characters to the lockfile name - is used; any pre-existing file of the same name is subject to deletion. -*/ -int acquire_lock_file( const char *lockfile, const int timeout, int force ); /** Returns the width of the terminal window, so that not all diff --git a/exec.cpp b/exec.cpp index d7f0e0b60..1dfad6f7c 100644 --- a/exec.cpp +++ b/exec.cpp @@ -198,6 +198,7 @@ void close_unused_internal_pipes( io_data_t *io ) */ static char *get_interpreter( const char *command, char *interpreter, size_t buff_size ) { + // OK to not use CLO_EXEC here because this is only called after fork int fd = open( command, O_RDONLY ); if( fd >= 0 ) { @@ -248,6 +249,7 @@ static void safe_launch_process( process_t *p, const char *actual_cmd, char **ar /bin/sh if encountered. This is a weird predecessor to the shebang that is still sometimes used since it is supported on Windows. */ + /* OK to not use CLO_EXEC here because this is called after fork and the file is immediately closed */ int fd = open(actual_cmd, O_RDONLY); if (fd >= 0) { @@ -414,6 +416,7 @@ static void io_untransmogrify( io_data_t * in, io_data_t *out ) */ static io_data_t *io_transmogrify( io_data_t * in ) { + ASSERT_IS_MAIN_THREAD(); if( !in ) return 0; @@ -876,6 +879,7 @@ void exec( parser_t &parser, job_t *j ) case IO_FILE: { + /* Do not set CLO_EXEC because child needs access */ builtin_stdin=wopen( in->filename, in->param2.flags, OPEN_MASK ); if( builtin_stdin == -1 ) diff --git a/fish.cpp b/fish.cpp index 12011651e..6bfd37ca0 100644 --- a/fish.cpp +++ b/fish.cpp @@ -326,13 +326,17 @@ int main( int argc, char **argv ) int i; int fd; wchar_t *rel_filename, *abs_filename; - + + if( ( fd = open(file, O_RDONLY) ) == -1 ) { wperror( L"open" ); return 1; } - + + // OK to not do this atomically since we cannot have gone multithreaded yet + set_cloexec(fd); + if( *(argv+my_optind)) { wcstring sb; diff --git a/fish_pager.cpp b/fish_pager.cpp index 08577c61a..a5f22b964 100644 --- a/fish_pager.cpp +++ b/fish_pager.cpp @@ -1006,6 +1006,7 @@ static void init( int mangle_descriptors, int out ) close(1); close(0); + /* OK to not use CLO_EXEC here because fish_pager is single threaded */ if( (in = open( ttyname(2), O_RDWR )) != -1 ) { if( dup2( 2, 1 ) == -1 ) diff --git a/fishd.cpp b/fishd.cpp index c3ad0d151..edaa762a8 100644 --- a/fishd.cpp +++ b/fishd.cpp @@ -73,6 +73,15 @@ time the original barrier request was sent have been received. #include "path.h" #include "print_help.h" +#ifndef HOST_NAME_MAX +/** + Maximum length of hostname return. It is ok if this is too short, + getting the actual hostname is not critical, so long as the string + is unique in the filesystem namespace. + */ +#define HOST_NAME_MAX 255 +#endif + /** Maximum length of socket filename */ @@ -188,6 +197,252 @@ static void handle_term( int signal ) } +/** + Writes a pid_t in decimal representation to str. + str must contain sufficient space. + The conservatively approximate maximum number of characters a pid_t will + represent is given by: (int)(0.31 * sizeof(pid_t) + CHAR_BIT + 1) + Returns the length of the string + */ +static int sprint_pid_t( pid_t pid, char *str ) +{ + int len, i = 0; + int dig; + + /* Store digits in reverse order into string */ + while( pid != 0 ) + { + dig = pid % 10; + str[i] = '0' + dig; + pid = ( pid - dig ) / 10; + i++; + } + len = i; + /* Reverse digits */ + i /= 2; + while( i ) + { + i--; + dig = str[i]; + str[i] = str[len - 1 - i]; + str[len - 1 - i] = dig; + } + return len; +} + + + +/** + Writes a pseudo-random number (between one and maxlen) of pseudo-random + digits into str. + str must point to an allocated buffer of size of at least maxlen chars. + Returns the number of digits written. + Since the randomness in part depends on machine time it has _some_ extra + strength but still not enough for use in concurrent locking schemes on a + single machine because gettimeofday may not return a different value on + consecutive calls when: + a) the OS does not support fine enough resolution + b) the OS is running on an SMP machine. + Additionally, gettimeofday errors are ignored. + Excludes chars other than digits since ANSI C only guarantees that digits + are consecutive. + */ +static int sprint_rand_digits( char *str, int maxlen ) +{ + int i, max; + struct timeval tv; + + /* + Seed the pseudo-random generator based on time - this assumes + that consecutive calls to gettimeofday will return different values + and ignores errors returned by gettimeofday. + Cast to unsigned so that wrapping occurs on overflow as per ANSI C. + */ + (void)gettimeofday( &tv, NULL ); + srand( (unsigned int)tv.tv_sec + (unsigned int)tv.tv_usec * 1000000UL ); + max = 1 + (maxlen - 1) * (rand() / (RAND_MAX + 1.0)); + for( i = 0; i < max; i++ ) + { + str[i] = '0' + 10 * (rand() / (RAND_MAX + 1.0)); + } + return i; +} + + +/** + Generate a filename unique in an NFS namespace by creating a copy of str and + appending .{hostname}.{pid} to it. If gethostname() fails then a pseudo- + random string is substituted for {hostname} - the randomness of the string + should be strong enough across different machines. The main assumption + though is that gethostname will not fail and this is just a "safe enough" + fallback. + The memory returned should be freed using free(). + */ +static char *gen_unique_nfs_filename( const char *filename ) +{ + int pidlen, hnlen, orglen = strlen( filename ); + char hostname[HOST_NAME_MAX + 1]; + char *newname; + + if ( gethostname( hostname, HOST_NAME_MAX + 1 ) == 0 ) + { + hnlen = strlen( hostname ); + } + else + { + hnlen = sprint_rand_digits( hostname, HOST_NAME_MAX ); + hostname[hnlen] = '\0'; + } + newname = (char *)malloc( orglen + 1 /* period */ + hnlen + 1 /* period */ + + /* max possible pid size: 0.31 ~= log(10)2 */ + (int)(0.31 * sizeof(pid_t) * CHAR_BIT + 1) + + 1 /* '\0' */ ); + + if ( newname == NULL ) + { + debug( 1, L"gen_unique_nfs_filename: %s", strerror( errno ) ); + return newname; + } + memcpy( newname, filename, orglen ); + newname[orglen] = '.'; + memcpy( newname + orglen + 1, hostname, hnlen ); + newname[orglen + 1 + hnlen] = '.'; + pidlen = sprint_pid_t( getpid(), newname + orglen + 1 + hnlen + 1 ); + newname[orglen + 1 + hnlen + 1 + pidlen] = '\0'; + /* debug( 1, L"gen_unique_nfs_filename returning with: newname = \"%s\"; " + L"HOST_NAME_MAX = %d; hnlen = %d; orglen = %d; " + L"sizeof(pid_t) = %d; maxpiddigits = %d; malloc'd size: %d", + newname, (int)HOST_NAME_MAX, hnlen, orglen, + (int)sizeof(pid_t), + (int)(0.31 * sizeof(pid_t) * CHAR_BIT + 1), + (int)(orglen + 1 + hnlen + 1 + + (int)(0.31 * sizeof(pid_t) * CHAR_BIT + 1) + 1) ); */ + return newname; +} + + +/** + The number of milliseconds to wait between polls when attempting to acquire + a lockfile + */ +#define LOCKPOLLINTERVAL 10 + +/** + Attempt to acquire a lock based on a lockfile, waiting LOCKPOLLINTERVAL + milliseconds between polls and timing out after timeout seconds, + thereafter forcibly attempting to obtain the lock if force is non-zero. + Returns 1 on success, 0 on failure. + To release the lock the lockfile must be unlinked. + A unique temporary file named by appending characters to the lockfile name + is used; any pre-existing file of the same name is subject to deletion. + */ +static int acquire_lock_file( const char *lockfile, const int timeout, int force ) +{ + int fd, timed_out = 0; + int ret = 0; /* early exit returns failure */ + struct timespec pollint; + struct timeval start, end; + double elapsed; + struct stat statbuf; + + /* + (Re)create a unique file and check that it has one only link. + */ + char *linkfile = gen_unique_nfs_filename( lockfile ); + if( linkfile == NULL ) + { + goto done; + } + (void)unlink( linkfile ); + /* OK to not use CLO_EXEC here because fishd is single threaded */ + if( ( fd = open( linkfile, O_CREAT|O_RDONLY, 0600 ) ) == -1 ) + { + debug( 1, L"acquire_lock_file: open: %s", strerror( errno ) ); + goto done; + } + /* + Don't need to check exit status of close on read-only file descriptors + */ + close( fd ); + if( stat( linkfile, &statbuf ) != 0 ) + { + debug( 1, L"acquire_lock_file: stat: %s", strerror( errno ) ); + goto done; + } + if ( statbuf.st_nlink != 1 ) + { + debug( 1, L"acquire_lock_file: number of hardlinks on unique " + L"tmpfile is %d instead of 1.", (int)statbuf.st_nlink ); + goto done; + } + if( gettimeofday( &start, NULL ) != 0 ) + { + debug( 1, L"acquire_lock_file: gettimeofday: %s", strerror( errno ) ); + goto done; + } + end = start; + pollint.tv_sec = 0; + pollint.tv_nsec = LOCKPOLLINTERVAL * 1000000; + do + { + /* + Try to create a hard link to the unique file from the + lockfile. This will only succeed if the lockfile does not + already exist. It is guaranteed to provide race-free + semantics over NFS which the alternative of calling + open(O_EXCL|O_CREAT) on the lockfile is not. The lock + succeeds if the call to link returns 0 or the link count on + the unique file increases to 2. + */ + if( link( linkfile, lockfile ) == 0 || + ( stat( linkfile, &statbuf ) == 0 && + statbuf.st_nlink == 2 ) ) + { + /* Successful lock */ + ret = 1; + break; + } + elapsed = end.tv_sec + end.tv_usec/1000000.0 - + ( start.tv_sec + start.tv_usec/1000000.0 ); + /* + The check for elapsed < 0 is to deal with the unlikely event + that after the loop is entered the system time is set forward + past the loop's end time. This would otherwise result in a + (practically) infinite loop. + */ + if( timed_out || elapsed >= timeout || elapsed < 0 ) + { + if ( timed_out == 0 && force ) + { + /* + Timed out and force was specified - attempt to + remove stale lock and try a final time + */ + (void)unlink( lockfile ); + timed_out = 1; + continue; + } + else + { + /* + Timed out and final try was unsuccessful or + force was not specified + */ + debug( 1, L"acquire_lock_file: timed out " + L"trying to obtain lockfile %s using " + L"linkfile %s", lockfile, linkfile ); + break; + } + } + nanosleep( &pollint, NULL ); + } while( gettimeofday( &end, NULL ) == 0 ); +done: + /* The linkfile is not needed once the lockfile has been created */ + (void)unlink( linkfile ); + free( linkfile ); + return ret; +} + /** Acquire the lock for the socket Returns the name of the lock file if successful or @@ -502,6 +757,7 @@ static void load_or_save( int save) save?"saving":"loading", name ); + /* OK to not use CLO_EXEC here because fishd is single threaded */ fd = open( name, save?(O_CREAT | O_TRUNC | O_WRONLY):O_RDONLY, 0600); free( name ); diff --git a/history.cpp b/history.cpp index 71ab1dbfa..c7a7549d4 100644 --- a/history.cpp +++ b/history.cpp @@ -494,7 +494,7 @@ void history_t::load_old_if_needed(void) if( ! filename.empty() ) { - if( ( fd = wopen( filename, O_RDONLY ) ) > 0 ) + if( ( fd = wopen_cloexec( filename, O_RDONLY ) ) > 0 ) { off_t len = lseek( fd, 0, SEEK_END ); if( len != (off_t)-1) diff --git a/iothread.cpp b/iothread.cpp index af6790835..dcb79fe13 100644 --- a/iothread.cpp +++ b/iothread.cpp @@ -12,8 +12,6 @@ #include #include -#define VOMIT_ON_FAILURE(a) do { if (0 != (a)) { int err = errno; fprintf(stderr, "%s failed on line %d in file %s: %d (%s). Break on debug_thread_error to debug.\n", #a, __LINE__, __FILE__, err, strerror(err)); debug_thread_error(); abort(); }} while (0) - #ifdef _POSIX_THREAD_THREADS_MAX #if _POSIX_THREAD_THREADS_MAX < 64 #define IO_MAX_THREADS _POSIX_THREAD_THREADS_MAX diff --git a/mimedb.cpp b/mimedb.cpp index 2639013f8..43681b365 100644 --- a/mimedb.cpp +++ b/mimedb.cpp @@ -196,6 +196,7 @@ char *my_strdup( const char *s ) */ static const char * search_ini( const char *filename, const char *match ) { + /* OK to not use CLO_EXEC here because mimedb is single threaded */ FILE *f = fopen( filename, "r" ); char buf[4096]; int len=strlen(match); @@ -584,6 +585,7 @@ static char *get_description( const char *mimetype ) return 0; } + /* OK to not use CLO_EXEC here because mimedb is single threaded */ fd = open( fn.c_str(), O_RDONLY ); // fprintf( stderr, "%s\n", fn ); diff --git a/parser.cpp b/parser.cpp index 6a53936db..53502e6ed 100644 --- a/parser.cpp +++ b/parser.cpp @@ -680,9 +680,7 @@ void parser_t::destroy() { if( profile ) { - /* - Save profiling information - */ + /* Save profiling information. OK to not use CLO_EXEC here because this is called while fish is dying (and hence will not fork) */ FILE *f = fopen( profile, "w" ); if( !f ) { diff --git a/postfork.cpp b/postfork.cpp index 425dc526f..d6da00a6d 100644 --- a/postfork.cpp +++ b/postfork.cpp @@ -169,6 +169,7 @@ static int handle_child_io( io_data_t *io ) case IO_FILE: { + // Here we definitely do not want to set CLO_EXEC because our child needs access if( (tmp=wopen( io->filename, io->param2.flags, OPEN_MASK ) )==-1 ) { diff --git a/wutil.cpp b/wutil.cpp index 8cbf7d432..af73c5ce1 100644 --- a/wutil.cpp +++ b/wutil.cpp @@ -71,6 +71,8 @@ void wutil_destroy() { } +static pthread_mutex_t readdir_lock = PTHREAD_MUTEX_INITIALIZER; + bool wreaddir_resolving(DIR *dir, const std::wstring &dir_path, std::wstring &out_name, bool *out_is_dir) { struct dirent *d = readdir( dir ); @@ -144,8 +146,39 @@ int wchdir( const wcstring &dir ) FILE *wfopen(const wcstring &path, const char *mode) { - cstring tmp = wcs2string(path); - return fopen(tmp.c_str(), mode); + int permissions = 0, options = 0; + switch (*mode++) { + case 'r': + permissions = O_RDONLY; + break; + case 'w': + permissions = O_WRONLY; + options = O_CREAT | O_TRUNC; + break; + case 'a': + permissions = O_WRONLY; + options = O_CREAT | O_APPEND; + break; + default: + errno = EINVAL; + return NULL; + break; + } + /* Skip binary */ + if (*mode == 'b') + mode++; + + /* Consider append option */ + if (*mode == '+') + permissions = O_RDWR; + + int fd = wopen_cloexec(path, permissions | options, 0666); + if (fd < 0) + return NULL; + FILE *result = fdopen(fd, mode); + if (result == NULL) + close(fd); + return result; } FILE *wfreopen(const wcstring &path, const char *mode, FILE *stream) @@ -154,23 +187,40 @@ FILE *wfreopen(const wcstring &path, const char *mode, FILE *stream) return freopen(tmp.c_str(), mode, stream); } -int wopen(const wcstring &pathname, int flags, ...) +bool set_cloexec(int fd) { + int flags = fcntl(fd, F_GETFD, 0); + if (flags < 0) { + return false; + } else if (flags & FD_CLOEXEC) { + return true; + } else { + return fcntl(fd, F_SETFD, flags | FD_CLOEXEC) >= 0; + } +} + +static int wopen_internal(const wcstring &pathname, int flags, mode_t mode, bool cloexec) { cstring tmp = wcs2string(pathname); - int res=-1; - va_list argp; - if( ! (flags & O_CREAT) ) - { - res = open(tmp.c_str(), flags); + int fd = ::open(tmp.c_str(), flags, mode); + if (cloexec && fd >= 0 && ! set_cloexec(fd)) { + close(fd); + fd = -1; } - else - { - va_start( argp, flags ); - res = open(tmp.c_str(), flags, va_arg(argp, int) ); - va_end( argp ); - } - return res; + return fd; + } +int wopen(const wcstring &pathname, int flags, mode_t mode) +{ + // off the main thread, always use wopen_cloexec + ASSERT_IS_MAIN_THREAD(); + return wopen_internal(pathname, flags, mode, false); +} + +int wopen_cloexec(const wcstring &pathname, int flags, mode_t mode) +{ + return wopen_internal(pathname, flags, mode, true); +} + int wcreat(const wcstring &pathname, mode_t mode) { diff --git a/wutil.h b/wutil.h index e2043023c..66a1bee38 100644 --- a/wutil.h +++ b/wutil.h @@ -42,19 +42,23 @@ void wutil_init(); void wutil_destroy(); /** - Wide character version of fopen(). + Wide character version of fopen(). This sets CLO_EXEC. */ FILE *wfopen(const wcstring &path, const char *mode); +/** Sets CLO_EXEC on a given fd */ +bool set_cloexec(int fd); + /** Wide character version of freopen(). */ FILE *wfreopen(const wcstring &path, const char *mode, FILE *stream); -/** - Wide character version of open(). -*/ -int wopen(const wcstring &pathname, int flags, ...); +/** Wide character version of open(). */ +int wopen(const wcstring &pathname, int flags, mode_t mode = 0); + +/** Wide character version of open() that also sets the close-on-exec flag (atomically when possible). */ +int wopen_cloexec(const wcstring &pathname, int flags, mode_t mode = 0); /** Wide character version of creat(). @@ -62,9 +66,7 @@ int wopen(const wcstring &pathname, int flags, ...); int wcreat(const wcstring &pathname, mode_t mode); -/** - Wide character version of opendir(). -*/ +/** Wide character version of opendir(). Note that opendir() is guaranteed to set close-on-exec by POSIX (hooray). */ DIR *wopendir(const wcstring &name); /** diff --git a/xdgmime.cpp b/xdgmime.cpp index 4a6278769..571c77c64 100644 --- a/xdgmime.cpp +++ b/xdgmime.cpp @@ -440,7 +440,8 @@ xdg_mime_get_mime_type_for_file (const char *file_name) data = (unsigned char *)malloc (max_extent); if (data == NULL) return XDG_MIME_TYPE_UNKNOWN; - + + /* OK to not use CLO_EXEC here because mimedb is single threaded */ file = fopen (file_name, "r"); if (file == NULL) { diff --git a/xdgmimealias.cpp b/xdgmimealias.cpp index 2b47f6717..e1701dbc7 100644 --- a/xdgmimealias.cpp +++ b/xdgmimealias.cpp @@ -124,6 +124,7 @@ _xdg_mime_alias_read_from_file (XdgAliasList *list, char line[255]; int alloc; + /* OK to not use CLO_EXEC here because mimedb is single threaded */ file = fopen (file_name, "r"); if (file == NULL) diff --git a/xdgmimeglob.cpp b/xdgmimeglob.cpp index 77f53a2cd..a485765ba 100644 --- a/xdgmimeglob.cpp +++ b/xdgmimeglob.cpp @@ -447,6 +447,7 @@ _xdg_mime_glob_read_from_file (XdgGlobHash *glob_hash, FILE *glob_file; char line[255]; + /* OK to not use CLO_EXEC here because mimedb is single threaded */ glob_file = fopen (file_name, "r"); if (glob_file == NULL) diff --git a/xdgmimemagic.cpp b/xdgmimemagic.cpp index d6ee9424b..5b624fe26 100644 --- a/xdgmimemagic.cpp +++ b/xdgmimemagic.cpp @@ -766,6 +766,7 @@ _xdg_mime_magic_read_from_file (XdgMimeMagic *mime_magic, FILE *magic_file; char header[12]; + /* OK to not use CLO_EXEC here because mimedb is single threaded */ magic_file = fopen (file_name, "r"); if (magic_file == NULL) diff --git a/xdgmimeparent.cpp b/xdgmimeparent.cpp index aef11348e..1585628a4 100644 --- a/xdgmimeparent.cpp +++ b/xdgmimeparent.cpp @@ -130,6 +130,7 @@ _xdg_mime_parent_read_from_file (XdgParentList *list, int i, alloc; XdgMimeParents *entry; + /* OK to not use CLO_EXEC here because mimedb is single threaded */ file = fopen (file_name, "r"); if (file == NULL)