[PATCH] qemu: Move virQEMUFileOpenAs to qemu_domain.c
Peter Krempa
pkrempa at redhat.com
Mon Aug 24 15:52:00 UTC 2020
Commit 43620689794507308fbd3def6992a68ee2f8fa97 moved the function to
util/virqemu.c which is compiled also on win32 and geteuid()/getegid()
doesn't exist there.
Move it to qemu_domain.c which is compiled only when the qemu driver is
enabled. Originally I didn't want to put it here as qemu_domain.c is a
code dump for helper functions but this is the least invasive fix.
Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
src/libvirt_private.syms | 1 -
src/qemu/qemu_domain.c | 124 +++++++++++++++++++++++++++++++++++++
src/qemu/qemu_domain.h | 7 +++
src/util/virqemu.c | 130 ---------------------------------------
src/util/virqemu.h | 7 ---
5 files changed, 131 insertions(+), 138 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index d737e4d9e4..01c2e710cd 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2939,7 +2939,6 @@ virQEMUBuildDriveCommandlineFromJSON;
virQEMUBuildNetdevCommandlineFromJSON;
virQEMUBuildObjectCommandlineFromJSON;
virQEMUBuildQemuImgKeySecretOpts;
-virQEMUFileOpenAs;
# util/virrandom.h
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 715815f224..b321722f0e 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -10828,6 +10828,130 @@ qemuDomainDiskBlockJobIsSupported(virDomainObjPtr vm,
}
+int
+virQEMUFileOpenAs(uid_t fallback_uid,
+ gid_t fallback_gid,
+ bool dynamicOwnership,
+ const char *path,
+ int oflags,
+ bool *needUnlink)
+{
+ struct stat sb;
+ bool is_reg = true;
+ bool need_unlink = false;
+ unsigned int vfoflags = 0;
+ int fd = -1;
+ int path_shared = virFileIsSharedFS(path);
+ uid_t uid = geteuid();
+ gid_t gid = getegid();
+
+ /* path might be a pre-existing block dev, in which case
+ * we need to skip the create step, and also avoid unlink
+ * in the failure case */
+ if (oflags & O_CREAT) {
+ need_unlink = true;
+
+ /* Don't force chown on network-shared FS
+ * as it is likely to fail. */
+ if (path_shared <= 0 || dynamicOwnership)
+ vfoflags |= VIR_FILE_OPEN_FORCE_OWNER;
+
+ if (stat(path, &sb) == 0) {
+ /* It already exists, we don't want to delete it on error */
+ need_unlink = false;
+
+ is_reg = !!S_ISREG(sb.st_mode);
+ /* If the path is regular file which exists
+ * already and dynamic_ownership is off, we don't
+ * want to change its ownership, just open it as-is */
+ if (is_reg && !dynamicOwnership) {
+ uid = sb.st_uid;
+ gid = sb.st_gid;
+ }
+ }
+ }
+
+ /* First try creating the file as root */
+ if (!is_reg) {
+ if ((fd = open(path, oflags & ~O_CREAT)) < 0) {
+ fd = -errno;
+ goto error;
+ }
+ } else {
+ if ((fd = virFileOpenAs(path, oflags, S_IRUSR | S_IWUSR, uid, gid,
+ vfoflags | VIR_FILE_OPEN_NOFORK)) < 0) {
+ /* If we failed as root, and the error was permission-denied
+ (EACCES or EPERM), assume it's on a network-connected share
+ where root access is restricted (eg, root-squashed NFS). If the
+ qemu user is non-root, just set a flag to
+ bypass security driver shenanigans, and retry the operation
+ after doing setuid to qemu user */
+ if ((fd != -EACCES && fd != -EPERM) || fallback_uid == geteuid())
+ goto error;
+
+ /* On Linux we can also verify the FS-type of the directory. */
+ switch (path_shared) {
+ case 1:
+ /* it was on a network share, so we'll continue
+ * as outlined above
+ */
+ break;
+
+ case -1:
+ virReportSystemError(-fd, oflags & O_CREAT
+ ? _("Failed to create file "
+ "'%s': couldn't determine fs type")
+ : _("Failed to open file "
+ "'%s': couldn't determine fs type"),
+ path);
+ goto cleanup;
+
+ case 0:
+ default:
+ /* local file - log the error returned by virFileOpenAs */
+ goto error;
+ }
+
+ /* If we created the file above, then we need to remove it;
+ * otherwise, the next attempt to create will fail. If the
+ * file had already existed before we got here, then we also
+ * don't want to delete it and allow the following to succeed
+ * or fail based on existing protections
+ */
+ if (need_unlink)
+ unlink(path);
+
+ /* Retry creating the file as qemu user */
+
+ /* Since we're passing different modes... */
+ vfoflags |= VIR_FILE_OPEN_FORCE_MODE;
+
+ if ((fd = virFileOpenAs(path, oflags,
+ S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
+ fallback_uid, fallback_gid,
+ vfoflags | VIR_FILE_OPEN_FORK)) < 0) {
+ virReportSystemError(-fd, oflags & O_CREAT
+ ? _("Error from child process creating '%s'")
+ : _("Error from child process opening '%s'"),
+ path);
+ goto cleanup;
+ }
+ }
+ }
+ cleanup:
+ if (needUnlink)
+ *needUnlink = need_unlink;
+ return fd;
+
+ error:
+ virReportSystemError(-fd, oflags & O_CREAT
+ ? _("Failed to create file '%s'")
+ : _("Failed to open file '%s'"),
+ path);
+ goto cleanup;
+}
+
+
/**
* qemuDomainOpenFile:
* @driver: driver object
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 70fdb48317..e6d4acb8d4 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -1024,6 +1024,13 @@ void qemuDomainRemoveInactiveJob(virQEMUDriverPtr driver,
void qemuDomainRemoveInactiveJobLocked(virQEMUDriverPtr driver,
virDomainObjPtr vm);
+int virQEMUFileOpenAs(uid_t fallback_uid,
+ gid_t fallback_gid,
+ bool dynamicOwnership,
+ const char *path,
+ int oflags,
+ bool *needUnlink);
+
int
qemuDomainOpenFile(virQEMUDriverPtr driver,
virDomainObjPtr vm,
diff --git a/src/util/virqemu.c b/src/util/virqemu.c
index e1c8673390..20cb09d878 100644
--- a/src/util/virqemu.c
+++ b/src/util/virqemu.c
@@ -22,18 +22,12 @@
#include <config.h>
-#include <sys/types.h>
-#include <sys/stat.h>
-#include <fcntl.h>
-#include <unistd.h>
-
#include "virbuffer.h"
#include "virerror.h"
#include "virlog.h"
#include "virqemu.h"
#include "virstring.h"
#include "viralloc.h"
-#include "virfile.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -447,127 +441,3 @@ virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf,
virBufferAddLit(buf, ",");
}
}
-
-
-int
-virQEMUFileOpenAs(uid_t fallback_uid,
- gid_t fallback_gid,
- bool dynamicOwnership,
- const char *path,
- int oflags,
- bool *needUnlink)
-{
- struct stat sb;
- bool is_reg = true;
- bool need_unlink = false;
- unsigned int vfoflags = 0;
- int fd = -1;
- int path_shared = virFileIsSharedFS(path);
- uid_t uid = geteuid();
- gid_t gid = getegid();
-
- /* path might be a pre-existing block dev, in which case
- * we need to skip the create step, and also avoid unlink
- * in the failure case */
- if (oflags & O_CREAT) {
- need_unlink = true;
-
- /* Don't force chown on network-shared FS
- * as it is likely to fail. */
- if (path_shared <= 0 || dynamicOwnership)
- vfoflags |= VIR_FILE_OPEN_FORCE_OWNER;
-
- if (stat(path, &sb) == 0) {
- /* It already exists, we don't want to delete it on error */
- need_unlink = false;
-
- is_reg = !!S_ISREG(sb.st_mode);
- /* If the path is regular file which exists
- * already and dynamic_ownership is off, we don't
- * want to change its ownership, just open it as-is */
- if (is_reg && !dynamicOwnership) {
- uid = sb.st_uid;
- gid = sb.st_gid;
- }
- }
- }
-
- /* First try creating the file as root */
- if (!is_reg) {
- if ((fd = open(path, oflags & ~O_CREAT)) < 0) {
- fd = -errno;
- goto error;
- }
- } else {
- if ((fd = virFileOpenAs(path, oflags, S_IRUSR | S_IWUSR, uid, gid,
- vfoflags | VIR_FILE_OPEN_NOFORK)) < 0) {
- /* If we failed as root, and the error was permission-denied
- (EACCES or EPERM), assume it's on a network-connected share
- where root access is restricted (eg, root-squashed NFS). If the
- qemu user is non-root, just set a flag to
- bypass security driver shenanigans, and retry the operation
- after doing setuid to qemu user */
- if ((fd != -EACCES && fd != -EPERM) || fallback_uid == geteuid())
- goto error;
-
- /* On Linux we can also verify the FS-type of the directory. */
- switch (path_shared) {
- case 1:
- /* it was on a network share, so we'll continue
- * as outlined above
- */
- break;
-
- case -1:
- virReportSystemError(-fd, oflags & O_CREAT
- ? _("Failed to create file "
- "'%s': couldn't determine fs type")
- : _("Failed to open file "
- "'%s': couldn't determine fs type"),
- path);
- goto cleanup;
-
- case 0:
- default:
- /* local file - log the error returned by virFileOpenAs */
- goto error;
- }
-
- /* If we created the file above, then we need to remove it;
- * otherwise, the next attempt to create will fail. If the
- * file had already existed before we got here, then we also
- * don't want to delete it and allow the following to succeed
- * or fail based on existing protections
- */
- if (need_unlink)
- unlink(path);
-
- /* Retry creating the file as qemu user */
-
- /* Since we're passing different modes... */
- vfoflags |= VIR_FILE_OPEN_FORCE_MODE;
-
- if ((fd = virFileOpenAs(path, oflags,
- S_IRUSR|S_IWUSR|S_IRGRP|S_IWGRP,
- fallback_uid, fallback_gid,
- vfoflags | VIR_FILE_OPEN_FORK)) < 0) {
- virReportSystemError(-fd, oflags & O_CREAT
- ? _("Error from child process creating '%s'")
- : _("Error from child process opening '%s'"),
- path);
- goto cleanup;
- }
- }
- }
- cleanup:
- if (needUnlink)
- *needUnlink = need_unlink;
- return fd;
-
- error:
- virReportSystemError(-fd, oflags & O_CREAT
- ? _("Failed to create file '%s'")
- : _("Failed to open file '%s'"),
- path);
- goto cleanup;
-}
diff --git a/src/util/virqemu.h b/src/util/virqemu.h
index e4e071f7c5..b1296cb657 100644
--- a/src/util/virqemu.h
+++ b/src/util/virqemu.h
@@ -63,10 +63,3 @@ void virQEMUBuildQemuImgKeySecretOpts(virBufferPtr buf,
virStorageEncryptionInfoDefPtr enc,
const char *alias)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3);
-
-int virQEMUFileOpenAs(uid_t fallback_uid,
- gid_t fallback_gid,
- bool dynamicOwnership,
- const char *path,
- int oflags,
- bool *needUnlink);
--
2.26.2
More information about the libvir-list
mailing list