[PATCH 1/3] networkUpdateState: do not assume dnsmasq_caps
Assume there's a dnsmasq running (because there's an active
virtual network that spawned it). Now, shut down the daemon,
remove the dnsmasq binary and start the daemon again. At this
point, networkUpdateState() is called, but dnsmasq_caps is NULL
(because networkStateInitialize() called earlier failed to set
them, rightfully though).
Now, the networkUpdateState() tries to read the dnsmasq's PID
file using virPidFileReadIfAlive() which takes a path to the
corresponding binary as one of its arguments. To provide that
path, dnsmasqCapsGetBinaryPath() is called, but since
dnsmasq_caps is NULL, it dereferences it and thus causes a crash.
It's true that virPidFileReadIfAlive() can deal with a removed
binary (well virPidFileReadPathIfAlive() which it calls can), but
iff the binary path is provided in its absolute form. Otherwise,
virFileResolveAllLinks() fails to canonicalize the path
(expected, the path doesn't exist anyway).
Therefore, reading dnsmasq's PID file didn't work before
v8.1.0-rc1~401 which introduced this crash. It was always set to
-1. But passing NULL as binary path instead, makes
virPidFileReadIfAlive() return early, right after the PID file is
read and it's confirmed the PID exists.
Yes, this may yield wrong results, as the PID might be of a
completely different binary. But this problem is preexistent and
until we start locking PID files, there's nothing we can do about
it. IOW, it would require rework of dnsmasq PID file handling.
Fixes: 4b68c982e283471575bacbf87302495864da46fe
Resolves: https://gitlab.com/libvirt/libvirt/-/issues/456
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/network/bridge_driver.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 97e099880b..7f2298a15e 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -493,15 +493,19 @@ networkUpdateState(virNetworkObj *obj,
/* Try and read dnsmasq pids of active networks */
if (virNetworkObjIsActive(obj) && def->ips && (def->nips >
0)) {
+ const char *binpath = NULL;
pid_t dnsmasqPid;
if (networkSetMacMap(cfg, obj) < 0)
return -1;
+ if (dnsmasq_caps)
+ binpath = dnsmasqCapsGetBinaryPath(dnsmasq_caps);
+
ignore_value(virPidFileReadIfAlive(cfg->pidDir,
def->name,
&dnsmasqPid,
- dnsmasqCapsGetBinaryPath(dnsmasq_caps)));
+ binpath));
virNetworkObjSetDnsmasqPid(obj, dnsmasqPid);
}
--
2.39.2