maint: prohibit access(,X_OK)

This simplifies several callers that were repeating checks already
guaranteed by util.c, and makes other callers more robust to now
reject directories.  remote_driver.c was over-strict - access(,R_OK)
is only needed to execute a script file; a binary only needs
access(,X_OK) (besides, it's unusual to see a file with x but not
r permissions, whether script or binary).

* cfg.mk (sc_prohibit_access_xok): New syntax-check rule.
(exclude_file_name_regexp--sc_prohibit_access_xok): Exempt one use.
* src/network/bridge_driver.c (networkStartRadvd): Fix offenders.
* src/qemu/qemu_capabilities.c (qemuCapsProbeMachineTypes)
(qemuCapsInitGuest, qemuCapsInit, qemuCapsExtractVersionInfo):
Likewise.
* src/remote/remote_driver.c (remoteFindDaemonPath): Likewise.
* src/uml/uml_driver.c (umlStartVMDaemon): Likewise.
* src/util/hooks.c (virHookCheck): Likewise.

diff --git a/cfg.mk b/cfg.mk
index 2a97f88e95cdbbe4eacc061d595262afc8dfb69b..ac419f74a8c3dbaf6789c5678b18b5376b2c3243 100644 (file)
--- a/cfg.mk
+++ b/cfg.mk
@@ -258,6 +258,13 @@ sc_prohibit_fork_wrappers:
        halt='use virCommand for child processes'                       \
          $(_sc_search_regexp)
 
+# access with X_OK accepts directories, but we can't exec() those.
+# access with F_OK or R_OK is okay, though.
+sc_prohibit_access_xok:
+       @prohibit='access''(at)? *\(.*X_OK'                             \
+       halt='use virFileIsExecutable instead of access''(,X_OK)'       \
+         $(_sc_search_regexp)
+
 # Similar to the gnulib maint.mk rule for sc_prohibit_strcmp
 # Use STREQLEN or STRPREFIX rather than comparing strncmp == 0, or != 0.
 sc_prohibit_strncmp:
@@ -551,6 +558,8 @@ exclude_file_name_regexp--sc_po_check = ^docs/
 exclude_file_name_regexp--sc_prohibit_VIR_ERR_NO_MEMORY = \
   ^(include/libvirt/virterror\.h|daemon/dispatch\.c|src/util/virterror\.c)$$
 
+exclude_file_name_regexp--sc_prohibit_access_xok = ^src/util/util\.c$$
+
 exclude_file_name_regexp--sc_prohibit_always_true_header_tests = \
   (^docs|^python/(libvirt-override|typewrappers)\.c$$)
 

diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index c30620a63b1d46f55f7e4529b05f67ba092d1b22..fb933a23fd1539d009a8680abece2400199625c6 100644 (file)
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -689,7 +689,7 @@ networkStartRadvd(virNetworkObjPtr network)
 
     network->radvdPid = -1;
 
