Compare commits

..

2 Commits

Author SHA1 Message Date
Cole Robinson 0aea5c88f2 libvirt-6.1.0-4
Fix libxl driver startup crash (bz #1842318)
2020-06-02 13:46:51 -04:00
Cole Robinson 1ab72850ed libvirt-6.1.0-3
Fix iptables No chain/target/match by that name (bz #1813830)
systemd: start libvirtd after firewalld/iptables services (bz #1697636)
2020-05-26 11:44:02 -04:00
14 changed files with 1429 additions and 2003 deletions
@@ -1,41 +0,0 @@
From 3499354e12a1c1832bf4030693a64e03ceb79d05 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Wed, 5 Jun 2024 11:16:21 +0100
Subject: [PATCH] interface: fix udev reference leak with invalid flags
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The udevInterfaceGetXMLDesc method takes a reference on the udev
driver as its first action. If the virCheckFlags() condition
fails, however, this reference is never released.
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
src/interface/interface_backend_udev.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c
index fdf11a8318..e1a50389c9 100644
--- a/src/interface/interface_backend_udev.c
+++ b/src/interface/interface_backend_udev.c
@@ -1027,12 +1027,14 @@ static char *
udevInterfaceGetXMLDesc(virInterfacePtr ifinfo,
unsigned int flags)
{
- struct udev *udev = udev_ref(driver->udev);
+ struct udev *udev = NULL;
g_autoptr(virInterfaceDef) ifacedef = NULL;
char *xmlstr = NULL;
virCheckFlags(VIR_INTERFACE_XML_INACTIVE, NULL);
+ udev = udev_ref(driver->udev);
+
/* Recursively build up the interface XML based on the requested
* interface name
*/
--
2.45.1
@@ -1,49 +0,0 @@
From 98f1cf88fa7e0f992d93f376418fbfb3996a9690 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Fri, 17 May 2024 14:55:24 +0100
Subject: [PATCH] rpc: avoid leak of GSource in use for interrupting main loop
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
We never release the reference on the GSource created for
interrupting the main loop, nor do we remove it from the
main context if our thread is woken up prior to the wakeup
callback firing.
This can result in a leak of GSource objects, along with an
ever growing list of GSources attached to the main context,
which will gradually slow down execution of the loop, as
several operations are O(N) for the number of attached GSource
objects.
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
src/rpc/virnetclient.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 147b0d661a..6d424eb599 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -1946,7 +1946,7 @@ static int virNetClientIO(virNetClient *client,
/* Check to see if another thread is dispatching */
if (client->haveTheBuck) {
/* Force other thread to wakeup from poll */
- GSource *wakeup = g_idle_source_new();
+ g_autoptr(GSource) wakeup = g_idle_source_new();
g_source_set_callback(wakeup, virNetClientIOWakeup, client->eventLoop, NULL);
g_source_attach(wakeup, client->eventCtx);
@@ -1968,6 +1968,7 @@ static int virNetClientIO(virNetClient *client,
return -1;
}
+ g_source_destroy(wakeup);
VIR_DEBUG("Woken up from sleep head=%p call=%p",
client->waitDispatch, thiscall);
/* Three reasons we can be woken up
--
2.45.1
@@ -1,99 +0,0 @@
From 8074d64dc2eca846d6a61efe1a9b7428a0ce1dd1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Tue, 30 Apr 2024 11:51:15 +0100
Subject: [PATCH] rpc: ensure temporary GSource is removed from client event
loop
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Users are seeing periodic segfaults from libvirt client apps,
especially thread heavy ones like virt-manager. A typical
stack trace would end up in the virNetClientIOEventFD method,
with illegal access to stale stack data. eg
==238721==ERROR: AddressSanitizer: stack-use-after-return on address 0x75cd18709788 at pc 0x75cd3111f907 bp 0x75cd181ff550 sp 0x75cd181ff548
WRITE of size 4 at 0x75cd18709788 thread T11
#0 0x75cd3111f906 in virNetClientIOEventFD /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:1634:15
#1 0x75cd3210d198 (/usr/lib/libglib-2.0.so.0+0x5a198) (BuildId: 0a2311dfbbc6c215dc36f4b6bdd2b4b6fbae55a2)
#2 0x75cd3216c3be (/usr/lib/libglib-2.0.so.0+0xb93be) (BuildId: 0a2311dfbbc6c215dc36f4b6bdd2b4b6fbae55a2)
#3 0x75cd3210ddc6 in g_main_loop_run (/usr/lib/libglib-2.0.so.0+0x5adc6) (BuildId: 0a2311dfbbc6c215dc36f4b6bdd2b4b6fbae55a2)
#4 0x75cd3111a47c in virNetClientIOEventLoop /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:1722:9
#5 0x75cd3111a47c in virNetClientIO /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:2002:10
#6 0x75cd3111a47c in virNetClientSendInternal /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:2170:11
#7 0x75cd311198a8 in virNetClientSendWithReply /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclient.c:2198:11
#8 0x75cd31111653 in virNetClientProgramCall /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/rpc/virnetclientprogram.c:318:9
#9 0x75cd31241c8f in callFull /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/remote/remote_driver.c:6054:10
#10 0x75cd31241c8f in call /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/remote/remote_driver.c:6076:12
#11 0x75cd31241c8f in remoteNetworkGetXMLDesc /usr/src/debug/libvirt/libvirt-10.2.0/build/src/remote/remote_client_bodies.h:5959:9
#12 0x75cd31410ff7 in virNetworkGetXMLDesc /usr/src/debug/libvirt/libvirt-10.2.0/build/../src/libvirt-network.c:952:15
The root cause is a bad assumption in the virNetClientIOEventLoop
method. This method is run by whichever thread currently owns the
buck, and is responsible for handling I/O. Inside a for(;;) loop,
this method creates a temporary GSource, adds it to the event loop
and runs g_main_loop_run(). When I/O is ready, the GSource callback
(virNetClientIOEventFD) will fire and call g_main_loop_quit(), and
return G_SOURCE_REMOVE which results in the temporary GSource being
destroyed. A g_autoptr() will then remove the last reference.
What was overlooked, is that a second thread can come along and
while it can't enter virNetClientIOEventLoop, it will register an
idle source that uses virNetClientIOWakeup to interrupt the
original thread's 'g_main_loop_run' call. When this happens the
virNetClientIOEventFD callback never runs, and so the temporary
GSource is not destroyed. The g_autoptr() will remove a reference,
but by virtue of still being attached to the event context, there
is an extra reference held causing GSource to be leaked. The
next time 'g_main_loop_run' is called, the original GSource will
trigger its callback, and access data that was allocated on the
stack by the previous thread, and likely SEGV.
To solve this, the thread calling 'g_main_loop_run' must call
g_source_destroy, immediately upon return, to guarantee that
the temporary GSource is removed.
CVE-2024-4418
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Martin Shirokov <shirokovmartin@gmail.com>
Tested-by: Martin Shirokov <shirokovmartin@gmail.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
src/rpc/virnetclient.c | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 68098b1c8d..147b0d661a 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -1657,7 +1657,7 @@ static int virNetClientIOEventLoop(virNetClient *client,
#endif /* !WIN32 */
int timeout = -1;
virNetMessage *msg = NULL;
- g_autoptr(GSource) G_GNUC_UNUSED source = NULL;
+ g_autoptr(GSource) source = NULL;
GIOCondition ev = 0;
struct virNetClientIOEventData data = {
.client = client,
@@ -1721,6 +1721,18 @@ static int virNetClientIOEventLoop(virNetClient *client,
g_main_loop_run(client->eventLoop);
+ /*
+ * If virNetClientIOEventFD ran, this GSource will already be
+ * destroyed due to G_SOURCE_REMOVE. It is harmless to re-destroy
+ * it, since we still own a reference.
+ *
+ * If virNetClientIOWakeup ran, it will have interrupted the
+ * g_main_loop_run call, before virNetClientIOEventFD could
+ * run, and thus the GSource is still registered, and we need
+ * to destroy it since it is referencing stack memory for 'data'
+ */
+ g_source_destroy(source);
+
#ifndef WIN32
ignore_value(pthread_sigmask(SIG_SETMASK, &oldmask, NULL));
#endif /* !WIN32 */
--
2.45.1
@@ -0,0 +1,36 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 9 Mar 2020 16:40:57 +0100
Subject: [PATCH] virDomainDiskTranslateSourcePool: Check for disk type
correctly
When rewriting the virDomainDiskTranslateSourcePool() function in
v6.1.0-rc1~184 a typo was introduced. Previously, we allowed
startup policy only for those volumes which translated to
VIR_STORAGE_TYPE_FILE. But starting with the referenced commit,
the value we checked for was changed to VIR_STORAGE_VOL_FILE
which comes from a different enum and has a different value too.
This is wrong, because virStorageSourceGetActualType() returns a
value from the original enum.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1811728
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
(cherry picked from commit 3918dbd84e4951b43f93fbf50ef52be00274850c)
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 17867eeece..fd2e8f4eb5 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -31746,7 +31746,7 @@ virDomainDiskTranslateSourcePool(virDomainDiskDefPtr def)
}
if (def->startupPolicy != 0 &&
- virStorageSourceGetActualType(def->src) != VIR_STORAGE_VOL_FILE) {
+ virStorageSourceGetActualType(def->src) != VIR_STORAGE_TYPE_FILE) {
virReportError(VIR_ERR_XML_ERROR, "%s",
_("'startupPolicy' is only valid for "
"'file' type volume"));
@@ -1,35 +0,0 @@
From adfdb79f1e01401349e1321d0f5059d7b6489f00 Mon Sep 17 00:00:00 2001
Message-ID: <adfdb79f1e01401349e1321d0f5059d7b6489f00.1724763718.git.crobinso@redhat.com>
From: Martin Kletzander <mkletzan@redhat.com>
Date: Fri, 16 Aug 2024 13:56:51 +0200
Subject: [PATCH 1/3] virarptable: Properly calculate rtattr length
Content-type: text/plain
Use convenience macro which does almost the same thing we were doing,
but also pads out the payload length to a multiple of NLMSG_ALIGNTO (4)
bytes.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
src/util/virarptable.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
index 299dddd664..d8e41c5a86 100644
--- a/src/util/virarptable.c
+++ b/src/util/virarptable.c
@@ -102,8 +102,7 @@ virArpTableGet(void)
return table;
VIR_WARNINGS_NO_CAST_ALIGN
- parse_rtattr(tb, NDA_MAX, NDA_RTA(r),
- nh->nlmsg_len - NLMSG_LENGTH(sizeof(*r)));
+ parse_rtattr(tb, NDA_MAX, NDA_RTA(r), NLMSG_PAYLOAD(nh, sizeof(*r)));
VIR_WARNINGS_RESET
if (tb[NDA_DST] == NULL || tb[NDA_LLADDR] == NULL)
--
2.46.0
@@ -0,0 +1,55 @@
From: Laine Stump <laine@redhat.com>
Date: Thu, 7 May 2020 22:32:59 -0400
Subject: [PATCH] network: make it safe to call networkSetupPrivateChains()
multiple times
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
networkSetupPrivateChains() is currently called only once per run of
libvirtd, so it can assume that errInitV4 and errInitV6 are empty/null
when it is called. In preparation for potentially calling this
function multiple times during one run, this patch moves the reset of
errInitV[46] to the top of the function, to assure no memory is
leaked.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit de110f110fb917a31b9f33ad8e4b3c1d3284766a)
---
src/network/bridge_driver_linux.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index 7bbde5c6a9..80bd2409e1 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -48,6 +48,10 @@ static void networkSetupPrivateChains(void)
VIR_DEBUG("Setting up global firewall chains");
createdChains = false;
+ virFreeError(errInitV4);
+ errInitV4 = NULL;
+ virFreeError(errInitV6);
+ errInitV6 = NULL;
rc = iptablesSetupPrivateChains(VIR_FIREWALL_LAYER_IPV4);
if (rc < 0) {
@@ -56,8 +60,6 @@ static void networkSetupPrivateChains(void)
errInitV4 = virSaveLastError();
virResetLastError();
} else {
- virFreeError(errInitV4);
- errInitV4 = NULL;
if (rc) {
VIR_DEBUG("Created global IPv4 chains");
createdChains = true;
@@ -73,8 +75,6 @@ static void networkSetupPrivateChains(void)
errInitV6 = virSaveLastError();
virResetLastError();
} else {
- virFreeError(errInitV6);
- errInitV6 = NULL;
if (rc) {
VIR_DEBUG("Created global IPv6 chains");
createdChains = true;
@@ -1,42 +0,0 @@
From 137779b894858bd958ea575cec260a0559b31e48 Mon Sep 17 00:00:00 2001
Message-ID: <137779b894858bd958ea575cec260a0559b31e48.1724763718.git.crobinso@redhat.com>
In-Reply-To: <adfdb79f1e01401349e1321d0f5059d7b6489f00.1724763718.git.crobinso@redhat.com>
References: <adfdb79f1e01401349e1321d0f5059d7b6489f00.1724763718.git.crobinso@redhat.com>
From: Martin Kletzander <mkletzan@redhat.com>
Date: Fri, 16 Aug 2024 13:59:15 +0200
Subject: [PATCH 2/3] virarptable: Fix check for message length
Content-type: text/plain
The previous check was all wrong since it calculated the how long would
the netlink message be if the netlink header was the payload and then
subtracted that from the whole message length, a variable that was not
used later in the code. This check can fail if there are no additional
payloads, struct rtattr in particular, which we are parsing later,
however the RTA_OK macro would've caught that anyway.
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
src/util/virarptable.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
index d8e41c5a86..45ee76766f 100644
--- a/src/util/virarptable.c
+++ b/src/util/virarptable.c
@@ -81,10 +81,9 @@ virArpTableGet(void)
for (; NLMSG_OK(nh, msglen); nh = NLMSG_NEXT(nh, msglen)) {
VIR_WARNINGS_RESET
struct ndmsg *r = NLMSG_DATA(nh);
- int len = nh->nlmsg_len;
void *addr;
- if ((len -= NLMSG_LENGTH(sizeof(*nh))) < 0) {
+ if (nh->nlmsg_len < NLMSG_SPACE(sizeof(*r))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("wrong nlmsg len"));
goto cleanup;
--
2.46.0
@@ -0,0 +1,265 @@
From: Laine Stump <laine@redhat.com>
Date: Thu, 7 May 2020 21:54:39 -0400
Subject: [PATCH] network: force re-creation of iptables private chains on
firewalld restart
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When firewalld is stopped, it removes *all* iptables rules and chains,
including those added by libvirt. Since restarting firewalld means
stopping and then starting it, any time it is restarted, libvirt needs
to recreate all the private iptables chains it uses, along with all
the rules it adds.
We already have code in place to call networkReloadFirewallRules() any
time we're notified of a firewalld start, and
networkReloadFirewallRules() will call
networkPreReloadFirewallRules(), which calls
networkSetupPrivateChains(); unfortunately that last call is called
using virOnce(), meaning that it will only be called the first time
through networkPreReloadFirewallRules() after libvirtd starts - so of
course when firewalld is later restarted, the call to
networkSetupPrivateChains() is skipped.
The neat and tidy way to fix this would be if there was a standard way
to reset a pthread_once_t object so that the next time virOnce was
called, it would think the function hadn't been called, and call it
again. Unfortunately, there isn't any official way of doing that (we
*could* just fill it with 0 and hope for the best, but that doesn't
seem very safe.
So instead, this patch just adds a static variable called
chainInitDone, which is set to true after networkSetupPrivateChains()
is called for the first time, and then during calls to
networkPreReloadFirewallRules(), if chainInitDone is set, we call
networkSetupPrivateChains() directly instead of via virOnce().
It may seem unsafe to directly call a function that is meant to be
called only once, but I think in this case we're safe - there's
nothing in the function that is inherently "once only" - it doesn't
initialize anything that can't safely be re-initialized (as long as
two threads don't try to do it at the same time), and it only happens
when responding to a dbus message that firewalld has been started (and
I don't think it's possible for us to be processing two of those at
once), and even then only if the initial call to the function has
already been completed (so we're safe if we receive a firewalld
restart call at a time when we haven't yet called it, or even if
another thread is already in the process of executing it. The only
problematic bit I can think of is if another thread is in the process
of adding an iptable rule at the time we're executing this function,
but 1) none of those threads will be trying to add chains, and 2) if
there was a concurrency problem with other threads adding iptables
rules while firewalld was being restarted, it would still be a problem
even without this change.
This is yet another patch that fixes an occurrence of this error:
COMMAND_FAILED: '/usr/sbin/iptables -w10 -w --table filter --insert LIBVIRT_INP --in-interface virbr0 --protocol tcp --destination-port 67 --jump ACCEPT' failed: iptables: No chain/target/match by that name.
In particular, this resolves: https://bugzilla.redhat.com/1813830
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit f5418b427e7d2f26803880309478de9103680826)
---
src/network/bridge_driver.c | 16 ++++---
src/network/bridge_driver_linux.c | 69 ++++++++++++++++++----------
src/network/bridge_driver_nop.c | 3 +-
src/network/bridge_driver_platform.h | 2 +-
4 files changed, 58 insertions(+), 32 deletions(-)
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 369e80a889..aaf14defe4 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -273,7 +273,9 @@ static int
networkShutdownNetworkExternal(virNetworkObjPtr obj);
static void
-networkReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup);
+networkReloadFirewallRules(virNetworkDriverStatePtr driver,
+ bool startup,
+ bool force);
static void
networkRefreshDaemons(virNetworkDriverStatePtr driver);
@@ -689,7 +691,7 @@ firewalld_dbus_filter_bridge(DBusConnection *connection G_GNUC_UNUSED,
if (reload) {
VIR_DEBUG("Reload in bridge_driver because of firewalld.");
- networkReloadFirewallRules(driver, false);
+ networkReloadFirewallRules(driver, false, true);
}
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
@@ -798,7 +800,7 @@ networkStateInitialize(bool privileged,
virNetworkObjListPrune(network_driver->networks,
VIR_CONNECT_LIST_NETWORKS_INACTIVE |
VIR_CONNECT_LIST_NETWORKS_TRANSIENT);
- networkReloadFirewallRules(network_driver, true);
+ networkReloadFirewallRules(network_driver, true, false);
networkRefreshDaemons(network_driver);
if (virDriverShouldAutostart(network_driver->stateDir, &autostart) < 0)
@@ -868,7 +870,7 @@ networkStateReload(void)
network_driver->networkConfigDir,
network_driver->networkAutostartDir,
network_driver->xmlopt);
- networkReloadFirewallRules(network_driver, false);
+ networkReloadFirewallRules(network_driver, false, false);
networkRefreshDaemons(network_driver);
virNetworkObjListForEach(network_driver->networks,
networkAutostartConfig,
@@ -2236,14 +2238,16 @@ networkReloadFirewallRulesHelper(virNetworkObjPtr obj,
static void
-networkReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup)
+networkReloadFirewallRules(virNetworkDriverStatePtr driver,
+ bool startup,
+ bool force)
{
VIR_INFO("Reloading iptables rules");
/* Ideally we'd not even register the driver when unprivilegd
* but until we untangle the virt driver that's not viable */
if (!driver->privileged)
return;
- networkPreReloadFirewallRules(driver, startup);
+ networkPreReloadFirewallRules(driver, startup, force);
virNetworkObjListForEach(driver->networks,
networkReloadFirewallRulesHelper,
NULL);
diff --git a/src/network/bridge_driver_linux.c b/src/network/bridge_driver_linux.c
index 80bd2409e1..b0bd207250 100644
--- a/src/network/bridge_driver_linux.c
+++ b/src/network/bridge_driver_linux.c
@@ -36,11 +36,14 @@ VIR_LOG_INIT("network.bridge_driver_linux");
#define PROC_NET_ROUTE "/proc/net/route"
static virOnceControl createdOnce;
-static bool createdChains;
+static bool chainInitDone; /* true iff networkSetupPrivateChains was ever called */
+static bool createdChains; /* true iff networkSetupPrivateChains created chains during most recent call */
static virErrorPtr errInitV4;
static virErrorPtr errInitV6;
-/* Only call via virOnce */
+/* Usually only called via virOnce, but can also be called directly in
+ * response to firewalld reload (if chainInitDone == true)
+ */
static void networkSetupPrivateChains(void)
{
int rc;
@@ -82,6 +85,8 @@ static void networkSetupPrivateChains(void)
VIR_DEBUG("Global IPv6 chains already exist");
}
}
+
+ chainInitDone = true;
}
@@ -111,7 +116,10 @@ networkHasRunningNetworks(virNetworkDriverStatePtr driver)
}
-void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup)
+void
+networkPreReloadFirewallRules(virNetworkDriverStatePtr driver,
+ bool startup,
+ bool force)
{
/*
* If there are any running networks, we need to
@@ -130,29 +138,42 @@ void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup
* of starting the network though as that makes them
* more likely to be seen by a human
*/
- if (!networkHasRunningNetworks(driver)) {
- VIR_DEBUG("Delayed global rule setup as no networks are running");
- return;
- }
+ if (chainInitDone && force) {
+ /* The Private chains have already been initialized once
+ * during this run of libvirtd, so 1) we can't do it again via
+ * virOnce(), and 2) we need to re-add the private chains even
+ * if there are currently no running networks, because the
+ * next time a network is started, libvirt will expect that
+ * the chains have already been added. So we call directly
+ * instead of via virOnce().
+ */
+ networkSetupPrivateChains();
- ignore_value(virOnce(&createdOnce, networkSetupPrivateChains));
+ } else {
+ if (!networkHasRunningNetworks(driver)) {
+ VIR_DEBUG("Delayed global rule setup as no networks are running");
+ return;
+ }
- /*
- * If this is initial startup, and we just created the
- * top level private chains we either
- *
- * - upgraded from old libvirt
- * - freshly booted from clean state
- *
- * In the first case we must delete the old rules from
- * the built-in chains, instead of our new private chains.
- * In the second case it doesn't matter, since no existing
- * rules will be present. Thus we can safely just tell it
- * to always delete from the builin chain
- */
- if (startup && createdChains) {
- VIR_DEBUG("Requesting cleanup of legacy firewall rules");
- iptablesSetDeletePrivate(false);
+ ignore_value(virOnce(&createdOnce, networkSetupPrivateChains));
+
+ /*
+ * If this is initial startup, and we just created the
+ * top level private chains we either
+ *
+ * - upgraded from old libvirt
+ * - freshly booted from clean state
+ *
+ * In the first case we must delete the old rules from
+ * the built-in chains, instead of our new private chains.
+ * In the second case it doesn't matter, since no existing
+ * rules will be present. Thus we can safely just tell it
+ * to always delete from the builin chain
+ */
+ if (startup && createdChains) {
+ VIR_DEBUG("Requesting cleanup of legacy firewall rules");
+ iptablesSetDeletePrivate(false);
+ }
}
}
diff --git a/src/network/bridge_driver_nop.c b/src/network/bridge_driver_nop.c
index 08d737511f..db89c10023 100644
--- a/src/network/bridge_driver_nop.c
+++ b/src/network/bridge_driver_nop.c
@@ -20,7 +20,8 @@
#include <config.h>
void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver G_GNUC_UNUSED,
- bool startup G_GNUC_UNUSED)
+ bool startup G_GNUC_UNUSED,
+ bool force G_GNUC_UNUSED)
{
}
diff --git a/src/network/bridge_driver_platform.h b/src/network/bridge_driver_platform.h
index 169417a6c0..48ab52c160 100644
--- a/src/network/bridge_driver_platform.h
+++ b/src/network/bridge_driver_platform.h
@@ -62,7 +62,7 @@ struct _virNetworkDriverState {
typedef struct _virNetworkDriverState virNetworkDriverState;
typedef virNetworkDriverState *virNetworkDriverStatePtr;
-void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup);
+void networkPreReloadFirewallRules(virNetworkDriverStatePtr driver, bool startup, bool force);
void networkPostReloadFirewallRules(bool startup);
int networkCheckRouteCollision(virNetworkDefPtr def);
@@ -1,54 +0,0 @@
From df2cefb31dab2fa56e0864fbd2b8ad468dee22c0 Mon Sep 17 00:00:00 2001
Message-ID: <df2cefb31dab2fa56e0864fbd2b8ad468dee22c0.1724763718.git.crobinso@redhat.com>
In-Reply-To: <adfdb79f1e01401349e1321d0f5059d7b6489f00.1724763718.git.crobinso@redhat.com>
References: <adfdb79f1e01401349e1321d0f5059d7b6489f00.1724763718.git.crobinso@redhat.com>
From: Martin Kletzander <mkletzan@redhat.com>
Date: Fri, 16 Aug 2024 14:02:48 +0200
Subject: [PATCH 3/3] virarptable: End parsing earlier in case of NLMSG_DONE
Content-type: text/plain
Check for the last multipart message right as the first thing. The
presumption probably was that the last message might still contain a
payload we want to parse. However that cannot be true since that would
have to be a type RTM_NEWNEIGH. This was not caught because older
kernels were note sending NLMSG_DONE and probably relied on the fact
that the parsing just stops after all the messages are walked through,
which the NLMSG_OK macro successfully did.
Resolves: https://issues.redhat.com/browse/RHEL-52449
Resolves: https://bugzilla.redhat.com/2302245
Fixes: a176d67cdfaf5b8237a7e3a80d8be0e6bdf2d8fd
Signed-off-by: Martin Kletzander <mkletzan@redhat.com>
Reviewed-by: Laine Stump <laine@redhat.com>
Signed-off-by: Cole Robinson <crobinso@redhat.com>
---
src/util/virarptable.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/util/virarptable.c b/src/util/virarptable.c
index 45ee76766f..20d11f97b0 100644
--- a/src/util/virarptable.c
+++ b/src/util/virarptable.c
@@ -83,6 +83,9 @@ virArpTableGet(void)
struct ndmsg *r = NLMSG_DATA(nh);
void *addr;
+ if (nh->nlmsg_type == NLMSG_DONE)
+ break;
+
if (nh->nlmsg_len < NLMSG_SPACE(sizeof(*r))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("wrong nlmsg len"));
@@ -97,9 +100,6 @@ virArpTableGet(void)
(!(r->ndm_state == NUD_STALE || r->ndm_state == NUD_REACHABLE)))
continue;
- if (nh->nlmsg_type == NLMSG_DONE)
- return table;
-
VIR_WARNINGS_NO_CAST_ALIGN
parse_rtattr(tb, NDA_MAX, NDA_RTA(r), NLMSG_PAYLOAD(nh, sizeof(*r)));
VIR_WARNINGS_RESET
--
2.46.0
@@ -0,0 +1,100 @@
From: Laine Stump <laine@redhat.com>
Date: Fri, 1 May 2020 00:05:50 -0400
Subject: [PATCH] systemd: start libvirtd after firewalld/iptables services
When a system has enabled the iptables/ip6tables services rather than
firewalld, there is no explicit ordering of the start of those
services vs. libvirtd. This creates a problem when libvirtd.service is
started before ip[6]tables, as the latter, when it finally is started,
will remove all of the iptables rules that had previously been added
by libvirt, including the custom chains where libvirt's rules are
kept. This results in an error message similar to the following when a
user subsequently tries to start a new libvirt network:
"Error while activating network: Call to virNetworkCreate failed:
internal error: Failed to apply firewall rules
/usr/sbin/ip6tables -w --table filter --insert LIBVIRT_FWO \
--in-interface virbr2 --jump REJECT:
ip6tables: No chain/target/match by that name."
(Prior to logging this error, it also would have caused failure to
forward (or block) traffic in some cases, e.g. for guests on a NATed
network, since libvirt's rules to forward/block had all been deleted
and libvirt didn't know about it, so it couldn't fix the problem)
When this happens, the problem can be remedied by simply restarting
libvirtd.service (which has the side-effect of reloading all
libvirt-generated firewall rules)
Instead, we can just explicitly stating in the libvirtd.service file
that libvirtd.service should start after ip6tables.service and
ip6tables.service, eliminating the race condition that leads to the
error.
There is also nothing (that I can see) in the systemd .service files
to guarantee that firewalld.service will be started (if enabled) prior
to libvirtd.service. The same error scenario given above would occur
if libvirtd.service started before firewalld.service. Even before
that, though libvirtd would have detected that firewalld.service was
disabled, and then turn off all firewalld support. So, for example,
firewalld's libvirt zone wouldn't be used, and most likely traffic
from guests would therefore be blocked (all with no external
indication of the source of the problem other than a debug-level log
when libvirtd was started saying that firewalld wasn't in use); also
libvirtd wouldn't notice when firewalld reloaded its rules (which also
simultaneously deletes all of libvirt's rules).
I'm not aware of any reports that have been traced back to
libvirtd.service starting before firewalld.service, but have seen that
error reported multiple times, and also don't see an existing
dependency that would guarantee firewalld.service starts before
libvirtd.service, so it's possible it's been happening and we just
haven't gotten to the bottom of it.
This patch adds an After= line to the libvirtd.service file for each
of iptables.service, ip6tables.service, and firewalld.servicee, which
should guarantee that libvirtd.service isn't started until systemd has
started whichever of the others is enabled.
This race was diagnosed, and patch proposed, by Jason Montleon in
https://bugzilla.redhat.com/1723698 . At the time (April 2019) danpb
agreed with him that this change to libvirtd.service was a reasonable
thing to do, but I guess everyone thought someone else was going to
post a patch, so in the end nobody did.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 0756415f147dda15a417bd79eef9a62027d176e6)
---
src/network/virtnetworkd.service.in | 3 +++
src/remote/libvirtd.service.in | 3 +++
2 files changed, 6 insertions(+)
diff --git a/src/network/virtnetworkd.service.in b/src/network/virtnetworkd.service.in
index 656e8b4f84..56182e1693 100644
--- a/src/network/virtnetworkd.service.in
+++ b/src/network/virtnetworkd.service.in
@@ -5,6 +5,9 @@ Requires=virtnetworkd.socket
Requires=virtnetworkd-ro.socket
Requires=virtnetworkd-admin.socket
After=network.target
+After=firewalld.service
+After=iptables.service
+After=ip6tables.service
After=dbus.service
After=apparmor.service
After=local-fs.target
diff --git a/src/remote/libvirtd.service.in b/src/remote/libvirtd.service.in
index 90b2cad5b0..cc0d4e3693 100644
--- a/src/remote/libvirtd.service.in
+++ b/src/remote/libvirtd.service.in
@@ -11,6 +11,9 @@ Wants=libvirtd-admin.socket
Wants=systemd-machined.service
Before=libvirt-guests.service
After=network.target
+After=firewalld.service
+After=iptables.service
+After=ip6tables.service
After=dbus.service
After=iscsid.service
After=apparmor.service
@@ -0,0 +1,43 @@
From: Jim Fehlig <jfehlig@suse.com>
Date: Fri, 3 Apr 2020 15:51:48 -0600
Subject: [PATCH] libxl: fix crash when initializing driver
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Commit 54a401af478 split out DriverConfigInit from DriverConfigNew, but
then called it a bit late from libxlStateInitialize. The cfg is used in
libxlDriverConfigLoadFile and when uninitialized results in a crash.
Calling DriverConfigInit immediately after DriverConfigNew fixes the
crash.
Signed-off-by: Jim Fehlig <jfehlig@suse.com>
Reviewed-by: Erik Skultety <eskultet@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
(cherry picked from commit 88011ed280c4f946a7b8e7ffcea2335eb075de60)
---
src/libxl/libxl_driver.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index f2387e2a20..c4fb791fa0 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -703,14 +703,14 @@ libxlStateInitialize(bool privileged,
if (!(cfg = libxlDriverConfigNew()))
goto error;
+ if (libxlDriverConfigInit(cfg) < 0)
+ goto error;
+
driverConf = g_strdup_printf("%s/libxl.conf", cfg->configBaseDir);
if (libxlDriverConfigLoadFile(cfg, driverConf) < 0)
goto error;
- if (libxlDriverConfigInit(cfg) < 0)
- goto error;
-
/* Register the callbacks providing access to libvirt's event loop */
libxl_osevent_register_hooks(cfg->ctx, &libxl_osevent_callbacks, cfg->ctx);
-31
View File
@@ -1,31 +0,0 @@
From 4c5b2e1e0d0d0cbbf8c6ed28ce77d055d5974f7f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A1n=20Tomko?= <jtomko@redhat.com>
Date: Wed, 6 Mar 2024 17:26:40 +0100
Subject: [PATCH] qemu: virtiofs: set correct label when creating the socket
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Use svirt_t instead of virtd_t, since virtd_t is not available in the
session mode and qemu with svirt_t won't be able to talk to unconfined_t
socket.
Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
---
src/qemu/qemu_virtiofs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/qemu/qemu_virtiofs.c b/src/qemu/qemu_virtiofs.c
index 15dea3bb57f..d80cddd3ba9 100644
--- a/src/qemu/qemu_virtiofs.c
+++ b/src/qemu/qemu_virtiofs.c
@@ -102,7 +102,7 @@ qemuVirtioFSOpenChardev(virQEMUDriver *driver,
chrdev->data.nix.listen = true;
chrdev->data.nix.path = g_strdup(socket_path);
- if (qemuSecuritySetDaemonSocketLabel(driver->securityManager, vm->def) < 0)
+ if (qemuSecuritySetSocketLabel(driver->securityManager, vm->def) < 0)
goto cleanup;
fd = qemuOpenChrChardevUNIXSocket(chrdev);
if (fd < 0) {
+929 -1651
View File
File diff suppressed because it is too large Load Diff
+1 -1
View File
@@ -1 +1 @@
SHA512 (libvirt-10.1.0.tar.xz) = 08e73ae15de5681430b62db85ec9901242dca5e9a4ca9685614f4a67092c6e28f27f9187144b3ceb18ad6b40e6eb1a90b1a4b056b0888724d04a62002ee2bc48
SHA512 (libvirt-6.1.0.tar.xz) = 17a2641f300a4a05149261bae74ac856e9a2511a259146595d2e2412c4a0601d88369b0544ba86edc80e433a47cf828317d8de38c6ec86a1b3efaca75294a606