[PATCH v2 for v10.6.0 0/2] Two pstore related fixes

This is a v2 of the following patch: https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/PP4WO... NB, 1/2 from the original series is pushed already. diff to v1: - adjusted args to ftruncate() - empty line cleanup - new patch 2/2 (thanks Andrea for testing!) Michal Prívozník (2): qemu: Pre-create pstore device file security: Allow RW access to pstore device src/qemu/qemu_process.c | 45 +++++++++++++++++++++++++++++++++ src/security/security_selinux.c | 2 +- 2 files changed, 46 insertions(+), 1 deletion(-) -- 2.44.2

So far we are relying on QEMU or sysadmin to create the file for pstore. This is suboptimal as in the case of the former we can not set proper seclabels (there's nothing to set seclabels on until QEMU is started). Therefore, make sure the file is created before launching QEMU and that it has the correct size. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_process.c | 45 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 6255ba92e7..cec739c984 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -6835,6 +6835,48 @@ qemuProcessPrepareLaunchSecurityGuestInput(virDomainObj *vm) } +static int +qemuProcessPreparePstore(virDomainObj *vm) +{ + virDomainPstoreDef *pstore = vm->def->pstore; + VIR_AUTOCLOSE fd = -1; + + if (!pstore) + return 0; + + switch (pstore->backend) { + case VIR_DOMAIN_PSTORE_BACKEND_ACPI_ERST: + if ((fd = open(pstore->path, O_WRONLY | O_CREAT, 0600)) < 0) { + virReportSystemError(errno, + _("cannot create file '%1$s'"), + pstore->path); + return -1; + } + + if (ftruncate(fd, pstore->size * 1024) < 0) { + virReportSystemError(errno, + _("Failed to truncate file '%1$s'"), + pstore->path); + return -1; + } + + if (VIR_CLOSE(fd) < 0) { + virReportSystemError(errno, + _("Unable to save '%1$s'"), + pstore->path); + return -1; + } + + break; + + case VIR_DOMAIN_PSTORE_BACKEND_LAST: + break; + } + + return 0; +} + + static int qemuProcessPrepareHostStorageSourceVDPA(virStorageSource *src, qemuDomainObjPrivate *priv) @@ -7333,6 +7375,9 @@ qemuProcessPrepareHost(virQEMUDriver *driver, if (qemuProcessPrepareLaunchSecurityGuestInput(vm) < 0) return -1; + if (qemuProcessPreparePstore(vm) < 0) + return -1; + return 0; } -- 2.44.2

The whole point of pstore device is that the guest writes crash dumps into it. But the way SELinux label is set on the corresponding file warrants RO access only. This is due to a copy-paste from code around: kernel/initrd/DTB/SLIC - these are RO indeed, but pstore MUST be writable too. In a sense it's closer to NVRAM/disks - hence set imagelagel on it. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/security/security_selinux.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c index ba0ce8fb9d..31df4d22db 100644 --- a/src/security/security_selinux.c +++ b/src/security/security_selinux.c @@ -3341,7 +3341,7 @@ virSecuritySELinuxSetAllLabel(virSecurityManager *mgr, if (def->pstore && virSecuritySELinuxSetFilecon(mgr, def->pstore->path, - data->content_context, true) < 0) + secdef->imagelabel, true) < 0) return -1; return 0; -- 2.44.2

On Tue, Jul 30, 2024 at 05:36:38PM GMT, Michal Privoznik wrote:
This is a v2 of the following patch:
https://lists.libvirt.org/archives/list/devel@lists.libvirt.org/thread/PP4WO...
NB, 1/2 from the original series is pushed already.
diff to v1: - adjusted args to ftruncate() - empty line cleanup - new patch 2/2 (thanks Andrea for testing!)
Michal Prívozník (2): qemu: Pre-create pstore device file security: Allow RW access to pstore device
src/qemu/qemu_process.c | 45 +++++++++++++++++++++++++++++++++ src/security/security_selinux.c | 2 +- 2 files changed, 46 insertions(+), 1 deletion(-)
Series Reviewed-by: Andrea Bolognani <abologna@redhat.com> and pushed for your convenience. -- Andrea Bolognani / Red Hat / Virtualization
participants (2)
-
Andrea Bolognani
-
Michal Privoznik