On a Friday in 2020, Daniel P. Berrangé wrote:
On Fri, Jun 19, 2020 at 01:56:55PM +0200, Ján Tomko wrote:On a Friday in 2020, Daniel P. Berrangé wrote: > The concept we're really testing for is whether QEMU supports > the seccomp syscall filter groups. We need to keep one place > using the old term to deal with upgrades from existing hosts > with running VMs. > > Signed-off-by: Daniel P. Berrangé <berra...@redhat.com> > --- > src/qemu/qemu.conf | 2 +- > src/qemu/qemu_capabilities.c | 4 ++-- > src/qemu/qemu_capabilities.h | 2 +- > src/qemu/qemu_command.c | 4 ++-- > src/qemu/qemu_domain.c | 10 +++++++--- > tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 2 +- > tests/qemucapabilitiesdata/caps_2.11.0.x86_64.xml | 2 +- > tests/qemucapabilitiesdata/caps_2.12.0.aarch64.xml | 2 +- > tests/qemucapabilitiesdata/caps_2.12.0.ppc64.xml | 2 +- > tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 2 +- > tests/qemucapabilitiesdata/caps_2.12.0.x86_64.xml | 2 +- > tests/qemucapabilitiesdata/caps_3.0.0.ppc64.xml | 2 +- > tests/qemucapabilitiesdata/caps_3.0.0.riscv32.xml | 2 +- > tests/qemucapabilitiesdata/caps_3.0.0.riscv64.xml | 2 +- > tests/qemucapabilitiesdata/caps_3.0.0.s390x.xml | 2 +- > tests/qemucapabilitiesdata/caps_3.0.0.x86_64.xml | 2 +- > tests/qemucapabilitiesdata/caps_3.1.0.ppc64.xml | 2 +- > tests/qemucapabilitiesdata/caps_3.1.0.x86_64.xml | 2 +- > tests/qemucapabilitiesdata/caps_4.0.0.aarch64.xml | 2 +- > tests/qemucapabilitiesdata/caps_4.0.0.ppc64.xml | 2 +- > tests/qemucapabilitiesdata/caps_4.0.0.riscv32.xml | 2 +- > tests/qemucapabilitiesdata/caps_4.0.0.riscv64.xml | 2 +- > tests/qemucapabilitiesdata/caps_4.0.0.s390x.xml | 2 +- > tests/qemucapabilitiesdata/caps_4.0.0.x86_64.xml | 2 +- > tests/qemucapabilitiesdata/caps_4.1.0.x86_64.xml | 2 +- > tests/qemucapabilitiesdata/caps_4.2.0.aarch64.xml | 2 +- > tests/qemucapabilitiesdata/caps_4.2.0.ppc64.xml | 2 +- > tests/qemucapabilitiesdata/caps_4.2.0.s390x.xml | 2 +- > tests/qemucapabilitiesdata/caps_4.2.0.x86_64.xml | 2 +- > tests/qemucapabilitiesdata/caps_5.0.0.aarch64.xml | 2 +- > tests/qemucapabilitiesdata/caps_5.0.0.ppc64.xml | 2 +- > tests/qemucapabilitiesdata/caps_5.0.0.riscv64.xml | 2 +- > tests/qemucapabilitiesdata/caps_5.0.0.x86_64.xml | 2 +- > tests/qemucapabilitiesdata/caps_5.1.0.x86_64.xml | 2 +- > tests/qemustatusxml2xmldata/backup-pull-in.xml | 2 +- > tests/qemustatusxml2xmldata/blockjob-blockdev-in.xml | 2 +- > tests/qemuxml2argvtest.c | 2 +- > 37 files changed, 45 insertions(+), 41 deletions(-) > > diff --git a/src/qemu/qemu.conf b/src/qemu/qemu.conf > index f89dbd2c3a..99b9ce53e5 100644 > --- a/src/qemu/qemu.conf > +++ b/src/qemu/qemu.conf > @@ -704,7 +704,7 @@ > # If it is unset (or -1), then seccomp will be enabled > # only if QEMU >= 2.11.0 is detected, otherwise it is > # left disabled. This ensures the default config gets > -# protection for new QEMU using the blacklist approach. > +# protection for new QEMU with filter groups. > # > #seccomp_sandbox = 1 > > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c > index 68fcbd3c4f..310be800e2 100644 > --- a/src/qemu/qemu_capabilities.c > +++ b/src/qemu/qemu_capabilities.c > @@ -468,7 +468,7 @@ VIR_ENUM_IMPL(virQEMUCaps, > /* 285 */ > "qcow2-luks", > "pcie-pci-bridge", > - "seccomp-blacklist", > + "seccomp-filter-groups", > "query-cpus-fast", > "disk-write-cache", > > @@ -3292,7 +3292,7 @@ static struct virQEMUCapsCommandLineProps virQEMUCapsCommandLine[] = { > { "vnc", "vnc", QEMU_CAPS_VNC_MULTI_SERVERS }, > { "chardev", "reconnect", QEMU_CAPS_CHARDEV_RECONNECT }, > { "sandbox", "enable", QEMU_CAPS_SECCOMP_SANDBOX }, > - { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_BLACKLIST }, > + { "sandbox", "elevateprivileges", QEMU_CAPS_SECCOMP_FILTER_GROUPS }, > { "chardev", "fd", QEMU_CAPS_CHARDEV_FD_PASS }, > { "overcommit", NULL, QEMU_CAPS_OVERCOMMIT }, > { "smp-opts", "dies", QEMU_CAPS_SMP_DIES }, > diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h > index ad93816d41..0ee3e357cb 100644 > --- a/src/qemu/qemu_capabilities.h > +++ b/src/qemu/qemu_capabilities.h > @@ -448,7 +448,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ > /* 285 */ > QEMU_CAPS_QCOW2_LUKS, /* qcow2 format support LUKS encryption */ > QEMU_CAPS_DEVICE_PCIE_PCI_BRIDGE, /* -device pcie-pci-bridge */ > - QEMU_CAPS_SECCOMP_BLACKLIST, /* -sandbox.elevateprivileges */ > + QEMU_CAPS_SECCOMP_FILTER_GROUPS, /* -sandbox.elevateprivileges */ > QEMU_CAPS_QUERY_CPUS_FAST, /* query-cpus-fast command */ > QEMU_CAPS_DISK_WRITE_CACHE, /* qemu block frontends support write-cache param */ > > diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c > index f27246b4c6..37113a433a 100644 > --- a/src/qemu/qemu_command.c > +++ b/src/qemu/qemu_command.c > @@ -9517,8 +9517,8 @@ qemuBuildSeccompSandboxCommandLine(virCommandPtr cmd, > return 0; > } > > - /* Use blacklist by default if supported */ > - if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SECCOMP_BLACKLIST)) { > + /* Block undesirable syscall groups by default if supported */ > + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_SECCOMP_FILTER_GROUPS)) {While 'filter groups' describes the underlying QEMU functionality better, we only use it to deny syscalls. So using 'blocklist' as proposed in the RFC you linked would better show the contrast between this and the old approach.I don't want to name it based on libvirt's /current/ usage, as we could alter that usage in future, hence naming it based on the QEMU conceptual feature.> virCommandAddArgList(cmd, "-sandbox", > "on,obsolete=deny,elevateprivileges=deny," > "spawn=deny,resourcecontrol=deny", > diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c > index 72874ee4fd..56ec5c0352 100644 > --- a/src/qemu/qemu_domain.c > +++ b/src/qemu/qemu_domain.c > @@ -3851,9 +3851,13 @@ qemuDomainObjPrivateXMLParse(xmlXPathContextPtr ctxt, > if (str) { > int flag = virQEMUCapsTypeFromString(str); > if (flag < 0) { > - virReportError(VIR_ERR_INTERNAL_ERROR, > - _("Unknown qemu capabilities flag %s"), str); > - goto error; > + if (g_str_equal(str, "seccomp-blacklist")) { > + flag = QEMU_CAPS_SECCOMP_FILTER_GROUPS; I'd just leave the XML as-is, to avoid introducing this special-casing.Renaming the capability lets us eliminate this from all the capabilities test data files we have (and the ones we cointinue to add in future), so I think it is a net win to just have this 2 line special case.
Sigh, Jano
signature.asc
Description: PGP signature
