Fix symbol resolution issues #48
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
devt2devname is under the default license and can be moved to libmultipath. Signed-off-by: Martin Wilck <[email protected]>
in libmpathutil, udev was only used by devt2devname. Remove it. Signed-off-by: Martin Wilck <[email protected]>
This symbol should live in libmpathutil only. Signed-off-by: Martin Wilck <[email protected]>
Using of __attribute__((weak)) here has no effect for dynamic linking. Quoting from ld.so(8): "Standard practice is that the distinction between weak and strong symbols should have effect only at static link time." See also the ELF specification, p. 1-18 "Symbol Table". We would only need this if we supported static linking for multipath-tools, which we haven't done for a long time.
Some symbols like get_multipath_config are exported by both libmpathutil and libmultipath. The symbols exported by `libmpathutil` are dummies, intended to be overridden by `libmultipath` using "breadth-first" symbol lookup search specified by ELF [1]. The dummy symbols are only used by programs not linking to libmultipath, i.e. multipathc. Because we're using ABI versioning, code in libmpathutil actually exports *and references* `get_multipath_config@LIBMULTIPATH_16.0.0`. That worked until 0.9.1, because both libraries were using the same ABI version, and `get_multipath_config@LIBMULTIPATH_16.0.0` from libmultipath overrides the symbol from libmpathutil. But in 0.9.2, we bumped the ABI version of libmultipath to 17.0.0. libmultipath now exports (only) `get_multipath_config@LIBMULTIPATH_17.0.0`, which does *not* override `get_multipath_config@LIBMULTIPATH_16.0.0` from libmpathutil. The effect is that references to these duplicate symbols from libmpathutil are resolved to its own dummy symbols, which is wrong and causes a coredump e.g. with "multipath -t", where the return value of get_multipath_config() is accessed in the call chain snprint_keyword()->snprint_defaults(). This happens only with multipath, because multipathd itself overrides get_multipath_config(). [1]: Executable and Linkable Format (ELF) (https://www.cs.cmu.edu/afs/cs/academic/class/15213-f00/docs/elf.pdf), p. 2-15 ("Shared Object Dependencies") Signed-off-by: Martin Wilck <[email protected]>
|
Compare to the example in #47 (comment):
|
|
I can confirm this does no longer crash for me. Thanks a lot! |
|
Will we see a new release with this? |
|
We haven't done that in the past. We usually accumulate some commits before we bump the version. @cvaroqui, would you bump to 0.9.3? |
|
Ok. Can you prepare the pr ? I will merge and tag asap |
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 @cvaroqui, hi @bmarzins,
This set fixes #47. The problem analysis can be seen in #47, and in the description of
6e8aec6. In short, if we use duplicate symbols in different shared objects, we need to keep
the versions of these symbols in sync, otherwise symbol lookup won't work as intended.
I've sent this via GitHub and not via
dm-develbecause it's really purely technical stuff thatmatters only little for the ML, and because people who upgrade multipath-tools because of
CVE-2022-41973 and CVE-2022-41974 will be looking here.
The solution for the problem is 3-fold:
udevandlibmpverbosityaren't duplicated any more, but defined only in libmultipath and libmpathutil, respectively.get_multipath_configandput_multipath_configas duplicate symbols. For these, we use a new ABI "version"LIBMPATHCOMMON_1.0.0, with comments that make it clear that the version of these symbols must be changed synchronously in the two libraries.Martin Wilck (5):
libmpathutil: move devt2devname() to libmultipath
libmpathutil: remove udev symbol
libmultipath: remove duplicate export of libmp_verbosity
libmpathutil: remove
__attribute__((weak))libmultipath/libmpathutil: use common ABI version for duplicate symbols
@xosevp for information.