src/security/apparmor/usr.sbin.libvirtd.in | 1 1 + 0 - 0 !
src/security/apparmor/usr.sbin.virtqemud.in | 1 1 + 0 - 0 !
2 files changed, 2 insertions(+)

 apparmor: allow umount(/dev)

Commit 379c0ce4bfed introduced a call to umount(/dev) performed
inside the namespace that we run QEMU in.

As a result of this, on machines using AppArmor, VM startup now
fails with

  internal error: Process exited prior to exec: libvirt:
  QEMU Driver error: failed to umount devfs on /dev: Permission denied

The corresponding denial is

  AVC apparmor="DENIED" operation="umount" profile="libvirtd"
      name="/dev/" pid=70036 comm="rpc-libvirtd"

Extend the AppArmor configuration for virtqemud and libvirtd so
that this operation is allowed.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>

src/qemu/qemu_interface.c | 5 3 + 2 - 0 !
1 file changed, 3 insertions(+), 2 deletions(-)

 qemu_interface: fix managed='no' case when creating an ethernet
 interface

In a recent commit of v9.0.0-rc1~192 I've tried to forbid case
where a TAP device already exists, but at the same time it's
managed by Libvirt (<interface type='ethernet'> <target
dev='tap0' managed='yes'/> </interface>). NB, if @managed
attribute is missing then it's assumed to be managed by Libvirt.

Anyway, I've mistakenly put setting of
VIR_NETDEV_TAP_CREATE_ALLOW_EXISTING flag into managed='yes'
branch instead of managed='no' branch in
qemuInterfaceEthernetConnect().

Move the setting of the flag into the correct branch.

Fixes: a2ae3d299cf9c5ada8aa42ec4271748eb479dc27
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

src/qemu/qemu_domain.c | 3 2 + 1 - 0 !
1 file changed, 2 insertions(+), 1 deletion(-)

 qemu_domain: don't unref null hash table in
 qemuDomainRefreshStatsSchema()
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

The g_hash_table_unref() function does not accept NULL. Passing
NULL results in a glib warning being triggered. Check whether the
hash table is not NULL and unref it only then.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

src/qemu/qemu_namespace.c | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 qemu: jump to cleanup label on umount failure

Similar to other error paths in qemuDomainUnshareNamespace(), jump to
the cleanup label on umount error instead of directly returning -1.

Signed-off-by: Jim Fehlig <jfehlig@suse.com>

src/qemu/qemu_namespace.c | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 qemu_namespace: deal with nested mounts when umount()-ing /dev

In one of recent commits (v9.0.0-rc1~106) I've made our QEMU
namespace code umount the original /dev. One of the reasons was
enhanced security, because previously we just mounted a tmpfs
over the original /dev. Thus a malicious QEMU could just
umount("/dev") and it would get to the original /dev with all
nodes.

Now, on some systems this introduced a regression:

   failed to umount devfs on /dev: Device or resource busy

But how this could be? We've moved all file systems mounted under
/dev to a temporary location. Or have we? As it turns out, not
quite. If there are two file systems mounted on the same target,
e.g. like this:

  mount -t tmpfs tmpfs /dev/shm/ && mount -t tmpfs tmpfs /dev/shm/

then only the top most (i.e. the last one) is moved. See
qemuDomainUnshareNamespace() for more info.

Now, we could enhance our code to deal with these "doubled" mount
points. Or, since it is the top most file system that is
accessible anyways (and this one is preserved), we can
umount("/dev") in a recursive fashion.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2167302
Fixes: 379c0ce4bfed8733dfbde557c359eecc5474ce38
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

src/qemu/qemu_process.c | 11 5 + 6 - 0 !
1 file changed, 5 insertions(+), 6 deletions(-)

 qemuprocessrefreshdisks: don't skip filling of disk information if
 tray state didn't change

Commit 5ef2582646eb98 added emitting of even when refreshign disk state,
where it wanted to avoid sending the event if disk state didn't change.
This was achieved by using 'continue' in the loop filling the
information. Unfortunately this skips extraction of whether the device
has a tray which is propagated into internal structures, which in turn
broke cdrom media change as the code thought there's no tray for the
device.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2166411
Fixes: 5ef2582646eb98af208ce37355f82bdef39931fa
Signed-off-by: Peter Krempa <pkrempa@redhat.com>

src/qemu/qemu_extdevice.c | 5 4 + 1 - 0 !
1 file changed, 4 insertions(+), 1 deletion(-)

 qemu_extdevice: do cleanup host only for
 VIR_DOMAIN_TPM_TYPE_EMULATOR
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

