Bug 1450593 - Gluster Python scripts do not check return value of find_library
Summary: Gluster Python scripts do not check return value of find_library
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: GlusterFS
Classification: Community
Component: core
Version: mainline
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Aravinda VK
QA Contact:
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2017-05-13 14:30 UTC by nh2
Modified: 2018-06-20 17:56 UTC (History)
3 users (show)

Fixed In Version: glusterfs-v4.1.0
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
Environment:
Last Closed: 2018-06-20 17:56:51 UTC
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Embargoed:


Attachments (Terms of Use)

Description nh2 2017-05-13 14:30:41 UTC
Gluster 3.10.1.

In many places, the gluster Pyton scripts, such as libgfchangelog.py (see https://github.com/gluster/glusterfs/blob/v3.10.1/geo-replication/syncdaemon/libgfchangelog.py#L18) use the pattern:

libgfc = CDLL(find_library("gfchangelog"), mode=RTLD_GLOBAL, use_errno=True)

This is improper error handling: You're supposed to check the return value of `find_library("yourlib")`. It can be `None`, which means the library was not found.

This can happen for packaging, messed up paths like in #1450588, wrong LD_LIBRARY_PATH, and so on.

CDLL also accepts `None` as first argument without throwing an exception, so the code just continues to run with a library that doesn't exist instead of failing early.

For exmaple, `cdll.LoadLibrary(find_library("nonexistent"))` returns a handle `<CDLL 'None', handle 7f4c46dac168 at 7f4c426fac50>`.

Gluster should check the return value of all find_library() calls, and error on failure.

Comment 1 nh2 2017-05-13 14:34:57 UTC
Note that in other places (such as tests), gluster checks the return value:

https://github.com/gluster/glusterfs/blob/v3.10.1/tests/features/ipctest.py#L9

Comment 2 nh2 2017-05-13 15:32:23 UTC
As documented in https://docs.python.org/2/library/ctypes.html#finding-shared-libraries for `ctypes.util.find_library(name)`:

"If no library can be found, returns None."

Comment 3 nh2 2017-05-13 15:37:04 UTC
After having looked more, `find_library()` is the wrong tool for the job.

It should not be used at all by gluster, because it does not consider LD_LIBRARY_PATH at all for finding libraries.

This has been a long-standing bug in Python since 2008, and it was finally fixed by Python 3.6:

https://bugs.python.org/issue9998
https://hg.python.org/cpython/rev/385181e809bc

But gluster doesn't require Python >= 3.6, it uses python2, which doesn't have this fix and will not receive it.

The right solution for gluster is probably to directly call

  ctypes.CDLL("libmylibrary.so", ...)

as is done in https://github.com/gluster/glusterfs/blob/v3.10.1/tests/features/ipctest.py#L10

Comment 4 nh2 2017-05-13 15:51:59 UTC
Note that care has to be taken with .so files that aren't real .so files but linker scripts; for example, `CDLL('libm.so')` will throw `invalid ELF header` because it's a text linker script; in contrast, `CDLL('libm.so.6')` will work.

Comment 5 nh2 2017-05-13 16:09:21 UTC
The same happens with `libc.so`, which gluster tries to load in libcxattr.py.

At least CDLL() fails loudly as long as it's not being passed `None`.

Comment 6 nh2 2017-09-27 20:45:45 UTC
Here is a patch for 3.10 that fixes it:

https://github.com/nh2/glusterfs/commit/d321df349d10f038f0c89b9c11f8059572264f1b

Comment 7 Worker Ant 2018-03-21 18:30:11 UTC
REVIEW: https://review.gluster.org/19762 (python: Remove all uses of find_library. Fixes #1450593) posted (#1) for review on master by Niklas Hambüchen

Comment 8 Worker Ant 2018-03-24 05:11:01 UTC
COMMIT: https://review.gluster.org/19762 committed in master by "Amar Tumballi" <amarts> with a commit message- python: Remove all uses of find_library. Fixes #1450593

`find_library()` doesn't consider LD_LIBRARY_PATH on Python < 3.6.

Change-Id: Iee26085cb5d14061001f19f032c2664d69a378a8
BUG: 1450593
Signed-off-by: Niklas Hambüchen <mail>

Comment 9 Shyamsundar 2018-06-20 17:56:51 UTC
This bug is getting closed because a release has been made available that should address the reported issue. In case the problem is still not fixed with glusterfs-v4.1.0, please open a new bug report.

glusterfs-v4.1.0 has been announced on the Gluster mailinglists [1], packages for several distributions should become available in the near future. Keep an eye on the Gluster Users mailinglist [2] and the update infrastructure for your distribution.

[1] http://lists.gluster.org/pipermail/announce/2018-June/000102.html
[2] https://www.gluster.org/pipermail/gluster-users/


Note You need to log in before you can comment on or make changes to this bug.