Bug 1450593

Summary:	Gluster Python scripts do not check return value of find_library
Product:	[Community] GlusterFS	Reporter:	nh2 <nh2-redhatbugzilla>
Component:	core	Assignee:	Aravinda VK <avishwan>
Status:	CLOSED CURRENTRELEASE	QA Contact:
Severity:	unspecified	Docs Contact:
Priority:	unspecified
Version:	mainline	CC:	avishwan, bugs, nh2-redhatbugzilla
Target Milestone:	---
Target Release:	---
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:	glusterfs-v4.1.0	Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:		Environment:
Last Closed:	2018-06-20 17:56:51 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:

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/