[dm-devel] [PATCH] libmultipath: fix NULL dereference in find_path_by_dev

[Martin Wilck]

On Sat, 2021-01-23 at 16:19 +0800, lixiaokeng wrote:
> When I test the 0.8.5 code with iscsi login/out, multipathd command
> and multipath command concurrently, there is a multipathd coredump.
> The stack is shown:
> 
> uxlsnrloop
>   ->cli_list_devices
>     ->show_devices
>       ->snprint_devices
>         ->find_path_by_dev
> 
> The reason is that devname is NULL in snprint_devices, then it will
> be dereference. Here we check dev in find_path_by_dev.
> ---
>  libmultipath/structs.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/libmultipath/structs.c b/libmultipath/structs.c
> index 464596f..a3f27fd 100644
> --- a/libmultipath/structs.c
> +++ b/libmultipath/structs.c
> @@ -453,12 +453,12 @@ find_mp_by_str (const struct _vector *mpvec,
> const char * str)
>  }
> 
>  struct path *
> -find_path_by_dev (const struct _vector *pathvec, const char * dev)
> +find_path_by_dev (const struct _vector *pathvec, const char *dev)
>  {
>     int i;
>     struct path * pp;
> 
> -   if (!pathvec)
> +   if (!pathvec || !dev)
>         return NULL;
> 
>     vector_foreach_slot (pathvec, pp, i)

Reviewed-by: Martin Wilck <mwilck at suse.de>

However, the actual issue is in snprint_devices():

	r = udev_enumerate_scan_devices(enm);
	first = udev_enumerate_get_list_entry(enm);
	udev_list_entry_foreach(item, first) {

		path = udev_list_entry_get_name(item);
		u_dev = udev_device_new_from_syspath(udev, path);
		devname = udev_device_get_sysname(u_dev);
                pp = find_path_by_dev(vecs->pathvec, devname);

if devname was NULL here, it means that the list returned by
udev_enumerate_scan_devices() contains devices that don't exist
(any more), even though the call to udev_enumerate_scan_devices() was
just a few LoC above. It's the kind of thing you don't expect, but you
should expect when you deal with udev :-/

I'll send a patch for that, too.

Martin