We only set up host for VIR_DOMAIN_TPM_TYPE_EMULATOR and thus
similarly, we should do cleanup for the same type. This also
fixes a crasher, in which qemuTPMEmulatorCleanupHost() accesses
tpm->data.emulator.storagepath which is NULL for
VIR_DOMAIN_TPM_TYPE_EXTERNAL.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2168762
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

src/qemu/qemu_block.c | 1 1 + 0 - 0 !
src/qemu/qemu_blockjob.c | 19 10 + 9 - 0 !
src/qemu/qemu_blockjob.h | 4 4 + 0 - 0 !
3 files changed, 15 insertions(+), 9 deletions(-)

 qemu: blockjob: handle 'pending' blockjob state only when we need it

The 'pending' state needs to be handled by the blockjob code only when
the snapshot code requests a block-commit without auto-finalization.

If we always handle it we fail to properly remove the blockjob data for
the 'blockdev-create' job as that also transitions trhough 'pending' but
we'd never update it once it reaches 'concluded' as the code already
thinks that the job has finished and is no longer watching it.

Introduce a 'processPending' property into block job data and set it
only when we know that we need to process 'pending'.

Fixes: 90d9bc9d74a5157167548b26c00b1a016655e295
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2168769
Signed-off-by: Peter Krempa <pkrempa@redhat.com>

src/rpc/virnetserverclient.c | 11 4 + 7 - 0 !
1 file changed, 4 insertions(+), 7 deletions(-)

 rpc: client: don't check return value of virnetmessagenew
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

virNetServerClientDispatchRead checked the return value but it's not
necessary any more as it can't return NULL nowadays.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>

src/remote/libvirtd.conf.in | 1 1 + 0 - 0 !
src/rpc/virnetserverclient.c | 3 2 + 1 - 0 !
2 files changed, 3 insertions(+), 1 deletion(-)

 rpc: don't warn about "max_client_requests" in single-threaded
 daemons
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

The warning about max_client_requests is hit inside virtlogd every time
a VM starts which spams the logs.

Emit the warning only when the client request limit is not 1 and add a
warning into the daemon config to not configure it too low instead.

Fixes: 031878c2364
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2145188
Signed-off-by: Peter Krempa <pkrempa@redhat.com>

src/conf/domain_conf.c | 39 37 + 2 - 0 !
tests/qemuxml2argvdata/firmware-manual-efi-features.x86_64-latest.args | 35 35 + 0 - 0 !
tests/qemuxml2argvtest.c | 6 5 + 1 - 0 !
tests/qemuxml2xmloutdata/firmware-manual-efi-features.x86_64-latest.xml | 32 32 + 0 - 0 !
tests/qemuxml2xmltest.c | 1 1 + 0 - 0 !
5 files changed, 110 insertions(+), 3 deletions(-)

 conf: fix migration in some firmware autoselection scenarios
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

Introduce a small kludge in the parser to avoid unnecessarily
blocking incoming migration from a range of recent libvirt
releases.

https://bugzilla.redhat.com/show_bug.cgi?id=2184966

Signed-off-by: Andrea Bolognani <abologna@redhat.com>

src/util/virpci.c | 1 1 + 0 - 0 !
1 file changed, 1 insertion(+)

 virpci: resolve leak in virpcivirtualfunctionlist cleanup
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

Repeatedly querying an SR-IOV PCI device's capabilities exposes a
memory leak caused by a failure to free the virPCIVirtualFunction
array within the parent struct's g_autoptr cleanup.

Valgrind output after getting a single interface's XML description
1000 times:

==325982== 256,000 bytes in 1,000 blocks are definitely lost in loss record 2,634 of 2,635
==325982==    at 0x4C3C096: realloc (vg_replace_malloc.c:1437)
==325982==    by 0x59D952D: g_realloc (in /usr/lib64/libglib-2.0.so.0.5600.4)
==325982==    by 0x4EE1F52: virReallocN (viralloc.c:52)
==325982==    by 0x4EE1FB7: virExpandN (viralloc.c:78)
==325982==    by 0x4EE219A: virInsertElementInternal (viralloc.c:183)
==325982==    by 0x4EE23B2: virAppendElement (viralloc.c:288)
==325982==    by 0x4F65D85: virPCIGetVirtualFunctionsFull (virpci.c:2389)
==325982==    by 0x4F65753: virPCIGetVirtualFunctions (virpci.c:2256)
==325982==    by 0x505CB75: virNodeDeviceGetPCISRIOVCaps (node_device_conf.c:2969)
==325982==    by 0x505D181: virNodeDeviceGetPCIDynamicCaps (node_device_conf.c:3099)
==325982==    by 0x505BC4E: virNodeDeviceUpdateCaps (node_device_conf.c:2677)
==325982==    by 0x260FCBB2: nodeDeviceGetXMLDesc (node_device_driver.c:355)

