From c6fe2aef82c3a9bac5d62fe9bd11c84d984a96a4 Mon Sep 17 00:00:00 2001 From: Chris Duf <101881860+dottspina@users.noreply.github.com> Date: Wed, 20 Jul 2022 17:46:49 +0200 Subject: [PATCH] [ISSUE-131] Library: patch possible misuse of find_library() (#132) On Linux, the Library.load_default() function fails to properly resolve the JLink shared library if not installed in `/opt/SEGGER/JLink`, regardless of the value of `LD_LIBRARY_PATH`. This is possibly caused by an improper use of the ctypes `find_library()` API, which: - On all platforms expects as parameter the library name without any prefix like lib, suffix like `.so`, `.dylib` or version number - Returns the full file path on MacOS and Windows, but only the file name (so name) on Linux. Changes introduced by this patch: - On Linux `find_library('jlinkarm')` will return the resolved soname, for e.g. `libjlinkarm.so.7`, and we'll use the native `dlinfo()` API to retrieve the full file path. --- pylink/library.py | 158 ++++++++++++++++++++++++++++++++++++- tests/unit/test_library.py | 124 +++++++++++++++++++++++++++++ 2 files changed, 279 insertions(+), 3 deletions(-) diff --git a/pylink/library.py b/pylink/library.py index 44a3e8d..211f101 100644 --- a/pylink/library.py +++ b/pylink/library.py @@ -17,6 +17,7 @@ import ctypes import ctypes.util as ctypes_util import os +import platform import sys import tempfile @@ -82,11 +83,29 @@ class Library(object): 'JLINK_SetFlashProgProgressCallback' ] - JLINK_SDK_NAME = 'libjlinkarm' + # Linux/MacOS: The JLink library name without any prefix like lib, + # suffix like .so, .dylib or version number. + JLINK_SDK_NAME = 'jlinkarm' + # Linux/MacOS: The library file name will start with 'libjlinkarm' + # Used by Library.find_library_{linux,darwin}() + JLINK_SDK_STARTS_WITH = 'libjlinkarm' + + # Windows: these are suitable for both the ctypes find_library() API, + # and for the directory scanning done in Library.find_library_windows() WINDOWS_32_JLINK_SDK_NAME = 'JLinkARM' WINDOWS_64_JLINK_SDK_NAME = 'JLink_x64' + # Linux: Represents the dlinfo(3) associated to the jlinkarm shared library. + # + # Poor man's singleton: having this as a class member avoids to create + # a new JLinkarmDlInfo() for each pylink.Library instance. + # + # For e.g., the simple 'pyocd list' command creates three instances + # of the pylink.Library class, and it's worth not repeating the dlinfo() + # dance three times. + _dlinfo = None + @classmethod def get_appropriate_windows_sdk_name(cls): """Returns the appropriate JLink SDK library name on Windows depending @@ -157,7 +176,7 @@ def find_library_linux(cls): The paths to the J-Link library files in the order that they are found. """ - dll = Library.JLINK_SDK_NAME + dll = Library.JLINK_SDK_STARTS_WITH root = os.path.join('/', 'opt', 'SEGGER') for (directory_name, subdirs, files) in os.walk(root): @@ -206,7 +225,7 @@ def find_library_darwin(cls): Returns: The path to the J-Link library files in the order they are found. """ - dll = Library.JLINK_SDK_NAME + dll = Library.JLINK_SDK_STARTS_WITH root = os.path.join('/', 'Applications', 'SEGGER') if not os.path.isdir(root): return @@ -283,7 +302,27 @@ def load_default(self): Returns: ``True`` if the DLL was loaded, otherwise ``False``. """ + + # Request the underlying operating system, through ctypes, + # to resolve the J-Link DLL "the standard way" by its + # library name. path = ctypes_util.find_library(self._sdk) + + # On Linux, find_library() actually returns the soname, + # so we've got something like path = 'libjlinkarm.so.7', + # and now have to retrieve the absolute file path. + if (path is not None) and sys.platform.startswith('linux'): + # For this, we'll rely on dlinfo(), which is not a POSIX API, + # but a GNU libc extension. + if platform.libc_ver()[0] == 'glibc': + if Library._dlinfo is None: + Library._dlinfo = JLinkarmDlInfo(path) + path = Library._dlinfo.path + else: + # When GNU libc extensions aren't available, + # continue as if find_library() had failed. + path = None + if path is None: # Couldn't find it the standard way. Fallback to the non-standard # way of finding the J-Link library. These methods are operating @@ -423,3 +462,116 @@ def dll(self): A ``ctypes`` DLL instance if one was loaded, otherwise ``None``. """ return self._lib + + +class JLinkarmDlInfo: + """Helper to retrieve the absolute path of the JLink library (aka DLL) + based on its soname. + + This is used on Linux, where ctypes.util.find_library() will not return + the library full file path, but only the file name (aka soname). + We'll then rely on the native dlinfo() API to retrieve the library absolute + file path. + + For e.g.: + - LD_LIBRARY_PATH=/mnt/platform/segger/JLink + - ctypes.util.find_library('jlinkarm') -> libjlinkarm.so.7 + - JLinkarmDlInfo('libjlinkarm.so.7').path -> /mnt/platform/segger/JLink/libjlinkarm.so.7 + + Note that unlike dlopen(), dlsym() and friends, dlinfo() is not a POSIX API, + but a GNU extension, available only on systems with the GNU libc implementation (glibc). + + The dlinfo() dance implementation is adapted from @cloudflightio, + https://github.com/cloudflightio/python-dlinfo. + """ + + # Request to obtain a pointer to the link_map structure corresponding + # to a given handle (Linux). + # See: man dlinfo(3) + RTLD_DI_LINKMAP = 2 + + # dlinfo(3): struct link_map, where l_name will be the file path. + class LinkMap(ctypes.Structure): + """ + Represents a C struct link_map (Linux). + See: man dlinfo(3) + """ + _fields_ = [ + ('l_addr', ctypes.c_void_p), + ('l_name', ctypes.c_char_p), + ('l_ld', ctypes.c_void_p), + ('l_next', ctypes.c_void_p), + ('l_prev', ctypes.c_void_p), + ] + + def __init__(self, jlinkarm_soname): + """Retrieves the absolute file path of the JLink library (aka DLL) + corresponding to the given soname. + + This runs the dlinfo() dance using the ctypes API: + - loads the JLink DLL shared object for given soname + - loads the dl library and lookup the dlinfo symbol + - calls dlinfo() with request RTLD_DI_LINKMAP to get the struct link_map + for the JLink library + - access the struct content to retrieve the library's absolute path + + The dlinfo() dance implementation does not try to hide any OSError + the ctypes API may raise, since this would likely hide a host + system configuration issue (aka "should not happen"). + + Args: + - jlinkarm_soname: The JLink DLL soname returned by find_library(), + for e.g. 'libjlinkarm.so.7'. + + Raises: + - OSError when the actual library file has been removed, is not readable, + is not a loadable shared object, etc. + """ + # JLink DLL's absolute file path. + self._dll_path = None + + # We're using the soname returned by a successful call to find_library(), + # hence we can expect LoadLibrary() to in turn successfully load the JLink DLL. + tmp_cdll_jlink = ctypes.cdll.LoadLibrary(jlinkarm_soname) + + # dlinfo() dance to retrieve the library's absolute file path. + # + # This code path should be involved only for POSIX systems + # with GNU libc, where: + # - libdl is available (POSIX) + # - dlinfo() is defined (glibc) + # + # Hence bellow calls to find_library(), LoadLibrary(), and dlinfo() should succeed. + dl_soname = ctypes_util.find_library('dl') + if dl_soname is not None: + tmp_cdll_dl = ctypes.cdll.LoadLibrary(dl_soname) + dlinfo = tmp_cdll_dl.dlinfo + dlinfo.argtypes = ctypes.c_void_p, ctypes.c_int, ctypes.c_void_p + dlinfo.restype = ctypes.c_int + + linkmap = ctypes.c_void_p() + if dlinfo(tmp_cdll_jlink._handle, JLinkarmDlInfo.RTLD_DI_LINKMAP, ctypes.byref(linkmap)) == 0: + linkmap = ctypes.cast(linkmap, ctypes.POINTER(JLinkarmDlInfo.LinkMap)) + self._dll_path = linkmap.contents.l_name.decode(sys.getdefaultencoding()) + + # "Free" tmp dl library + del tmp_cdll_dl + tmp_cdll_dl = None + + else: + # Should not happen. + pass + + # "Free" tmp jlinkarm library + del tmp_cdll_jlink + tmp_cdll_jlink = None + + @property + def path(self): + """Answers the JLink DLL file path. + + Returns the JLink DLL's absolute path, + or None when the dl library is unavailable despite the system + presenting itself as POSIX. + """ + return self._dll_path diff --git a/tests/unit/test_library.py b/tests/unit/test_library.py index c0d27c4..94ca901 100644 --- a/tests/unit/test_library.py +++ b/tests/unit/test_library.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +from platform import platform import pylink.library as library import pylink.util as util @@ -949,6 +950,129 @@ def test_linux_empty(self, mock_os, mock_load_library, mock_find_library, mock_o self.assertEqual(1, mock_find_library.call_count) self.assertEqual(0, mock_load_library.call_count) + @mock.patch('os.name', new='posix') + @mock.patch('sys.platform', new='linux') + @mock.patch('tempfile.NamedTemporaryFile', new=mock.Mock()) + @mock.patch('os.remove', new=mock.Mock()) + @mock.patch('pylink.library.open') + @mock.patch('pylink.library.os') + @mock.patch('pylink.util.is_os_64bit', return_value=True) + @mock.patch('pylink.platform.libc_ver', return_value=('libc', '1.0')) + @mock.patch('ctypes.util.find_library', return_value='libjlinkarm.so.7') + @mock.patch('pylink.library.JLinkarmDlInfo.__init__') + @mock.patch('ctypes.cdll.LoadLibrary') + def test_linux_glibc_unavailable(self, mock_load_library, mock_dlinfo_ctr, mock_find_library, + mock_libc_ver, mock_is_os_64bit, mock_os, mock_open): + """Confirms the whole JLinkarmDlInfo code path is not involved when GNU libc + extensions are unavailable on a Linux system, and that we'll successfully fallback + to the "search by file name". + + Test case: + - initial find_library('jlinkarm') succeeds + - but the host system does not provide GNU libc extensions + - we should then skip the dlinfo() dance and proceed + to the "search by file name" code path, aka find_library_linux() + - and "successfully load" a mock library file from /opt/SEGGER/JLink + """ + directories = [ + # Library.find_library_linux() should find this. + '/opt/SEGGER/JLink/libjlinkarm.so.6' + ] + self.mock_directories(mock_os, directories, '/') + + lib = library.Library() + lib.unload = mock.Mock() + + mock_find_library.assert_called_once_with(library.Library.JLINK_SDK_NAME) + # JLinkarmDlInfo has not been instantiated. + self.assertEquals(0, mock_dlinfo_ctr.call_count) + # Fallback to "search by file name" has succeeded. + self.assertEquals(1, mock_load_library.call_count) + self.assertEqual(directories[0], lib._path) + + @mock.patch('os.name', new='posix') + @mock.patch('sys.platform', new='linux') + @mock.patch('tempfile.NamedTemporaryFile', new=mock.Mock()) + @mock.patch('os.remove', new=mock.Mock()) + @mock.patch('pylink.library.open') + @mock.patch('pylink.library.os') + @mock.patch('pylink.util.is_os_64bit', return_value=True) + @mock.patch('pylink.platform.libc_ver', return_value=('glibc', '2.34')) + @mock.patch('ctypes.util.find_library') + @mock.patch('ctypes.cdll.LoadLibrary') + def test_linux_dl_unavailable(self, mock_load_library, mock_find_library, mock_libc_ver, + mock_is_os_64bit, mock_os, mock_open): + """Confirms we successfully fallback to the "search by file name" code path when libdl is + unavailable despite the host system presenting itself as POSIX (GNU/Linux). + + Test case: + - initial find_library('jlinkarm') succeeds + - the host system presents itself as GNU/Linux, but does not provide libdl + - we should then skip the dlinfo() dance and proceed + to the "search by file name" code path, aka find_library_linux() + - and "successfully load" a mock library file from /opt/SEGGER/JLink + """ + mock_find_library.side_effect = [ + # find_library('jlinkarm') + 'libjlinkarm.so.6', + # find_library('dl') + None + ] + + directories = [ + '/opt/SEGGER/JLink/libjlinkarm.so.6' + ] + self.mock_directories(mock_os, directories, '/') + + lib = library.Library() + lib.unload = mock.Mock() + + mock_find_library.assert_any_call(library.Library.JLINK_SDK_NAME) + mock_find_library.assert_any_call('dl') + self.assertEquals(2, mock_find_library.call_count) + # Called once in JLinkarmDlInfo and once in Library. + self.assertEquals(2, mock_load_library.call_count) + # The dlinfo() dance silently failed, but will answer None resolved path. + self.assertIsNone(library.Library._dlinfo.path) + # Fallback to "search by file name" has succeeded. + self.assertEqual(directories[0], lib._path) + + @mock.patch('os.name', new='posix') + @mock.patch('sys.platform', new='linux') + @mock.patch('pylink.platform.libc_ver', return_value=('glibc', '2.34')) + @mock.patch('ctypes.util.find_library') + @mock.patch('ctypes.cdll.LoadLibrary') + def test_linux_dl_oserror(self, mock_load_library, mock_find_library, mock_libc_ver): + """Confirms ctype API exceptions actually propagate from JLinkarmDlInfo to call site. + + Test case: + - initial find_library('jlinkarm') succeeds + - the host system presents itself as GNU/Linux, and we successfully load libdl + - but loading libdl raises OSError + """ + + mock_find_library.side_effect = [ + # find_library('jlinkarm') + 'libjlinkarm.so.6', + # find_library('dl') + 'libdl.so.2' + ] + mock_load_library.side_effect = [ + # load JLink DLL + mock.Mock(), + # load libdl + OSError() + ] + + with self.assertRaises(OSError): + lib = library.Library() + lib.unload = mock.Mock() + + mock_find_library.assert_any_call(library.Library.JLINK_SDK_NAME) + mock_find_library.assert_any_call('dl') + self.assertEquals(2, mock_find_library.call_count) + self.assertEquals(2, mock_load_library.call_count) + if __name__ == '__main__': unittest.main()