[PATCH] interface: fix udev_device_get_sysattr_value return value check
Peter Krempa
pkrempa at redhat.com
Fri Sep 8 10:20:14 UTC 2023
On Fri, Sep 08, 2023 at 13:06:43 +0300, Dmitry Frolov wrote:
> 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.
>
> Signed-off-by: Dmitry Frolov <frolov at swemel.ru>
> ---
> src/interface/interface_backend_udev.c | 11 ++++++-----
> 1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/src/interface/interface_backend_udev.c b/src/interface/interface_backend_udev.c
> index a0485ddd21..c820b3ccdf 100644
> --- a/src/interface/interface_backend_udev.c
> +++ b/src/interface/interface_backend_udev.c
> @@ -355,10 +355,11 @@ udevConnectListAllInterfaces(virConnectPtr conn,
> g_autoptr(virInterfaceDef) def = NULL;
>
> path = udev_list_entry_get_name(dev_entry);
> - dev = udev_device_new_from_syspath(udev, path);
> + if (!(dev = udev_device_new_from_syspath(udev, path)))
> + continue;
IMO this warants at least a VIR_DEBUG stating that the returned
'dev' struct is NULL and thus we're skipping a whole interface.
> name = udev_device_get_sysname(dev);
This is also documented as returning NULL on error, but virGetInterface
actually checks it before use.
> macaddr = udev_device_get_sysattr_value(dev, "address");
> - status = STREQ(udev_device_get_sysattr_value(dev, "operstate"), "up");
> + status = STREQ(NULLSTR(udev_device_get_sysattr_value(dev, "operstate")), "up");
For comparing strings which can be NULL we have STREQ_NULLABLE and
STRNEQ_NULLABLE.
More information about the libvir-list
mailing list