Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multipath-tools PR Q1/2021 #4

Merged
merged 153 commits into from Apr 1, 2021
Merged

multipath-tools PR Q1/2021 #4

merged 153 commits into from Apr 1, 2021

Conversation

mwilck
Copy link
Contributor

@mwilck mwilck commented Mar 30, 2021

Hi Christophe,

I think it's about time for a new release.

Changelog

Except for the bottom 3 patches, which are minor fixes for github CI and the README, everything has been reviewed on dm-devel.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Since f0462f0 ("libmultipath: use vector for for pp->hwe and mp->hwe"),
we've been trying to fix issues caused by paths getting freed and mpp->hwe
dangling. This approach couldn't work because we need mpp->hwe to persist,
even if all paths are removed from the map. Before f0462f0, a simple
assignment worked, because the lifetime of the hwe wasn't bound to the
path. But now, we need to copy the vector. It turns out that we need to set
mpp->hwe only in two places, add_map_with_path() and setup_map(), and
that the code is simplified overall.

Even now, it can happen that a map is added with add_map_without_paths(),
and has no paths. In that case, calling do_set_from_hwe() with a NULL
pointer is not a bug, so remove the message.

Fixes: f0462f0 ("libmultipath: use vector for for pp->hwe and mp->hwe")
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
libmultipath shouldn't call dm_task_create() directly any more.

Fixes: 90535a3 ("multipath: centralize validation code")
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
These tests don't compile with -Werror=unused-parameter. Fix it.
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
If compiled with -DLOAD_ALL_SHARED_LIBS, all known prioritizers
and checkers will be loaded immediately on startup. This is useful
for determining symbol usage (start executables with LD_BIND_NOW=1).

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Rely more on "make" functionality than on sequential command execution.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
The unit tests use a lot of internal symbols that we don't want
to add to the ABI if we don't have to. As long as we don't
have to make incompatible changes to functions, we can work around
that by simply using a non-versioned library for the unit tests.
Therefore we add a seperate rule here. Do this before actually
adding a version script, to avoid breakage during bisection.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
This version script documents the current ABI of libmultipath,
as used by multipath, multipathd, (lib)mpathpersist, and the
dynamically loaded libraries (prioritizers, checkers, and foreign).
The initial version string is set to "LIBMULTIPATH_1.0.0".

This reduces the amount of exported symbols of libmultipath.so.0
by more than a half (199 vs. 434).

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
This defines the ABI of libmpathpersist in the current state.
The initial version is set to "LIBMPATHPERSIST_1.0.0".

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
For completeness, this isn't really necessary.
The version string is set to "LIBMPATHCMD_1.0.0".

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
In __mpath_persistent_reserve_out, we call select_all_tg_pt(),
which requires mpp->hwe to be set. Initialize it in get_mpvec().

Fixes: 5b54e77 ("mpathpersist: add all_tg_pt option")
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
reconfigure() can be a long-running operation; both initial path
discovery and initial map setup can take a long time. Allow
the main program to indicate that the process should be
interrupted if a shutdown signal was received.

We take advantage of the dynamic linker's symbol lookup ordering
here. The default implementation of should_exit never returns
true, but callers (like multipathd) can override it.

Cc: Chongyun Wu <wu.chongyun@h3c.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
If multipathd gets a shutdown request during initial reconfigure(),
it shouldn't send "READY=1" to systemd. Ensure this by sending
"READY=1" via post_config_state().

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Inform systemd that the daemon is shutting down. See sd_notify(3).

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
The logic is as follows: child() sets DAEMON_IDLE status after
DAEMON_CONFIGURE when reconfigure() has finished. The only other state change
that can race with that is DAEMON_SHUTDOWN. Other state changes will wait for
DAEMON_IDLE first (see set_config_state()). When DAEMON_CONFIGURE is entered,
and we are not just starting up, send a "RELOADING=1" message to
systemd. After that, we must send "READY=1" when we're done reloading. Also
do that on startup, when DAEMON_IDLE is set for the first time.
See sd_notify(3).

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
While we access running_state only under the config_lock,
we sometimes do in a loop. Use the volatile qualifier to make
sure compilers can't optimize away the loads.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
It's unlikely but not impossible that other threads change the state
while we're waiting, and if we grab the lock again, it's still not
what we wanted. We need to continue waiting until either the condition
is met, or time timeout expired.

