diff --git a/common.c b/common.c index ee9b2d9f5..8768f8e0c 100644 --- a/common.c +++ b/common.c @@ -55,10 +55,10 @@ parts of fish. #define ERROR_MAX_COUNT 1 /** - The number of nanoseconds to wait between polls when attempting to acquire + The number of microseconds to wait between polls when attempting to acquire a lockfile */ -#define LOCKPOLLINTERVAL 1000000 +#define LOCKPOLLINTERVAL 10 struct termios shell_modes; @@ -1070,36 +1070,161 @@ wchar_t *unescape( wchar_t * in, int escape_special ) return in; } +/** + Convert a pid_t to a decimal string representation. + The str parameter must contain sufficient space. + The conservatively approximate maximum number of characters a pid_t will + represent is given by: (int)(3.1 * sizeof(pid_t) + CHAR_BIT + 1) + Returns the length of the string +*/ +static int pidtostr( 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; +} + +/** + Create a copy of str adding this process's decimalised pid + The memory returned should be freed using free() +*/ +static char *dup_str_with_pid( const char *str ) +{ + int pidlen, len = strlen( str ); + /* + allocate enough extra space for max size of pid when converted to a + string; 3.1 ~= log(10)2 + */ + char *newstr = malloc( len + 1 + + (int)(3.1 * sizeof(pid_t) * CHAR_BIT + 1) ); + + if ( newstr == NULL ) + { + return newstr; + } + memcpy( newstr, str, len ); + pidlen = pidtostr( getpid(), newstr + len ); + newstr[len + pidlen] = '\0'; + return newstr; +} + /** Attempt to acquire a lock based on a lockfile, waiting LOCKPOLLINTERVAL nanoseconds between polls and timing out after timeout seconds, - thereafter forcibly attempting to obtain the lock. Returns the opened - lockfile's descriptor or -1 if not possible to open the file or on error. - Contains a race condition when the lockfile is on an NFS filesystem - according to Linux manpage open(2) + thereafter forcibly attempting to obtain the lock. + Returns 1 on success, 0 on failure. + A temporary file based on adding the pid to the lockfile name is used, so + be aware that such a file will be deleted if it already exists. + To remove the lock the lockfile must be unlinked. */ -int acquire_lock_file(const char *lockfile, const int timeout) +int acquire_lock_file( const char *lockfile, const int timeout, int force ) { - int ret = -1; + int fd, timed_out = 0; + int ret = 0; /* early exit returns failure */ struct timespec pollint; struct timeval start, end; double elapsed; + struct stat statbuf; - if (gettimeofday(&start, NULL) == 0) { - pollint.tv_sec = 0; - pollint.tv_nsec = LOCKPOLLINTERVAL; - while ((ret = open(lockfile, O_EXCL|O_CREAT|O_RDONLY)) == -1) { - if (gettimeofday(&end, NULL) != 0) - break; - elapsed = end.tv_sec + end.tv_usec/1000000.0 - - (start.tv_sec + start.tv_usec/1000000.0); - if (elapsed >= timeout) { - (void)unlink(lockfile); - ret = open(lockfile, O_EXCL|O_CREAT|O_RDONLY); + /* + (Re)create a unique file and check that it has one only link. + The check would be subject to a race if the filename were not unique. + */ + char *linkfile = dup_str_with_pid( lockfile ); + if( linkfile == NULL ) + { + goto done; + } + (void)unlink( linkfile ); + if( ( fd = open( linkfile, O_CREAT|O_RDONLY ) ) == -1 ) + { + goto done; + } + close( fd ); + if( stat( linkfile, &statbuf ) != 0 || statbuf.st_nlink != 1 ) + { + goto done; + } + + if( gettimeofday( &start, NULL ) != 0 ) + { + 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 + */ break; } - (void)nanosleep(&pollint, NULL); - } - } + } + 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; } diff --git a/common.h b/common.h index 9e2ea82e2..c5d60a8a7 100644 --- a/common.h +++ b/common.h @@ -247,4 +247,4 @@ wchar_t *unescape( wchar_t * in, int escape_special ); void block(); void unblock(); -int acquire_lock_file(const char *lockfile, const int timeout); +int acquire_lock_file( const char *lockfile, const int timeout, int force ); diff --git a/fishd.c b/fishd.c index 8d26a109f..2ef0df0a5 100644 --- a/fishd.c +++ b/fishd.c @@ -73,106 +73,120 @@ static int sock; /** Constructs the fish socket filename */ -static char *get_socket_filename(void) +static char *get_socket_filename() { char *name; - char *dir = getenv("FISHD_SOCKET_DIR"); - char *uname = getenv("USER"); + char *dir = getenv( "FISHD_SOCKET_DIR" ); + char *uname = getenv( "USER" ); - if (dir == NULL) + if( dir == NULL ) + { dir = "/tmp"; + } - if (uname == NULL) { + if( uname == NULL ) + { struct passwd *pw; - pw = getpwuid(getuid()); - uname = strdup(pw->pw_name); + pw = getpwuid( getuid() ); + uname = strdup( pw->pw_name ); } - name = malloc(strlen(dir)+ strlen(uname)+ strlen(SOCK_FILENAME) + 2); - if (name == NULL) { - perror("get_socket_filename"); - exit(EXIT_FAILURE); + name = malloc( strlen(dir)+ strlen(uname)+ strlen(SOCK_FILENAME) + 2 ); + if( name == NULL ) + { + wperror( L"get_socket_filename" ); + exit( EXIT_FAILURE ); } - strcpy(name, dir); - strcat(name, "/"); - strcat(name, SOCK_FILENAME); - strcat(name, uname); + strcpy( name, dir ); + strcat( name, "/" ); + strcat( name, SOCK_FILENAME ); + strcat( name, uname ); - if (strlen(name) >= UNIX_PATH_MAX) { - debug(1, L"Filename too long: '%s'", name); - exit(EXIT_FAILURE); + if( strlen( name ) >= UNIX_PATH_MAX ) + { + debug( 1, L"Filename too long: '%s'", name ); + exit( EXIT_FAILURE ); } return name; } /** - Acquire the lock file for the socket - Returns an fd that should be closed to unlock + Acquire the lock for the socket + Returns the name of the lock file if successful or + NULL if unable to obtain lock. + The returned string must be free()d after unlink()ing the file to release + the lock */ -int acquire_socket_lock_file(const char *sock_name, char **lockfile) +static char *acquire_socket_lock( const char *sock_name ) { - int fd; - int len = strlen(sock_name); - - *lockfile = malloc(len + strlen(LOCKPOSTFIX) + 1); - if (*lockfile == NULL) { - perror("get_socket"); - exit(EXIT_FAILURE); + int len = strlen( sock_name ); + char *lockfile = malloc( len + strlen( LOCKPOSTFIX ) + 1 ); + + if( lockfile == NULL ) + { + wperror( L"acquire_socket_lock" ); + exit( EXIT_FAILURE ); } - (void)strcpy(*lockfile, sock_name); - (void)strcpy(*lockfile + len, LOCKPOSTFIX); - if ((fd = acquire_lock_file(*lockfile, LOCKTIMEOUT)) == -1) { - fprintf(stderr, "Unable to obtain lockfile %s, exiting", - *lockfile); - exit(EXIT_FAILURE); + strcpy( lockfile, sock_name ); + strcpy( lockfile + len, LOCKPOSTFIX ); + if ( !acquire_lock_file( lockfile, LOCKTIMEOUT, 1 ) ) + { + free( lockfile ); + lockfile = NULL; } - return fd; + return lockfile; } /** Connects to the fish socket and starts listening for connections */ -static int get_socket(void) +static int get_socket() { int s, len, doexit = 0; int exitcode = EXIT_FAILURE; struct sockaddr_un local; - char *lockfile; char *sock_name = get_socket_filename(); - int fd_lockfile; - /** - Start critical section protected by lock file + /* + Start critical section protected by lock */ - fd_lockfile = acquire_socket_lock_file(sock_name, &lockfile); - debug(1, L"Acquired lockfile %s", lockfile); + char *lockfile = acquire_socket_lock( sock_name ); + if( lockfile == NULL ) + { + fwprintf( stderr, L"Unable to obtain lock on socket, exiting" ); + exit( EXIT_FAILURE ); + } + debug( 1, L"Acquired lockfile: %s", lockfile ); local.sun_family = AF_UNIX; - strcpy(local.sun_path, sock_name); - len = strlen(local.sun_path) + sizeof(local.sun_family); + strcpy( local.sun_path, sock_name ); + len = strlen( local.sun_path ) + sizeof( local.sun_family ); debug(1, L"Connect to socket at %s", sock_name); - if ((s = socket(AF_UNIX, SOCK_STREAM, 0)) == -1) { - perror("socket"); + if( ( s = socket( AF_UNIX, SOCK_STREAM, 0 ) ) == -1 ) + { + wperror( L"socket" ); doexit = 1; goto unlock; } - /** + /* First check whether the socket has been opened by another fishd; if so, exit with success status */ - if (connect(s, (struct sockaddr *)&local, len) == 0) { - debug(1, L"Socket already exists, exiting"); + if( connect( s, (struct sockaddr *)&local, len ) == 0 ) + { + debug( 1, L"Socket already exists, exiting" ); doexit = 1; exitcode = 0; goto unlock; } - unlink(local.sun_path); - if (bind(s, (struct sockaddr *)&local, len) == -1) { - perror("bind"); + unlink( local.sun_path ); + if( bind( s, (struct sockaddr *)&local, len ) == -1 ) + { + wperror( L"bind" ); doexit = 1; goto unlock; } @@ -185,29 +199,32 @@ static int get_socket(void) } */ - if( fcntl( s, F_SETFL, O_NONBLOCK ) != 0 ) { + if( fcntl( s, F_SETFL, O_NONBLOCK ) != 0 ) + { wperror( L"fcntl" ); close( s ); doexit = 1; - } else if (listen(s, 64) == -1) { - wperror(L"listen"); + } else if( listen( s, 64 ) == -1 ) + { + wperror( L"listen" ); doexit = 1; } unlock: - (void)close(fd_lockfile); - (void)unlink(lockfile); - debug(1, L"Released lockfile %s", lockfile); - /** - End critical section protected by lock file + (void)unlink( lockfile ); + debug( 1, L"Released lockfile: %s", lockfile ); + /* + End critical section protected by lock */ - free(lockfile); + free( lockfile ); - free(sock_name); + free( sock_name ); - if (doexit) - exit(exitcode); + if( doexit ) + { + exit( exitcode ); + } return s; }