[libvirt] [PATCH 2/2] maint: prohibit access(,X_OK)
Eric Blake
eblake at redhat.com
Fri Mar 18 20:46:50 UTC 2011
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
More information about the libvir-list
mailing list