Moreover, generalize this code so that it can also be used in
set_config_state().

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Use __post_config_state() and __wait_for_state_change(). This
way __post_config_state() is the only place where running_state
is ever changed, and we avoid code duplication.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Cancel the other threads before taking vecs->lock. This avoids
delays during shutdown caused e.g. by the checker thread holding
the vecs lock.

Note: this makes it possible that cancelled threads leak memory,
because they can now be cancelled before having released the vecs
lock. I believe this is acceptable, as only threads are affected
that are cancelled during multipathd shutdown.

Cc: Chongyun Wu <wu.chongyun@h3c.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
The purpose of dm_lib_release() is to release stacked device node
operations in libdevmapper. This is functionality we don't need and
use any more, as we rely on udev to set up device nodes and symlinks.

We always set DM_UDEV_DISABLE_LIBRARY_FALLBACK when we run dm tasks.
In the standard CREATE and REMOVE cases, libdevmapper doesn't
stack any operations if this flag is set. The only exceptions are

 a) RESUME operations with DM_ADD_NODE_ON_RESUME set. This happens
implicity when we create new maps
 b) RENAME operations

In both cases, we call dm_udev_wait() after the libdm operation, which
calls update_devs() and thus has the same effect as dm_lib_release(),
cleaning out stacked operations.

OTOH, dm_lib_releases() accesses static variables in libdevmapper, so
calling it might be racy.

Drop the calls to dm_lib_release().

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
As one step towards bundling all possibly racy libdm init calls into a single
function, split the code for determining and checking versions of
libdm and kernel components. Provide a generic helper
libmp_get_version() that makes sure the versions are "lazily" initialized.

External callers may use dm_prereq(), like before.
Minor API change: dm_prereq() does not nullify the argument any more
if an error is encountered.

Remove the conf->version field, which isn't needed any more after this
change. This makes it necessary to fixup the hwtable test. Also, it
represents an incompatible ABI change as offsets in "struct config" are
changed, and two symbols removed. Bump the ABI major version to 2.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
dm_udev_wait() and dm_task_run() may access global / static state
in libdevmapper. They need to be protected by a lock in in our
multithreaded library.

The modified call sequence requires fixing the dmevents test:
devmapper.c must be added to dmevents-test_OBJDEPS to catch calls
to dm_task_run(). Also, the call to dmevent_poll_supported() in
setup() will cause init_versions() to be called, which requires
bypassing the wrappers in the test setup phase.

Cc: lixiaokeng@huawei.com

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Add an implementation of get_multipath_config() and put_multipath_config()
to libmultipath. The linker's symbol lookup rules will make sure that
applications can override these functions if they need to. Defining
these functions in libmultipath avoids the need to provide stubs
for these functions in every appliation linking to libmultipath.

libmultipath's internal functions simply refer to a static "struct config".
It must be initialized with init_config() rather than load_config(),
which always allocates a new struct for backward compatibility, and must
be teared down using uninit_config().

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Provide an alternative init function libmpathpersist_init() which
avoids allocating a new struct config, simply using libmultipath's
internal implementation. This causes a minor version bump for
libmpathpersist.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
With these symbols added, applications using libmultipath don't
need to define global variables "udev" and "logsink" any more.
This comes at the cost of having to call an init function.
Currently, libmultipath_init() does nothing but initialize
"udev".

The linker's symbol lookup order still allows applications to use
their own "logsink" and "udev" variables, which will take precendence
over libmultipath's internal ones. In this case, calling
libmultipath_init() can be skipped, but like before,
udev should be initialized (using udev_new()) before making any
libmultipath calls.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
We can use libmultipath's symbols now.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
mwilck and others added 27 commits March 22, 2021 16:01
gcc on alpine Linux/i386 throws errors because the "tv_sec" element
of struct timespec is a time_t, which is a "long long" in that
environment. In general, time_t is signed. As we only use CLOCK_MONOTONIC,
which starts at boot time, a cast to long should be no problem, even
in 32bit environments.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
gcc complained about a possible negative value of "nr" in the
memcpy() call. I consider that a false positive, but it's easily
fixed.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
If "kpartx -l" is run on a file that doesn't already have a loop device
associated with it, it will create a loop device to run the command.
Starting with da59d15 ("Fix loopback file with kpartx -av"), it will
not free the loop device when exitting. This is because it checks if the
the file it stat()ed is a regular file, before freeing the loop device.
However, after da59d15, stat() is rerun on the loop device itself, so
the check fails.  There is no need to check this, if loopcreated is
true, then the file will be a kpartx created loop device, and should be
freed.

