[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH 2/2] 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(,X_OK)
is not strictly needed to execute a file (although its unusual to see
a file with X_OK but not R_OK).

* 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.
---

This patch depends on the unreviewed:
https://www.redhat.com/archives/libvir-list/2011-March/msg00770.html

 cfg.mk                       |    9 +++++++++
 src/network/bridge_driver.c  |    2 +-
 src/qemu/qemu_capabilities.c |   15 +++++----------
 src/remote/remote_driver.c   |    2 +-
 src/uml/uml_driver.c         |    2 +-
 src/util/hooks.c             |    4 ++--
 6 files changed, 19 insertions(+), 15 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 2a97f88..ac419f7 100644
--- 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 c30620a..fb933a2 100644
--- 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 d8aa9cd..f86e7f5 100644
--- 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 b844d9a..a8fb52d 100644
--- 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 538d5f7..2af53ff 100644
--- 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 5ba2036..8231349 100644
--- 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
@@ -113,7 +113,7 @@ virHookCheck(int no, const char *driver) {
         ret = 0;
         VIR_DEBUG("No hook script %s", path);
     } else {
-        if ((access(path, X_OK) != 0) || (!S_ISREG(sb.st_mode))) {
+        if (!virFileIsExecutable(path)) {
             ret = 0;
             VIR_WARN("Non executable hook script %s", path);
         } else {
-- 
1.7.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]