[dm-devel] libmultipath: fix NULL dereference in get_be64

[lixiaokeng]

On 2021/2/2 13:26, Benjamin Marzinski wrote:
> So, I think the main issue here is that filter_property appears to be
> broken.  It only filters if uid_attribute is set, but that will never be
> set the first time it's called in pathinfo.  This means that it will
> pass in the pathinfo call in cli_add_path, and the path will get stored
> in the pathvec.
> 

  Yes! The pathinfo returns PATHINFO_OK in store_pathinfo but returns
PATHINFO_SKIPPED in adopt_paths. I'm sorry for not mentioning it in the
previous e-mails. I just focus on where return PATHINFO_SKIPPED in second
pathinfo.

Gdb second pathinfo in adopt_paths:
(gdb)
2106			if (hidden && !strcmp(hidden, "1")) {
(gdb)
2103			const char *hidden =
(gdb)
2106			if (hidden && !strcmp(hidden, "1")) {
(gdb)
2110			if (is_claimed_by_foreign(pp->udev) ||
(gdb)
2111			    filter_property(conf, pp->udev, 4, pp->uid_attribute) > 0)
(gdb)
2110			if (is_claimed_by_foreign(pp->udev) ||
(gdb)
2146				return PATHINFO_SKIPPED;
(gdb)
2260	}
  I'm not sure filter_property makes pathinfo return PATHINFO_SKIPPED from gdb.
Ben’s analysis resolves my doubts.

> However, it will fail in the pathinfo call from adopt_paths, so the path
> won't be added to the multipath device.  This means adopt paths doesn't
> actually adopt any paths potentially, but that in itself doesn't cause
> it to fail. This check
> 
>         if (adopt_paths(vecs->pathvec, mpp) ||
>             find_slot(vecs->pathvec, pp) == -1)
>                 goto out;
> 
> passes, since we only check if the path is on the pathvec, not part of
> the multipath device, and since filter_property let the path past the
> first time, it is. So add_map_with_path() will create a multipath
> device, but the path won't be added to it, and pp->mpp == NULL.
> 
> So, add_map_with_path() should probably check that we actually created a
> map that included the path that got added. But more importantly,
> filter_property shouldn't return different results the when it's called
> the first time.  That would have avoid the entire situation.
	if (adopt_paths(vecs->pathvec, mpp) ||
	    find_slot(vecs->pathvec, pp) == -1 ||
	    !pp->mpp)
		goto out;

This is better than my first patch to avoid this problem. However, it
is beyond my ability to slove the problem of filter_property returning
different values.

Regards,
Lixiaokeng