Also, keep kpartx from printing that the loop device has been removed
at normal verbosity.

Fixes: da59d15 ("Fix loopback file with kpartx -av")
Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
…tus()

Since 378cb66 ("multipath: use update_pathvec_from_dm()"),
we remove paths and even pathgroups from multipathd's data structures
in update_multipath_table() if these paths are found to be non-existent.
But update_multipath_status() is called afterwards, and it
uses the kernel's mapping of pathgroups and paths, which won't match
any more if any members had been removed. disassemble_status() returns
an error if the number of path groups doesn't match, causing the
entire structure setup to fail. And because disassemble_status()
doesn't check the dev_t against the corresponding values in multipathd's
data structures, it may assign wrong DM state to paths.

Fix this by calling disassemble_status() before making any changes to
the data structure in update_pathvec_from_dm(). This can be easily
done, because every call to update_multipath_status() is preceded
by a call to update_multipath_table() anyway, and vice versa. So
we simply merge the two functions into one. This actually simplifies
the code for all callers.

As we remove a symbol, the major library version must be bumped.

Fixes: 378cb66 ("multipath: use update_pathvec_from_dm()")
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
In cases where some path devices are temporarily unavailable (e.g. failover),
high amounts of error messages such as these are seen:

Feb 27 08:02:03 ictm1608s02h1 multipath[1420]: get_udev_device: failed to look up 65:224 with type 1
Feb 27 08:02:03 ictm1608s02h1 multipath[1420]: 3600a098000aada210000f1625de51ed9: discarding non-existing path 65:224

This is because every invocation of "multipath -U" prints these messages
at the default log level (-v2). In the case of "multipath -U", these
messages aren't important, and in failover situations, "multipath -U" is
run pretty often, spamming the log with many similar messages.