-    if (access(RADVD, X_OK) < 0) {
+    if (!virFileIsExecutable(RADVD)) {
         virReportSystemError(errno,
                              _("Cannot find %s - "
                                "Possibly the package isn't installed"),

diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index d8aa9cdc0c6d0b7a56ff257df2ad4418cd703b25..f86e7f5efbacaffed9d24fbce00af2dbcb8825a0 100644 (file)
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -178,7 +178,7 @@ qemuCapsProbeMachineTypes(const char *binary,
      * Technically we could catch the exec() failure, but that's
      * in a sub-process so it's hard to feed back a useful error.
      */
-    if (access(binary, X_OK) < 0) {
+    if (!virFileIsExecutable(binary)) {
         virReportSystemError(errno, _("Cannot find QEMU binary %s"), binary);
         return -1;
     }
@@ -451,12 +451,9 @@ qemuCapsInitGuest(virCapsPtr caps,
      */
     binary = virFindFileInPath(info->binary);
 
-    if (binary == NULL || access(binary, X_OK) != 0) {
+    if (binary == NULL || !virFileIsExecutable(binary)) {
         VIR_FREE(binary);
         binary = virFindFileInPath(info->altbinary);
-
-        if (binary != NULL && access(binary, X_OK) != 0)
-            VIR_FREE(binary);
     }
 
     /* Can use acceleration for KVM/KQEMU if
@@ -475,10 +472,8 @@ qemuCapsInitGuest(virCapsPtr caps,
             for (i = 0; i < ARRAY_CARDINALITY(kvmbins); ++i) {
                 kvmbin = virFindFileInPath(kvmbins[i]);
 
-                if (kvmbin == NULL || access(kvmbin, X_OK) != 0) {
-                    VIR_FREE(kvmbin);
+                if (!kvmbin)
                     continue;
-                }
 
                 haskvm = 1;
                 if (!binary)
@@ -753,7 +748,7 @@ virCapsPtr qemuCapsInit(virCapsPtr old_caps)
     /* Then possibly the Xen paravirt guests (ie Xenner */
     xenner = virFindFileInPath("xenner");
 
-    if (xenner != NULL && access(xenner, X_OK) == 0 &&
+    if (xenner != NULL && virFileIsExecutable(xenner) == 0 &&
         access("/dev/kvm", F_OK) == 0) {
         for (i = 0 ; i < ARRAY_CARDINALITY(arch_info_xen) ; i++)
             /* Allow Xen 32-on-32, 32-on-64 and 64-on-64 */
@@ -1147,7 +1142,7 @@ int qemuCapsExtractVersionInfo(const char *qemu, const char *arch,
      * Technically we could catch the exec() failure, but that's
      * in a sub-process so it's hard to feed back a useful error.
      */
-    if (access(qemu, X_OK) < 0) {
+    if (!virFileIsExecutable(qemu)) {
         virReportSystemError(errno, _("Cannot find QEMU binary %s"), qemu);
         return -1;
     }

diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 40da5c233d0918c4514f6d394c5ad968ca499535..b05bbcbecc27e84f1e63f75d5dc5e2994937e831 100644 (file)
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -306,7 +306,7 @@ remoteFindDaemonPath(void)
         return(customDaemon);
 
     for (i = 0; serverPaths[i]; i++) {
-        if (access(serverPaths[i], X_OK | R_OK) == 0) {
+        if (virFileIsExecutable(serverPaths[i])) {
             return serverPaths[i];
         }
     }

diff --git a/src/uml/uml_driver.c b/src/uml/uml_driver.c
index f19a4a8202a3d91159b755c0ae9fc09d5573d96e..551448743a0558965bb04fcc626e9d2dc9c51fe8 100644 (file)
--- a/src/uml/uml_driver.c
+++ b/src/uml/uml_driver.c
@@ -829,7 +829,7 @@ static int umlStartVMDaemon(virConnectPtr conn,
      * Technically we could catch the exec() failure, but that's
      * in a sub-process so its hard to feed back a useful error
      */
-    if (access(vm->def->os.kernel, X_OK) < 0) {
+    if (!virFileIsExecutable(vm->def->os.kernel)) {
         virReportSystemError(errno,
                              _("Cannot find UML kernel %s"),
                              vm->def->os.kernel);

diff --git a/src/util/hooks.c b/src/util/hooks.c
index 62446eadbb9725e12a6dc56ac37c0b7eef033749..99dddc4c0551df820ffe82001a328403c38e5bb4 100644 (file)
--- a/src/util/hooks.c
+++ b/src/util/hooks.c
@@ -1,7 +1,7 @@
 /*
  * hooks.c: implementation of the synchronous hooks support
  *
- * Copyright (C) 2010 Red Hat, Inc.
+ * Copyright (C) 2010-2011 Red Hat, Inc.
  * Copyright (C) 2010 Daniel Veillard
  *
  * This library is free software; you can redistribute it and/or
@@ -94,13 +94,12 @@ static int virHooksFound = -1;
 static int
 virHookCheck(int no, const char *driver) {
     char *path;
-    struct stat sb;
     int ret;
 
     if (driver == NULL) {
         virHookReportError(VIR_ERR_INTERNAL_ERROR,
                    _("Invalid hook name for #%d"), no);
-        return(-1);
+        return -1;
     }
 
     ret = virBuildPath(&path, LIBVIRT_HOOK_DIR, driver);
@@ -108,24 +107,19 @@ virHookCheck(int no, const char *driver) {
         virHookReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Failed to build path for %s hook"),
                            driver);
-        return(-1);
+        return -1;
     }
 
-    if (stat(path, &sb) < 0) {
+    if (!virFileIsExecutable(path)) {
         ret = 0;
-        VIR_DEBUG("No hook script %s", path);
+        VIR_WARN("Missing or non-executable hook script %s", path);
     } else {
-        if ((access(path, X_OK) != 0) || (!S_ISREG(sb.st_mode))) {
-            ret = 0;
-            VIR_WARN("Non executable hook script %s", path);
-        } else {
-            ret = 1;
-            VIR_DEBUG("Found hook script %s", path);
-        }
+        ret = 1;
+        VIR_DEBUG("Found hook script %s", path);
     }
 
     VIR_FREE(path);
-    return(ret);
+    return ret;
 }
 
 /*