New pull request from "queue" branch #9
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If checker_cleanup_thread is called after cleanup_checkers, the checker_class will not be freed. Here, we use free_checker_class instead of checker_class_unref in checker_cleanup_thread. Signed-off-by: Lixiaokeng <[email protected]> Reviewed-by: Benjamin Marzinski <[email protected]> Reviewed-by: Martin Wilck <[email protected]>
To avoid race conditions with pending RCU callbacks on exit, it's necessary to call rcu_barrier() in cleanup_rcu() (see https://lists.lttng.org/pipermail/lttng-dev/2021-May/029958.html and follow-ups). rcu_barrier() is only available in User-space RCU v0.8 and newer. Fix it by reverting 5d0dae6 ("multipathd: Fix liburcu memory leak") if an older version of liburcu is detected. Fixes: 5d0dae6 ("multipathd: Fix liburcu memory leak") Reviewed-by: Benjamin Marzinski <[email protected]>
In ev_remove_path(), if update_mpp_paths() fails, we delete the entire map. However, since update_mpp_paths() happens before we call set_path_removed(), pp->initialized isn't set to INIT_REMOVED, so remove_map_and_stop_waiter() doesn't remove the path when in removes the map. But with the map removed, there's nothing to keep us from removing the path. Call set_path_removed() before update_mpp_paths() to avoid the odd case of ev_remove_path() removing the map but not the path. Signed-off-by: Benjamin Marzinski <[email protected]> Reviewed-by: Martin Wilck <[email protected]>
remove_map_and_stop_waiter() already calls orphan_paths() so flush_map() doesn't need to call orphan_paths() before calling remove_map_and_stop_waiter(). Signed-off-by: Benjamin Marzinski <[email protected]> Reviewed-by: Martin Wilck <[email protected]>
When ev_remove_path() returned success, callers assumed that the path (and possibly the map) had been removed. When ev_remove_path() returned failure, callers assumed that the path had not been removed. However, the path could be removed on both success or failure. This could cause callers to dereference the path after it was removed. To deal with this, make ev_remove_path() return a different symbolic value for each outcome, and make the callers react appropriately for the different values. Found by coverity. Signed-off-by: Benjamin Marzinski <[email protected]> Reviewed-by: Martin Wilck <[email protected]>
configure() can retry multiple times, each time reallocing a maps and paths vector, and leaking the previous ones. Fix this by always freeing the vectors before configure() exits. Found by coverity. Signed-off-by: Benjamin Marzinski <[email protected]> Reviewed-by: Martin Wilck <[email protected]>
If a new block was allocated, but couldn't be filled, getblock will discard it. When it does so, it needs to free the block to avoid leaking memory. Found by coverity. Signed-off-by: Benjamin Marzinski <[email protected]> Reviewed-by: Martin Wilck <[email protected]>
If get_uid() is returning a different wwid in uev_update_path(), then the uid_attribute must have already gotten updated, which was the purpose behind calling rescan_path() in the first place. Signed-off-by: Benjamin Marzinski <[email protected]> Reviewed-by: Martin Wilck <[email protected]>
Create a macro for setting the reply length for string literals correctly, and use it where necessary. In cli_del_path(), don't change the function's return code if just the buffer allocation for the reply failed. Reviewed-by: Benjamin Marzinski <[email protected]>
By setting (*reply)[19] = '\0', we always truncated a possible ":aptpl" suffix. Fix it, and use the return value of snprintf() as length. Reviewed-by: Benjamin Marzinski <[email protected]>
Allow the compiler to catch possible format string overflows. Two were found by gcc 10. Reviewed-by: Benjamin Marzinski <[email protected]>
Use the latest commit timestamp of the "libdmmp.h" file as the timestamp for the man pages. This should avoid spurious rebuilds of the documentation. Reviewed-by: Benjamin Marzinski <[email protected]>
Cc: Martin Wilck <[email protected]> Cc: Benjamin Marzinski <[email protected]> Cc: Christophe Varoqui <[email protected]> Cc: DM-DEVEL ML <[email protected]> Signed-off-by: Xose Vazquez Perez <[email protected]> Reviewed-by: Martin Wilck <[email protected]>
Do not attempt to start multipath-tools in containers, should switch for on-demand udev/socket based activation in the future. Originally reported as: https://bugs.launchpad.net/ubuntu/+source/multipath-tools/+bug/1823093 Author: Dimitri John Ledkov <[email protected]> Co-Author: Utkarsh Gupta <[email protected]> Reviewed-by: Martin Wilck <[email protected]>
Build fails on distributions that don't support DM_DEFERRED_REMOVE (libdevmapper < 1.02.89). Fix it. Resolves: opensvc#7 Tested-by: Paul Menzel <[email protected]> Reviewed-by: Benjamin Marzinski <[email protected]>
SCSI LUNs are 64bit unsigned integers, and have been exposed as such by the kernel for years. Storage using the full 8 bytes is fortunately rare. Still, we should handle this properly.
Cc: Martin Wilck <[email protected]> Cc: Benjamin Marzinski <[email protected]> Cc: Christophe Varoqui <[email protected]> Cc: DM-DEVEL ML <[email protected]> Signed-off-by: Xose Vazquez Perez <[email protected]> Reviewed-by: Martin Wilck <[email protected]>"
And add recommended no_path_retry and pgfailback values. Info from: - RHEL https://download.huawei.com/edownload/e/download.do?actionFlag=download&nid=EDOC1100113070&partNo=6001&mid=SUPE_DOC&_t=1612885511000 - SLES https://download.huawei.com/edownload/e/download.do?actionFlag=download&nid=EDOC1100117892&partNo=6001&mid=SUPE_DOC&_t=1612885538000 - without HyperMetro: vendor "HUAWEI" product "XSG1" path_grouping_policy multibus no_path_retry 15 - with HyperMetro: vendor "HUAWEI" product "XSG1" path_grouping_policy group_by_prio prio alua failback immediate no_path_retry 15 ALUA is only used with HyperMetro(cluster of two arrays). Suggested-by: Martin Wilck <[email protected]> Cc: Zhouweigang (Jack) <[email protected]> Cc: Zou Ming <[email protected]> Cc: Benjamin Marzinski <[email protected]> Cc: Martin Wilck <[email protected]> Cc: Christophe Varoqui <[email protected]> Cc: DM-DEVEL ML <[email protected]> Signed-off-by: Xose Vazquez Perez <[email protected]> Reviewed-by: Martin Wilck <[email protected]>"
mwilck
referenced
this pull request
in openSUSE/multipath-tools
Dec 1, 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 <[email protected]>
Tested-by: Lixiaokeng <[email protected]>
Reviewed-by: Benjamin Marzinski <[email protected]>
mwilck
referenced
this pull request
in openSUSE/multipath-tools
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 <[email protected]>
Tested-by: Lixiaokeng <[email protected]>
Reviewed-by: Benjamin Marzinski <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hi Christophe,
we only have a few basic fixes this time, but I think it's time for a release.
Regards
Martin