]> git.ipfire.org Git - thirdparty/libvirt.git/commit
networkUpdateState: do not assume dnsmasq_caps
authorMichal Privoznik <mprivozn@redhat.com>
Mon, 17 Apr 2023 08:09:51 +0000 (10:09 +0200)
committerMichal Privoznik <mprivozn@redhat.com>
Wed, 19 Apr 2023 12:58:33 +0000 (14:58 +0200)
commit212dfa94eeec88eb1a0bcf0c935a0ce17984306a
tree10295aebbc7666e0174a85e3114cc0eb580cf3ef
parent03094f8c65f71b8cc55b73d1c2af575b0e84a23d
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@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
src/network/bridge_driver.c