Compare commits

..

5 Commits

Author SHA1 Message Date
Daniel P. Berrangé 5ff9ee11bb Rebuild for wireshark soname change rhbz#2031322
Now with missing patches

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-12-13 15:33:14 +00:00
Daniel P. Berrangé a5abe2ec98 Rebuild for wireshark soname change rhbz#2031322
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
2021-12-13 15:21:21 +00:00
Cole Robinson 3e09e1c917 libvirt-7.0.0-7
More CAP_SETPCAP warning fixes (bz #1924218)
Handle unknown firmware.json errors
CVE-2021-3667: Fix deadlock on virStoragePoolLookupByTargetPath failure (bz #1986113)
2021-07-27 14:18:27 -04:00
Cole Robinson aa95d82b58 libvirt-7.0.0-6
CVE-2021-3631 libvirt: insecure sVirt label generation (bz #1977760)
2021-07-02 16:32:25 -04:00
Cole Robinson 476c3411a3 libvirt-7.0.0-5
Crash in udev driver populate_vendor (bz #1966851)
Fix CAP_SETPCAP syslog warning (bz #1924218)
2021-06-29 09:51:22 -04:00
18 changed files with 1254 additions and 1847 deletions
@@ -0,0 +1,56 @@
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(),
@@ -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
@@ -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,72 @@
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);
@@ -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,53 @@
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,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,51 @@
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,43 @@
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)
@@ -0,0 +1,57 @@
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;
}
@@ -0,0 +1,33 @@
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);
@@ -0,0 +1,81 @@
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
@@ -0,0 +1,33 @@
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
-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) {
+774 -1495
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-7.0.0.tar.xz) = dd6db5ec4971cf4c6059795fd81d5a3a889b10740e34c3c92271eda1c683c99df2c8f923398065d8a7c4f987a20eb1da617d5297ba8ea5a31f154412af50c343