|
| 1 | +From 2ca94317ac642a70921947150ced8acc674ccdc8 Mon Sep 17 00:00:00 2001 |
| 2 | +From: Dmitry Frolov <frolov@swemel.ru> |
| 3 | +Date: Tue, 12 Sep 2023 15:56:47 +0300 |
| 4 | +Subject: [PATCH] interface: fix udev_device_get_sysattr_value return value |
| 5 | + check |
| 6 | + |
| 7 | +Reviewing the code I found that return value of function |
| 8 | +udev_device_get_sysattr_value() is dereferenced without a check. |
| 9 | +udev_device_get_sysattr_value() may return NULL by number of reasons. |
| 10 | + |
| 11 | +v2: VIR_DEBUG added, replaced STREQ(NULLSTR()) with STREQ_NULLABLE() |
| 12 | +v3: More checks added, to skip earlier. More verbose VIR_DEBUG. |
| 13 | + |
| 14 | +Signed-off-by: Dmitry Frolov <frolov@swemel.ru> |
| 15 | +Reviewed-by: Martin Kletzander <mkletzan@redhat.com> |
| 16 | +--- |
| 17 | + src/interface/interface_backend_udev.c | 26 +++++++++++++++++++------- |
| 18 | + 1 file changed, 19 insertions(+), 7 deletions(-) |
| 19 | + |
| 20 | +diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c |
| 21 | +index a0485ddd21..fb6799ed94 100644 |
| 22 | +--- a/src/interface/interface_backend_udev.c |
| 23 | ++++ b/src/interface/interface_backend_udev.c |
| 24 | +@@ -23,6 +23,7 @@ |
| 25 | + #include <dirent.h> |
| 26 | + #include <libudev.h> |
| 27 | + |
| 28 | ++#include "virlog.h" |
| 29 | + #include "virerror.h" |
| 30 | + #include "virfile.h" |
| 31 | + #include "datatypes.h" |
| 32 | +@@ -40,6 +41,8 @@ |
| 33 | + |
| 34 | + #define VIR_FROM_THIS VIR_FROM_INTERFACE |
| 35 | + |
| 36 | ++VIR_LOG_INIT("interface.interface_backend_udev"); |
| 37 | ++ |
| 38 | + struct udev_iface_driver { |
| 39 | + struct udev *udev; |
| 40 | + /* pid file FD, ensures two copies of the driver can't use the same root */ |
| 41 | +@@ -354,11 +357,20 @@ udevConnectListAllInterfaces(virConnectPtr conn, |
| 42 | + const char *macaddr; |
| 43 | + g_autoptr(virInterfaceDef) def = NULL; |
| 44 | + |
| 45 | +- path = udev_list_entry_get_name(dev_entry); |
| 46 | +- dev = udev_device_new_from_syspath(udev, path); |
| 47 | +- name = udev_device_get_sysname(dev); |
| 48 | ++ if (!(path = udev_list_entry_get_name(dev_entry))) { |
| 49 | ++ VIR_DEBUG("Skipping interface, path == NULL"); |
| 50 | ++ continue; |
| 51 | ++ } |
| 52 | ++ if (!(dev = udev_device_new_from_syspath(udev, path))) { |
| 53 | ++ VIR_DEBUG("Skipping interface '%s', dev == NULL", path); |
| 54 | ++ continue; |
| 55 | ++ } |
| 56 | ++ if (!(name = udev_device_get_sysname(dev))) { |
| 57 | ++ VIR_DEBUG("Skipping interface '%s', name == NULL", path); |
| 58 | ++ continue; |
| 59 | ++ } |
| 60 | + macaddr = udev_device_get_sysattr_value(dev, "address"); |
| 61 | +- status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up"); |
| 62 | ++ status = STREQ_NULLABLE(udev_device_get_sysattr_value(dev, "operstate"), "up"); |
| 63 | + |
| 64 | + def = udevGetMinimalDefForDevice(dev); |
| 65 | + if (!virConnectListAllInterfacesCheckACL(conn, def)) { |
| 66 | +@@ -964,9 +976,9 @@ udevGetIfaceDef(struct udev *udev, const char *name) |
| 67 | + |
| 68 | + /* MTU */ |
| 69 | + mtu_str = udev_device_get_sysattr_value(dev, "mtu"); |
| 70 | +- if (virStrToLong_ui(mtu_str, NULL, 10, &mtu) < 0) { |
| 71 | ++ if (!mtu_str || virStrToLong_ui(mtu_str, NULL, 10, &mtu) < 0) { |
| 72 | + virReportError(VIR_ERR_INTERNAL_ERROR, |
| 73 | +- _("Could not parse MTU value '%s'"), mtu_str); |
| 74 | ++ _("Could not parse MTU value '%s'"), NULLSTR(mtu_str)); |
| 75 | + goto error; |
| 76 | + } |
| 77 | + ifacedef->mtu = mtu; |
| 78 | +@@ -1089,7 +1101,7 @@ udevInterfaceIsActive(virInterfacePtr ifinfo) |
| 79 | + goto cleanup; |
| 80 | + |
| 81 | + /* Check if it's active or not */ |
| 82 | +- status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up"); |
| 83 | ++ status = STREQ_NULLABLE(udev_device_get_sysattr_value(dev, "operstate"), "up"); |
| 84 | + |
| 85 | + udev_device_unref(dev); |
| 86 | + |
| 87 | +-- |
| 88 | +GitLab |
0 commit comments