10:02 a.m.
The Linux kernel offers a way to mitigate side channel attacks on Hyper
Threads (e.g. MDS and L1TF). Long story short, userspace can define
groups of processes (aka trusted groups) and only processes within one
group can run on sibling Hyper Threads. The group membership is
automatically preserved on fork() and exec().
Now, there is one scenario which I don't cover in my series and I'd like
to hear proposal: if there are two guests with odd number of vCPUs they
can no longer run on sibling Hyper Threads because my patches create
separate group for each QEMU. This is a performance penalty. Ideally, we
would have a knob inside domain XML that would place two or more domains
into the same trusted group. But since there's pre-existing example (of
sharing a piece of information between two domains) I've failed to come
up with something usable.
Also, it's worth noting, that on kernel level, group membership is
expressed by so called 'cookie' which is effectively an unique UL
number, but there's no API that would "set this number on given
process", so we may have to go with some abstraction layer.
Michal Prívozník (10):
qemu_tpm: Make APIs work over a single virDomainTPMDef
qemu_dbus: Separate PID read code into qemuDBusGetPID
qemu_vhost_user_gpu: Export qemuVhostUserGPUGetPid()
qemu_tpm: Expose qemuTPMEmulatorGetPid()
qemu_virtiofs: Separate PID read code into qemuVirtioFSGetPid
virprocess: Core Scheduling support
virCommand: Introduce APIs for core scheduling
qemu_conf: Introduce a knob to turn off SCHED_CORE
qemu: Enable SCHED_CORE for domains and helper processes
qemu: Place helper processes into the same trusted group
src/libvirt_private.syms | 6 +
src/qemu/libvirtd_qemu.aug | 1 +
src/qemu/qemu.conf.in | 5 +
src/qemu/qemu_conf.c | 24 ++++
src/qemu/qemu_conf.h | 2 +
src/qemu/qemu_dbus.c | 42 ++++---
src/qemu/qemu_dbus.h | 4 +
src/qemu/qemu_extdevice.c | 171 ++++++++++++++++++++++++++---
src/qemu/qemu_extdevice.h | 3 +
src/qemu/qemu_process.c | 9 ++
src/qemu/qemu_security.c | 4 +
src/qemu/qemu_tpm.c | 91 +++++----------
src/qemu/qemu_tpm.h | 18 ++-
src/qemu/qemu_vhost_user_gpu.c | 2 +-
src/qemu/qemu_vhost_user_gpu.h | 8 ++
src/qemu/qemu_virtiofs.c | 41 ++++---
src/qemu/qemu_virtiofs.h | 5 +
src/qemu/test_libvirtd_qemu.aug.in | 1 +
src/util/vircommand.c | 74 +++++++++++++
src/util/vircommand.h | 5 +
src/util/virprocess.c | 124 +++++++++++++++++++++
src/util/virprocess.h | 8 ++
22 files changed, 538 insertions(+), 110 deletions(-)
--
2.35.1
10:02 a.m.
New subject: [PATCH RFC 01/10] qemu_tpm: Make APIs work over a single virDomainTPMDef
In qemu_extdevice.c lives code that handles helper daemons that
are required for some types of devices (e.g. virtiofsd,
vhost-user-gpu, swtpm, etc.). These devices have their own
handling code in separate files, with only a very basic functions
exposed (e.g. for starting/stopping helper process, placing it
into given CGroup, etc.). And these functions all work over a
single instance of device (virDomainVideoDef *, virDomainFSDef *,
etc.), except for TPM handling code which takes virDomainDef *
and iterates over it inside its module.
Remove this oddness and make qemuExtTPM*() functions look closer
to the rest of the code.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_extdevice.c | 51 ++++++++++++++++------
src/qemu/qemu_tpm.c | 89 +++++++++++----------------------------
src/qemu/qemu_tpm.h | 11 +++--
3 files changed, 69 insertions(+), 82 deletions(-)
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 537b130394..234815c075 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -73,8 +73,15 @@ static int
qemuExtDevicesInitPaths(virQEMUDriver *driver,
virDomainDef *def)
{
- if (def->ntpms > 0)
- return qemuExtTPMInitPaths(driver, def);
+ size_t i;
+
+ for (i = 0; i < def->ntpms; i++) {
+ virDomainTPMDef *tpm = def->tpms[i];
+
+ if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
+ qemuExtTPMInitPaths(driver, def, tpm) < 0)
+ return -1;
+ }
return 0;
}
@@ -135,9 +142,13 @@ qemuExtDevicesPrepareHost(virQEMUDriver *driver,
if (qemuExtDevicesInitPaths(driver, def) < 0)
return -1;
- if (def->ntpms > 0 &&
- qemuExtTPMPrepareHost(driver, def) < 0)
- return -1;
+ for (i = 0; i < def->ntpms; i++) {
+ virDomainTPMDef *tpm = def->tpms[i];
+
+ if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
+ qemuExtTPMPrepareHost(driver, def, tpm) < 0)
+ return -1;
+ }
for (i = 0; i < def->nnets; i++) {
virDomainNetDef *net = def->nets[i];
@@ -155,11 +166,14 @@ void
qemuExtDevicesCleanupHost(virQEMUDriver *driver,
virDomainDef *def)
{
+ size_t i;
+
if (qemuExtDevicesInitPaths(driver, def) < 0)
return;
- if (def->ntpms > 0)
- qemuExtTPMCleanupHost(def);
+ for (i = 0; i < def->ntpms; i++) {
+ qemuExtTPMCleanupHost(def->tpms[i]);
+ }
}
@@ -180,8 +194,13 @@ qemuExtDevicesStart(virQEMUDriver *driver,
}
}
- if (def->ntpms > 0 && qemuExtTPMStart(driver, vm, incomingMigration)
< 0)
- return -1;
+ for (i = 0; i < def->ntpms; i++) {
+ virDomainTPMDef *tpm = def->tpms[i];
+
+ if (tpm->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
+ qemuExtTPMStart(driver, vm, tpm, incomingMigration) < 0)
+ return -1;
+ }
for (i = 0; i < def->nnets; i++) {
virDomainNetDef *net = def->nets[i];
@@ -222,8 +241,10 @@ qemuExtDevicesStop(virQEMUDriver *driver,
qemuExtVhostUserGPUStop(driver, vm, video);
}
- if (def->ntpms > 0)
- qemuExtTPMStop(driver, vm);
+ for (i = 0; i < def->ntpms; i++) {
+ if (def->tpms[i]->type == VIR_DOMAIN_TPM_TYPE_EMULATOR)
+ qemuExtTPMStop(driver, vm);
+ }
for (i = 0; i < def->nnets; i++) {
virDomainNetDef *net = def->nets[i];
@@ -299,9 +320,11 @@ qemuExtDevicesSetupCgroup(virQEMUDriver *driver,
return -1;
}
- if (def->ntpms > 0 &&
- qemuExtTPMSetupCgroup(driver, def, cgroup) < 0)
- return -1;
+ for (i = 0; i < def->ntpms; i++) {
+ if (def->tpms[i]->type == VIR_DOMAIN_TPM_TYPE_EMULATOR &&
+ qemuExtTPMSetupCgroup(driver, def, cgroup) < 0)
+ return -1;
+ }
for (i = 0; i < def->nfss; i++) {
virDomainFSDef *fs = def->fss[i];
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 56bccee128..086780edcd 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -971,86 +971,59 @@ qemuTPMEmulatorStart(virQEMUDriver *driver,
int
qemuExtTPMInitPaths(virQEMUDriver *driver,
- virDomainDef *def)
+ virDomainDef *def,
+ virDomainTPMDef *tpm)
{
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
- size_t i;
- for (i = 0; i < def->ntpms; i++) {
- if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR)
- continue;
-
- return qemuTPMEmulatorInitPaths(def->tpms[i],
- cfg->swtpmStorageDir,
- cfg->swtpmLogDir,
- def->name,
- def->uuid);
- }
-
- return 0;
+ return qemuTPMEmulatorInitPaths(tpm,
+ cfg->swtpmStorageDir,
+ cfg->swtpmLogDir,
+ def->name,
+ def->uuid);
}
int
qemuExtTPMPrepareHost(virQEMUDriver *driver,
- virDomainDef *def)
+ virDomainDef *def,
+ virDomainTPMDef *tpm)
{
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
g_autofree char *shortName = virDomainDefGetShortName(def);
- size_t i;
if (!shortName)
return -1;
- for (i = 0; i < def->ntpms; i++) {
- if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR)
- continue;
-
- return qemuTPMEmulatorPrepareHost(def->tpms[i], cfg->swtpmLogDir,
- cfg->swtpm_user,
- cfg->swtpm_group,
- cfg->swtpmStateDir, cfg->user,
- shortName);
- }
-
- return 0;
+ return qemuTPMEmulatorPrepareHost(tpm,
+ cfg->swtpmLogDir,
+ cfg->swtpm_user,
+ cfg->swtpm_group,
+ cfg->swtpmStateDir,
+ cfg->user,
+ shortName);
}
void
-qemuExtTPMCleanupHost(virDomainDef *def)
+qemuExtTPMCleanupHost(virDomainTPMDef *tpm)
{
- size_t i;
-
- for (i = 0; i < def->ntpms; i++) {
- if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR)
- continue;
-
- qemuTPMEmulatorCleanupHost(def->tpms[i]);
- }
+ qemuTPMEmulatorCleanupHost(tpm);
}
int
qemuExtTPMStart(virQEMUDriver *driver,
virDomainObj *vm,
+ virDomainTPMDef *tpm,
bool incomingMigration)
{
g_autofree char *shortName = virDomainDefGetShortName(vm->def);
- size_t i;
if (!shortName)
return -1;
- for (i = 0; i < vm->def->ntpms; i++) {
- if (vm->def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR)
- continue;
-
- return qemuTPMEmulatorStart(driver, vm, shortName, vm->def->tpms[i],
- incomingMigration);
- }
-
- return 0;
+ return qemuTPMEmulatorStart(driver, vm, shortName, tpm, incomingMigration);
}
@@ -1060,20 +1033,12 @@ qemuExtTPMStop(virQEMUDriver *driver,
{
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
g_autofree char *shortName = virDomainDefGetShortName(vm->def);
- size_t i;
if (!shortName)
return;
- for (i = 0; i < vm->def->ntpms; i++) {
- if (vm->def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR)
- continue;
-
- qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName);
- qemuSecurityCleanupTPMEmulator(driver, vm);
- }
-
- return;
+ qemuTPMEmulatorStop(cfg->swtpmStateDir, shortName);
+ qemuSecurityCleanupTPMEmulator(driver, vm);
}
@@ -1084,18 +1049,12 @@ qemuExtTPMSetupCgroup(virQEMUDriver *driver,
{
g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
g_autofree char *shortName = virDomainDefGetShortName(def);
- size_t i;
if (!shortName)
return -1;
- for (i = 0; i < def->ntpms; i++) {
- if (def->tpms[i]->type != VIR_DOMAIN_TPM_TYPE_EMULATOR)
- continue;
-
- if (qemuExtTPMEmulatorSetupCgroup(cfg->swtpmStateDir, shortName, cgroup) <
0)
- return -1;
- }
+ if (qemuExtTPMEmulatorSetupCgroup(cfg->swtpmStateDir, shortName, cgroup) < 0)
+ return -1;
return 0;
}
diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h
index da8ce4c369..9951f025a6 100644
--- a/src/qemu/qemu_tpm.h
+++ b/src/qemu/qemu_tpm.h
@@ -23,22 +23,27 @@
#include "vircommand.h"
int qemuExtTPMInitPaths(virQEMUDriver *driver,
- virDomainDef *def)
+ virDomainDef *def,
+ virDomainTPMDef *tpm)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
G_GNUC_WARN_UNUSED_RESULT;
int qemuExtTPMPrepareHost(virQEMUDriver *driver,
- virDomainDef *def)
+ virDomainDef *def,
+ virDomainTPMDef *tpm)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
+ ATTRIBUTE_NONNULL(3)
G_GNUC_WARN_UNUSED_RESULT;
-void qemuExtTPMCleanupHost(virDomainDef *def)
+void qemuExtTPMCleanupHost(virDomainTPMDef *tpm)
ATTRIBUTE_NONNULL(1);
int qemuExtTPMStart(virQEMUDriver *driver,
virDomainObj *vm,
+ virDomainTPMDef *def,
bool incomingMigration)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
+ ATTRIBUTE_NONNULL(3)
G_GNUC_WARN_UNUSED_RESULT;
void qemuExtTPMStop(virQEMUDriver *driver,
--
2.35.1
11:49 a.m.
New subject: [PATCH RFC 01/10] qemu_tpm: Make APIs work over a single virDomainTPMDef
On Mon, May 09, 2022 at 05:02:08PM +0200, Michal Privoznik wrote:
In qemu_extdevice.c lives code that handles helper daemons that
are required for some types of devices (e.g. virtiofsd,
vhost-user-gpu, swtpm, etc.). These devices have their own
handling code in separate files, with only a very basic functions
exposed (e.g. for starting/stopping helper process, placing it
into given CGroup, etc.). And these functions all work over a
single instance of device (virDomainVideoDef *, virDomainFSDef *,
etc.), except for TPM handling code which takes virDomainDef *
and iterates over it inside its module.
Remove this oddness and make qemuExtTPM*() functions look closer
to the rest of the code.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_extdevice.c | 51 ++++++++++++++++------
src/qemu/qemu_tpm.c | 89 +++++++++++----------------------------
src/qemu/qemu_tpm.h | 11 +++--
3 files changed, 69 insertions(+), 82 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
9:17 a.m.
New subject: [PATCH RFC 01/10] qemu_tpm: Make APIs work over a single virDomainTPMDef
On 5/23/22 18:49, Daniel P. Berrangé wrote:
On Mon, May 09, 2022 at 05:02:08PM +0200, Michal Privoznik wrote:
> In qemu_extdevice.c lives code that handles helper daemons that
> are required for some types of devices (e.g. virtiofsd,
> vhost-user-gpu, swtpm, etc.). These devices have their own
> handling code in separate files, with only a very basic functions
> exposed (e.g. for starting/stopping helper process, placing it
> into given CGroup, etc.). And these functions all work over a
> single instance of device (virDomainVideoDef *, virDomainFSDef *,
> etc.), except for TPM handling code which takes virDomainDef *
> and iterates over it inside its module.
>
> Remove this oddness and make qemuExtTPM*() functions look closer
> to the rest of the code.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/qemu/qemu_extdevice.c | 51 ++++++++++++++++------
> src/qemu/qemu_tpm.c | 89 +++++++++++----------------------------
> src/qemu/qemu_tpm.h | 11 +++--
> 3 files changed, 69 insertions(+), 82 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
PSA: I'm going to push this one regardless of ongoing discussion on the
design, because it's independent of the rest and actually makes TPM code
consistent with the rest of devices.
Michal
10:02 a.m.
New subject: [PATCH RFC 02/10] qemu_dbus: Separate PID read code into qemuDBusGetPID
In near future it will be necessary to know the PID of DBus
daemon started for QEMU. Move the code into a separate function
(qemuDBusGetPID()) and export it in the header file.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_dbus.c | 42 +++++++++++++++++++++++++++++-------------
src/qemu/qemu_dbus.h | 4 ++++
2 files changed, 33 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
index 2ed8f8640d..0eae1aa2fe 100644
--- a/src/qemu/qemu_dbus.c
+++ b/src/qemu/qemu_dbus.c
@@ -146,28 +146,44 @@ qemuDBusStop(virQEMUDriver *driver,
}
+int
+qemuDBusGetPID(virQEMUDriver *driver,
+ virDomainObj *vm,
+ pid_t *pid)
+{
+ g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+ qemuDomainObjPrivate *priv = vm->privateData;
+ g_autofree char *shortName = NULL;
+ g_autofree char *pidfile = NULL;
+
+ if (!priv->dbusDaemonRunning)
+ return 0;
+
+ if (!(shortName = virDomainDefGetShortName(vm->def)))
+ return -1;
+ pidfile = qemuDBusCreatePidFilename(cfg, shortName);
+ if (virPidFileReadPath(pidfile, pid) < 0) {
+ VIR_WARN("Unable to get DBus PID");
+ return -1;
+ }
+
+ return 0;
+}
+
+
int
qemuDBusSetupCgroup(virQEMUDriver *driver,
virDomainObj *vm,
virCgroup *cgroup)
{
- g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
- qemuDomainObjPrivate *priv = vm->privateData;
- g_autofree char *shortName = NULL;
- g_autofree char *pidfile = NULL;
pid_t cpid = -1;
- if (!priv->dbusDaemonRunning)
+ if (qemuDBusGetPID(driver, vm, &cpid) < 0)
+ return -1;
+
+ if (cpid == -1)
return 0;
- if (!(shortName = virDomainDefGetShortName(vm->def)))
- return -1;
- pidfile = qemuDBusCreatePidFilename(cfg, shortName);
- if (virPidFileReadPath(pidfile, &cpid) < 0) {
- VIR_WARN("Unable to get DBus PID");
- return -1;
- }
-
return virCgroupAddProcess(cgroup, cpid);
}
diff --git a/src/qemu/qemu_dbus.h b/src/qemu/qemu_dbus.h
index b27f38a591..a079976aa4 100644
--- a/src/qemu/qemu_dbus.h
+++ b/src/qemu/qemu_dbus.h
@@ -34,6 +34,10 @@ void qemuDBusVMStateAdd(virDomainObj *vm, const char *id);
void qemuDBusVMStateRemove(virDomainObj *vm, const char *id);
+int qemuDBusGetPID(virQEMUDriver *driver,
+ virDomainObj *vm,
+ pid_t *pid);
+
int qemuDBusSetupCgroup(virQEMUDriver *driver,
virDomainObj *vm,
virCgroup *cgroup);
--
2.35.1
11:50 a.m.
New subject: [PATCH RFC 02/10] qemu_dbus: Separate PID read code into qemuDBusGetPID
On Mon, May 09, 2022 at 05:02:09PM +0200, Michal Privoznik wrote:
In near future it will be necessary to know the PID of DBus
daemon started for QEMU. Move the code into a separate function
(qemuDBusGetPID()) and export it in the header file.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_dbus.c | 42 +++++++++++++++++++++++++++++-------------
src/qemu/qemu_dbus.h | 4 ++++
2 files changed, 33 insertions(+), 13 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:02 a.m.
New subject: [PATCH RFC 03/10] qemu_vhost_user_gpu: Export qemuVhostUserGPUGetPid()
In near future it will be necessary to know the PID of
vhost-user-gpu process for QEMU. Export the function that does
just that (qemuVhostUserGPUGetPid()).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_vhost_user_gpu.c | 2 +-
src/qemu/qemu_vhost_user_gpu.h | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_vhost_user_gpu.c b/src/qemu/qemu_vhost_user_gpu.c
index 6f601cebde..d108566976 100644
--- a/src/qemu/qemu_vhost_user_gpu.c
+++ b/src/qemu/qemu_vhost_user_gpu.c
@@ -63,7 +63,7 @@ qemuVhostUserGPUCreatePidFilename(const char *stateDir,
* If the PID was not still alive, zero will be returned, and @pid will be
* set to -1;
*/
-static int
+int
qemuVhostUserGPUGetPid(const char *stateDir,
const char *shortName,
const char *alias,
diff --git a/src/qemu/qemu_vhost_user_gpu.h b/src/qemu/qemu_vhost_user_gpu.h
index 0d50dd2464..bde7104af6 100644
--- a/src/qemu/qemu_vhost_user_gpu.h
+++ b/src/qemu/qemu_vhost_user_gpu.h
@@ -40,6 +40,14 @@ void qemuExtVhostUserGPUStop(virQEMUDriver *driver,
virDomainVideoDef *video)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int
+qemuVhostUserGPUGetPid(const char *stateDir,
+ const char *shortName,
+ const char *alias,
+ pid_t *pid)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3)
+ G_GNUC_WARN_UNUSED_RESULT;
+
int
qemuExtVhostUserGPUSetupCgroup(virQEMUDriver *driver,
virDomainDef *def,
--
2.35.1
11:50 a.m.
New subject: [PATCH RFC 03/10] qemu_vhost_user_gpu: Export qemuVhostUserGPUGetPid()
On Mon, May 09, 2022 at 05:02:10PM +0200, Michal Privoznik wrote:
In near future it will be necessary to know the PID of
vhost-user-gpu process for QEMU. Export the function that does
just that (qemuVhostUserGPUGetPid()).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_vhost_user_gpu.c | 2 +-
src/qemu/qemu_vhost_user_gpu.h | 8 ++++++++
2 files changed, 9 insertions(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:02 a.m.
New subject: [PATCH RFC 04/10] qemu_tpm: Expose qemuTPMEmulatorGetPid()
In near future it will be necessary to know the PID of swtpm
process for QEMU. Export the function that does just that
(qemuTPMEmulatorGetPid()).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_tpm.c | 2 +-
src/qemu/qemu_tpm.h | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_tpm.c b/src/qemu/qemu_tpm.c
index 086780edcd..bf86f2fe39 100644
--- a/src/qemu/qemu_tpm.c
+++ b/src/qemu/qemu_tpm.c
@@ -143,7 +143,7 @@ qemuTPMEmulatorPidFileBuildPath(const char *swtpmStateDir,
* If the PID was not still alive, zero will be returned, and @pid will be
* set to -1;
*/
-static int
+int
qemuTPMEmulatorGetPid(const char *swtpmStateDir,
const char *shortName,
pid_t *pid)
diff --git a/src/qemu/qemu_tpm.h b/src/qemu/qemu_tpm.h
index 9951f025a6..9f4d01f60b 100644
--- a/src/qemu/qemu_tpm.h
+++ b/src/qemu/qemu_tpm.h
@@ -50,6 +50,13 @@ void qemuExtTPMStop(virQEMUDriver *driver,
virDomainObj *vm)
ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2);
+int qemuTPMEmulatorGetPid(const char *swtpmStateDir,
+ const char *shortName,
+ pid_t *pid)
+ ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
+ ATTRIBUTE_NONNULL(3)
+ G_GNUC_WARN_UNUSED_RESULT;
+
int qemuExtTPMSetupCgroup(virQEMUDriver *driver,
virDomainDef *def,
virCgroup *cgroup)
--
2.35.1
11:50 a.m.
New subject: [PATCH RFC 04/10] qemu_tpm: Expose qemuTPMEmulatorGetPid()
On Mon, May 09, 2022 at 05:02:11PM +0200, Michal Privoznik wrote:
In near future it will be necessary to know the PID of swtpm
process for QEMU. Export the function that does just that
(qemuTPMEmulatorGetPid()).
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_tpm.c | 2 +-
src/qemu/qemu_tpm.h | 7 +++++++
2 files changed, 8 insertions(+), 1 deletion(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:02 a.m.
New subject: [PATCH RFC 05/10] qemu_virtiofs: Separate PID read code into qemuVirtioFSGetPid
In near future it will be necessary to know the PID of virtiofsd
started for QEMU. Move the code into a separate function
(qemuVirtioFSGetPid()) and export it in the header file.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_virtiofs.c | 38 +++++++++++++++++++++++++-------------
src/qemu/qemu_virtiofs.h | 5 +++++
2 files changed, 30 insertions(+), 13 deletions(-)
diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index 7e3324b017..b3a2d2990a 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -319,26 +319,38 @@ qemuVirtioFSStop(virQEMUDriver *driver G_GNUC_UNUSED,
}
+
+int
+qemuVirtioFSGetPid(virDomainObj *vm,
+ virDomainFSDef *fs,
+ pid_t *pid)
+{
+ g_autofree char *pidfile = NULL;
+ int rc;
+
+ if (!(pidfile = qemuVirtioFSCreatePidFilename(vm, fs->info.alias)))
+ return -1;
+
+ rc = virPidFileReadPathIfAlive(pidfile, pid, NULL);
+ if (rc < 0 || *pid == (pid_t) -1) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("virtiofsd died unexpectedly"));
+ return -1;
+ }
+
+ return 0;
+}
+
+
int
qemuVirtioFSSetupCgroup(virDomainObj *vm,
virDomainFSDef *fs,
virCgroup *cgroup)
{
- g_autofree char *pidfile = NULL;
pid_t pid = -1;
- int rc;
- if (!(pidfile = qemuVirtioFSCreatePidFilename(vm, fs->info.alias)))
- return -1;
-
- rc = virPidFileReadPathIfAlive(pidfile, &pid, NULL);
- if (rc < 0 || pid == (pid_t) -1) {
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
- _("virtiofsd died unexpectedly"));
- return -1;
- }
-
- if (virCgroupAddProcess(cgroup, pid) < 0)
+ if (qemuVirtioFSGetPid(vm, fs, &pid) < 0 ||
+ virCgroupAddProcess(cgroup, pid) < 0)
return -1;
return 0;
diff --git a/src/qemu/qemu_virtiofs.h b/src/qemu/qemu_virtiofs.h
index 5463acef98..dd3fbfa555 100644
--- a/src/qemu/qemu_virtiofs.h
+++ b/src/qemu/qemu_virtiofs.h
@@ -35,6 +35,11 @@ qemuVirtioFSStop(virQEMUDriver *driver,
virDomainObj *vm,
virDomainFSDef *fs);
+int
+qemuVirtioFSGetPid(virDomainObj *vm,
+ virDomainFSDef *fs,
+ pid_t *pid);
+
int
qemuVirtioFSSetupCgroup(virDomainObj *vm,
virDomainFSDef *fs,
--
2.35.1
11:57 a.m.
New subject: [PATCH RFC 05/10] qemu_virtiofs: Separate PID read code into qemuVirtioFSGetPid
On Mon, May 09, 2022 at 05:02:12PM +0200, Michal Privoznik wrote:
In near future it will be necessary to know the PID of virtiofsd
started for QEMU. Move the code into a separate function
(qemuVirtioFSGetPid()) and export it in the header file.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_virtiofs.c | 38 +++++++++++++++++++++++++-------------
src/qemu/qemu_virtiofs.h | 5 +++++
2 files changed, 30 insertions(+), 13 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:02 a.m.
New subject: [PATCH RFC 06/10] virprocess: Core Scheduling support
Since its 5.14 release the Linux kernel allows userspace to
define trusted groups of processes/threads that can run on
sibling Hyper Threads (HT) at the same time. This is to mitigate
side channel attacks like L1TF or MDS. If there are no tasks to
fully utilize all HTs, then a HT will idle instead of running a
task from another (un-)trusted group.
On low level, this is implemented by cookies (effectively an UL
value): processes in the same trusted group share the same cookie
and cookie is unique to the group. There are four basic
operations:
1) PR_SCHED_CORE_GET -- get cookie of given PID,
2) PR_SCHED_CORE_CREATE -- create a new unique cookie for PID,
3) PR_SCHED_CORE_SHARE_TO -- push cookie of the caller onto
another PID,
4) PR_SCHED_CORE_SHARE_FROM -- pull cookie of another PID into
the caller.
Since a system where the code is built can be different to the
one where the code is ran let's provide declaration of some
values. It's not unusual for distros to ship older linux-headers
than the actual kernel.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt_private.syms | 4 ++
src/util/virprocess.c | 124 +++++++++++++++++++++++++++++++++++++++
src/util/virprocess.h | 8 +++
3 files changed, 136 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 97bfca906b..252d7e029f 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -3129,6 +3129,10 @@ virProcessKillPainfullyDelay;
virProcessNamespaceAvailable;
virProcessRunInFork;
virProcessRunInMountNamespace;
+virProcessSchedCoreAvailable;
+virProcessSchedCoreCreate;
+virProcessSchedCoreShareFrom;
+virProcessSchedCoreShareTo;
virProcessSchedPolicyTypeFromString;
virProcessSchedPolicyTypeToString;
virProcessSetAffinity;
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 36d7df050a..cd4f3fc7e7 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -57,6 +57,10 @@
# include <windows.h>
#endif
+#if WITH_CAPNG
+# include <sys/prctl.h>
+#endif
+
#include "virprocess.h"
#include "virerror.h"
#include "viralloc.h"
@@ -1906,3 +1910,123 @@ virProcessGetSchedInfo(unsigned long long *cpuWait,
return 0;
}
#endif /* __linux__ */
+
+#ifdef __linux__
+# ifndef PR_SCHED_CORE
+/* Copied from linux/prctl.h */
+# define PR_SCHED_CORE 62
+# define PR_SCHED_CORE_GET 0
+# define PR_SCHED_CORE_CREATE 1 /* create unique core_sched cookie */
+# define PR_SCHED_CORE_SHARE_TO 2 /* push core_sched cookie to pid */
+# define PR_SCHED_CORE_SHARE_FROM 3 /* pull core_sched cookie to pid */
+# endif
+
+/* Unfortunately, kernel-headers forgot to export these. */
+# ifndef PR_SCHED_CORE_SCOPE_THREAD
+# define PR_SCHED_CORE_SCOPE_THREAD 0
+# define PR_SCHED_CORE_SCOPE_THREAD_GROUP 1
+# define PR_SCHED_CORE_SCOPE_PROCESS_GROUP 2
+# endif
+
+/**
+ * virProcessSchedCoreAvailable:
+ *
+ * Check whether kernel supports Core Scheduling (CONFIG_SCHED_CORE), i.e. only
+ * a defined set of PIDs/TIDs can run on sibling Hyper Threads at the same
+ * time.
+ *
+ * Returns: 1 if Core Scheduling is available,
+ * 0 if Core Scheduling is NOT available,
+ * -1 otherwise.
+ */
+int
+virProcessSchedCoreAvailable(void)
+{
+ unsigned long cookie = 0;
+ int rc;
+
+ /* Let's just see if we can get our own sched cookie, and if yes we can
+ * safely assume CONFIG_SCHED_CORE kernel is available. */
+ rc = prctl(PR_SCHED_CORE, PR_SCHED_CORE_GET, 0,
+ PR_SCHED_CORE_SCOPE_THREAD, &cookie);
+
+ return rc == 0 ? 1 : errno == EINVAL ? 0 : -1;
+}
+
+/**
+ * virProcessSchedCoreCreate:
+ *
+ * Creates a new trusted group for the caller process.
+ *
+ * Returns: 0 on success,
+ * -1 otherwise, with errno set.
+ */
+int
+virProcessSchedCoreCreate(void)
+{
+ /* pid = 0 (3rd argument) means the calling process. */
+ return prctl(PR_SCHED_CORE, PR_SCHED_CORE_CREATE, 0,
+ PR_SCHED_CORE_SCOPE_THREAD_GROUP, 0);
+}
+
+/**
+ * virProcessSchedCoreShareFrom:
+ * @pid: PID to share group with
+ *
+ * Places the current caller process into the trusted group of @pid.
+ *
+ * Returns: 0 on success,
+ * -1 otherwise, with errno set.
+ */
+int
+virProcessSchedCoreShareFrom(pid_t pid)
+{
+ return prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_FROM, pid,
+ PR_SCHED_CORE_SCOPE_THREAD, 0);
+}
+
+/**
+ * virProcessSchedCoreShareTo:
+ * @pid: PID to share group with
+ *
+ * Places foreign @pid into the trusted group of the current caller process.
+ *
+ * Returns: 0 on success,
+ * -1 otherwise, with errno set.
+ */
+int
+virProcessSchedCoreShareTo(pid_t pid)
+{
+ return prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, pid,
+ PR_SCHED_CORE_SCOPE_THREAD, 0);
+}
+
+#else /* !__linux__ */
+
+int
+virProcessSchedCoreAvailable(void)
+{
+ return 0;
+}
+
+int
+virProcessSchedCoreCreate(void)
+{
+ errno = ENOSYS;
+ return -1;
+}
+
+int
+virProcessSchedCoreShareFrom(pid_t pid G_GNUC_UNUSED)
+{
+ errno = ENOSYS;
+ return -1;
+}
+
+int
+virProcessSchedCoreShareTo(pid_t pid G_GNUC_UNUSED)
+{
+ errno = ENOSYS;
+ return -1;
+}
+#endif /* !__linux__ */
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 086fbe0e4d..e01f9a24ee 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -202,3 +202,11 @@ int virProcessGetStatInfo(unsigned long long *cpuTime,
int virProcessGetSchedInfo(unsigned long long *cpuWait,
pid_t pid,
pid_t tid);
+
+int virProcessSchedCoreAvailable(void);
+
+int virProcessSchedCoreCreate(void);
+
+int virProcessSchedCoreShareFrom(pid_t pid);
+
+int virProcessSchedCoreShareTo(pid_t pid);
--
2.35.1
noon
New subject: [PATCH RFC 06/10] virprocess: Core Scheduling support
On Mon, May 09, 2022 at 05:02:13PM +0200, Michal Privoznik wrote:
Since its 5.14 release the Linux kernel allows userspace to
define trusted groups of processes/threads that can run on
sibling Hyper Threads (HT) at the same time. This is to mitigate
side channel attacks like L1TF or MDS. If there are no tasks to
fully utilize all HTs, then a HT will idle instead of running a
task from another (un-)trusted group.
On low level, this is implemented by cookies (effectively an UL
value): processes in the same trusted group share the same cookie
and cookie is unique to the group. There are four basic
operations:
1) PR_SCHED_CORE_GET -- get cookie of given PID,
2) PR_SCHED_CORE_CREATE -- create a new unique cookie for PID,
3) PR_SCHED_CORE_SHARE_TO -- push cookie of the caller onto
another PID,
4) PR_SCHED_CORE_SHARE_FROM -- pull cookie of another PID into
the caller.
Since a system where the code is built can be different to the
one where the code is ran let's provide declaration of some
values. It's not unusual for distros to ship older linux-headers
than the actual kernel.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt_private.syms | 4 ++
src/util/virprocess.c | 124 +++++++++++++++++++++++++++++++++++++++
src/util/virprocess.h | 8 +++
3 files changed, 136 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
diff --git a/src/util/virprocess.c b/src/util/virprocess.c
index 36d7df050a..cd4f3fc7e7 100644
--- a/src/util/virprocess.c
+++ b/src/util/virprocess.c
@@ -57,6 +57,10 @@
# include <windows.h>
#endif
+#if WITH_CAPNG
This feels odd - what relation has CAPNG got with prctl ?
+# include <sys/prctl.h>
+#endif
+
#include "virprocess.h"
#include "virerror.h"
#include "viralloc.h"
@@ -1906,3 +1910,123 @@ virProcessGetSchedInfo(unsigned long long *cpuWait,
return 0;
}
#endif /* __linux__ */
+
+#ifdef __linux__
+# ifndef PR_SCHED_CORE
+/* Copied from linux/prctl.h */
+# define PR_SCHED_CORE 62
+# define PR_SCHED_CORE_GET 0
+# define PR_SCHED_CORE_CREATE 1 /* create unique core_sched cookie */
+# define PR_SCHED_CORE_SHARE_TO 2 /* push core_sched cookie to pid */
+# define PR_SCHED_CORE_SHARE_FROM 3 /* pull core_sched cookie to pid */
+# endif
+
+/* Unfortunately, kernel-headers forgot to export these. */
+# ifndef PR_SCHED_CORE_SCOPE_THREAD
+# define PR_SCHED_CORE_SCOPE_THREAD 0
+# define PR_SCHED_CORE_SCOPE_THREAD_GROUP 1
+# define PR_SCHED_CORE_SCOPE_PROCESS_GROUP 2
+# endif
+
+/**
+ * virProcessSchedCoreAvailable:
+ *
+ * Check whether kernel supports Core Scheduling (CONFIG_SCHED_CORE), i.e. only
+ * a defined set of PIDs/TIDs can run on sibling Hyper Threads at the same
+ * time.
+ *
+ * Returns: 1 if Core Scheduling is available,
+ * 0 if Core Scheduling is NOT available,
+ * -1 otherwise.
+ */
+int
+virProcessSchedCoreAvailable(void)
+{
+ unsigned long cookie = 0;
+ int rc;
+
+ /* Let's just see if we can get our own sched cookie, and if yes we can
+ * safely assume CONFIG_SCHED_CORE kernel is available. */
+ rc = prctl(PR_SCHED_CORE, PR_SCHED_CORE_GET, 0,
+ PR_SCHED_CORE_SCOPE_THREAD, &cookie);
+
+ return rc == 0 ? 1 : errno == EINVAL ? 0 : -1;
+}
+
+/**
+ * virProcessSchedCoreCreate:
+ *
+ * Creates a new trusted group for the caller process.
+ *
+ * Returns: 0 on success,
+ * -1 otherwise, with errno set.
+ */
+int
+virProcessSchedCoreCreate(void)
+{
+ /* pid = 0 (3rd argument) means the calling process. */
+ return prctl(PR_SCHED_CORE, PR_SCHED_CORE_CREATE, 0,
+ PR_SCHED_CORE_SCOPE_THREAD_GROUP, 0);
+}
+
+/**
+ * virProcessSchedCoreShareFrom:
+ * @pid: PID to share group with
+ *
+ * Places the current caller process into the trusted group of @pid.
+ *
+ * Returns: 0 on success,
+ * -1 otherwise, with errno set.
+ */
+int
+virProcessSchedCoreShareFrom(pid_t pid)
+{
+ return prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_FROM, pid,
+ PR_SCHED_CORE_SCOPE_THREAD, 0);
+}
+
+/**
+ * virProcessSchedCoreShareTo:
+ * @pid: PID to share group with
+ *
+ * Places foreign @pid into the trusted group of the current caller process.
+ *
+ * Returns: 0 on success,
+ * -1 otherwise, with errno set.
+ */
+int
+virProcessSchedCoreShareTo(pid_t pid)
+{
+ return prctl(PR_SCHED_CORE, PR_SCHED_CORE_SHARE_TO, pid,
+ PR_SCHED_CORE_SCOPE_THREAD, 0);
+}
+
+#else /* !__linux__ */
+
+int
+virProcessSchedCoreAvailable(void)
+{
+ return 0;
+}
+
+int
+virProcessSchedCoreCreate(void)
+{
+ errno = ENOSYS;
+ return -1;
+}
+
+int
+virProcessSchedCoreShareFrom(pid_t pid G_GNUC_UNUSED)
+{
+ errno = ENOSYS;
+ return -1;
+}
+
+int
+virProcessSchedCoreShareTo(pid_t pid G_GNUC_UNUSED)
+{
+ errno = ENOSYS;
+ return -1;
+}
+#endif /* !__linux__ */
diff --git a/src/util/virprocess.h b/src/util/virprocess.h
index 086fbe0e4d..e01f9a24ee 100644
--- a/src/util/virprocess.h
+++ b/src/util/virprocess.h
@@ -202,3 +202,11 @@ int virProcessGetStatInfo(unsigned long long *cpuTime,
int virProcessGetSchedInfo(unsigned long long *cpuWait,
pid_t pid,
pid_t tid);
+
+int virProcessSchedCoreAvailable(void);
+
+int virProcessSchedCoreCreate(void);
+
+int virProcessSchedCoreShareFrom(pid_t pid);
+
+int virProcessSchedCoreShareTo(pid_t pid);
--
2.35.1
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
4:50 a.m.
New subject: [PATCH RFC 06/10] virprocess: Core Scheduling support
On 5/23/22 19:00, Daniel P. Berrangé wrote:
On Mon, May 09, 2022 at 05:02:13PM +0200, Michal Privoznik wrote:
> Since its 5.14 release the Linux kernel allows userspace to
> define trusted groups of processes/threads that can run on
> sibling Hyper Threads (HT) at the same time. This is to mitigate
> side channel attacks like L1TF or MDS. If there are no tasks to
> fully utilize all HTs, then a HT will idle instead of running a
> task from another (un-)trusted group.
>
> On low level, this is implemented by cookies (effectively an UL
> value): processes in the same trusted group share the same cookie
> and cookie is unique to the group. There are four basic
> operations:
>
> 1) PR_SCHED_CORE_GET -- get cookie of given PID,
> 2) PR_SCHED_CORE_CREATE -- create a new unique cookie for PID,
> 3) PR_SCHED_CORE_SHARE_TO -- push cookie of the caller onto
> another PID,
> 4) PR_SCHED_CORE_SHARE_FROM -- pull cookie of another PID into
> the caller.
>
> Since a system where the code is built can be different to the
> one where the code is ran let's provide declaration of some
> values. It's not unusual for distros to ship older linux-headers
> than the actual kernel.
>
> Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
> ---
> src/libvirt_private.syms | 4 ++
> src/util/virprocess.c | 124 +++++++++++++++++++++++++++++++++++++++
> src/util/virprocess.h | 8 +++
> 3 files changed, 136 insertions(+)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
> diff --git a/src/util/virprocess.c b/src/util/virprocess.c
> index 36d7df050a..cd4f3fc7e7 100644
> --- a/src/util/virprocess.c
> +++ b/src/util/virprocess.c
> @@ -57,6 +57,10 @@
> # include <windows.h>
> #endif
>
> +#if WITH_CAPNG
This feels odd - what relation has CAPNG got with prctl ?
Nothing, it's a blind copy from virutil.c O:-)
Consider changed to #ifdef __linux__
Michal
10:02 a.m.
New subject: [PATCH RFC 07/10] virCommand: Introduce APIs for core scheduling
There are two modes of core scheduling that are handy wrt
virCommand:
1) create new trusted group when executing a virCommand
2) place freshly executed virCommand into the trusted group of
another process.
Therefore, implement these two new operations as new APIs:
virCommandSetRunAlone() and virCommandSetRunAmong(),
respectively.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt_private.syms | 2 ++
src/util/vircommand.c | 74 ++++++++++++++++++++++++++++++++++++++++
src/util/vircommand.h | 5 +++
3 files changed, 81 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 252d7e029f..8f2b789cee 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2079,6 +2079,8 @@ virCommandSetOutputBuffer;
virCommandSetOutputFD;
virCommandSetPidFile;
virCommandSetPreExecHook;
+virCommandSetRunAlone;
+virCommandSetRunAmong;
virCommandSetSELinuxLabel;
virCommandSetSendBuffer;
virCommandSetUID;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 41cf552d7b..db20620f7c 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -148,6 +148,9 @@ struct _virCommand {
#endif
int mask;
+ bool schedCore;
+ pid_t schedCorePID;
+
virCommandSendBuffer *sendBuffers;
size_t numSendBuffers;
};
@@ -434,6 +437,22 @@ virCommandHandshakeChild(virCommand *cmd)
static int
virExecCommon(virCommand *cmd, gid_t *groups, int ngroups)
{
+ /* Do this before dropping capabilities. */
+ if (cmd->schedCore &&
+ virProcessSchedCoreCreate() < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to set SCHED_CORE"));
+ return -1;
+ }
+
+ if (cmd->schedCorePID >= 0 &&
+ virProcessSchedCoreShareFrom(cmd->schedCorePID) < 0) {
+ virReportSystemError(errno,
+ _("Unable to run among %llu"),
+ (unsigned long long) cmd->schedCorePID);
+ return -1;
+ }
+
if (cmd->uid != (uid_t)-1 || cmd->gid != (gid_t)-1 ||
cmd->capabilities || (cmd->flags & VIR_EXEC_CLEAR_CAPS)) {
VIR_DEBUG("Setting child uid:gid to %d:%d with caps %llx",
@@ -964,6 +983,7 @@ virCommandNewArgs(const char *const*args)
cmd->pid = -1;
cmd->uid = -1;
cmd->gid = -1;
+ cmd->schedCorePID = -1;
virCommandAddArgSet(cmd, args);
@@ -3437,3 +3457,57 @@ virCommandRunNul(virCommand *cmd G_GNUC_UNUSED,
return -1;
}
#endif /* WIN32 */
+
+/**
+ * virCommandSetRunAlone:
+ *
+ * Create new trusted group when running the command. In other words, the
+ * process won't be scheduled to run on a core among with processes from
+ * another, untrusted group.
+ */
+void
+virCommandSetRunAlone(virCommand *cmd)
+{
+ if (virCommandHasError(cmd))
+ return;
+
+ if (cmd->schedCorePID >= 0) {
+ /* Can't mix these two. */
+ cmd->has_error = -1;
+ VIR_DEBUG("cannot mix with virCommandSetRunAmong()");
+ return;
+ }
+
+ cmd->schedCore = true;
+}
+
+/**
+ * virCommandSetRunAmong:
+ * @pid: pid from a trusted group
+ *
+ * When spawning the command place it into the trusted group of @pid so that
+ * these two processes can run on Hyper Threads of a single core at the same
+ * time.
+ */
+void
+virCommandSetRunAmong(virCommand *cmd,
+ pid_t pid)
+{
+ if (virCommandHasError(cmd))
+ return;
+
+ if (cmd->schedCore) {
+ /* Can't mix these two. */
+ VIR_DEBUG("cannot mix with virCommandSetRunAlone()");
+ cmd->has_error = -1;
+ return;
+ }
+
+ if (pid < 0) {
+ VIR_DEBUG("invalid pid value: %lld", (long long) pid);
+ cmd->has_error = -1;
+ return;
+ }
+
+ cmd->schedCorePID = pid;
+}
diff --git a/src/util/vircommand.h b/src/util/vircommand.h
index 600806a987..0b03ea005c 100644
--- a/src/util/vircommand.h
+++ b/src/util/vircommand.h
@@ -225,4 +225,9 @@ int virCommandRunNul(virCommand *cmd,
virCommandRunNulFunc func,
void *data);
+void virCommandSetRunAlone(virCommand *cmd);
+
+void virCommandSetRunAmong(virCommand *cmd,
+ pid_t pid);
+
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virCommand, virCommandFree);
--
2.35.1
11:45 a.m.
New subject: [PATCH RFC 07/10] virCommand: Introduce APIs for core scheduling
On Mon, May 09, 2022 at 05:02:14PM +0200, Michal Privoznik wrote:
There are two modes of core scheduling that are handy wrt
virCommand:
1) create new trusted group when executing a virCommand
2) place freshly executed virCommand into the trusted group of
another process.
Therefore, implement these two new operations as new APIs:
virCommandSetRunAlone() and virCommandSetRunAmong(),
respectively.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/libvirt_private.syms | 2 ++
src/util/vircommand.c | 74 ++++++++++++++++++++++++++++++++++++++++
src/util/vircommand.h | 5 +++
3 files changed, 81 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 252d7e029f..8f2b789cee 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -2079,6 +2079,8 @@ virCommandSetOutputBuffer;
virCommandSetOutputFD;
virCommandSetPidFile;
virCommandSetPreExecHook;
+virCommandSetRunAlone;
+virCommandSetRunAmong;
virCommandSetSELinuxLabel;
virCommandSetSendBuffer;
virCommandSetUID;
diff --git a/src/util/vircommand.c b/src/util/vircommand.c
index 41cf552d7b..db20620f7c 100644
--- a/src/util/vircommand.c
+++ b/src/util/vircommand.c
@@ -148,6 +148,9 @@ struct _virCommand {
#endif
int mask;
+ bool schedCore;
+ pid_t schedCorePID;
+
virCommandSendBuffer *sendBuffers;
size_t numSendBuffers;
};
@@ -434,6 +437,22 @@ virCommandHandshakeChild(virCommand *cmd)
static int
virExecCommon(virCommand *cmd, gid_t *groups, int ngroups)
{
+ /* Do this before dropping capabilities. */
+ if (cmd->schedCore &&
+ virProcessSchedCoreCreate() < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to set SCHED_CORE"));
+ return -1;
+ }
+
+ if (cmd->schedCorePID >= 0 &&
+ virProcessSchedCoreShareFrom(cmd->schedCorePID) < 0) {
+ virReportSystemError(errno,
+ _("Unable to run among %llu"),
+ (unsigned long long) cmd->schedCorePID);
+ return -1;
+ }
+
if (cmd->uid != (uid_t)-1 || cmd->gid != (gid_t)-1 ||
cmd->capabilities || (cmd->flags & VIR_EXEC_CLEAR_CAPS)) {
VIR_DEBUG("Setting child uid:gid to %d:%d with caps %llx",
@@ -964,6 +983,7 @@ virCommandNewArgs(const char *const*args)
cmd->pid = -1;
cmd->uid = -1;
cmd->gid = -1;
+ cmd->schedCorePID = -1;
virCommandAddArgSet(cmd, args);
@@ -3437,3 +3457,57 @@ virCommandRunNul(virCommand *cmd G_GNUC_UNUSED,
return -1;
}
#endif /* WIN32 */
+
+/**
+ * virCommandSetRunAlone:
+ *
+ * Create new trusted group when running the command. In other words, the
+ * process won't be scheduled to run on a core among with processes from
+ * another, untrusted group.
+ */
+void
+virCommandSetRunAlone(virCommand *cmd)
+{
+ if (virCommandHasError(cmd))
+ return;
+
+ if (cmd->schedCorePID >= 0) {
+ /* Can't mix these two. */
+ cmd->has_error = -1;
+ VIR_DEBUG("cannot mix with virCommandSetRunAmong()");
+ return;
+ }
+
+ cmd->schedCore = true;
+}
+
+/**
+ * virCommandSetRunAmong:
+ * @pid: pid from a trusted group
+ *
+ * When spawning the command place it into the trusted group of @pid so that
+ * these two processes can run on Hyper Threads of a single core at the same
+ * time.
+ */
+void
+virCommandSetRunAmong(virCommand *cmd,
+ pid_t pid)
+{
+ if (virCommandHasError(cmd))
+ return;
+
+ if (cmd->schedCore) {
+ /* Can't mix these two. */
+ VIR_DEBUG("cannot mix with virCommandSetRunAlone()");
+ cmd->has_error = -1;
+ return;
+ }
+
+ if (pid < 0) {
+ VIR_DEBUG("invalid pid value: %lld", (long long) pid);
+ cmd->has_error = -1;
+ return;
+ }
+
+ cmd->schedCorePID = pid;
+}
It strikes me that we can handle this with only 1 variable and
thus avoid the possibility of mutually incompatible settings.
PID == 0 is not a valid PID for sharing a group with so we
can have
pid_t schedCore;
with
0 == no core scheduling
1 == don't be silly we shouldn't ever copy 'init's PID :-)
1 == copy scheduling group from said PID
-1 == create a new
scheduling group
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:02 a.m.
New subject: [PATCH RFC 08/10] qemu_conf: Introduce a knob to turn off SCHED_CORE
Ideally, we would just pick the best default and users wouldn't
have to intervene at all. But in some cases it may be handy to
not bother with SCHED_CORE at all and thus let users turn the
feature off in qemu.conf.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/libvirtd_qemu.aug | 1 +
src/qemu/qemu.conf.in | 5 +++++
src/qemu/qemu_conf.c | 24 ++++++++++++++++++++++++
src/qemu/qemu_conf.h | 2 ++
src/qemu/test_libvirtd_qemu.aug.in | 1 +
5 files changed, 33 insertions(+)
diff --git a/src/qemu/libvirtd_qemu.aug b/src/qemu/libvirtd_qemu.aug
index 0f18775121..28a8db2b43 100644
--- a/src/qemu/libvirtd_qemu.aug
+++ b/src/qemu/libvirtd_qemu.aug
@@ -110,6 +110,7 @@ module Libvirtd_qemu =
| bool_entry "dump_guest_core"
| str_entry "stdio_handler"
| int_entry "max_threads_per_process"
+ | bool_entry "sched_core"
let device_entry = bool_entry "mac_filter"
| bool_entry "relaxed_acs_check"
diff --git a/src/qemu/qemu.conf.in b/src/qemu/qemu.conf.in
index 04b7740136..ece822edc3 100644
--- a/src/qemu/qemu.conf.in
+++ b/src/qemu/qemu.conf.in
@@ -952,3 +952,8 @@
# DO NOT use in production.
#
#deprecation_behavior = "none"
+
+# If this is set then QEMU and its threads will run with SCHED_CORE set,
+# meaning no other foreign process will share Hyper Threads of a single core
+# with QEMU nor with any of its helper process.
+#sched_core = 1
diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c
index c22cf79cbe..03d8da0157 100644
--- a/src/qemu/qemu_conf.c
+++ b/src/qemu/qemu_conf.c
@@ -286,6 +286,8 @@ virQEMUDriverConfig *virQEMUDriverConfigNew(bool privileged,
cfg->deprecationBehavior = g_strdup("none");
+ cfg->schedCore = virProcessSchedCoreAvailable() == 1;
+
return g_steal_pointer(&cfg);
}
@@ -634,6 +636,8 @@ virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfig *cfg,
g_auto(GStrv) hugetlbfs = NULL;
g_autofree char *stdioHandler = NULL;
g_autofree char *corestr = NULL;
+ bool schedCore;
+ int rc;
size_t i;
if (virConfGetValueStringList(conf, "hugetlbfs_mount", true,
@@ -711,6 +715,26 @@ virQEMUDriverConfigLoadProcessEntry(virQEMUDriverConfig *cfg,
}
}
+ if ((rc = virConfGetValueBool(conf, "sched_core", &schedCore)) < 0)
{
+ return -1;
+ } else if (rc > 0) {
+ if (schedCore) {
+ int rv = virProcessSchedCoreAvailable();
+
+ if (rv < 0) {
+ virReportSystemError(errno, "%s",
+ _("Unable to detect SCHED_CORE"));
+ return -1;
+ } else if (rv == 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+ _("SCHED_CORE not supported by kernel"));
+ return -1;
+ }
+ }
+
+ cfg->schedCore = schedCore;
+ }
+
return 0;
}
diff --git a/src/qemu/qemu_conf.h b/src/qemu/qemu_conf.h
index c71a666aea..32899859c0 100644
--- a/src/qemu/qemu_conf.h
+++ b/src/qemu/qemu_conf.h
@@ -223,6 +223,8 @@ struct _virQEMUDriverConfig {
char **capabilityfilters;
char *deprecationBehavior;
+
+ bool schedCore;
};
G_DEFINE_AUTOPTR_CLEANUP_FUNC(virQEMUDriverConfig, virObjectUnref);
diff --git a/src/qemu/test_libvirtd_qemu.aug.in b/src/qemu/test_libvirtd_qemu.aug.in
index 757d21c33f..9f3f98d524 100644
--- a/src/qemu/test_libvirtd_qemu.aug.in
+++ b/src/qemu/test_libvirtd_qemu.aug.in
@@ -116,3 +116,4 @@ module Test_libvirtd_qemu =
{ "1" = "capname" }
}
{ "deprecation_behavior" = "none" }
+{ "sched_core" = "1" }
--
2.35.1
10:02 a.m.
New subject: [PATCH RFC 09/10] qemu: Enable SCHED_CORE for domains and helper processes
Despite all mitigations, side channel attacks when two processes
run at two Hyper Threads of the same core are still possible.
Fortunately, the Linux kernel came up with a solution: userspace
can create so called trusted groups, which are sets of processes
and only processes of the same group can run on sibling Hyper
Threads. Of course, two processes of different groups can run on
different cores, because there's no known side channel attack.
It's only Hyper Threads that are affected.
Having said that, it's a clear security win for users when
enabled for QEMU.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_process.c | 5 +++++
src/qemu/qemu_security.c | 4 ++++
src/qemu/qemu_virtiofs.c | 3 +++
3 files changed, 12 insertions(+)
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b0b00eb0a2..0a49008124 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -2923,6 +2923,9 @@ qemuProcessStartManagedPRDaemon(virDomainObj *vm)
* qemu (so that it shares the same view of the system). */
virCommandSetPreExecHook(cmd, qemuProcessStartPRDaemonHook, vm);
+ if (cfg->schedCore && vm->pid != -1)
+ virCommandRunAmong(cmd, vm->pid);
+
if (virCommandRun(cmd, NULL) < 0)
goto cleanup;
@@ -7472,6 +7475,8 @@ qemuProcessLaunch(virConnectPtr conn,
virCommandSetMaxProcesses(cmd, cfg->maxProcesses);
if (cfg->maxFiles > 0)
virCommandSetMaxFiles(cmd, cfg->maxFiles);
+ if (cfg->schedCore)
+ virCommandRunAlone(cmd);
/* In this case, however, zero means that core dumps should be
* disabled, and so we always need to set the limit explicitly */
diff --git a/src/qemu/qemu_security.c b/src/qemu/qemu_security.c
index 3be1766764..0fe1555406 100644
--- a/src/qemu/qemu_security.c
+++ b/src/qemu/qemu_security.c
@@ -683,6 +683,8 @@ qemuSecurityCommandRun(virQEMUDriver *driver,
int *exitstatus,
int *cmdret)
{
+ g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+
if (virSecurityManagerSetChildProcessLabel(driver->securityManager,
vm->def, cmd) < 0)
return -1;
@@ -691,6 +693,8 @@ qemuSecurityCommandRun(virQEMUDriver *driver,
virCommandSetUID(cmd, uid);
if (gid != (gid_t) -1)
virCommandSetGID(cmd, gid);
+ if (cfg->schedCore && vm->pid != -1)
+ virCommandRunAmong(cmd, vm->pid);
if (virSecurityManagerPreFork(driver->securityManager) < 0)
return -1;
diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index b3a2d2990a..0a3548065f 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -248,6 +248,9 @@ qemuVirtioFSStart(virQEMUDriver *driver,
virCommandNonblockingFDs(cmd);
virCommandDaemonize(cmd);
+ if (cfg->schedCore && vm->pid != -1)
+ virCommandRunAmong(cmd, vm->pid);
+
if (qemuExtDeviceLogCommand(driver, vm, cmd, "virtiofsd") < 0)
goto error;
--
2.35.1
10:02 a.m.
New subject: [PATCH RFC 10/10] qemu: Place helper processes into the same trusted group
Since the level of trust that QEMU has is the same level of trust
that helper processes have there's no harm in placing all of them
into the same group.
Unfortunately, since these processes are started before QEMU we
can't use brand new virCommand*() APIs (those are used on hotplug
though) and have to use the low level virProcess*() APIs.
Moreover, because there no (kernel) API that would copy cookie
from one process to another WITHOUT modifying the cookie of the
process that's doing the copy, we have to fork() and use
available copy APIs.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/qemu/qemu_extdevice.c | 120 ++++++++++++++++++++++++++++++++++++++
src/qemu/qemu_extdevice.h | 3 +
src/qemu/qemu_process.c | 4 ++
3 files changed, 127 insertions(+)
diff --git a/src/qemu/qemu_extdevice.c b/src/qemu/qemu_extdevice.c
index 234815c075..611ea8d640 100644
--- a/src/qemu/qemu_extdevice.c
+++ b/src/qemu/qemu_extdevice.c
@@ -337,3 +337,123 @@ qemuExtDevicesSetupCgroup(virQEMUDriver *driver,
return 0;
}
+
+
+static int
+qemuExtDevicesSetupSchedHelper(pid_t ppid G_GNUC_UNUSED,
+ void *opaque)
+{
+ GSList *pids = opaque;
+ GSList *next;
+ pid_t vmPid;
+
+ /* The first item on the list is special: it's the PID of the
+ * QEMU that has the cookie we want to copy to the rest. */
+ vmPid = GPOINTER_TO_INT(pids->data);
+ if (virProcessSchedCoreShareFrom(vmPid) < 0) {
+ virReportSystemError(errno,
+ _("Unable to get core group of: %lld"),
+ (long long) vmPid);
+ return -1;
+ }
+
+ VIR_DEBUG("SCHED_CORE: vmPid = %lld", (long long) vmPid);
+
+ for (next = pids->next; next; next = next->next) {
+ pid_t pid = GPOINTER_TO_INT(next->data);
+
+ VIR_DEBUG("SCHED_CORE: share to %lld", (long long) pid);
+ if (virProcessSchedCoreShareTo(pid) < 0) {
+ virReportSystemError(errno,
+ _("Unable to share core group to: %lld"),
+ (long long) pid);
+ return -1;
+ }
+ }
+
+ return 0;
+}
+
+
+int
+qemuExtDevicesSetupSched(virQEMUDriver *driver,
+ virDomainObj *vm)
+{
+ g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
+ virDomainDef *def = vm->def;
+ g_autofree char *shortname = NULL;
+ g_autoptr(GSList) pids = NULL;
+ size_t i;
+ pid_t cpid = -1;
+
+ if (cfg->schedCore == false)
+ return 0;
+
+ shortname = virDomainDefGetShortName(def);
+ if (!shortname)
+ return -1;
+
+ if (qemuDBusGetPID(driver, vm, &cpid) < 0)
+ return -1;
+
+ if (cpid != -1)
+ pids = g_slist_prepend(pids, GINT_TO_POINTER(cpid));
+
+ for (i = 0; i < def->nvideos; i++) {
+ virDomainVideoDef *video = def->videos[i];
+
+ if (video->backend != VIR_DOMAIN_VIDEO_BACKEND_TYPE_VHOSTUSER)
+ continue;
+
+ if (qemuVhostUserGPUGetPid(cfg->stateDir, shortname, video->info.alias,
&cpid) < 0)
+ return -1;
+
+ if (cpid != -1)
+ pids = g_slist_prepend(pids, GINT_TO_POINTER(cpid));
+ }
+
+ for (i = 0; i < def->nnets; i++) {
+ virDomainNetDef *net = def->nets[i];
+ qemuSlirp *slirp = QEMU_DOMAIN_NETWORK_PRIVATE(net)->slirp;
+
+ if (slirp && slirp->pid != -1)
+ pids = g_slist_prepend(pids, GINT_TO_POINTER(slirp->pid));
+ }
+
+ for (i = 0; i < def->ntpms; i++) {
+ virDomainTPMDef *tpm = def->tpms[i];
+
+ if (tpm->type != VIR_DOMAIN_TPM_TYPE_EMULATOR)
+ continue;
+
+ if (qemuTPMEmulatorGetPid(cfg->swtpmStateDir, shortname, &cpid) < 0)
+ return -1;
+
+ if (cpid != -1)
+ pids = g_slist_prepend(pids, GINT_TO_POINTER(cpid));
+ }
+
+ for (i = 0; i < def->nfss; i++) {
+ virDomainFSDef *fs = def->fss[i];
+
+ if (fs->sock ||
+ fs->fsdriver != VIR_DOMAIN_FS_DRIVER_TYPE_VIRTIOFS)
+ continue;
+
+ if (qemuVirtioFSGetPid(vm, fs, &cpid) < 0)
+ return -1;
+
+ if (cpid != -1)
+ pids = g_slist_prepend(pids, GINT_TO_POINTER(cpid));
+ }
+
+ /* Exit early if there's nothing to do, to avoid needless fork. */
+ if (!pids)
+ return 0;
+
+ pids = g_slist_prepend(pids, GINT_TO_POINTER(vm->pid));
+
+ /* Unfortunately, there's no better way of copying scheduling
+ * cookies than fork(). */
+ return virProcessRunInFork(qemuExtDevicesSetupSchedHelper, pids);
+}
diff --git a/src/qemu/qemu_extdevice.h b/src/qemu/qemu_extdevice.h
index 43d2a4dfff..02397adc6c 100644
--- a/src/qemu/qemu_extdevice.h
+++ b/src/qemu/qemu_extdevice.h
@@ -59,3 +59,6 @@ bool qemuExtDevicesHasDevice(virDomainDef *def);
int qemuExtDevicesSetupCgroup(virQEMUDriver *driver,
virDomainObj *vm,
virCgroup *cgroup);
+
+int qemuExtDevicesSetupSched(virQEMUDriver *driver,
+ virDomainObj *vm);
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 0a49008124..515f12fb41 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -7552,6 +7552,10 @@ qemuProcessLaunch(virConnectPtr conn,
if (qemuSetupCgroupForExtDevices(vm, driver) < 0)
goto cleanup;
+ VIR_DEBUG("Setting SCHED_CORE for external devices (if required)");
+ if (qemuExtDevicesSetupSched(driver, vm) < 0)
+ goto cleanup;
+
VIR_DEBUG("Setting up resctrl");
if (qemuProcessResctrlCreate(driver, vm) < 0)
goto cleanup;
--
2.35.1
11:30 a.m.
New subject: [PATCH RFC 10/10] qemu: Place helper processes into the same trusted group
On Mon, May 09, 2022 at 05:02:17PM +0200, Michal Privoznik wrote:
Since the level of trust that QEMU has is the same level of trust
that helper processes have there's no harm in placing all of them
into the same group.
This assumption feels like it might be a bit of a stretch. I
recall discussing this with Paolo to some extent a long time
back, but let me recap my understanding.
IIUC, the attack scenario is that a guest vCPU thread is scheduled
on a SMT sibling with another thread that is NOT running guest OS
code. "another thread" in this context refers to many things
- Random host OS processes
- QEMU vCPU threads from a different geust
- QEMU emulator threads from any guest
- QEMU helper process threads from any guest
Consider for example, if the QEMU emulator thread contains a password
used for logging into a remote RBD/Ceph server. That is a secret
credential that the guest OS should not have permission to access.
Consider alternatively that the QEMU emulator is making a TLS connection
to some service, and there are keys negotiated for the TLS session. While
some of the data transmitted over the session is known to the guest OS,
we shouldn't assume it all is.
Now in the case of QEMU emulator threads I think you can make a somewhat
decent case that we don't have to worry about it. Most of the keys/passwds
are used once at cold boot, so there's no attack window for vCPUs at that
point. There is a small window of risk when hotplugging. If someone is
really concerned about this though, they shouldn't have let QEMU have
these credentials in the first place, as its already vulnerable to a
guest escape. eg use kernel RBD instead of letting QEMU directly login
to RBD.
IOW, on balance of probabilities it is reasonable to let QEMU emulator
threads be in the same core scheduling domain as vCPU threads.
In the case of external QEMU helper processes though, I think it is
a far less clearcut decision. There are a number of reasons why helper
processes are used, but at least one significant motivating factor is
security isolation between QEMU & the helper - they can only communicate
and share information through certain controlled mechanisms.
With this in mind I think it is risky to assume that it is safe to
run QEMU and helper processes in the same core scheduling group. At
the same time there are likely cases where it is also just fine to
do so.
If we separate helper processes from QEMU vCPUs this is not as wasteful
as it sounds. Some the helper processes are running trusted code, there
is no need for helper processes from different guests to be isolated.
They can all just live in the default core scheduling domain.
I feel like I'm talking myself into suggesting the core scheduling
host knob in qemu.conf needs to be more than just a single boolean.
Either have two knobs - one to turn it on/off and one to control
whether helpers are split or combined - or have one knob and make
it an enumeration.
One possible complication comes if we consider a guest that is
pinned, but not on the fine grained per-vCPU basis.
eg if guest is set to allow floating over a sub-set of host CPUs
we need to make sure that it is possible to actually execute the
guest still. ie if entire guest is pinned to 1 host CPU but our
config implies use of 2 distinct core scheduling domains, we have
an unsolvable constraint.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
4:50 a.m.
New subject: [PATCH RFC 10/10] qemu: Place helper processes into the same trusted group
On 5/23/22 18:30, Daniel P. Berrangé wrote:
On Mon, May 09, 2022 at 05:02:17PM +0200, Michal Privoznik wrote:
> Since the level of trust that QEMU has is the same level of trust
> that helper processes have there's no harm in placing all of them
> into the same group.
This assumption feels like it might be a bit of a stretch. I
recall discussing this with Paolo to some extent a long time
back, but let me recap my understanding.
IIUC, the attack scenario is that a guest vCPU thread is scheduled
on a SMT sibling with another thread that is NOT running guest OS
code. "another thread" in this context refers to many things
- Random host OS processes
- QEMU vCPU threads from a different geust
- QEMU emulator threads from any guest
- QEMU helper process threads from any guest
Consider for example, if the QEMU emulator thread contains a password
used for logging into a remote RBD/Ceph server. That is a secret
credential that the guest OS should not have permission to access.
Consider alternatively that the QEMU emulator is making a TLS connection
to some service, and there are keys negotiated for the TLS session. While
some of the data transmitted over the session is known to the guest OS,
we shouldn't assume it all is.
Now in the case of QEMU emulator threads I think you can make a somewhat
decent case that we don't have to worry about it. Most of the keys/passwds
are used once at cold boot, so there's no attack window for vCPUs at that
point. There is a small window of risk when hotplugging. If someone is
really concerned about this though, they shouldn't have let QEMU have
these credentials in the first place, as its already vulnerable to a
guest escape. eg use kernel RBD instead of letting QEMU directly login
to RBD.
IOW, on balance of probabilities it is reasonable to let QEMU emulator
threads be in the same core scheduling domain as vCPU threads.
In the case of external QEMU helper processes though, I think it is
a far less clearcut decision. There are a number of reasons why helper
processes are used, but at least one significant motivating factor is
security isolation between QEMU & the helper - they can only communicate
and share information through certain controlled mechanisms.
With this in mind I think it is risky to assume that it is safe to
run QEMU and helper processes in the same core scheduling group. At
the same time there are likely cases where it is also just fine to
do so.
If we separate helper processes from QEMU vCPUs this is not as wasteful
as it sounds. Some the helper processes are running trusted code, there
is no need for helper processes from different guests to be isolated.
They can all just live in the default core scheduling domain.
I feel like I'm talking myself into suggesting the core scheduling
host knob in qemu.conf needs to be more than just a single boolean.
Either have two knobs - one to turn it on/off and one to control
whether helpers are split or combined - or have one knob and make
it an enumeration.
Seems reasonable. And the default should be QEMU's emulator + vCPU
threads in one sched group, and all helper processes in another, right?
One possible complication comes if we consider a guest that is
pinned, but not on the fine grained per-vCPU basis.
eg if guest is set to allow floating over a sub-set of host CPUs
we need to make sure that it is possible to actually execute the
guest still. ie if entire guest is pinned to 1 host CPU but our
config implies use of 2 distinct core scheduling domains, we have
an unsolvable constraint.
Do we? Since we're placing emulator + vCPUs into one group and helper
processes into another these would never run at the same time, but that
would be the case anyways - if emulator write()-s into a helper's socket
it would be blocked because the helper isn't running. This "bottleneck"
is result of pinning everything onto a single CPU and exists regardless
of scheduling groups.
The only case where scheduling groups would make the bottleneck worse is
if emulator and vCPUs were in different groups, but we don't intent to
allow that.
Michal
5:33 a.m.
New subject: [PATCH RFC 10/10] qemu: Place helper processes into the same trusted group
On Tue, May 24, 2022 at 11:50:50AM +0200, Michal Prívozník wrote:
On 5/23/22 18:30, Daniel P. Berrangé wrote:
> On Mon, May 09, 2022 at 05:02:17PM +0200, Michal Privoznik wrote:
>> Since the level of trust that QEMU has is the same level of trust
>> that helper processes have there's no harm in placing all of them
>> into the same group.
>
> This assumption feels like it might be a bit of a stretch. I
> recall discussing this with Paolo to some extent a long time
> back, but let me recap my understanding.
>
> IIUC, the attack scenario is that a guest vCPU thread is scheduled
> on a SMT sibling with another thread that is NOT running guest OS
> code. "another thread" in this context refers to many things
>
> - Random host OS processes
> - QEMU vCPU threads from a different geust
> - QEMU emulator threads from any guest
> - QEMU helper process threads from any guest
>
> Consider for example, if the QEMU emulator thread contains a password
> used for logging into a remote RBD/Ceph server. That is a secret
> credential that the guest OS should not have permission to access.
>
> Consider alternatively that the QEMU emulator is making a TLS connection
> to some service, and there are keys negotiated for the TLS session. While
> some of the data transmitted over the session is known to the guest OS,
> we shouldn't assume it all is.
>
> Now in the case of QEMU emulator threads I think you can make a somewhat
> decent case that we don't have to worry about it. Most of the keys/passwds
> are used once at cold boot, so there's no attack window for vCPUs at that
> point. There is a small window of risk when hotplugging. If someone is
> really concerned about this though, they shouldn't have let QEMU have
> these credentials in the first place, as its already vulnerable to a
> guest escape. eg use kernel RBD instead of letting QEMU directly login
> to RBD.
>
> IOW, on balance of probabilities it is reasonable to let QEMU emulator
> threads be in the same core scheduling domain as vCPU threads.
>
> In the case of external QEMU helper processes though, I think it is
> a far less clearcut decision. There are a number of reasons why helper
> processes are used, but at least one significant motivating factor is
> security isolation between QEMU & the helper - they can only communicate
> and share information through certain controlled mechanisms.
>
> With this in mind I think it is risky to assume that it is safe to
> run QEMU and helper processes in the same core scheduling group. At
> the same time there are likely cases where it is also just fine to
> do so.
>
> If we separate helper processes from QEMU vCPUs this is not as wasteful
> as it sounds. Some the helper processes are running trusted code, there
> is no need for helper processes from different guests to be isolated.
> They can all just live in the default core scheduling domain.
>
> I feel like I'm talking myself into suggesting the core scheduling
> host knob in qemu.conf needs to be more than just a single boolean.
> Either have two knobs - one to turn it on/off and one to control
> whether helpers are split or combined - or have one knob and make
> it an enumeration.
Seems reasonable. And the default should be QEMU's emulator + vCPU
threads in one sched group, and all helper processes in another, right?
Not quite. I'm suggesting that helper processes can remain in the
host's default core scheduling group, since the helpers are all
executing trusted machine code.
> One possible complication comes if we consider a guest that is
> pinned, but not on the fine grained per-vCPU basis.
>
> eg if guest is set to allow floating over a sub-set of host CPUs
> we need to make sure that it is possible to actually execute the
> guest still. ie if entire guest is pinned to 1 host CPU but our
> config implies use of 2 distinct core scheduling domains, we have
> an unsolvable constraint.
Do we? Since we're placing emulator + vCPUs into one group and helper
processes into another these would never run at the same time, but that
would be the case anyways - if emulator write()-s into a helper's socket
it would be blocked because the helper isn't running. This "bottleneck"
is result of pinning everything onto a single CPU and exists regardless
of scheduling groups.
The only case where scheduling groups would make the bottleneck worse is
if emulator and vCPUs were in different groups, but we don't intent to
allow that.
Do we actually pin the helper processes at all ?
I was thinking of a scenario where we implicitly pin helper processes to
the same CPUs as the emulator threads and/or QEMU process-global pinning
mask. eg
If we only had
<vcpu placement='static' cpuset="2-3"
current="1">2</vcpu>
Traditionally the emulator threads, i/o threads, vCPU threads will
all float across host CPUs 2 & 3. I was assuming we also placed
helper processes in these same 2 host CPUs. Not sure if that's right
or not. Assuming we do, then...
Lets say CPUs 2 & 3 are SMT siblings.
We have helper processes in the default core scheduling
domain and QEMU in a dedicated core scheduling domain. We
loose 100% of concurrency between the vCPUs and helper
processes.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:35 a.m.
New subject: [PATCH RFC 10/10] qemu: Place helper processes into the same trusted group
On 5/24/22 12:33, Daniel P. Berrangé wrote:
On Tue, May 24, 2022 at 11:50:50AM +0200, Michal Prívozník wrote:
> On 5/23/22 18:30, Daniel P. Berrangé wrote:
>> On Mon, May 09, 2022 at 05:02:17PM +0200, Michal Privoznik wrote:
>>> Since the level of trust that QEMU has is the same level of trust
>>> that helper processes have there's no harm in placing all of them
>>> into the same group.
>>
>> This assumption feels like it might be a bit of a stretch. I
>> recall discussing this with Paolo to some extent a long time
>> back, but let me recap my understanding.
>>
>> IIUC, the attack scenario is that a guest vCPU thread is scheduled
>> on a SMT sibling with another thread that is NOT running guest OS
>> code. "another thread" in this context refers to many things
>>
>> - Random host OS processes
>> - QEMU vCPU threads from a different geust
>> - QEMU emulator threads from any guest
>> - QEMU helper process threads from any guest
>>
>> Consider for example, if the QEMU emulator thread contains a password
>> used for logging into a remote RBD/Ceph server. That is a secret
>> credential that the guest OS should not have permission to access.
>>
>> Consider alternatively that the QEMU emulator is making a TLS connection
>> to some service, and there are keys negotiated for the TLS session. While
>> some of the data transmitted over the session is known to the guest OS,
>> we shouldn't assume it all is.
>>
>> Now in the case of QEMU emulator threads I think you can make a somewhat
>> decent case that we don't have to worry about it. Most of the keys/passwds
>> are used once at cold boot, so there's no attack window for vCPUs at that
>> point. There is a small window of risk when hotplugging. If someone is
>> really concerned about this though, they shouldn't have let QEMU have
>> these credentials in the first place, as its already vulnerable to a
>> guest escape. eg use kernel RBD instead of letting QEMU directly login
>> to RBD.
>>
>> IOW, on balance of probabilities it is reasonable to let QEMU emulator
>> threads be in the same core scheduling domain as vCPU threads.
>>
>> In the case of external QEMU helper processes though, I think it is
>> a far less clearcut decision. There are a number of reasons why helper
>> processes are used, but at least one significant motivating factor is
>> security isolation between QEMU & the helper - they can only communicate
>> and share information through certain controlled mechanisms.
>>
>> With this in mind I think it is risky to assume that it is safe to
>> run QEMU and helper processes in the same core scheduling group. At
>> the same time there are likely cases where it is also just fine to
>> do so.
>>
>> If we separate helper processes from QEMU vCPUs this is not as wasteful
>> as it sounds. Some the helper processes are running trusted code, there
>> is no need for helper processes from different guests to be isolated.
>> They can all just live in the default core scheduling domain.
>>
>> I feel like I'm talking myself into suggesting the core scheduling
>> host knob in qemu.conf needs to be more than just a single boolean.
>> Either have two knobs - one to turn it on/off and one to control
>> whether helpers are split or combined - or have one knob and make
>> it an enumeration.
>
> Seems reasonable. And the default should be QEMU's emulator + vCPU
> threads in one sched group, and all helper processes in another, right?
Not quite. I'm suggesting that helper processes can remain in the
host's default core scheduling group, since the helpers are all
executing trusted machine code.
>> One possible complication comes if we consider a guest that is
>> pinned, but not on the fine grained per-vCPU basis.
>>
>> eg if guest is set to allow floating over a sub-set of host CPUs
>> we need to make sure that it is possible to actually execute the
>> guest still. ie if entire guest is pinned to 1 host CPU but our
>> config implies use of 2 distinct core scheduling domains, we have
>> an unsolvable constraint.
>
> Do we? Since we're placing emulator + vCPUs into one group and helper
> processes into another these would never run at the same time, but that
> would be the case anyways - if emulator write()-s into a helper's socket
> it would be blocked because the helper isn't running. This
"bottleneck"
> is result of pinning everything onto a single CPU and exists regardless
> of scheduling groups.
>
> The only case where scheduling groups would make the bottleneck worse is
> if emulator and vCPUs were in different groups, but we don't intent to
> allow that.
Do we actually pin the helper processes at all ?
Yes, we do. Into the same CGroup as emulator thread:
qemuSetupCgroupForExtDevices().
I was thinking of a scenario where we implicitly pin helper processes to
the same CPUs as the emulator threads and/or QEMU process-global pinning
mask. eg
If we only had
<vcpu placement='static' cpuset="2-3"
current="1">2</vcpu>
Traditionally the emulator threads, i/o threads, vCPU threads will
all float across host CPUs 2 & 3. I was assuming we also placed
helper processes in these same 2 host CPUs. Not sure if that's right
or not. Assuming we do, then...
Lets say CPUs 2 & 3 are SMT siblings.
We have helper processes in the default core scheduling
domain and QEMU in a dedicated core scheduling domain. We
loose 100% of concurrency between the vCPUs and helper
processes.
So in this case users might want to have helpers and emulator in the
same group. Therefore, in qemu.conf we should allow something like:
sched_core = "none" // off, no SCHED_CORE
"emulator" // default, place only emulator & vCPU threads
// into the group
"helpers" // place emulator & vCPU & helpers into the
// group
I agree that "helpers" is terrible name, maybe "emulator+helpers"? Or
something completely different? Maybe:
sched_core = [] // off
["emulator"] // enumlator & vCPU threads
["emulator","helpers"] // emulator + helpers
We can refine "helpers" in future (if needed) to say "virtiofsd",
"dbus", "swtpm" allowing users to fine tune what helper processes are
part of the group.
Michal
11:29 a.m.
New subject: [PATCH RFC 10/10] qemu: Place helper processes into the same trusted group
On Tue, May 24, 2022 at 05:35:03PM +0200, Michal Prívozník wrote:
On 5/24/22 12:33, Daniel P. Berrangé wrote:
> On Tue, May 24, 2022 at 11:50:50AM +0200, Michal Prívozník wrote:
>> On 5/23/22 18:30, Daniel P. Berrangé wrote:
>>> On Mon, May 09, 2022 at 05:02:17PM +0200, Michal Privoznik wrote:
>>>> Since the level of trust that QEMU has is the same level of trust
>>>> that helper processes have there's no harm in placing all of them
>>>> into the same group.
>>>
>>> This assumption feels like it might be a bit of a stretch. I
>>> recall discussing this with Paolo to some extent a long time
>>> back, but let me recap my understanding.
>>>
>>> IIUC, the attack scenario is that a guest vCPU thread is scheduled
>>> on a SMT sibling with another thread that is NOT running guest OS
>>> code. "another thread" in this context refers to many things
>>>
>>> - Random host OS processes
>>> - QEMU vCPU threads from a different geust
>>> - QEMU emulator threads from any guest
>>> - QEMU helper process threads from any guest
>>>
>>> Consider for example, if the QEMU emulator thread contains a password
>>> used for logging into a remote RBD/Ceph server. That is a secret
>>> credential that the guest OS should not have permission to access.
>>>
>>> Consider alternatively that the QEMU emulator is making a TLS connection
>>> to some service, and there are keys negotiated for the TLS session. While
>>> some of the data transmitted over the session is known to the guest OS,
>>> we shouldn't assume it all is.
>>>
>>> Now in the case of QEMU emulator threads I think you can make a somewhat
>>> decent case that we don't have to worry about it. Most of the
keys/passwds
>>> are used once at cold boot, so there's no attack window for vCPUs at
that
>>> point. There is a small window of risk when hotplugging. If someone is
>>> really concerned about this though, they shouldn't have let QEMU have
>>> these credentials in the first place, as its already vulnerable to a
>>> guest escape. eg use kernel RBD instead of letting QEMU directly login
>>> to RBD.
>>>
>>> IOW, on balance of probabilities it is reasonable to let QEMU emulator
>>> threads be in the same core scheduling domain as vCPU threads.
>>>
>>> In the case of external QEMU helper processes though, I think it is
>>> a far less clearcut decision. There are a number of reasons why helper
>>> processes are used, but at least one significant motivating factor is
>>> security isolation between QEMU & the helper - they can only
communicate
>>> and share information through certain controlled mechanisms.
>>>
>>> With this in mind I think it is risky to assume that it is safe to
>>> run QEMU and helper processes in the same core scheduling group. At
>>> the same time there are likely cases where it is also just fine to
>>> do so.
>>>
>>> If we separate helper processes from QEMU vCPUs this is not as wasteful
>>> as it sounds. Some the helper processes are running trusted code, there
>>> is no need for helper processes from different guests to be isolated.
>>> They can all just live in the default core scheduling domain.
>>>
>>> I feel like I'm talking myself into suggesting the core scheduling
>>> host knob in qemu.conf needs to be more than just a single boolean.
>>> Either have two knobs - one to turn it on/off and one to control
>>> whether helpers are split or combined - or have one knob and make
>>> it an enumeration.
>>
>> Seems reasonable. And the default should be QEMU's emulator + vCPU
>> threads in one sched group, and all helper processes in another, right?
>
> Not quite. I'm suggesting that helper processes can remain in the
> host's default core scheduling group, since the helpers are all
> executing trusted machine code.
>
>>> One possible complication comes if we consider a guest that is
>>> pinned, but not on the fine grained per-vCPU basis.
>>>
>>> eg if guest is set to allow floating over a sub-set of host CPUs
>>> we need to make sure that it is possible to actually execute the
>>> guest still. ie if entire guest is pinned to 1 host CPU but our
>>> config implies use of 2 distinct core scheduling domains, we have
>>> an unsolvable constraint.
>>
>> Do we? Since we're placing emulator + vCPUs into one group and helper
>> processes into another these would never run at the same time, but that
>> would be the case anyways - if emulator write()-s into a helper's socket
>> it would be blocked because the helper isn't running. This
"bottleneck"
>> is result of pinning everything onto a single CPU and exists regardless
>> of scheduling groups.
>>
>> The only case where scheduling groups would make the bottleneck worse is
>> if emulator and vCPUs were in different groups, but we don't intent to
>> allow that.
>
> Do we actually pin the helper processes at all ?
Yes, we do. Into the same CGroup as emulator thread:
qemuSetupCgroupForExtDevices().
>
> I was thinking of a scenario where we implicitly pin helper processes to
> the same CPUs as the emulator threads and/or QEMU process-global pinning
> mask. eg
>
> If we only had
>
> <vcpu placement='static' cpuset="2-3"
current="1">2</vcpu>
>
> Traditionally the emulator threads, i/o threads, vCPU threads will
> all float across host CPUs 2 & 3. I was assuming we also placed
> helper processes in these same 2 host CPUs. Not sure if that's right
> or not. Assuming we do, then...
>
> Lets say CPUs 2 & 3 are SMT siblings.
>
> We have helper processes in the default core scheduling
> domain and QEMU in a dedicated core scheduling domain. We
> loose 100% of concurrency between the vCPUs and helper
> processes.
So in this case users might want to have helpers and emulator in the
same group. Therefore, in qemu.conf we should allow something like:
sched_core = "none" // off, no SCHED_CORE
"emulator" // default, place only emulator & vCPU threads
// into the group
"helpers" // place emulator & vCPU & helpers into the
// group
I agree that "helpers" is terrible name, maybe "emulator+helpers"?
Or
something completely different? Maybe:
A scalar is nice, but we can just call it "full" or "all", as in
the opposite of "none".
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
11:13 a.m.
On Mon, May 09, 2022 at 05:02:07PM +0200, Michal Privoznik wrote:
The Linux kernel offers a way to mitigate side channel attacks on
Hyper
Threads (e.g. MDS and L1TF). Long story short, userspace can define
groups of processes (aka trusted groups) and only processes within one
group can run on sibling Hyper Threads. The group membership is
automatically preserved on fork() and exec().
Now, there is one scenario which I don't cover in my series and I'd like
to hear proposal: if there are two guests with odd number of vCPUs they
can no longer run on sibling Hyper Threads because my patches create
separate group for each QEMU. This is a performance penalty. Ideally, we
would have a knob inside domain XML that would place two or more domains
into the same trusted group. But since there's pre-existing example (of
sharing a piece of information between two domains) I've failed to come
up with something usable.
Right now users have two choices
- Run with SMT enabled. 100% of CPUs available. VMs are vulnerable
- Run with SMT disabled. 50% of CPUs available. VMs are safe
What the core scheduling gives is somewhere inbetween, depending on
the vCPU count. If we assume all guests have even CPUs then
- Run with SMT enabled + core scheduling. 100% of CPUs available.
100% of CPUs are used, VMs are safe
This is the ideal scenario, and probably the fairly common scenario
too as IMHO even number CPU counts are likely to be typical.
If we assume the worst case, of entirely 1 vCPU guests then we have
- Run with SMT enabled + core scheduling. 100% of CPUs available.
50% of CPUs are used, VMs are safe
This feels highly unlikely though, as all except tiny workloads
want > 1 vCPU.
With entirely 3 vCPU guests then we have
- Run with SMT enabled + core scheduling. 100% of CPUs available.
75% of CPUs are used, VMs are safe
With entirely 5 vCPU guests then we have
- Run with SMT enabled + core scheduling. 100% of CPUs available.
83% of CPUs are used, VMs are safe
If we have a mix of even and odd numbered vCPU guests, with mostly
even numbered, then I think utilization will be high enough that
almost no one will care about the last few %.
While we could try to come up with a way to express sharing of
cores between VMs I don't think its worth it, in the absence of
someone presenting compelling data why it'll be needed in a non
niche use case. Bear in mind, that users can also resort to
pinning VMs explicitly to get sharing.
In terms of defaults I'd very much like us to default to enabling
core scheduling, so that we have a secure deployment out of the box.
The only caveat is that this does have the potential to be interpreted
as a regression for existing deployments in some cases. Perhaps we
should make it a meson option for distros to decide whether to ship
with it turned on out of the box or not ?
I don't think we need core scheduling to be a VM XML config option,
because security is really a host level matter IMHO, such that it
does't make sense to have both secure & insecure VMs co-located.
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
4:50 a.m.
On 5/23/22 18:13, Daniel P. Berrangé wrote:
On Mon, May 09, 2022 at 05:02:07PM +0200, Michal Privoznik wrote:
> The Linux kernel offers a way to mitigate side channel attacks on Hyper
> Threads (e.g. MDS and L1TF). Long story short, userspace can define
> groups of processes (aka trusted groups) and only processes within one
> group can run on sibling Hyper Threads. The group membership is
> automatically preserved on fork() and exec().
>
> Now, there is one scenario which I don't cover in my series and I'd like
> to hear proposal: if there are two guests with odd number of vCPUs they
> can no longer run on sibling Hyper Threads because my patches create
> separate group for each QEMU. This is a performance penalty. Ideally, we
> would have a knob inside domain XML that would place two or more domains
> into the same trusted group. But since there's pre-existing example (of
> sharing a piece of information between two domains) I've failed to come
> up with something usable.
Right now users have two choices
- Run with SMT enabled. 100% of CPUs available. VMs are vulnerable
- Run with SMT disabled. 50% of CPUs available. VMs are safe
What the core scheduling gives is somewhere inbetween, depending on
the vCPU count. If we assume all guests have even CPUs then
- Run with SMT enabled + core scheduling. 100% of CPUs available.
100% of CPUs are used, VMs are safe
This is the ideal scenario, and probably the fairly common scenario
too as IMHO even number CPU counts are likely to be typical.
If we assume the worst case, of entirely 1 vCPU guests then we have
- Run with SMT enabled + core scheduling. 100% of CPUs available.
50% of CPUs are used, VMs are safe
This feels highly unlikely though, as all except tiny workloads
want > 1 vCPU.
With entirely 3 vCPU guests then we have
- Run with SMT enabled + core scheduling. 100% of CPUs available.
75% of CPUs are used, VMs are safe
With entirely 5 vCPU guests then we have
- Run with SMT enabled + core scheduling. 100% of CPUs available.
83% of CPUs are used, VMs are safe
If we have a mix of even and odd numbered vCPU guests, with mostly
even numbered, then I think utilization will be high enough that
almost no one will care about the last few %.
While we could try to come up with a way to express sharing of
cores between VMs I don't think its worth it, in the absence of
someone presenting compelling data why it'll be needed in a non
niche use case. Bear in mind, that users can also resort to
pinning VMs explicitly to get sharing.
In terms of defaults I'd very much like us to default to enabling
core scheduling, so that we have a secure deployment out of the box.
The only caveat is that this does have the potential to be interpreted
as a regression for existing deployments in some cases. Perhaps we
should make it a meson option for distros to decide whether to ship
with it turned on out of the box or not ?
Alternatively, distros can just patch qemu_conf.c which enables the
option in cfg (virQEMUDriverConfigNew()).
Michal
7:01 a.m.
On Mon, 2022-05-23 at 17:13 +0100, Daniel P. Berrangé wrote:
On Mon, May 09, 2022 at 05:02:07PM +0200, Michal Privoznik wrote:
In terms of defaults I'd very much like us to default to enabling
core scheduling, so that we have a secure deployment out of the box.
The only caveat is that this does have the potential to be
interpreted
as a regression for existing deployments in some cases. Perhaps we
should make it a meson option for distros to decide whether to ship
with it turned on out of the box or not ?
I think, as Michal said, with the qemu.conf knob from patch 8, we will
already have that. I.e., distros will ship a qemu.conf with sched_core
equal to 1 or 0, depending on what they want as a default behavior.
I don't think we need core scheduling to be a VM XML config
option,
because security is really a host level matter IMHO, such that it
does't make sense to have both secure & insecure VMs co-located.
Mmm... I can't say that I have any concrete example, but I guess I can
picture a situation where someone has "sensitive" VMs, which he/she
would want to make sure they're protected from the possibility that
other VMs steal their secrets, and "less sensitive" ones, for which
it's not a concern if they share cores and (potentially) steal secrets
among each others (as far as none of those can steal from any
"sensitive" one, but this does not happen, if we set core scheduling
for the latter).
Another scenario would be if core-scheduling is (ab)used for limiting
the interference. Like some sort of flexible and dynamic form of vcpu-
pinning. That is, if I set core-scheduling for VM1, I'm sure that VM1's
vcpus will never share cores with any other VMs. Which is good for
performance and determinism, because it means that it can't happen that
vcpu1 of VM3 runs on the same core of vcpu0 of VM1 and, when VM3-vcpu1
is busy, VM1-vcpu0 slows down as well. Imagine that VM1 and VM3 are
owned by different customers, core-scheduling would allow me to make
sure that whatever customer A is doing in VM3, it can't slow down
customer B, who owns VM1, without having to resort to do vcpu-pinning,
which is unflexible and. And again, maybe we do want this "dynamic
interference shielding" property for some VMs, but not for all... E.g.,
we can offer it as an higher SLA, and ask more money for a VM that has
it.
Thoughts?
In any case, even if we decide that we do want per-VM core-scheduling,
e.g., for the above mentioned reasons, I guess it can come later, as a
further improvement (and I'd be happy to help making it happen).
Regards
--
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)
9 a.m.
On Thu, 2022-05-26 at 14:01 +0200, Dario Faggioli wrote:
Thoughts?
Oh, and there are even a couple of other (potential) use case, for
having an (even more!) fine grained control of core-scheduling.
So, right now, giving a virtual topology to a VM, pretty much only
makes sense if the VM has its vcpus pinned. Well, actually, there's
something that we can do even if that is not the case, especially if we
define at least *some* constraints on where the vcpus can run, even if
we don't have strict and static 1-to-1 pinning... But for sure we
shouldn't define an SMT topology, if we don't have that (i.e., if we
don't have strict and static 1-to-1 pinning). And yet, the vcpus will
run on cores and threads!
Now, if we implement per-vcpu core-scheduling (which means being able
to put not necessarily whole VMs, but single vcpus [although, of the
same VM], in trusted groups), then we can:
- put vcpu0 and vcpu1 of VM1 in a group
- put vcpu2 and vcpu3 of VM1 in a(nother!) group
- define, in the virtual topology of VM1, vcpu0 and vcpu1 as
SMT-threads of the same core
- define, in the virtual topology of VM1, vcpu2 and vcpu3 as
SMT-threads of the same core
From the perspective of the accuracy of the mapping between virtual and
physical topology (and hence, most likely, of performance), it's still
a mixed bag. I.e., on an idle or lightly loaded system, vcpu0 and vcpu1
can still run on two different cores. So, if the guest kernel and apps
assume that the two vcpus are SMT-siblings, and optimize for that,
well, that still might be false/wrong (like it would be without any
core-scheduling, without any pinning, etc). At least, when they run on
different cores, they run there alone, which is nice (but achievable
with per-VM core-scheduling already).
On a heavily loaded system, instead, vcpu0 and vcpu1 should (when they
both want to run) have much higher chances of actually ending up
running on the same core. [Of couse, not necessarily always on one same
specific core --like when we do pinning-- but always on the one core.]
So, in-guest workloads operating under the assumption that those two
vcpus are SMT-siblings, will hopefully benefit from that.
And for the lightly loaded case, well, I believe that combining
per-vcpu core-scheduling + SMT virtual topology with *some* kind of
vcpu affinity (and I mean something more flexible and less wasteful
than 1-to-1 pinning, of course!) and/or with something like numad, will
actually bring some performance and determinism benefits, even in such
a scenario... But, of course, we need data for that, and I don't have
any yet. :-)
Anyway, let's now consider the case where the user/customer wants to be
able use core-scheduling _inside_ of the guest, e.g., for protecting
and/or shielding, some sensitive workload that he/she is running inside
of the VM itself, from all the other tasks. But for using core-
scheduling inside of the guest we need the guest to have cores and
threads. And for the protection/shielding to be effective, we need to
be sure that, say, if two guest tasks are in the same trusted group and
are running on two vcpus that are virtual SMT-siblings, these two vcpus
either (1) run on two actual physical SMT-siblings pCPUs on the host
(i.e., on they run on the same core), or (2) run on different host
cores, each one on a thread, with no other vCPU from any other VM (and
no host task, for what matters) running on the other thread. And this
is exactly what per-vcpu core-scheduling + SMT virtual topology gives
us. :-D
Of course, as in the previous message, I think that it's perfectly fine
for something like this to not be implemented immediately, and come
later. At least as far as we don't do anything at this stage that will
prevent/make it difficult to implement such extensions in future.
Which I guess is, after all, the main point of these very long emails
(sorry!) that I am writing. I.e., _if_ we agree that it might be
interesting to have per-VM, or even per-vcpu, core-scheduling in the
future, let's just try to make sure that what we put together now
(especially at the interface level) is easy to extend in that
direction. :-)
Thanks and Regards
--
Dario Faggioli, Ph.D
http://about.me/dario.faggioli
Virtualization Software Engineer
SUSE Labs, SUSE https://www.suse.com/
-------------------------------------------------------------------
<<This happens because _I_ choose it to happen!>> (Raistlin Majere)
4:47 a.m.
On 5/26/22 16:00, Dario Faggioli wrote:
On Thu, 2022-05-26 at 14:01 +0200, Dario Faggioli wrote:
> Thoughts?
>
Oh, and there are even a couple of other (potential) use case, for
having an (even more!) fine grained control of core-scheduling.
So, right now, giving a virtual topology to a VM, pretty much only
makes sense if the VM has its vcpus pinned. Well, actually, there's
something that we can do even if that is not the case, especially if we
define at least *some* constraints on where the vcpus can run, even if
we don't have strict and static 1-to-1 pinning... But for sure we
shouldn't define an SMT topology, if we don't have that (i.e., if we
don't have strict and static 1-to-1 pinning). And yet, the vcpus will
run on cores and threads!
Now, if we implement per-vcpu core-scheduling (which means being able
to put not necessarily whole VMs, but single vcpus [although, of the
same VM], in trusted groups), then we can:
- put vcpu0 and vcpu1 of VM1 in a group
- put vcpu2 and vcpu3 of VM1 in a(nother!) group
- define, in the virtual topology of VM1, vcpu0 and vcpu1 as
SMT-threads of the same core
- define, in the virtual topology of VM1, vcpu2 and vcpu3 as
SMT-threads of the same core
These last two we can't do ourselves really. It has to come from domain
definition. Otherwise we might break guest ABI, because unless
configured in domain XML all vCPUs are different cores (e.g.
<vcpu>4</vcpu> gives you a 4 core vCPU).
What we could do is to utilize cpu topology, regardless of pinning. I
mean, for the following config:
<vcpu>4</vcpu>
<cpu>
<topology sockets='1' dies='1' cores='2'
threads='2'/>
</cpu>
which gives you two threads in two cores. Now, we could place threads of
one core into one group, and the other two threads of the other core
into another group.
Ideally, I'd like to avoid computing an intersection with pinning
because that will get hairy pretty quickly (as you demonstrated in this
e-mail). For properly pinned vCPUs this won't be any performance penalty
(yes, it's still possible to come up with an artificial counter
example), and for "misconfigured" pinning, well tough luck.
Michal
4:47 a.m.
On 5/26/22 14:01, Dario Faggioli wrote:
On Mon, 2022-05-23 at 17:13 +0100, Daniel P. Berrangé wrote:
> On Mon, May 09, 2022 at 05:02:07PM +0200, Michal Privoznik wrote:
> In terms of defaults I'd very much like us to default to enabling
> core scheduling, so that we have a secure deployment out of the box.
> The only caveat is that this does have the potential to be
> interpreted
> as a regression for existing deployments in some cases. Perhaps we
> should make it a meson option for distros to decide whether to ship
> with it turned on out of the box or not ?
>
I think, as Michal said, with the qemu.conf knob from patch 8, we will
already have that. I.e., distros will ship a qemu.conf with sched_core
equal to 1 or 0, depending on what they want as a default behavior.
> I don't think we need core scheduling to be a VM XML config option,
> because security is really a host level matter IMHO, such that it
> does't make sense to have both secure & insecure VMs co-located.
>
Mmm... I can't say that I have any concrete example, but I guess I can
picture a situation where someone has "sensitive" VMs, which he/she
would want to make sure they're protected from the possibility that
other VMs steal their secrets, and "less sensitive" ones, for which
it's not a concern if they share cores and (potentially) steal secrets
among each others (as far as none of those can steal from any
"sensitive" one, but this does not happen, if we set core scheduling
for the latter).
Another scenario would be if core-scheduling is (ab)used for limiting
the interference. Like some sort of flexible and dynamic form of vcpu-
pinning. That is, if I set core-scheduling for VM1, I'm sure that VM1's
vcpus will never share cores with any other VMs. Which is good for
performance and determinism, because it means that it can't happen that
vcpu1 of VM3 runs on the same core of vcpu0 of VM1 and, when VM3-vcpu1
is busy, VM1-vcpu0 slows down as well. Imagine that VM1 and VM3 are
owned by different customers, core-scheduling would allow me to make
sure that whatever customer A is doing in VM3, it can't slow down
customer B, who owns VM1, without having to resort to do vcpu-pinning,
which is unflexible and. And again, maybe we do want this "dynamic
interference shielding" property for some VMs, but not for all... E.g.,
we can offer it as an higher SLA, and ask more money for a VM that has
it.
Thoughts?
I'd expect the host scheduler to work around this problem, e.g. it could
run vCPUs of different VMs on different cores. Of course, this assumes
that they are allowed to run on different cores (i.e. they are not
pinned onto the same physical CPU). And if they are then that's
obviously a misconfig on admin side.
Michal
906
days inactive
928
days old
32 comments
4 participants
participants (4)
-
Daniel P. Berrangé -
Dario Faggioli -
Michal Privoznik -
Michal Prívozník