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 | 46 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 46 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 6255ba92e7..7e8fdca43e 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6835,6 +6835,49 @@ 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) < 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 +7376,9 @@ qemuProcessPrepareHost(virQEMUDriver *driver,
if (qemuProcessPrepareLaunchSecurityGuestInput(vm) < 0)
return -1;
+ if (qemuProcessPreparePstore(vm) < 0)
+ return -1;
+
return 0;
}
--
2.44.2On Mon, Jul 29, 2024 at 11:31:36AM GMT, Michal Privoznik wrote:
> +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) < 0) {
> + virReportSystemError(errno,
> + _("Failed to truncate file '%1$s'"),
> + pstore->path);
> + return -1;
> + }
The size is stored in KiB but ftruncate(2) expect it in bytes, so you
need to multiply it by 1024 or QEMU will (correctly) complain about
the size mismatch on startup.
> + if (VIR_CLOSE(fd) < 0) {
> + virReportSystemError(errno,
> + _("Unable to save '%1$s'"),
> + pstore->path);
> + return -1;
> + }
> +
> +
> + break;
Unnecessary empty line.
Even with the fix mentioned above applied, I still get an error on
startup on my SELinux-enabled system. Running `setenforce 0` makes it
go away, but clearly that's not acceptable.
The diff below seems to do the trick, but I'm not entirely confident
that it's the right fix rather than a mere kludge.
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;
--
Andrea Bolognani / Red Hat / Virtualization
On 7/30/24 16:04, Andrea Bolognani wrote:
> On Mon, Jul 29, 2024 at 11:31:36AM GMT, Michal Privoznik wrote:
>> +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) < 0) {
>> + virReportSystemError(errno,
>> + _("Failed to truncate file '%1$s'"),
>> + pstore->path);
>> + return -1;
>> + }
>
> The size is stored in KiB but ftruncate(2) expect it in bytes, so you
> need to multiply it by 1024 or QEMU will (correctly) complain about
> the size mismatch on startup.
>
>> + if (VIR_CLOSE(fd) < 0) {
>> + virReportSystemError(errno,
>> + _("Unable to save '%1$s'"),
>> + pstore->path);
>> + return -1;
>> + }
>> +
>> +
>> + break;
>
> Unnecessary empty line.
>
> Even with the fix mentioned above applied, I still get an error on
> startup on my SELinux-enabled system. Running `setenforce 0` makes it
> go away, but clearly that's not acceptable.
>
> The diff below seems to do the trick, but I'm not entirely confident
> that it's the right fix rather than a mere kludge.
>
>
>
> 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)
Ah, correct. I don't run SELinux enabled system, hence I did not catch
this in my testing. The thing is, pstore is somewhat similar to NVRAM
store - QEMU must be able to not just read it but also write to it.
Hence, it's different to other files around this line (kernel image,
initrd, DTB, SLIC table) - those are RO. Looking into what NVRAM would
use - it in fact boils down to secdef->imagelabel.
I'll post this as a separate patch.
Thanks for catching this and prompt review!
Michal
© 2016 - 2024 Red Hat, Inc.