Signed-off-by: Tim Shearer <tshearer@adva.com>

tools/virsh-domain.c | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 virsh: make domif-setlink work more than once
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

In virsh, we have this convenient domif-setlink command, which is
just a wrapper over virDomainUpdateDeviceFlags() and which allows
setting link state of given guest NIC. It does so by fetching
corresponding <interface/> XML snippet and either putting <link
state=''/> into it, OR if the element already exists setting the
attribute to desired value. The XML is then fed into the update
API.

There's, however, a small bug in detecting the pre-existence of
the element and its attribute. The code looks at "link"
attribute, while in fact, the attribute is called "state".

Resolves: https://gitlab.com/libvirt/libvirt/-/issues/426
Fixes: e575bf082ed4889280be07c986375f1ca15bb7ee
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

src/conf/virstorageobj.c | 7 6 + 1 - 0 !
1 file changed, 6 insertions(+), 1 deletion(-)

 storage: fix returning of locked objects from
 'virStoragePoolObjListSearch'
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

CVE-2023-3750

'virStoragePoolObjListSearch' explicitly documents that it's returning
a pointer to a locked and ref'd pool that maches the lookup function.

This was not the case as in commit 0c4b391e2a9 (released in
libvirt-8.3.0) the code was accidentally converted to use 'VIR_LOCK_GUARD'
which auto-unlocked it when leaving the scope, even when the code was
originally "leaking" the lock.

Revert the corresponding conversion and add a comment that this function
is intentionally leaking a locked object.

Fixes: 0c4b391e2a9
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2221851
Signed-off-by: Peter Krempa <pkrempa@redhat.com>

src/interface/interface_backend_udev.c | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 fix off-by-one error in udevlistinterfacesbystatus
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

Ever since this function was introduced in 2012 it could've tried
filling in an extra interface name.  That was made worse in 2019 when
the caller functions started accepting NULL arrays of size 0.

This is assigned CVE-2024-1441.

Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reported-by: Alexander Kuznetsov <kuznetsovam@altlinux.org>
Fixes: 5a33366f5c0b18c93d161bd144f9f079de4ac8ca
Fixes: d6064e2759a24e0802f363e3a810dc5a7d7ebb15

src/remote/remote_daemon_dispatch.c | 65 65 + 0 - 0 !
src/rpc/gendispatch.pl | 5 5 + 0 - 0 !
2 files changed, 70 insertions(+)

 remote: check for negative array lengths before allocation
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit

While the C API entry points will validate non-negative lengths
for various parameters, the RPC server de-serialization code
will need to allocate memory for arrays before entering the C
API. These allocations will thus happen before the non-negative
length check is performed.

Passing a negative length to the g_new0 function will usually
result in a crash due to the negative length being treated as
a huge positive number.

This was found and diagnosed by ALT Linux Team with AFLplusplus.

CVE-2024-2494

src/interface/interface_backend_udev.c | 26 19 + 7 - 0 !
1 file changed, 19 insertions(+), 7 deletions(-)

 interface: fix udev_device_get_sysattr_value return value check

Reviewing the code I found that return value of function
udev_device_get_sysattr_value() is dereferenced without a check.
udev_device_get_sysattr_value() may return NULL by number of reasons.

v2: VIR_DEBUG added, replaced STREQ(NULLSTR()) with STREQ_NULLABLE()
v3: More checks added, to skip earlier. More verbose VIR_DEBUG.

Signed-off-by: Dmitry Frolov <frolov@swemel.ru>

src/qemu/qemu_domain.c | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 qemu: domain: fix logic when tainting domain

Originally the code was skipping all repeated taints with the same taint
flag but a logic bug introduced in commit 30626ed15b239c424ae inverted
the condition. This caused that actually the first occurence was NOT
logged but any subsequent was.

This was noticed when going through oVirt logs as they use custom guest
agent commands and the logs are totally spammed with this message.

Fixes: 30626ed15b239c424ae891f096057a696eadd715
Signed-off-by: Peter Krempa <pkrempa@redhat.com>

