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
16 changed files with 839 additions and 820 deletions
@@ -1,56 +0,0 @@
From: Laine Stump <laine@redhat.com>
Date: Thu, 21 Jan 2021 16:01:06 -0500
Subject: [PATCH] build: support explicitly disabling netcf
placing "-Dnetcf=disabled" on the meson commandline was ignored,
meaning that even with that option the build would get WITH_NETCF if
the netcf-devel package was found - the only way to disable it was to
uninstall netcf-devel.
This patch adds the small bit of logic to check the netcf meson
commandline option (in addition to whether netcf-devel is installed)
before defining WITH_NETCF.
Signed-off-by: Laine Stump <laine@redhat.com>
Reviewed-by: Neal Gompa <ngompa13@gmail.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 06169a115d46d8870a96d293c2faf6ea87e71020)
---
meson.build | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/meson.build b/meson.build
index b5164f68ed..e9d6d9f82e 100644
--- a/meson.build
+++ b/meson.build
@@ -1155,8 +1155,10 @@ libm_dep = cc.find_library('m', required : false)
netcf_version = '0.1.8'
netcf_dep = dependency('netcf', version: '>=' + netcf_version, required: get_option('netcf'))
-if netcf_dep.found()
- conf.set('WITH_NETCF', 1)
+if not get_option('netcf').disabled()
+ if netcf_dep.found()
+ conf.set('WITH_NETCF', 1)
+ endif
endif
have_gnu_gettext_tools = false
@@ -1550,7 +1552,7 @@ elif get_option('driver_hyperv').enabled()
error('openwsman is required for the Hyper-V driver')
endif
-if not get_option('driver_interface').disabled() and conf.has('WITH_LIBVIRTD') and (udev_dep.found() or netcf_dep.found())
+if not get_option('driver_interface').disabled() and conf.has('WITH_LIBVIRTD') and (udev_dep.found() or conf.has('WITH_NETCF'))
conf.set('WITH_INTERFACE', 1)
elif get_option('driver_interface').enabled()
error('Requested the Interface driver without netcf or udev and libvirtd support')
@@ -2362,7 +2364,7 @@ libs_summary = {
'libssh': libssh_dep.found(),
'libssh2': libssh2_dep.found(),
'libutil': libutil_dep.found(),
- 'netcf': netcf_dep.found(),
+ 'netcf': conf.has('WITH_NETCF'),
'NLS': have_gnu_gettext_tools,
'numactl': numactl_dep.found(),
'openwsman': openwsman_dep.found(),
@@ -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"));
@@ -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,72 +0,0 @@
From: wangjian <wangjian161@huawei.com>
Date: Fri, 26 Mar 2021 11:21:16 +0800
Subject: [PATCH] node_device_udev: Serialize access to pci_get_strings)_
Since the functions provided by libpciaccess are not thread-safe,
when the udev-event and nodedev-init threads of libvirt call the
pci_get_strings function provided by libpaciaccess at the same
time the following can happen:
nodedev-init thread:
nodeStateInitializeEnumerate ->
udevEnumerateDevices->
udevProcessDeviceListEntry ->
udevAddOneDevice ->
udevGetDeviceDetails->
udevProcessPCI ->
udevTranslatePCIIds ->
pci_get_strings -> (libpciaccess)
find_device_name ->
populate_vendor ->
d = realloc( vend->devices, (vend->num_devices + 1), * sizeof( struct pci_device_leaf ) );
vend->num_devices++;
udev-event thread:
udevEventHandleThread ->
udevHandleOneDevice ->
udevAddOneDevice->
udevGetDeviceDetails->
udevProcessPCI ->
udevTranslatePCIIds ->
pci_get_strings -> (libpciaccess)
find_device_name ->
populate_vendor ->
d = realloc( vend->devices, (vend->num_devices + 1), * sizeof( struct pci_device_leaf ) );
vend->num_devices++;
Signed-off-by: WangJian <wangjian161@huawei.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 59788a5caea5f292c86e07a31ee2b853d68db87e)
---
src/node_device/node_device_udev.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
index 55a2731681..6f0defe908 100644
--- a/src/node_device/node_device_udev.c
+++ b/src/node_device/node_device_udev.c
@@ -328,6 +328,7 @@ udevGenerateDeviceName(struct udev_device *device,
return 0;
}
+static virMutex pciaccessMutex = VIR_MUTEX_INITIALIZER;
static int
udevTranslatePCIIds(unsigned int vendor,
@@ -346,12 +347,14 @@ udevTranslatePCIIds(unsigned int vendor,
m.device_class_mask = 0;
m.match_data = 0;
- /* pci_get_strings returns void */
+ /* pci_get_strings returns void and unfortunately is not thread safe. */
+ virMutexLock(&pciaccessMutex);
pci_get_strings(&m,
&device_name,
&vendor_name,
NULL,
NULL);
+ virMutexUnlock(&pciaccessMutex);
*vendor_string = g_strdup(vendor_name);
*product_string = g_strdup(device_name);
@@ -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,53 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Thu, 24 Jun 2021 16:58:09 +0200
Subject: [PATCH] virSetUIDGIDWithCaps: Don't drop CAP_SETPCAP right away
There are few cases where we execute a virCommand with all caps
cleared (virCommandClearCaps()). For instance
dnsmasqCapsRefreshInternal() does just that. This means, that
after fork() and before exec() the virSetUIDGIDWithCaps() is
called. But since the caller did not want to change anything,
just drop capabilities, these are the values of arguments:
virSetUIDGIDWithCaps (uid=-1, gid=-1, groups=0x0, ngroups=0,
capBits=0, clearExistingCaps=true)
This means that indeed all capabilities will be dropped,
including CAP_SETPCAP. But this capability controls whether
capabilities can be set, IOW whether capng_apply() succeeds.
There are two calls of capng_apply() in the function. The
CAP_SETPCAP is dropped after the first call and thus the other
call (capng_apply(CAPNG_SELECT_BOUNDS);) fails.
The solution is to keep the capability for as long as needed
(just like CAP_SETGID and CAP_SETUID) and drop it only at the
very end (just like CAP_SETGID and CAP_SETUID).
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1949388
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Martin Kletzander <mkletzan@redhat.com>
(cherry picked from commit 438b50dda8a863fdc988e9ab612f097cc1626e8a)
---
src/util/virutil.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/src/util/virutil.c b/src/util/virutil.c
index a0cd0f1bcd..7ae23a7061 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1202,12 +1202,10 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t *groups, int ngroups,
}
# ifdef PR_CAPBSET_DROP
/* If newer kernel, we need also need setpcap to change the bounding set */
- if ((capBits || need_setgid || need_setuid) &&
- !capng_have_capability(CAPNG_EFFECTIVE, CAP_SETPCAP)) {
+ if (!capng_have_capability(CAPNG_EFFECTIVE, CAP_SETPCAP)) {
need_setpcap = true;
- }
- if (need_setpcap)
capng_update(CAPNG_ADD, CAPNG_EFFECTIVE|CAPNG_PERMITTED, CAP_SETPCAP);
+ }
# endif
/* Tell system we want to keep caps across uid change */
@@ -1,51 +0,0 @@
From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= <berrange@redhat.com>
Date: Mon, 28 Jun 2021 13:09:04 +0100
Subject: [PATCH] security: fix SELinux label generation logic
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
A process can access a file if the set of MCS categories
for the file is equal-to *or* a subset-of, the set of
MCS categories for the process.
If there are two VMs:
a) svirt_t:s0:c117
b) svirt_t:s0:c117,c720
Then VM (b) is able to access files labelled for VM (a).
IOW, we must discard case where the categories are equal
because that is a subset of many other valid category pairs.
Fixes: https://gitlab.com/libvirt/libvirt/-/issues/153
CVE-2021-3631
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 15073504dbb624d3f6c911e85557019d3620fdb2)
---
src/security/security_selinux.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/src/security/security_selinux.c b/src/security/security_selinux.c
index 2fc6ef2616..61a871ec3d 100644
--- a/src/security/security_selinux.c
+++ b/src/security/security_selinux.c
@@ -389,7 +389,15 @@ virSecuritySELinuxMCSFind(virSecurityManagerPtr mgr,
VIR_DEBUG("Try cat %s:c%d,c%d", sens, c1 + catMin, c2 + catMin);
if (c1 == c2) {
- mcs = g_strdup_printf("%s:c%d", sens, catMin + c1);
+ /*
+ * A process can access a file if the set of MCS categories
+ * for the file is equal-to *or* a subset-of, the set of
+ * MCS categories for the process.
+ *
+ * IOW, we must discard case where the categories are equal
+ * because that is a subset of other category pairs.
+ */
+ continue;
} else {
if (c1 > c2) {
int t = c1;
@@ -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);
@@ -1,43 +0,0 @@
From: Michal Privoznik <mprivozn@redhat.com>
Date: Thu, 22 Jul 2021 14:26:00 +0200
Subject: [PATCH] virSetUIDGIDWithCaps: Set bounding capabilities only with
CAP_SETPCAP
In one of my previous patches I've tried to postpone dropping
CAP_SETPCAP until the very end because it's needed for
capng_apply(). What I did not realize back then was that we might
not have the capability to begin with. Because of unknown reasons
capng_apply() pollutes logs only for CAPNG_SELECT_BOUNDS and not
for CAPNG_SELECT_CAPS.
Reproducer is really simple: run libvirtd as a regular user.
During its initialization, libvirtd will spawn some binaries
(dnsmasq, qemu-*, etc.) and while doing so it will try to drop
capabilities.
Anyway, let's call capng_apply(CAPNG_SELECT_BOUNDS) only if we
have the CAP_SETPCAP (which is tracked in need_setpcap variable).
Fixes: 438b50dda8a863fdc988e9ab612f097cc1626e8a
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1924218
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
(cherry picked from commit a2476f37a7789eb9315b77bb451f4754ef4ef15b)
---
src/util/virutil.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/src/util/virutil.c b/src/util/virutil.c
index 7ae23a7061..333f99e91d 100644
--- a/src/util/virutil.c
+++ b/src/util/virutil.c
@@ -1269,7 +1269,8 @@ virSetUIDGIDWithCaps(uid_t uid, gid_t gid, gid_t *groups, int ngroups,
* do this if we failed to get the capability above, so ignore the
* return value.
*/
- capng_apply(CAPNG_SELECT_BOUNDS);
+ if (!need_setpcap)
+ capng_apply(CAPNG_SELECT_BOUNDS);
/* Drop the caps that allow setuid/gid (unless they were requested) */
if (need_setgid)
@@ -1,57 +0,0 @@
From: Pavel Hrdina <phrdina@redhat.com>
Date: Mon, 10 May 2021 15:07:09 +0200
Subject: [PATCH] qemu_firmware: don't error out for unknown firmware features
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
When QEMU introduces new firmware features libvirt will fail until we
list that feature in our code as well which doesn't sound right.
We should simply ignore the new feature until we add a proper support
for it.
Reported-by: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Pavel Hrdina <phrdina@redhat.com>
Reviewed-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
(cherry picked from commit 61d95a1073833ec4323c1ef28e71e913c55aa7b9)
---
src/qemu/qemu_firmware.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/src/qemu/qemu_firmware.c b/src/qemu/qemu_firmware.c
index 639cff7459..e602de22e3 100644
--- a/src/qemu/qemu_firmware.c
+++ b/src/qemu/qemu_firmware.c
@@ -573,6 +573,7 @@ qemuFirmwareFeatureParse(const char *path,
virJSONValuePtr featuresJSON;
g_autoptr(qemuFirmwareFeature) features = NULL;
size_t nfeatures;
+ size_t nparsed = 0;
size_t i;
if (!(featuresJSON = virJSONValueObjectGetArray(doc, "features"))) {
@@ -592,17 +593,16 @@ qemuFirmwareFeatureParse(const char *path,
int tmp;
if ((tmp = qemuFirmwareFeatureTypeFromString(tmpStr)) <= 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("unknown feature %s"),
- tmpStr);
- return -1;
+ VIR_DEBUG("ignoring unknown QEMU firmware feature '%s'", tmpStr);
+ continue;
}
- features[i] = tmp;
+ features[nparsed] = tmp;
+ nparsed++;
}
fw->features = g_steal_pointer(&features);
- fw->nfeatures = nfeatures;
+ fw->nfeatures = nparsed;
return 0;
}
@@ -1,33 +0,0 @@
From: Peter Krempa <pkrempa@redhat.com>
Date: Wed, 21 Jul 2021 11:22:25 +0200
Subject: [PATCH] storage_driver: Unlock object on ACL fail in
storagePoolLookupByTargetPath
'virStoragePoolObjListSearch' returns a locked and refed object, thus we
must release it on ACL permission failure.
Fixes: 7aa0e8c0cb8
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1984318
Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 447f69dec47e1b0bd15ecd7cd49a9fd3b050fb87)
---
src/storage/storage_driver.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 16bc53aa46..2787c1671b 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -1739,8 +1739,10 @@ storagePoolLookupByTargetPath(virConnectPtr conn,
storagePoolLookupByTargetPathCallback,
cleanpath))) {
def = virStoragePoolObjGetDef(obj);
- if (virStoragePoolLookupByTargetPathEnsureACL(conn, def) < 0)
+ if (virStoragePoolLookupByTargetPathEnsureACL(conn, def) < 0) {
+ virStoragePoolObjEndAPI(&obj);
return NULL;
+ }
pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL);
virStoragePoolObjEndAPI(&obj);
@@ -1,81 +0,0 @@
From 7e299ba649b1288d529c7595c0e6060c9ae0ff2a Mon Sep 17 00:00:00 2001
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 29 Nov 2021 09:57:49 +0100
Subject: [PATCH 1/2] wireshark: Switch to tvb_bytes_to_str()
When the dissector sees a byte sequence that is either an opaque
data (xdr_opaque) or a byte sequence (xdr_bytes) it formats the
bytes as a hex numbers using our own implementation. But
wireshark already provides a function for it: tvb_bytes_to_str().
NB, the reason why it returns a const string is so that callers
don't try to free it - the string is allocated using an allocator
which will decide when to free it.
The wireshark formatter was introduced in wireshark commit of
v1.99.2~479 and thus is present in the version we require at
least (2.6.0).
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
---
tools/wireshark/src/packet-libvirt.c | 30 ++++++++--------------------
1 file changed, 8 insertions(+), 22 deletions(-)
diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c
index f43919b05d..cb922b8070 100644
--- a/tools/wireshark/src/packet-libvirt.c
+++ b/tools/wireshark/src/packet-libvirt.c
@@ -158,24 +158,6 @@ dissect_xdr_string(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf,
}
}
-static const gchar *
-format_xdr_bytes(guint8 *bytes, guint32 length)
-{
- gchar *buf;
- guint32 i;
-
- if (length == 0)
- return "";
- buf = wmem_alloc(wmem_packet_scope(), length*2 + 1);
- for (i = 0; i < length; i++) {
- /* We know that buf has enough size to contain
- 2 * length + '\0' characters. */
- g_snprintf(buf, 2*(length - i) + 1, "%02x", bytes[i]);
- buf += 2;
- }
- return buf - length*2;
-}
-
static gboolean
dissect_xdr_opaque(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf,
guint32 size)
@@ -187,8 +169,10 @@ dissect_xdr_opaque(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf,
val = g_malloc(size);
start = xdr_getpos(xdrs);
if ((rc = xdr_opaque(xdrs, (caddr_t)val, size))) {
- proto_tree_add_bytes_format_value(tree, hf, tvb, start, xdr_getpos(xdrs) - start,
- NULL, "%s", format_xdr_bytes(val, size));
+ gint len = xdr_getpos(xdrs) - start;
+ const char *s = tvb_bytes_to_str(wmem_packet_scope(), tvb, start, len);
+
+ proto_tree_add_bytes_format_value(tree, hf, tvb, start, len, NULL, "%s", s);
} else {
proto_tree_add_item(tree, hf_libvirt_unknown, tvb, start, -1, ENC_NA);
}
@@ -207,8 +191,10 @@ dissect_xdr_bytes(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf,
start = xdr_getpos(xdrs);
if (xdr_bytes(xdrs, (char **)&val, &length, maxlen)) {
- proto_tree_add_bytes_format_value(tree, hf, tvb, start, xdr_getpos(xdrs) - start,
- NULL, "%s", format_xdr_bytes(val, length));
+ gint len = xdr_getpos(xdrs) - start;
+ const char *s = tvb_bytes_to_str(wmem_packet_scope(), tvb, start, len);
+
+ proto_tree_add_bytes_format_value(tree, hf, tvb, start, len, NULL, "%s", s);
/* Seems I can't call xdr_free() for this case.
It will raises SEGV by referencing out of bounds call stack */
free(val);
--
2.33.1
@@ -1,33 +0,0 @@
From 010613cfd8dae6d85602a84c5c95b2d441e1b3d1 Mon Sep 17 00:00:00 2001
From: Michal Privoznik <mprivozn@redhat.com>
Date: Mon, 29 Nov 2021 10:20:05 +0100
Subject: [PATCH 2/2] wireshark: Drop needless comment in dissect_xdr_bytes()
In the dissect_xdr_bytes() there's a comment that the string
allocated by xdr_bytes() can't be freed using xdr_free(). Well,
that is expected because xdr_bytes() used plain calloc() AND the
string is not an XDR struct but plain 'char *' type. Passing it
to xdr_free() must result in weird things happening.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
---
tools/wireshark/src/packet-libvirt.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/wireshark/src/packet-libvirt.c b/tools/wireshark/src/packet-libvirt.c
index cb922b8070..eeacbcdf0e 100644
--- a/tools/wireshark/src/packet-libvirt.c
+++ b/tools/wireshark/src/packet-libvirt.c
@@ -195,8 +195,6 @@ dissect_xdr_bytes(tvbuff_t *tvb, proto_tree *tree, XDR *xdrs, int hf,
const char *s = tvb_bytes_to_str(wmem_packet_scope(), tvb, start, len);
proto_tree_add_bytes_format_value(tree, hf, tvb, start, len, NULL, "%s", s);
- /* Seems I can't call xdr_free() for this case.
- It will raises SEGV by referencing out of bounds call stack */
free(val);
return TRUE;
} else {
--
2.33.1
+339 -340
View File
File diff suppressed because it is too large Load Diff
+1 -1
View File
@@ -1 +1 @@
SHA512 (libvirt-7.0.0.tar.xz) = dd6db5ec4971cf4c6059795fd81d5a3a889b10740e34c3c92271eda1c683c99df2c8f923398065d8a7c4f987a20eb1da617d5297ba8ea5a31f154412af50c343
SHA512 (libvirt-6.1.0.tar.xz) = 17a2641f300a4a05149261bae74ac856e9a2511a259146595d2e2412c4a0601d88369b0544ba86edc80e433a47cf828317d8de38c6ec86a1b3efaca75294a606