Improve execvpe(3) logic and run shell on ENOEXEC.

libc/unistd/execvpe.cpp

@@ -23,11 +23,13 @@
 *******************************************************************************/
 
 #include <errno.h>
+#include <limits.h>
+#include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
 
-// TODO: Move this to some generic environment interface!
+// TODO: Move this to some generic environment interface.
 static const char* LookupEnvironment(const char* name, char* const* envp)
 {
 	size_t equalpos = strcspn(name, "=");
@@ -45,21 +47,66 @@ static const char* LookupEnvironment(const char* name, char* const* envp)
 	return NULL;
 }
 
-// TODO: Provide an interface that allows user-space to find out which command
-// would have been executed (according to PATH) had execvpe been called now.
-// This is of value to programs such as which(1), instead of repeating much of
-// this logic there.
-extern "C" int execvpe(const char* filename, char* const* argv,
+static
+int execvpe_attempt(const char* filename,
+                    const char* original,
+                    char* const* argv,
                     char* const* envp)
 {
+	execve(filename, argv, envp);
+
+	if ( errno != ENOEXEC )
+		return -1;
+
+	// Prevent attempting to run the shell using itself in an endless loop if it
+	// happens to be an unknown format or a shell script itself.
+	if ( !strcmp(original, "sh") )
+		return errno = ENOEXEC, -1;
+
+	int argc = 0;
+	while ( argv[argc] )
+		argc++;
+
+	if ( INT_MAX - argc < 1 + 1 )
+		return errno = EOVERFLOW, -1;
+
+	int new_argc = 1 + argc;
+	char** new_argv = (char**) malloc(sizeof(char*) * (new_argc + 1));
+	if ( !new_argv )
+		return -1;
+
+	new_argv[0] = (char*) "sh";
+	new_argv[1] = (char*) filename;
+	for ( int i = 1; i < argc; i++ )
+		new_argv[1 + i] = argv[i];
+	new_argv[new_argc] = (char*) NULL;
+
+	execvpe(new_argv[0], new_argv, envp);
+
+	free(new_argv);
+
+	return errno = ENOEXEC, -1;
+}
+
+// NOTE: The PATH-searching logic is repeated multiple places. Until this logic
+//       can be shared somehow, you need to keep this comment in sync as well
+//       as the logic in these files:
+//         * libc/unistd/execvpe.cpp
+//         * utils/which.cpp
+
+extern "C"
+int execvpe(const char* filename, char* const* argv, char* const* envp)
+{
+	if ( !filename || !filename[0] )
+		return errno = ENOENT;
+
 	const char* path = LookupEnvironment("PATH", envp);
-	// TODO: Should there be a default PATH value?
-	if ( strchr(filename, '/') || !path )
-		return execve(filename, argv, envp);
+	bool search_path = !strchr(filename, '/') && path;
+	bool any_tries = false;
+	bool any_eacces = false;
 
 	// Search each directory in the PATH variable for a suitable file.
-	size_t filename_len = strlen(filename);
-	while ( *path )
+	while ( search_path && *path )
 	{
 		size_t len = strcspn(path, ":");
 		if ( !len )
@@ -68,33 +115,61 @@ extern "C" int execvpe(const char* filename, char* const* argv,
 			// directory. While it does inductively make sense, the common
 			// kernel interfaces such as openat doesn't accept it and software
 			// often just prefix their directories and a colon to PATH without
-			// regard to whether it's already empty. S
+			// regard to whether it's already empty.
 			path++;
 			continue;
 		}
 
-		// Determine the full path to the file if it is in the directory.
-		size_t fullpath_len = len + 1 + filename_len + 1;
-		char* fullpath = (char*) malloc(fullpath_len * sizeof(char));
-		if ( !fullpath )
+		any_tries = true;
+
+		// Determine the directory prefix.
+		char* dirpath = strndup(path, len);
+		if ( !dirpath )
 			return -1;
-		stpcpy(stpcpy(stpncpy(fullpath, path, len), "/"), filename);
 		if ( (path += len)[0] == ':' )
 			path++;
+		while ( len && dirpath[len - 1] == '/' )
+			dirpath[--len] = '\0';
+
+		// Determine the full path to the file inside the directory.
+		char* fullpath;
+		if ( asprintf(&fullpath, "%s/%s", dirpath, filename) < 0 )
+			return free(dirpath), -1;
+
+		execvpe_attempt(fullpath, filename, argv, envp);
 
-		execve(fullpath, argv, envp);
 		free(fullpath);
+		free(dirpath);
 
-		// TODO: There may be some security concerns here as to whether to
-		// continue or abort execution. For instance, if a directory in the
-		// start of the PATH has permissions set up too restrictively, then
-		// it would never look in the later directories (and you can't execute
-		// anything without absolute paths). And other situations.
-		if ( errno == EACCES )
-			return -1;
+		// Proceed to the next PATH entry if the file didn't exist.
 		if ( errno == ENOENT )
 			continue;
 
+		// Ignore errors related to path resolution where the cause is a bad
+		// entry in the PATH as opposed to security issues.
+		if ( errno == ELOOP ||
+		     errno == EISDIR ||
+		     errno == ENAMETOOLONG ||
+		     errno == ENOTDIR )
+			continue;
+
+		// Remember permission denied errors and report that errno value if the
+		// entire PATH search fails rather than the error of the last attempt.
+		if ( errno == EACCES )
+		{
+			any_eacces = true;
+			continue;
 		}
+
+		// Any other errors are treated as fatal and we stop the search.
+		break;
+	}
+
+	if ( !any_tries )
+		execvpe_attempt(filename, filename, argv, envp);
+
+	if ( any_eacces )
+		errno = EACCES;
+
 	return -1;
 }

utils/which.cpp

@@ -1,6 +1,6 @@
 /*******************************************************************************
 
-    Copyright(C) Jonas 'Sortie' Termansen 2012.
+    Copyright(C) Jonas 'Sortie' Termansen 2012, 2014.
 
     This program is free software: you can redistribute it and/or modify it
     under the terms of the GNU General Public License as published by the Free
@@ -35,54 +35,90 @@
 #define VERSIONSTR "unknown version"
 #endif
 
-bool Which(const char* cmd, const char* path, bool all)
+// NOTE: The PATH-searching logic is repeated multiple places. Until this logic
+//       can be shared somehow, you need to keep this comment in sync as well
+//       as the logic in these files:
+//         * libc/unistd/execvpe.cpp
+//         * utils/which.cpp
+// NOTO: See comments in execvpe() for algorithmic commentary.
+
+bool Which(const char* filename, const char* path, bool all)
 {
-	if ( strchr(cmd, '/') )
+	bool found = false;
+
+	bool search_path = !strchr(filename, '/') && path;
+	bool any_tries = false;
+	bool any_eacces = false;
+
+	while ( search_path && *path )
 	{
-		struct stat st;
-		if ( stat(path, &st) )
+		size_t len = strcspn(path, ":");
+		if ( !len )
 		{
-			printf("%s\n", cmd);
-			return true;
-		}
-		return false;
+			path++;
+			continue;
 		}
 
-	// Sortix doesn't support that the empty string means current directory.
-	bool found = false;
-	char* dirname = NULL;
-	while ( *path )
-	{
-		if ( dirname ) { free(dirname); dirname = NULL; }
-		size_t len = strcspn(path, ":");
-		if ( !len ) { path++; continue; }
-		dirname = (char*) malloc((len+1) * sizeof(char));
-		if ( !dirname )
-			error(1, errno, "malloc");
-		memcpy(dirname, path, len * sizeof(char));
-		dirname[len] = '\0';
+		any_tries = true;
+
+		char* dirpath = strndup(path, len);
+		if ( !dirpath )
+			return -1;
 		if ( (path += len)[0] == ':' )
 			path++;
-		int dirfd = open(dirname, O_RDONLY | O_DIRECTORY);
-		if ( dirfd < 0 )
+		while ( len && dirpath[len - 1] == '/' )
+			dirpath[--len] = '\0';
+
+		char* fullpath;
+		if ( asprintf(&fullpath, "%s/%s", dirpath, filename) < 0 )
+			return free(dirpath), -1;
+
+		if ( access(fullpath, X_OK) == 0 )
 		{
-			if ( errno == EACCES )
-				error(1, errno, "%s", dirname);
-			// TODO: May be a security concern to continue;
-			if ( errno == ENOENT )
-				continue;
-			continue;
-		}
-		struct stat st;
-		int ret = fstatat(dirfd, cmd, &st, 0);
-		if ( ret != 0 )
-			continue;
-		printf("%s/%s\n", dirname, cmd);
 			found = true;
-		if ( !all )
+			printf("%s\n", fullpath);
+			free(fullpath);
+			free(dirpath);
+			if ( all )
+				continue;
 			break;
 		}
-	free(dirname);
+
+		free(fullpath);
+		free(dirpath);
+
+		if ( errno == ENOENT )
+			continue;
+
+		if ( errno == ELOOP ||
+		     errno == EISDIR ||
+		     errno == ENAMETOOLONG ||
+		     errno == ENOTDIR )
+			continue;
+
+		if ( errno == EACCES )
+		{
+			any_eacces = true;
+			continue;
+		}
+
+		if ( all )
+			continue;
+
+		break;
+	}
+
+	if ( !any_tries )
+	{
+		if ( access(filename, X_OK) == 0 )
+		{
+			found = true;
+			printf("%s\n", filename);
+		}
+	}
+
+	(void) any_eacces;
+
 	return found;
 }
 
@@ -144,11 +180,6 @@ int main(int argc, char* argv[])
 	}
 
 	const char* path = getenv("PATH");
-	if ( !path )
-	{
-		fprintf(stderr, "%s: PATH variable is not set\n", argv0);
-		exit(1);
-	}
 
 	bool success = true;
 	for ( int i = 1; i < argc; i++ )