[PATCH 1/5] qemuOpenFileAs: Move into util/virqemu.c
Peter Krempa
pkrempa at redhat.com
Thu Aug 6 09:55:12 UTC 2020
Signed-off-by: Peter Krempa <pkrempa at redhat.com>
---
src/libvirt_private.syms | 1 +
src/qemu/qemu_driver.c | 138 ++-------------------------------------
src/util/virqemu.c | 130 ++++++++++++++++++++++++++++++++++++
src/util/virqemu.h | 7 ++
4 files changed, 144 insertions(+), 132 deletions(-)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 01c2e710cd..d737e4d9e4 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2939,6 +2939,7 @@ virQEMUBuildDriveCommandlineFromJSON;
virQEMUBuildNetdevCommandlineFromJSON;
virQEMUBuildObjectCommandlineFromJSON;
virQEMUBuildQemuImgKeySecretOpts;
+virQEMUFileOpenAs;
# util/virrandom.h
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 0f98243fe4..a667eb21bf 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -148,11 +148,6 @@ static int qemuDomainObjStart(virConnectPtr conn,
static int qemuDomainManagedSaveLoad(virDomainObjPtr vm,
void *opaque);
-static int qemuOpenFileAs(uid_t fallback_uid, gid_t fallback_gid,
- bool dynamicOwnership,
- const char *path, int oflags,
- bool *needUnlink);
-
static virQEMUDriverPtr qemu_driver;
/* Looks up the domain object from snapshot and unlocks the
@@ -3062,129 +3057,8 @@ qemuOpenFile(virQEMUDriverPtr driver,
(virParseOwnershipIds(seclabel->label, &user, &group) < 0))
return -1;
- return qemuOpenFileAs(user, group, dynamicOwnership,
- path, oflags, needUnlink);
-}
-
-static int
-qemuOpenFileAs(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;
+ return virQEMUFileOpenAs(user, group, dynamicOwnership,
+ path, oflags, needUnlink);
}
@@ -3247,9 +3121,9 @@ qemuDomainSaveMemory(virQEMUDriverPtr driver,
}
}
- fd = qemuOpenFileAs(cfg->user, cfg->group, false, path,
- O_WRONLY | O_TRUNC | O_CREAT | directFlag,
- &needUnlink);
+ fd = virQEMUFileOpenAs(cfg->user, cfg->group, false, path,
+ O_WRONLY | O_TRUNC | O_CREAT | directFlag,
+ &needUnlink);
if (fd < 0)
goto cleanup;
@@ -3824,7 +3698,7 @@ doCoreDump(virQEMUDriverPtr driver,
/* Core dumps usually imply last-ditch analysis efforts are
* desired, so we intentionally do not unlink even if a file was
* created. */
- if ((fd = qemuOpenFileAs(cfg->user, cfg->group, false, path,
+ if ((fd = virQEMUFileOpenAs(cfg->user, cfg->group, false, path,
O_CREAT | O_TRUNC | O_WRONLY | directFlag,
NULL)) < 0)
goto cleanup;
diff --git a/src/util/virqemu.c b/src/util/virqemu.c
index 20cb09d878..e1c8673390 100644
--- a/src/util/virqemu.c
+++ b/src/util/virqemu.c
@@ -22,12 +22,18 @@
#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
@@ -441,3 +447,127 @@ 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 b1296cb657..e4e071f7c5 100644
--- a/src/util/virqemu.h
+++ b/src/util/virqemu.h
@@ -63,3 +63,10 @@ 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