From nobody Sun Feb 8 22:07:52 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 207.211.31.120 as permitted sender) client-ip=207.211.31.120; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-1.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 207.211.31.120 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1596707736; cv=none; d=zohomail.com; s=zohoarc; b=dcy5/zKbUnlEo+nW5lUPmQOd+dqrqdclph9se/eVU01l/oTbAFIW0PvyBnEB6KAWKJHosXEz7YDbmhuhLcsurPGG68XKA3YogQZ0GaCdEyFsinVmsd83J1dW3YHYLSfr0LH0HbhOueOg78WXMMUod26e01lcklmJNmUI7b4m1qY= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1596707736; h=Content-Type:Content-Transfer-Encoding:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To; bh=dEOAn9ec+ZZNb3B0n4744kQfEHdOj9PPNhawdVyvmNE=; b=MieOU2gHbW533LUVfIonv1cGMnTWh9E/H87CDGNphoYThzuqwlWsSQ5zO2spoWo43cP2+o39rWdMbK9Linl63h57cb0OoVipIw1RPHuNf+rFiTlQ72ib5aSlV53Gy4qLeXm7885D1MTlLfYoorZMfoDP2wddMY6KtIZounhO1U0= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 207.211.31.120 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [207.211.31.120]) by mx.zohomail.com with SMTPS id 1596707736920803.7135375610995; Thu, 6 Aug 2020 02:55:36 -0700 (PDT) Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-505-K52mS5jINMivjbisquNMbA-1; Thu, 06 Aug 2020 05:55:33 -0400 Received: from smtp.corp.redhat.com (int-mx05.intmail.prod.int.phx2.redhat.com [10.5.11.15]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A8010100CCC0; Thu, 6 Aug 2020 09:55:27 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 7630865C7C; Thu, 6 Aug 2020 09:55:27 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id C61B99692A; Thu, 6 Aug 2020 09:55:24 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id 0769tMRJ024668 for ; Thu, 6 Aug 2020 05:55:22 -0400 Received: by smtp.corp.redhat.com (Postfix) id E120860E1C; Thu, 6 Aug 2020 09:55:22 +0000 (UTC) Received: from speedmetal.redhat.com (unknown [10.40.208.38]) by smtp.corp.redhat.com (Postfix) with ESMTP id 03A6460BF3 for ; Thu, 6 Aug 2020 09:55:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1596707735; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=dEOAn9ec+ZZNb3B0n4744kQfEHdOj9PPNhawdVyvmNE=; b=JUc/c8kXgRa49eMMJrJL/ZPJz+qlKSyc6Grb4hT5NKkDU1YLHcgaaA8yoKmJF/ef3HRU87 vaHGGGGaZMNLt3s4RbiM3Z6WSOypedwIODGTGqAuSbUvLPXSugG/TjlKb95PFGpIQ+jBh/ mW586ZI0+bkWSMWPFWOPfKiyeidZq78= X-MC-Unique: K52mS5jINMivjbisquNMbA-1 From: Peter Krempa To: libvir-list@redhat.com Subject: [PATCH 1/5] qemuOpenFileAs: Move into util/virqemu.c Date: Thu, 6 Aug 2020 11:55:12 +0200 Message-Id: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-loop: libvir-list@redhat.com X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.15 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) Content-Type: text/plain; charset="utf-8" Signed-off-by: Peter Krempa --- 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 =3D true; - bool need_unlink =3D false; - unsigned int vfoflags =3D 0; - int fd =3D -1; - int path_shared =3D virFileIsSharedFS(path); - uid_t uid =3D geteuid(); - gid_t gid =3D 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 =3D true; - - /* Don't force chown on network-shared FS - * as it is likely to fail. */ - if (path_shared <=3D 0 || dynamicOwnership) - vfoflags |=3D VIR_FILE_OPEN_FORCE_OWNER; - - if (stat(path, &sb) =3D=3D 0) { - /* It already exists, we don't want to delete it on error */ - need_unlink =3D false; - - is_reg =3D !!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 =3D sb.st_uid; - gid =3D sb.st_gid; - } - } - } - - /* First try creating the file as root */ - if (!is_reg) { - if ((fd =3D open(path, oflags & ~O_CREAT)) < 0) { - fd =3D -errno; - goto error; - } - } else { - if ((fd =3D virFileOpenAs(path, oflags, S_IRUSR | S_IWUSR, uid, gi= d, - 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 !=3D -EACCES && fd !=3D -EPERM) || fallback_uid =3D=3D= 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 virFileOpenA= s */ - 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 |=3D VIR_FILE_OPEN_FORCE_MODE; - - if ((fd =3D 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 creatin= g '%s'") - : _("Error from child process opening= '%s'"), - path); - goto cleanup; - } - } - } - cleanup: - if (needUnlink) - *needUnlink =3D 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 =3D qemuOpenFileAs(cfg->user, cfg->group, false, path, - O_WRONLY | O_TRUNC | O_CREAT | directFlag, - &needUnlink); + fd =3D 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 =3D qemuOpenFileAs(cfg->user, cfg->group, false, path, + if ((fd =3D 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 +#include +#include +#include +#include + #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 =3D true; + bool need_unlink =3D false; + unsigned int vfoflags =3D 0; + int fd =3D -1; + int path_shared =3D virFileIsSharedFS(path); + uid_t uid =3D geteuid(); + gid_t gid =3D 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 =3D true; + + /* Don't force chown on network-shared FS + * as it is likely to fail. */ + if (path_shared <=3D 0 || dynamicOwnership) + vfoflags |=3D VIR_FILE_OPEN_FORCE_OWNER; + + if (stat(path, &sb) =3D=3D 0) { + /* It already exists, we don't want to delete it on error */ + need_unlink =3D false; + + is_reg =3D !!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 =3D sb.st_uid; + gid =3D sb.st_gid; + } + } + } + + /* First try creating the file as root */ + if (!is_reg) { + if ((fd =3D open(path, oflags & ~O_CREAT)) < 0) { + fd =3D -errno; + goto error; + } + } else { + if ((fd =3D virFileOpenAs(path, oflags, S_IRUSR | S_IWUSR, uid, gi= d, + 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 !=3D -EACCES && fd !=3D -EPERM) || fallback_uid =3D=3D= 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 virFileOpenA= s */ + 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 |=3D VIR_FILE_OPEN_FORCE_MODE; + + if ((fd =3D 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 creatin= g '%s'") + : _("Error from child process opening= '%s'"), + path); + goto cleanup; + } + } + } + cleanup: + if (needUnlink) + *needUnlink =3D 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); --=20 2.26.2