From 13ea81b22cde0a429aa1de8b58655296084ce8d7 Mon Sep 17 00:00:00 2001 From: Dmitry Frolov Date: Tue, 12 Sep 2023 15:56:47 +0300 Subject: [PATCH] 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 (cherry picked from commit 2ca94317ac642a70921947150ced8acc674ccdc8) --- 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 54b43fb999..ef334f175b 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" @@ -40,6 +41,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 */ @@ -354,11 +357,20 @@ udevConnectListAllInterfaces(virConnectPtr conn, const char *macaddr; g_autoptr(virInterfaceDef) def = NULL; - 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)) { @@ -962,9 +974,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; @@ -1087,7 +1099,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.43.0