diff --git a/interface-fix-udev_device_get_sysattr_value-return-v.patch b/interface-fix-udev_device_get_sysattr_value-return-v.patch new file mode 100644 index 0000000000000000000000000000000000000000..6747c8105dd23c9691876b607620264c01f5709e --- /dev/null +++ b/interface-fix-udev_device_get_sysattr_value-return-v.patch @@ -0,0 +1,89 @@ +From 0d5c04534f0e041e9923da1cb37a431ae0d463a8 Mon Sep 17 00:00:00 2001 +From: Dmitry Frolov +Date: Tue, 12 Sep 2023 15:56:47 +0300 +Subject: [PATCH 8/8] interface: fix udev_device_get_sysattr_value return value + check + +Reviewing the code I found that return value of function +udev_device_get_sysattr_value() is dereferenced without a check. +udev_device_get_sysattr_value() may return NULL by number of reasons. + +v2: VIR_DEBUG added, replaced STREQ(NULLSTR()) with STREQ_NULLABLE() +v3: More checks added, to skip earlier. More verbose VIR_DEBUG. + +Signed-off-by: Dmitry Frolov +Reviewed-by: Martin Kletzander +--- + src/interface/interface_backend_udev.c | 26 +++++++++++++++++++------- + 1 file changed, 19 insertions(+), 7 deletions(-) + +diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c +index dde88860d3..00eeee6cc4 100644 +--- a/src/interface/interface_backend_udev.c ++++ b/src/interface/interface_backend_udev.c +@@ -23,6 +23,7 @@ + #include + #include + ++#include "virlog.h" + #include "virerror.h" + #include "virfile.h" + #include "datatypes.h" +@@ -41,6 +42,8 @@ + + #define VIR_FROM_THIS VIR_FROM_INTERFACE + ++VIR_LOG_INIT("interface.interface_backend_udev"); ++ + struct udev_iface_driver { + struct udev *udev; + /* pid file FD, ensures two copies of the driver can't use the same root */ +@@ -371,11 +374,20 @@ udevConnectListAllInterfaces(virConnectPtr conn, + const char *macaddr; + virInterfaceDefPtr def; + +- path = udev_list_entry_get_name(dev_entry); +- dev = udev_device_new_from_syspath(udev, path); +- name = udev_device_get_sysname(dev); ++ if (!(path = udev_list_entry_get_name(dev_entry))) { ++ VIR_DEBUG("Skipping interface, path == NULL"); ++ continue; ++ } ++ if (!(dev = udev_device_new_from_syspath(udev, path))) { ++ VIR_DEBUG("Skipping interface '%s', dev == NULL", path); ++ continue; ++ } ++ if (!(name = udev_device_get_sysname(dev))) { ++ VIR_DEBUG("Skipping interface '%s', name == NULL", path); ++ continue; ++ } + macaddr = udev_device_get_sysattr_value(dev, "address"); +- status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up"); ++ status = STREQ_NULLABLE(udev_device_get_sysattr_value(dev, "operstate"), "up"); + + def = udevGetMinimalDefForDevice(dev); + if (!virConnectListAllInterfacesCheckACL(conn, def)) { +@@ -1000,9 +1012,9 @@ udevGetIfaceDef(struct udev *udev, const char *name) + + /* MTU */ + mtu_str = udev_device_get_sysattr_value(dev, "mtu"); +- if (virStrToLong_ui(mtu_str, NULL, 10, &mtu) < 0) { ++ if (!mtu_str || virStrToLong_ui(mtu_str, NULL, 10, &mtu) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, +- _("Could not parse MTU value '%s'"), mtu_str); ++ _("Could not parse MTU value '%s'"), NULLSTR(mtu_str)); + goto error; + } + ifacedef->mtu = mtu; +@@ -1129,7 +1141,7 @@ udevInterfaceIsActive(virInterfacePtr ifinfo) + goto cleanup; + + /* Check if it's active or not */ +- status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up"); ++ status = STREQ_NULLABLE(udev_device_get_sysattr_value(dev, "operstate"), "up"); + + udev_device_unref(dev); + +-- +2.27.0 + diff --git a/libvirt.spec b/libvirt.spec index 23b97b7fa90222d47f6fd8b96141172b9565f0cd..a59a0da378418440447acebe48a511571816ed68 100644 --- a/libvirt.spec +++ b/libvirt.spec @@ -101,7 +101,7 @@ Summary: Library providing a simple virtualization API Name: libvirt Version: 6.2.0 -Release: 21 +Release: 24 License: LGPLv2+ URL: https://libvirt.org/ @@ -156,6 +156,9 @@ Patch0043: virsh-Display-vhostuser-socket-path-in-domblklist.patch Patch0044: nwfilter-fix-crash-when-counting-number-of-network-f.patch Patch0045: qemu-Add-missing-lock-in-qemuProcessHandleMonitorEOF.patch Patch0046: update-the-Chinese-translation-of-nwfilter.patch +Patch0047: virsh-Fix-off-by-one-error-in-udevListInterfacesBySt.patch +Patch0048: remote-check-for-negative-array-lengths-before-alloc.patch +Patch0049: interface-fix-udev_device_get_sysattr_value-return-v.patch Requires: libvirt-daemon = %{version}-%{release} Requires: libvirt-daemon-config-network = %{version}-%{release} @@ -1890,6 +1893,15 @@ exit 0 %changelog +* Wed Apr 10 2024 caozhongwang +- interface: fix udev_device_get_sysattr_value return value check (CVE-2024-2496) + +* Wed Apr 10 2024 caozhongwang +- remote: check for negative array lengths before allocation (CVE-2024-2494) + +* Wed Apr 10 2024 caozhongwang +- vish:Fix off-by-one error in udevListInterfacesByStatus (CVE-2024-1441) + * Sat Dec 10 2022 yezengruan - update the Chinese translation of nwfilter diff --git a/remote-check-for-negative-array-lengths-before-alloc.patch b/remote-check-for-negative-array-lengths-before-alloc.patch new file mode 100644 index 0000000000000000000000000000000000000000..b8b2a09921d334cd19bbd28808e5a3434d4fe505 --- /dev/null +++ b/remote-check-for-negative-array-lengths-before-alloc.patch @@ -0,0 +1,216 @@ +From e8245ba93fe1d7d345d10ff9a189b6f5188682b1 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= +Date: Fri, 15 Mar 2024 10:47:50 +0000 +Subject: [PATCH 7/8] remote: check for negative array lengths before + allocation +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +While the C API entry points will validate non-negative lengths +for various parameters, the RPC server de-serialization code +will need to allocate memory for arrays before entering the C +API. These allocations will thus happen before the non-negative +length check is performed. + +Passing a negative length to the g_new0 function will usually +result in a crash due to the negative length being treated as +a huge positive number. + +This was found and diagnosed by ALT Linux Team with AFLplusplus. + +Reviewed-by: Michal Privoznik +Found-by: Alexandr Shashkin +Co-developed-by: Alexander Kuznetsov +Signed-off-by: Daniel P. Berrangé +--- + src/remote/remote_daemon_dispatch.c | 65 +++++++++++++++++++++++++++++ + src/rpc/gendispatch.pl | 5 +++ + 2 files changed, 70 insertions(+) + +diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c +index c5506c2e11..5a1a317452 100644 +--- a/src/remote/remote_daemon_dispatch.c ++++ b/src/remote/remote_daemon_dispatch.c +@@ -2307,6 +2307,10 @@ remoteDispatchDomainGetSchedulerParameters(virNetServerPtr server G_GNUC_UNUSED, + if (!conn) + goto cleanup; + ++ if (args->nparams < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative")); ++ goto cleanup; ++ } + if (args->nparams > REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; +@@ -2355,6 +2359,10 @@ remoteDispatchDomainGetSchedulerParametersFlags(virNetServerPtr server G_GNUC_UN + if (!conn) + goto cleanup; + ++ if (args->nparams < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative")); ++ goto cleanup; ++ } + if (args->nparams > REMOTE_DOMAIN_SCHEDULER_PARAMETERS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; +@@ -2516,6 +2524,10 @@ remoteDispatchDomainBlockStatsFlags(virNetServerPtr server G_GNUC_UNUSED, + goto cleanup; + flags = args->flags; + ++ if (args->nparams < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative")); ++ goto cleanup; ++ } + if (args->nparams > REMOTE_DOMAIN_BLOCK_STATS_PARAMETERS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; +@@ -2750,6 +2762,14 @@ remoteDispatchDomainGetVcpuPinInfo(virNetServerPtr server G_GNUC_UNUSED, + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + ++ if (args->ncpumaps < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("ncpumaps must be non-negative")); ++ goto cleanup; ++ } ++ if (args->maplen < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("maplen must be non-negative")); ++ goto cleanup; ++ } + if (args->ncpumaps > REMOTE_VCPUINFO_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("ncpumaps > REMOTE_VCPUINFO_MAX")); + goto cleanup; +@@ -2845,6 +2865,11 @@ remoteDispatchDomainGetEmulatorPinInfo(virNetServerPtr server G_GNUC_UNUSED, + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + ++ if (args->maplen < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("maplen must be non-negative")); ++ goto cleanup; ++ } ++ + /* Allocate buffers to take the results */ + if (args->maplen > 0 && + VIR_ALLOC_N(cpumaps, args->maplen) < 0) +@@ -2893,6 +2918,14 @@ remoteDispatchDomainGetVcpus(virNetServerPtr server G_GNUC_UNUSED, + if (!(dom = get_nonnull_domain(conn, args->dom))) + goto cleanup; + ++ if (args->maxinfo < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo must be non-negative")); ++ goto cleanup; ++ } ++ if (args->maplen < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo must be non-negative")); ++ goto cleanup; ++ } + if (args->maxinfo > REMOTE_VCPUINFO_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo > REMOTE_VCPUINFO_MAX")); + goto cleanup; +@@ -3140,6 +3173,10 @@ remoteDispatchDomainGetMemoryParameters(virNetServerPtr server G_GNUC_UNUSED, + + flags = args->flags; + ++ if (args->nparams < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative")); ++ goto cleanup; ++ } + if (args->nparams > REMOTE_DOMAIN_MEMORY_PARAMETERS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; +@@ -3200,6 +3237,10 @@ remoteDispatchDomainGetNumaParameters(virNetServerPtr server G_GNUC_UNUSED, + + flags = args->flags; + ++ if (args->nparams < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative")); ++ goto cleanup; ++ } + if (args->nparams > REMOTE_DOMAIN_NUMA_PARAMETERS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; +@@ -3260,6 +3301,10 @@ remoteDispatchDomainGetBlkioParameters(virNetServerPtr server G_GNUC_UNUSED, + + flags = args->flags; + ++ if (args->nparams < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative")); ++ goto cleanup; ++ } + if (args->nparams > REMOTE_DOMAIN_BLKIO_PARAMETERS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; +@@ -3321,6 +3366,10 @@ remoteDispatchNodeGetCPUStats(virNetServerPtr server G_GNUC_UNUSED, + + flags = args->flags; + ++ if (args->nparams < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative")); ++ goto cleanup; ++ } + if (args->nparams > REMOTE_NODE_CPU_STATS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; +@@ -3389,6 +3438,10 @@ remoteDispatchNodeGetMemoryStats(virNetServerPtr server G_GNUC_UNUSED, + + flags = args->flags; + ++ if (args->nparams < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative")); ++ goto cleanup; ++ } + if (args->nparams > REMOTE_NODE_MEMORY_STATS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; +@@ -3570,6 +3623,10 @@ remoteDispatchDomainGetBlockIoTune(virNetServerPtr server G_GNUC_UNUSED, + if (!conn) + goto cleanup; + ++ if (args->nparams < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative")); ++ goto cleanup; ++ } + if (args->nparams > REMOTE_DOMAIN_BLOCK_IO_TUNE_PARAMETERS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; +@@ -5128,6 +5185,10 @@ remoteDispatchDomainGetInterfaceParameters(virNetServerPtr server G_GNUC_UNUSED, + + flags = args->flags; + ++ if (args->nparams < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative")); ++ goto cleanup; ++ } + if (args->nparams > REMOTE_DOMAIN_INTERFACE_PARAMETERS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; +@@ -5350,6 +5411,10 @@ remoteDispatchNodeGetMemoryParameters(virNetServerPtr server G_GNUC_UNUSED, + + flags = args->flags; + ++ if (args->nparams < 0) { ++ virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams must be non-negative")); ++ goto cleanup; ++ } + if (args->nparams > REMOTE_NODE_MEMORY_PARAMETERS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("nparams too large")); + goto cleanup; +diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl +index 590a46ef66..02612f1856 100755 +--- a/src/rpc/gendispatch.pl ++++ b/src/rpc/gendispatch.pl +@@ -1074,6 +1074,11 @@ elsif ($mode eq "server") { + print "\n"; + + if ($single_ret_as_list) { ++ print " if (args->$single_ret_list_max_var < 0) {\n"; ++ print " virReportError(VIR_ERR_RPC,\n"; ++ print " \"%s\", _(\"max$single_ret_list_name must be non-negative\"));\n"; ++ print " goto cleanup;\n"; ++ print " }\n"; + print " if (args->$single_ret_list_max_var > $single_ret_list_max_define) {\n"; + print " virReportError(VIR_ERR_RPC,\n"; + print " \"%s\", _(\"max$single_ret_list_name > $single_ret_list_max_define\"));\n"; +-- +2.27.0 + diff --git a/virsh-Fix-off-by-one-error-in-udevListInterfacesBySt.patch b/virsh-Fix-off-by-one-error-in-udevListInterfacesBySt.patch new file mode 100644 index 0000000000000000000000000000000000000000..de70e005858c0cee819bced391506da9b8eb1dc4 --- /dev/null +++ b/virsh-Fix-off-by-one-error-in-udevListInterfacesBySt.patch @@ -0,0 +1,39 @@ +From 001ede185f96d359481495a4016fcd0cffb2e1b0 Mon Sep 17 00:00:00 2001 +From: Martin Kletzander +Date: Tue, 27 Feb 2024 16:20:12 +0100 +Subject: [PATCH 6/8] virsh:Fix off-by-one error in udevListInterfacesByStatus +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Ever since this function was introduced in 2012 it could've tried +filling in an extra interface name. That was made worse in 2019 when +the caller functions started accepting NULL arrays of size 0. + +This is assigned CVE-2024-1441. + +Signed-off-by: Martin Kletzander +Reported-by: Alexander Kuznetsov +Fixes: 5a33366f5c0b18c93d161bd144f9f079de4ac8ca +Fixes: d6064e2759a24e0802f363e3a810dc5a7d7ebb15 +Reviewed-by: Ján Tomko +--- + src/interface/interface_backend_udev.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c +index e388f98536..dde88860d3 100644 +--- a/src/interface/interface_backend_udev.c ++++ b/src/interface/interface_backend_udev.c +@@ -221,7 +221,7 @@ udevListInterfacesByStatus(virConnectPtr conn, + virInterfaceDefPtr def; + + /* Ensure we won't exceed the size of our array */ +- if (count > names_len) ++ if (count >= names_len) + break; + + path = udev_list_entry_get_name(dev_entry); +-- +2.27.0 +