Generally reducing the log level of these messages would be wrong,
because they are important for multipathd's operation, to verify that
multipathd does the right thing when discovering a discrepancy between the dm
state and the devices present in the system. Therefore, just decrease the
verbosity with which we invoke "multipath -U" in the udev rules.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Since f131e31 ("multipath-tools: devt test: avoid failure when run in
containers"), we check the existence of /sys/dev/block before running
the devt test. It turns out that on recent releases of podman (3.0.1),
this check is insufficient, because /sys/dev/block exists now in
containers, albeit empty. So we need to check for actual entries
in the directory.

Fixes: f131e31 ("multipath-tools: devt test: avoid failure when run in containers")

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Normally "add map" will be used to add a map which doesn't exist
yet. Thus not finding this map in the first place is not a problem
indicator and should be logged at level 3 only.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
For max_sectors_kb, replace "device dependent" with its sysfs path.
And use <dev> as wildcard for device in paths.

Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
When iscsi login/logout and multipath command are executed
concurrently, there is a coredump.

The reason is:
check_path
    ->update_multipath_strings
        ->sync_paths
            ->orphan_path    //pp->mpp is set to NULL
        ->update_multipath_status
            ->dm_get_status  //return DMP_NOT_FOUND
    ->condlog //pp->mpp->alias, NULL dereference

Here we don't dereference pp-> mpp if it is NULL.

Signed-off-by: Lixiaokeng<lixiaokeng@huawei.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
…r arrays

Cc: Zou Ming <zouming.zouming@huawei.com>
Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
EMC tool "Drop-down list" differs across product generations.

Cc: Martin Wilck <mwilck@suse.com>
Cc: Benjamin Marzinski <bmarzins@redhat.com>
Cc: Christophe Varoqui <christophe.varoqui@opensvc.com>
Cc: DM-DEVEL ML <dm-devel@redhat.com>
Signed-off-by: Xose Vazquez Perez <xose.vazquez@gmail.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
If a device with a scsi name identifier has an unknown prefix,
parse_vpd_pg83() needs to advance to the next identifier, instead of
simply trying the same one again in an infinite loop.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
The priorities for the EUI-64 (0x02) and NAME (0x08) scsi identifiers in
parse_vpd_pg83() don't match their priorities in 55-scsi-sg3_id.rules.
Switch them so that they match.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
Instead of looping through parents and checking, just call
udev_device_get_parent_with_subsystem_devtype() to get the
right one.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
If the wwid changed, the device is no longer the same, so sending add
events to the devices partitions doesn't make any sense.

Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
Reviewed-by: Martin Wilck <mwilck@suse.com>
This is useful for building in a cross-compilation environment.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
"make test" builds and runs test programs. For multiarch / cross-compilation
environments, it's useful to be able to separate these steps.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Add the kernel-doc generated manpages of libdmmp to git, and
change the libdmmp Makefile to regenerate them (only) when
necessary.

This allows us to drop perl as a build-time requirement.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
"make install" in a clean directory fails. Fix it by making "all"
a dependency of "install".

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Don't run closedir() if opendir() failed.

Fixes: "multipath-tools tests: check if /sys/dev/block is non-empty"

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Fixes the following warning with clang 3.5:

io_err_stat.c:613:31: error: missing field 'slot' initializer
[-Werror,-Wmissing-field-initializers]
        struct _vector _pathvec = {0,};

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
After b7aae60 ("multipathd: improve "add missing path" handling"),
a path that failed to initialize after multiple udev retriggers
would still be checked in check_path(). However, if a path is up,
has been checked more than once, the failback WWID methods have
been tried, and still there is no usable WWID, we may conclude
that something is fishy and we shouldn't keep trying.

Without this patch, totally WWID-less devices (seen e.g. on ESXi)
will cause a "add missing path" message in every checker iteration.

Fixes: b7aae60 ("multipathd: improve "add missing path" handling")

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Until now, our CI only tested builds on Ubuntu, the default github
runner. This commit adds containerized build/test workflows, which can cover a
much larger range of distributions and environments.

I've distinguished "native" runs, where the architecture is compatible with
the architecture of the runners (assumed to be x86_64), and "foreign" runs
with different architectures. The former can run "make test" just like in
any ordinary environment. To run tests in "foreign arch" environments,
I use qemu-user for runtime emulation. That's done with the help of the
"multiarch/qemu-user-static" container. Runtime containers for testing need
to have the qemu-user-static binary (compiled for x86_64) for their target
architecture built in. In theory, we could do the same thing for build
containers too, but it would be grossly inefficient. Instead, I've focused
on Debian environments for the foreign architectures, relying on Debian's
nice multiarch / cross-compilation features. Compilation is run in a dedicated
cross-build container, and only the test runs are carried out in the emulation.

The set of tests currently includes:

native:
 - Debian Jessie, x86_64/i386 (gcc 4.9, clang 3.5, glibc 2.19)
 - Debian Buster, x86_64/i386 (gcc 8.3, clang 7.0, glibc 2.28)
 - Debian Sid, x86_64/i386 (gcc 10.2, clang 11.0, glibc 2.31)
 - Alpine, x86_64/i386 (gcc 10.2, clang 10.0, musl libc 1.2)
 - Fedora 34, x86_64 (gcc 11.0, clang 12.0, glibc 2.33)

foreign:
 - Debian Buster, ppc64le/aarch64/s390x

This covers a rather broad range of compiler and C library versions and
should be fine for some time to come.

Note: In theory, it would be possible to just fetch base containers
via github actions, and install the dependencies as part of the build
procedure. But that would be quite resource-intensive and slow. Therefore
I've decided to use pre-built containers. The current container setup
fetches containers from my docker hub repository. The containers there
(multipath-build-$os-$arch and multipath-run-$os-$arch) come from my
"build-multipath" repository (https://github.com/mwilck/build-multipath),
and are created via github actions, too. The upload of the built container
images to docker hub requires the use of tokens and secrets.

I'd considered adding these container definitions and workflows to the
multipath-tools repository. We'd just need to create reasonable rules
for running the respective workflows. I expect these container images
to remain relatively stable; it makes no sense to rebuild the images
for every multipath-tools commit.

Tell me if you want this in the multipath-tools repo, and if you're ok
with hosting the images in my docker hub repo.

Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
This is not so important for the openSUSE repo, but it will
be for the main repo. No reason not to do it here.
This allows people to review the CI status quickly.
Github markdown doesn't support RST-style underlining with '*'.
@cvaroqui cvaroqui merged commit be60ecd into opensvc:master Apr 1, 2021
cvaroqui pushed a commit that referenced this pull request Dec 2, 2021
... by the paths and pg vectors of the map to be removed.

Original bug report from Lixiaokeng ("libmultipath: clear removed path from mpp"):

multipathd[3525635]: ==3525635==ERROR: AddressSanitizer: heap-use-after-free on address 0xffffa4902fc0 at pc 0xffffac7d5b88 bp 0xffffa948dac0 sp 0xffffa948dae0
multipathd[3525635]: READ of size 8 at 0xffffa4902fc0 thread T7
multipathd[3525635]:    #0 0xffffac7d5b87 in free_multipath (/usr/lib64/libmultipath.so.0+0x4bb87)
multipathd[3525635]:    #1 0xaaaad6cf7057  (/usr/sbin/multipathd+0x17057)
multipathd[3525635]:    #2 0xaaaad6cf78eb  (/usr/sbin/multipathd+0x178eb)
multipathd[3525635]:    #3 0xaaaad6cff4df  (/usr/sbin/multipathd+0x1f4df)
multipathd[3525635]:    #4 0xaaaad6cfffe7  (/usr/sbin/multipathd+0x1ffe7)
multipathd[3525635]:    #5 0xffffac807be3 in uevent_dispatch (/usr/lib64/libmultipath.so.0+0x7dbe3)
multipathd[3525635]:    #6 0xaaaad6cf563f  (/usr/sbin/multipathd+0x1563f)
multipathd[3525635]:    #7 0xffffac6877af  (/usr/lib64/libpthread.so.0+0x87af)
multipathd[3525635]:    #8 0xffffac44118b  (/usr/lib64/libc.so.6+0xd518b)
multipathd[3525635]: 0xffffa4902fc0 is located 1344 bytes inside of 1440-byte region [0xffffa4902a80,0xffffa4903020)
multipathd[3525635]: freed by thread T7 here:
multipathd[3525635]:    #0 0xffffac97d703 in free (/usr/lib64/libasan.so.4+0xd0703)
multipathd[3525635]:    #1 0xffffac824827 in orphan_paths (/usr/lib64/libmultipath.so.0+0x9a827)
multipathd[3525635]:    #2 0xffffac824a43 in remove_map (/usr/lib64/libmultipath.so.0+0x9aa43)
multipathd[3525635]:    #3 0xaaaad6cf7057  (/usr/sbin/multipathd+0x17057)
multipathd[3525635]:    #4 0xaaaad6cf78eb  (/usr/sbin/multipathd+0x178eb)
multipathd[3525635]:    #5 0xaaaad6cff4df  (/usr/sbin/multipathd+0x1f4df)
multipathd[3525635]:    #6 0xaaaad6cfffe7  (/usr/sbin/multipathd+0x1ffe7)
multipathd[3525635]:    #7 0xffffac807be3 in uevent_dispatch (/usr/lib64/libmultipath.so.0+0x7dbe3)
multipathd[3525635]:    #8 0xaaaad6cf563f  (/usr/sbin/multipathd+0x1563f)
multipathd[3525635]:    #9 0xffffac6877af  (/usr/lib64/libpthread.so.0+0x87af)
multipathd[3525635]:    #10 0xffffac44118b  (/usr/lib64/libc.so.6+0xd518b)

When mpp only has one path and log out the path, there is an asan error.
In remove_mpp, the pp is freed firstly in orphan_path but is accessed,
changed in free_multipath later. Before free_path(pp), the pp should be
cleared from pp->mpp.

Reported-by: Lixiaokeng <lixiaokeng@huawei.com>
Tested-by: Lixiaokeng <lixiaokeng@huawei.com>
Reviewed-by: Benjamin Marzinski <bmarzins@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants