[libvirt PATCH v2 00/13] Automatic mutex management - part 4
by Tim Wiederhake
Use the recently implemented VIR_LOCK_GUARD and VIR_WITH_MUTEX_LOCK_GUARD
to simplify mutex management.
V1: https://listman.redhat.com/archives/libvir-list/2022-March/229144.html
Changes since V1:
* Renamed mutex in nwfilter_driver.c
* Removed all instances where a lock guard had to be initialized with NULL
(i.e. referencing no mutex).
Tim Wiederhake (13):
nwfilter_driver: Statically initialize mutex
nwfilter_driver: Split up nwfilterStateCleanup
nwfilter_driver: Use automatic mutex management
nwfilter_gentech: Use automatic mutex management
nwfilter_dhcpsnoop: Replace virNWFilterSnoopActiveLock macros
nwfilter_dhcpsnoop: Replace virNWFilterSnoopLock macros
nwfilter_dhcpsnoop: Replace virNWFilterSnoopReqLock functions
nwfilter_learnipaddr: Use automatic mutex management
nwfilter_ipaddrmap: Use automatic mutex management
virNetlinkEventAddClient: Remove goto
virnetlink: Use automatic memory management
remote_daemon_stream: Use automatic memory management
qemu_conf: Use automatic memory management
src/conf/nwfilter_ipaddrmap.c | 80 ++---
src/conf/virnwfilterobj.h | 1 -
src/nwfilter/nwfilter_dhcpsnoop.c | 396 ++++++++-----------------
src/nwfilter/nwfilter_driver.c | 175 +++++------
src/nwfilter/nwfilter_gentech_driver.c | 33 +--
src/nwfilter/nwfilter_learnipaddr.c | 83 ++----
src/qemu/qemu_conf.c | 70 ++---
src/remote/remote_daemon_stream.c | 34 +--
src/util/virnetlink.c | 226 +++++++-------
9 files changed, 423 insertions(+), 675 deletions(-)
--
2.31.1
2 years, 8 months
sigfault in virObjectLockGuard
by Claudio Fontana
Hello all,
while experimenting with upstream libvirt, I encountered the following segfault when trying to virt-install a centos7 guest:
----
#! /bin/bash
ISO=CentOS-7-x86_64-Minimal-2009.iso
DISK=centos7.qcow2
rm ${DISK}
qemu-img create -f qcow2 ${DISK} 30G
virt-install --virt-type=kvm --name=centos7 --vcpus=2 --memory=30720 --location ${ISO} --disk path=${DISK},format=qcow2 --network default --graphics none --os-variant=centos7.0 --extra-args console=ttyS0
-----
This triggers a segfault in libvirtd as follows:
Thread 5 "rpc-libvirtd" received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0x7fca0b7fe640 (LWP 55658)]
___pthread_mutex_lock (mutex=mutex@entry=0x18) at pthread_mutex_lock.c:80
80 unsigned int type = PTHREAD_MUTEX_TYPE_ELISION (mutex);
(gdb) bt
#0 ___pthread_mutex_lock (mutex=mutex@entry=0x18) at pthread_mutex_lock.c:80
#1 0x00007fca132b8275 in virMutexLock (m=m@entry=0x18) at ../src/util/virthread.c:91
#2 0x00007fca132b829a in virLockGuardLock (m=0x18) at ../src/util/virthread.c:102
#3 0x00007fca1329c519 in virObjectLockGuard (anyobj=anyobj@entry=0x7fca13539310 <virNetDevGenNames+16>) at ../src/util/virobject.c:441
#4 0x00007fca1328b5f1 in virNetDevGenerateName (ifname=ifname@entry=0x7fca00018158, type=type@entry=VIR_NET_DEV_GEN_NAME_VNET)
at ../src/util/virnetdev.c:3619
#5 0x00007fca13293b9d in virNetDevTapCreate
(ifname=ifname@entry=0x7fca00018158, tunpath=tunpath@entry=0x7fca0845accd "/dev/net/tun", tapfd=tapfd@entry=0x7fca00043fc0, tapfdSize=tapfdSize@entry=1, flags=flags@entry=3) at ../src/util/virnetdevtap.c:183
#6 0x00007fca13294501 in virNetDevTapCreateInBridgePort
(brname=brname@entry=0x7fca000163c0 "br0", ifname=ifname@entry=0x7fca00018158, macaddr=macaddr@entry=0x7fca00018034, vmuuid=vmuuid@entry=0x7fca000156b8 "\232T\020\250~\024Mn\211\227\062\"\264\001\025\030", tunpath=tunpath@entry=0x7fca0845accd "/dev/net/tun", tapfd=tapfd@entry=0x7fca00043fc0, tapfdSize=1, virtPortProfile=0x0, virtVlan=0x0, isolatedPort=VIR_TRISTATE_BOOL_ABSENT, coalesce=0x0, mtu=0, actualMTU=0x0, flags=3) at ../src/util/virnetdevtap.c:602
#7 0x00007fca083d66f5 in qemuInterfaceBridgeConnect
(def=def@entry=0x7fca000156b0, driver=driver@entry=0x7fc9c0050980, net=net@entry=0x7fca00018030, tapfd=tapfd@entry=0x7fca00043fc0, tapfdSize=tapfdSize@entry=0x7fca0b7fd410) at ../src/qemu/qemu_interface.c:564
#8 0x00007fca08370f35 in qemuBuildInterfaceCommandLine
(vm=<optimized out>, vm=<optimized out>, nicindexes=0x7fca0b7fd570, nnicindexes=0x7fca0b7fd568, standalone=false, vmop=VIR_NETDEV_VPORT_PROFILE_OP_CREATE, qemuCaps=0x7fc9fc0145a0 [virQEMUCaps], net=0x7fca00018030, cmd=0x7fca00041300, driver=0x7fc9c0050980)
at ../src/qemu/qemu_command.c:8669
#9 qemuBuildNetCommandLine
(nicindexes=0x7fca0b7fd570, nnicindexes=0x7fca0b7fd568, standalone=<optimized out>, vmop=VIR_NETDEV_VPORT_PROFILE_OP_CREATE, qemuCaps=0x7fc9fc0145a0 [virQEMUCaps], cmd=0x7fca00041300, vm=<optimized out>, driver=0x7fc9c0050980) at ../src/qemu/qemu_command.c:8958
#10 qemuBuildCommandLine (driver=driver@entry=0x7fc9c0050980, vm=0x7fc9fc31f5e0 [virDomainObj],
vm@entry=0x2200000000, migrateURI=0x0, snapshot=0x0,
snapshot@entry=0x7fc9c0013190, vmop=vmop@entry=VIR_NETDEV_VPORT_PROFILE_OP_CREATE, standalone=standalone@entry=false, enableFips=false, nnicindexes=0x7fca0b7fd568, nicindexes=0x7fca0b7fd570, flags=0) at ../src/qemu/qemu_command.c:10548
#11 0x00007fca0841adc2 in qemuProcessLaunch
(conn=0x7fc9c0022070, driver=0x7fc9c0050980, vm=0x2200000000, asyncJob=QEMU_ASYNC_JOB_NONE, incoming=0x0, snapshot=0x7fc9c0013190, vmop=VIR_NETDEV_VPORT_PROFILE_OP_CREATE, flags=17) at ../src/qemu/qemu_process.c:7435
#12 0x00007fca08420669 in qemuProcessStart
(conn=conn@entry=0x7fc9c0013190, driver=driver@entry=0x7fc9c0050980, vm=0x7fc9fc31f5e0 [virDomainObj], updatedCPU=updatedCPU@entry=0x0, asyncJob=asyncJob@entry=QEMU_ASYNC_JOB_START, migrateFrom=migrateFrom@entry=0x0, migrateFd=-1, migratePath=0x0, snapshot=0x0, vmop=VIR_NETDE--Type <RET> for more, q to quit, c to continue without paging--
V_VPORT_PROFILE_OP_CREATE, flags=<optimized out>) at ../src/qemu/qemu_process.c:7855
#13 0x00007fca083b3e10 in qemuDomainCreateXML (conn=0x7fc9c0013190, xml=<optimized out>, flags=0) at ../src/qemu/qemu_driver.c:1628
#14 0x00007fca133e46f0 in virDomainCreateXML
(conn=0x7fc9c0013190, xmlDesc=0x7fca00008760 "<domain type=\"kvm\">\n <name>centos7</name>\n <uuid>9a5410a8-7e14-4d6e-8997-3222b4011518</uuid>\n <metadata>\n <libosinfo:libosinfo xmlns:libosinfo=\"http://libosinfo.org/xmlns/libvirt/domain/1.0\">\n "..., flags=0)
at ../src/libvirt-domain.c:180
#15 0x0000564bb33d631f in remoteDispatchDomainCreateXML
(server=0x564bb3dd1880 [virNetServer], msg=0x564bb3e81620, ret=0x7fca0000bfa0, args=0x7fca00002510, rerr=0x7fca0b7fd9a0, client=0x564bb3e80030 [virNetServerClient]) at src/remote/remote_daemon_dispatch_stubs.h:5083
#16 remoteDispatchDomainCreateXMLHelper
(server=0x564bb3dd1880 [virNetServer], client=0x564bb3e80030 [virNetServerClient], msg=0x564bb3e81620, rerr=0x7fca0b7fd9a0, args=0x7fca00002510, ret=0x7fca0000bfa0) at src/remote/remote_daemon_dispatch_stubs.h:5064
#17 0x00007fca13374426 in virNetServerProgramDispatchCall
(msg=0x564bb3e81620, client=0x564bb3e80030 [virNetServerClient], server=0x564bb3dd1880 [virNetServer], prog=0x564bb3e3c010 [virNetServerProgram]) at ../src/rpc/virnetserverprogram.c:428
#18 virNetServerProgramDispatch
(prog=0x564bb3e3c010 [virNetServerProgram], server=server@entry=0x564bb3dd1880 [virNetServer], client=0x564bb3e80030 [virNetServerClient], msg=0x564bb3e81620) at ../src/rpc/virnetserverprogram.c:302
#19 0x00007fca133796d4 in virNetServerProcessMsg
(msg=<optimized out>, prog=<optimized out>, client=<optimized out>, srv=0x564bb3dd1880 [virNetServer]) at ../src/rpc/virnetserver.c:140
#20 virNetServerHandleJob (jobOpaque=0x564bb3e7fd50, opaque=0x564bb3dd1880) at ../src/rpc/virnetserver.c:160
#21 0x00007fca132b8eaf in virThreadPoolWorker (opaque=<optimized out>) at ../src/util/virthreadpool.c:164
#22 0x00007fca132b84f5 in virThreadHelper (data=<optimized out>) at ../src/util/virthread.c:256
#23 0x00007fca12c362ba in start_thread (arg=<optimized out>) at pthread_create.c:442
#24 0x00007fca12cc0460 in clone3 () at ../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
----
In order to work around the problem this is the hack I used (to get things to work), but clearly it is no solution:
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 5df48af60c..e22d3cb5c5 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -3616,7 +3616,7 @@ virNetDevGenerateName(char **ifname, virNetDevGenNameType type)
g_autofree char *try = NULL;
int id = 0;
- VIR_WITH_OBJECT_LOCK_GUARD(&virNetDevGenNames[type].mutex) {
+ /* VIR_WITH_OBJECT_LOCK_GUARD(&virNetDevGenNames[type].mutex) */ {
id = ++virNetDevGenNames[type].lastID;
/* reset before overflow */
----
With this applied things run fine for me, install is successful and domain is functional.
Ciao,
Claudio
--
Claudio Fontana
Engineering Manager Virtualization, SUSE Labs Core
SUSE Software Solutions Italy Srl
2 years, 8 months
[PATCH] nwfilter: fix crash when counting number of network filters
by Daniel P. Berrangé
The virNWFilterObjListNumOfNWFilters method iterates over the
driver->nwfilters, accessing virNWFilterObj instances. As such
it needs to be protected against concurrent modification of
the driver->nwfilters object.
This API allows unprivileged users to connect, so users with
read-only access to libvirt can cause a denial of service
crash if they are able to race with a call of virNWFilterUndefine.
Since network filters are usually statically defined, this is
considered a low severity problem.
This is assigned CVE-2022-0897.
Reviewed-by: Eric Blake <eblake(a)redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com>
---
Pushed as a pre-reviewed patch from the security list
src/nwfilter/nwfilter_driver.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c
index 3ce8fce7f9..a493205c80 100644
--- a/src/nwfilter/nwfilter_driver.c
+++ b/src/nwfilter/nwfilter_driver.c
@@ -478,11 +478,15 @@ nwfilterLookupByName(virConnectPtr conn,
static int
nwfilterConnectNumOfNWFilters(virConnectPtr conn)
{
+ int ret;
if (virConnectNumOfNWFiltersEnsureACL(conn) < 0)
return -1;
- return virNWFilterObjListNumOfNWFilters(driver->nwfilters, conn,
- virConnectNumOfNWFiltersCheckACL);
+ nwfilterDriverLock();
+ ret = virNWFilterObjListNumOfNWFilters(driver->nwfilters, conn,
+ virConnectNumOfNWFiltersCheckACL);
+ nwfilterDriverUnlock();
+ return ret;
}
--
2.35.1
2 years, 8 months
[libvirt PATCH v2 00/10] Automatic mutex management - part 3
by Tim Wiederhake
Use the recently implemented VIR_LOCK_GUARD and VIR_WITH_MUTEX_LOCK_GUARD
to simplify mutex management.
v1: https://listman.redhat.com/archives/libvir-list/2022-February/msg00674.html
Changed since v1:
* Removed locking / unlocking in storage driver initialization and cleanup
instead of working around the issue of the lifetime of the mutex.
Tim Wiederhake (10):
test: Use automatic mutex management
openvz: Use automatic mutex management
remote_daemon_dispatch: Use automatic mutex management
netdev: Use automatic mutex management
nodesuspend: Use automatic mutex management
admin: Use automatic mutex management
esx_stream: Use automatic mutex management
esx_vi: Use automatic mutex management
storage: Removing mutex locking in initialization and cleanup
storage: Use automatic mutex management
src/admin/admin_server_dispatch.c | 3 +-
src/esx/esx_stream.c | 65 ++++------
src/esx/esx_vi.c | 109 +++++++---------
src/openvz/openvz_driver.c | 91 +++++---------
src/remote/remote_daemon_dispatch.c | 187 +++++++++-------------------
src/storage/storage_driver.c | 32 ++---
src/test/test_driver.c | 15 +--
src/util/virnetdev.c | 20 ++-
src/util/virnodesuspend.c | 54 +++-----
9 files changed, 193 insertions(+), 383 deletions(-)
--
2.31.1
2 years, 8 months
[PATCH v2 0/4] nodedev: dynamic parent update on mdev definitions
by Boris Fiuczynski
Mdev definitions can be created regardless of the existence of there
parent devices in mdevctl. Parent objects of mdev definitions can also
vanish dynamically. This series adds the missing support for these
scenarios.
Boris Fiuczynski (4):
nodedev: fix typo in mdevctl update warning
virnodedeviceobj: export virNodeDeviceObjHasCap
nodedev: update mdevs on parent change
nodedev: trigger mdev device definition update on udev add and remove
src/conf/virnodedeviceobj.c | 3 +--
src/conf/virnodedeviceobj.h | 4 ++++
src/libvirt_private.syms | 1 +
src/node_device/node_device_driver.c | 4 +++-
src/node_device/node_device_udev.c | 11 ++++++++++-
5 files changed, 19 insertions(+), 4 deletions(-)
--
2.33.1
2 years, 8 months
[libvirt PATCH 0/4] Functional CI - GitLab enablement
by Erik Skultety
RFC here [1]
Since RFC:
- renamed the stage to integration_tests
- dropped both the test child pipeline as well as triggering a
multi-project CI pipeline to build the Perl bindings and instead use the
latest bindings artifacts we have available from the libvirt-perl project.
-> basically now we only have a regular CI stage with no extra jobs
that just spins up a bunch of private runners and uploads RPM
artifacts into them and runs the TCK tests
- replaced sed invocations to edit daemon logging settings with augtool
- used a pattern for log filters from [2]
- used the "$CI_JOB_STATUS" variable in the after_script to determine
whether the main script failed so as not to try moving and collecting
logs on successful jobs (the logs would have been published as artifacts
on failures anyway)
Demo pipeline: https://gitlab.com/eskultety/libvirt/-/pipelines/491838828
[1] https://listman.redhat.com/archives/libvir-list/2022-January/227947.html
[2] https://libvirt.org/kbase/debuglogs.html#less-verbose-logging-for-qemu-vms
Erik Skultety (4):
ci: gitlab: Refresh gitlab.yml
ci: manifest: Publish RPMs as artifacts on CentOS Stream and Fedoras
gitlab-ci: Introduce new 'integration_tests' pipeline stage
DO NOT MERGE: Demo a functional CI pipeline running Fedoras CentOS
only
.gitlab-ci.yml | 99 +-------
ci/gitlab.yml | 606 ++-------------------------------------------
ci/integration.yml | 99 ++++++++
ci/manifest.yml | 21 +-
4 files changed, 142 insertions(+), 683 deletions(-)
create mode 100644 ci/integration.yml
--
2.34.1
2 years, 8 months
[PATCH] virnetdev: Use VIR_WITH_MUTEX_LOCK_GUARD in virNetDevGenerateName()
by Michal Privoznik
The virNetDevGenerateName() function uses a global array of
virNetDevGenName structs to find next unused name for network
device. This obviously needs some locking and in fact each member
of the array has its own lock. However, these members are not
virObjects, they are just plain structs, therefore
VIR_WITH_MUTEX_LOCK_GUARD() must be used instead of
VIR_WITH_OBJECT_LOCK_GUARD() to lock individual mutexes.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/util/virnetdev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/util/virnetdev.c b/src/util/virnetdev.c
index 5df48af60c..1c15a475fa 100644
--- a/src/util/virnetdev.c
+++ b/src/util/virnetdev.c
@@ -3616,7 +3616,7 @@ virNetDevGenerateName(char **ifname, virNetDevGenNameType type)
g_autofree char *try = NULL;
int id = 0;
- VIR_WITH_OBJECT_LOCK_GUARD(&virNetDevGenNames[type].mutex) {
+ VIR_WITH_MUTEX_LOCK_GUARD(&virNetDevGenNames[type].mutex) {
id = ++virNetDevGenNames[type].lastID;
/* reset before overflow */
--
2.34.1
2 years, 8 months
[PATCH] nodedev: fix typo in mdevctl update warning
by Boris Fiuczynski
Signed-off-by: Boris Fiuczynski <fiuczy(a)linux.ibm.com>
---
src/node_device/node_device_udev.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index b0a5e6302c..17b6bb9299 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -2035,7 +2035,7 @@ mdevctlHandlerThread(void *opaque G_GNUC_UNUSED)
VIR_LOCK_GUARD lock = virLockGuardLock(&priv->mdevctlLock);
if (nodeDeviceUpdateMediatedDevices() < 0)
- VIR_WARN("mdevctl failed to updated mediated devices");
+ VIR_WARN("mdevctl failed to update mediated devices");
}
--
2.33.1
2 years, 8 months
[PATCH 0/3] nodedev: dynamic parent update on mdev definitions
by Boris Fiuczynski
Mdev definitions can be created regardless of the existence of there
parent devices in mdevctl. Parent objects of mdev definitions can also
vanish dynamically. This series adds the missing support for these
scenarios.
Boris Fiuczynski (3):
virnodedeviceobj: export virNodeDeviceObjHasCap
nodedev: update mdevs on parent change
nodedev: trigger mdev device definition update on udev add and remove
src/conf/virnodedeviceobj.c | 3 +--
src/conf/virnodedeviceobj.h | 4 ++++
src/libvirt_private.syms | 1 +
src/node_device/node_device_driver.c | 4 +++-
src/node_device/node_device_udev.c | 9 +++++++++
5 files changed, 18 insertions(+), 3 deletions(-)
--
2.33.1
2 years, 8 months