src/qemu/qemu_process.c | 27 27 + 0 - 0 !
1 file changed, 27 insertions(+)

 qemu_process: skip over non-virtio non-tap nic models when
 refreshing rx-filter

After guest is started, or we are reconnecting to already running
one (after daemon restart), qemuProcessRefreshRxFilters() is
called to refresh rx-filters (basically MAC addresses of guest
NICs) as they might have changed while we were not running (for
the case when reconnecting to an already running guest), or we
need to enable them by running a command (for freshly started
guest - see processNicRxFilterChangedEvent()).

Now, our XML parser allowed trustGuestRxFilters attribute for all
types and models of <interface/> while in reality, only virtio
model AND TUN/TAP based types can see MAC address changes. For
other combinations, QEMU reports an error.

This all means that when the daemon is restarted and it
reconnects to a guest with, well invalid configuration, or when
such guest is restored from a saved image, or migrated then we
issue the monitor command, to which QEMU replies with an error
which is then propagated to users:

  error: internal error: unable to execute QEMU command 'query-rx-filter': invalid net client name: hostdev0

While on one hand users should fix their configuration (and after
v10.0.0-rc1~123 they can do that even on live domains), libvirt
can also has some logic built in that prevent issuing the command
in the first place (for obviously wrong cases).

Fixes: 060d4c83ef436cf56abfad51a4d64c39448e199d
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

src/security/apparmor/libvirt-qemu | 3 3 + 0 - 0 !
1 file changed, 3 insertions(+)

 apparmor: allow running i686 vms on debian 12

In Debian 12, the qemu-system-i386 binary in /usr/bin is a wrapper
script, with the actual executable living in /usr/libexec instead.
This makes it impossible to run i686 VMs when AppArmor is enabled.

Allow running the actual binary.

https://bugs.debian.org/1030926

Signed-off-by: Andrea Bolognani <abologna@redhat.com>

tests/vircgrouptest.c | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 skip vircgrouptest

We don't have a mock for nodeGetCPUCount yet so we fail in a chroot
without sysfs mounted.

src/util/virutil.c | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 reduce udevadm settle timeout to 10 seconds

This isn't a proper fix but it will make virt-manager at least start.

Closes: #663931

src/rpc/virnetsocket.c | 2 2 + 0 - 0 !
1 file changed, 2 insertions(+)

 pass gpg_tty env var to the ssh binary

gpg-agent(1) can emulate the OpenSSH Agent protocol (which provides
pubkey-authentication using an authentication-capable OpenPGP key, in
addition to the usual identity files).  However for a console-based
password prompt to work, the 'GPG_TTY' environment variable needs to be
set to the current TTY.  Furthermore, curses-based password prompts also
require the 'TERM' environment variable to be set to the terminal type.

docs/manpages/libvirt-guests.rst | 4 2 + 2 - 0 !
tools/libvirt-guests.sh.in | 21 10 + 11 - 0 !
2 files changed, 12 insertions(+), 13 deletions(-)

 debianize libvirt-guests

src/locking/virtlockd.service.in | 2 1 + 1 - 0 !
src/logging/virtlogd.service.in | 2 1 + 1 - 0 !
src/remote/libvirtd.service.in | 2 1 + 1 - 0 !
tools/libvirt-guests.service.in | 2 1 + 1 - 0 !
4 files changed, 4 insertions(+), 4 deletions(-)

 debianize systemd service files

src/security/apparmor/libvirt-lxc | 3 3 + 0 - 0 !
src/security/apparmor/libvirt-qemu | 3 3 + 0 - 0 !
src/security/apparmor/usr.lib.libvirt.virt-aa-helper.in | 1 1 + 0 - 0 !
src/security/apparmor/usr.sbin.libvirtd.in | 3 3 + 0 - 0 !
4 files changed, 10 insertions(+)

 apparmor_profiles_local_include

Include local apparmor profile

tools/vsh.c | 2 1 + 1 - 0 !
1 file changed, 1 insertion(+), 1 deletion(-)

 use sensible-editor by default

It is the reasonable default for Debian.

src/conf/domain_conf.c | 6 6 + 0 - 0 !
tests/qemuxml2argvtest.c | 1 0 + 1 - 0 !
tests/qemuxml2xmltest.c | 1 0 + 1 - 0 !
3 files changed, 6 insertions(+), 2 deletions(-)

 disable passt support

Rejecting the value from the XML parser ensures that all the
code related to passt, which is left untouched, is never
executed.

The error message reported in this case is the same that
virXMLPropEnum() would generate when encountering a value
that doesn't exist in the enum.