virFindFileInPath: only find executable non-directory

Without this patch, at least tests/daemon-conf (which sticks
$builddir/src in the PATH) tries to execute the directory
$builddir/src/qemu rather than a real qemu binary.

* src/util/util.h (virFileExists): Adjust prototype.
(virFileIsExecutable): New prototype.
* src/util/util.c (virFindFileInPath): Reject non-executables and
directories.  Avoid huge stack allocation.
(virFileExists): Use lighter-weight syscall.
(virFileIsExecutable): New function.
* src/libvirt_private.syms (util.h): Export new function.

src/libvirt_private.syms

@@ -845,6 +845,7 @@ virFileDeletePid;
 virFileExists;
 virFileFindMountPoint;
 virFileHasSuffix;
+virFileIsExecutable;
 virFileLinkPointsTo;
 virFileMakePath;
 virFileMatchesNameSuffix;

src/util/util.c

@@ -1255,7 +1255,7 @@ int virFileResolveLink(const char *linkpath,
 }
 
 /*
- * Finds a requested file in the PATH env. e.g.:
+ * Finds a requested executable file in the PATH env. e.g.:
  * "kvm-img" will return "/usr/bin/kvm-img"
  *
  * You must free the result
@@ -1263,19 +1263,18 @@ int virFileResolveLink(const char *linkpath,
 char *virFindFileInPath(const char *file)
 {
     char *path;
-    char pathenv[PATH_MAX];
+    char *pathiter;
-    char *penv = pathenv;
     char *pathseg;
-    char fullpath[PATH_MAX];
+    char *fullpath = NULL;
 
     if (file == NULL)
         return NULL;
 
     /* if we are passed an absolute path (starting with /), return a
-     * copy of that path
+     * copy of that path, after validating that it is executable
      */
-    if (file[0] == '/') {
+    if (IS_ABSOLUTE_FILE_NAME(file)) {
-        if (virFileExists(file))
+        if (virFileIsExecutable(file))
             return strdup(file);
         else
             return NULL;
@@ -1284,27 +1283,45 @@ char *virFindFileInPath(const char *file)
     /* copy PATH env so we can tweak it */
     path = getenv("PATH");
 
-    if (path == NULL || virStrcpyStatic(pathenv, path) == NULL)
+    if (path == NULL || (path = strdup(path)) == NULL)
         return NULL;
 
     /* for each path segment, append the file to search for and test for
      * it. return it if found.
      */
-    while ((pathseg = strsep(&penv, ":")) != NULL) {
+    pathiter = path;
-       snprintf(fullpath, PATH_MAX, "%s/%s", pathseg, file);
+    while ((pathseg = strsep(&pathiter, ":")) != NULL) {
-       if (virFileExists(fullpath))
+        if (virAsprintf(&fullpath, "%s/%s", pathseg, file) < 0 ||
-           return strdup(fullpath);
+            virFileIsExecutable(fullpath))
+            break;
+        VIR_FREE(fullpath);
     }
 
-    return NULL;
+    VIR_FREE(path);
+    return fullpath;
 }
-int virFileExists(const char *path)
-{
-    struct stat st;
 
-    if (stat(path, &st) >= 0)
+bool virFileExists(const char *path)
-        return(1);
+{
-    return(0);
+    return access(path, F_OK) == 0;
+}
+
+/* Check that a file is regular and has executable bits.
+ *
+ * Note: In the presence of ACLs, this may return true for a file that
+ * would actually fail with EACCES for a given user, or false for a
+ * file that the user could actually execute, but setups with ACLs
+ * that weird are unusual. */
+bool
+virFileIsExecutable(const char *file)
+{
+    struct stat sb;
+
+    /* We would also want to check faccessat if we cared about ACLs,
+     * but we don't.  */
+    return (stat(file, &sb) == 0 &&
+            S_ISREG(sb.st_mode) &&
+            (sb.st_mode & 0111) != 0);
 }
 
 #ifndef WIN32

src/util/util.h

@@ -1,8 +1,7 @@
-
 /*
  * utils.h: common, generic utility functions
  *
- * Copyright (C) 2010 Red Hat, Inc.
+ * Copyright (C) 2010-2011 Red Hat, Inc.
  * Copyright (C) 2006, 2007 Binary Karma
  * Copyright (C) 2006 Shuveb Hussain
  *
@@ -32,6 +31,7 @@
 # include <sys/select.h>
 # include <sys/types.h>
 # include <stdarg.h>
+# include <stdbool.h>
 
 # ifndef MIN
 #  define MIN(a, b) ((a) < (b) ? (a) : (b))
@@ -120,7 +120,8 @@ int virFileResolveLink(const char *linkpath,
 
 char *virFindFileInPath(const char *file);
 
-int virFileExists(const char *path);
+bool virFileExists(const char *file) ATTRIBUTE_NONNULL(1);
+bool virFileIsExecutable(const char *file) ATTRIBUTE_NONNULL(1);
 
 char *virFileSanitizePath(const char *path);