Dlopen the actual linked libpython #1592

spbnick · Apr 25, 2016

In rlm_python, dlopen libpython shared library at the actual path it was
linked with on loading, instead of just the version-less SONAME.

This removes the need to have the version-less SONAME symlink (e.g.
"libpython2.7.so") in library directory, which is normally installed
only with the development packages. I.e. this removes the requirement of
having python-devel/libpython-dev installed, when loading rlm_python.

Ahem?

Hi Arran,

I'm not sure what you mean, but I'll make a guess :)
This is matching the beginning of the actual linked filename.
E.g. pattern might be:

/libpython2.7.so

And the matched path might be:

/usr/lib/x86_64-linux-gnu/libpython2.7.so.1.0

This is to make it match the exact python version we were building with (not really necessary, as it's linked already) and to prevent matching against something like libpython-blah.so (which, e.g. can be linked by libpython in its own turn), if we would only use "/libpython" pattern, without the version.

What about when we're not on Linux :)

Ugh, looks like that was pre-existing, OK, i'll fix that.

Well, regarding path matching, the filename is still the same as before the patch (e.g. "libpython2.7.so"), only the slash is added, which should be the same on all platforms FreeRADIUS targets, AFAIK.

I'm not sure about dl_iterate_phdr, but it seems it is widely supported.

Or will it by libpython2.7.dll on Cygwin and dlopen replaces the .so for us? If so, then we can drop the extension.

talloc_free

talloc_strdup(NULL, info->dlpi_name)

It's better to use talloc functions, because it allows us to track all the allocations done by us when debugging memory leaks.

Sure, I agree with the usefulness of tracking. I'll switch the patch to talloc.

talloc_free

spbnick · Apr 25, 2016

Alright, I switched the patch to talloc and removed the "so" from the end of the pattern to try to handle Cygwin.

I wonder, should I also remove the slash?

arr2036 · Apr 25, 2016

Not sure, was hunting for docs on dlopen to see if it included the extension... I guess try it and see? :)

spbnick · Apr 26, 2016

Right, I was wrong about dl_iterate_phdr being widely available. Apparently wishful thinking doesn't work :)

So, it's not on Cygwin, and the other platform support appears to be spotty.

Do we have any options except scrapping the whole patch?

arr2036 · Apr 26, 2016

POSIX only give us:

dlclose - close a dlopen object 
dlerror - get error
dlopen - gain access to an executable object file 
dlsym - obtain the address of a symbol from a dlopen object

So doesn't look like it. Could pass in -l as a -D and use that?

spbnick · Apr 26, 2016

Yeah, POSIX is limited and it's not there. No, -l wouldn't help as it only (but perfectly correctly) has "python2.7", which is essentially what you already ask for in your dlopen call.

I suspect there are similar interfaces on all the target platforms, but branching and using them all would be too much.

One last idea that I have is to check for dl_iterate_phdr at configure time and compile the lookup conditionally.

arr2036 · Apr 26, 2016

Situation isn't that bad actually.

Linux, FreeBSD and Solaris 11 all support dl_iterate_phdr

For OSX you have:

 uint32_t _dyld_image_count(void);
const char* _dyld_get_image_name(uint32_t image_index);

https://developer.apple.com/library/mac/documentation/Darwin/Reference/ManPages/man3/dyld.3.html

spbnick · Apr 26, 2016

Yeah, older FreeBSD and Solaris releases don't have it.
So, would you like to do the build-time condition?
I can start with checking for dl_iterate_phdr. However, I don't have OS X to test on.

spbnick · Apr 27, 2016

Alright, I'll make a patch which does the same this one does, but only if dh_iterate_phdr is present. Otherwise it will fallback to what we already have. I'd like to do the Mac OS X support, but I wasn't able to find anywhere to test it.

Expect a new patch not later than tomorrow.

spbnick · Apr 28, 2016

Alright, this version uses dl_iterate_phdr only if it is found by configure and otherwise falls back to the old way of loading libpython.

That needs to be conditional on HAVE_DL_ITERATE_PHDR

spbnick · Apr 29, 2016

Doh, yes, thank you, Arran. Fixed.

spbnick · May 13, 2016

@arr2036, is there anything you'd like me to do to this pull request, or could it be merged?

spbnick · Feb 16, 2017

Repeating here why I think this should still be merged, posted earlier on the maillist.

First, this is not very OS-specific. This is a problem on all Linux distros. The version-less libraries are only for development, and at runtime you shouldn't expect them to be present and load the fully-versioned library instead.

For example, on Debian Testing, without libpython2.7-dev installed, I get the
same issue:

  # Instantiating module "python" from file /etc/freeradius/3.0/mods-enabled/python
Python version: 2.7.13 (default, Jan 19 2017, 14:48:08)  [GCC 6.3.0 20170118]
Failed loading libpython symbols into global symbol table: libpython2.7.so: cannot open shared object file: No such file or directory

Second, even if we add it as a build-time patch, it will be a pain to maintain, especially since it touches "configure" and "config.h.in" files, which are generated.

alandekok merged commit 1d9a88a into FreeRADIUS:v3.0.x Feb 27, 2017
1 check passed

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@@ -205,6 +206,58 @@ static void mod_error(void)
      	Py_XDECREF(pTraceback);
      }
+    +static int dlopen_linked_libpython_cb(struct dl_phdr_info *info,
+    +					UNUSED size_t size, void *data)
+    +{
+    +	const char *pattern = "/libpython"
+    +				STRINGIFY(PY_MAJOR_VERSION) "."
+    +				STRINGIFY(PY_MINOR_VERSION) ".so";

@@ -205,6 +206,56 @@ static void mod_error(void)
      	Py_XDECREF(pTraceback);
      }
+    +static int dlopen_linked_libpython_cb(struct dl_phdr_info *info,
+    +					UNUSED size_t size, void *data)
+    +{
+    +	const char *pattern = "/libpython"
+    +				STRINGIFY(PY_MAJOR_VERSION) "."
+    +				STRINGIFY(PY_MINOR_VERSION) ".so";
+    +	char **ppath = (char **)data;
+    +
+    +	if (strstr(info->dlpi_name, pattern) != NULL) {
+    +		if (*ppath != NULL) {
+    +			free(*ppath);

@@ -205,6 +206,56 @@ static void mod_error(void)
      	Py_XDECREF(pTraceback);
      }
+    +static int dlopen_linked_libpython_cb(struct dl_phdr_info *info,
+    +					UNUSED size_t size, void *data)
+    +{
+    +	const char *pattern = "/libpython"
+    +				STRINGIFY(PY_MAJOR_VERSION) "."
+    +				STRINGIFY(PY_MINOR_VERSION) ".so";
+    +	char **ppath = (char **)data;
+    +
+    +	if (strstr(info->dlpi_name, pattern) != NULL) {
+    +		if (*ppath != NULL) {
+    +			free(*ppath);
+    +			*ppath = NULL;
+    +			return EEXIST;
+    +		} else {
+    +			*ppath = strdup(info->dlpi_name);

+    +	rc = dl_iterate_phdr(dlopen_linked_libpython_cb, &path);
+    +	if (rc != 0) {
+    +		WARN("Failed searching for libpython "
+    +			"among linked libraries: %s", strerror(rc));
+    +		return NULL;
+    +	} else if (path == NULL) {
+    +		WARN("Libpython is not found among linked libraries");
+    +		return NULL;
+    +	}
+    +
+    +	/* Dlopen the found library */
+    +	handle = dlopen(path, flags);
+    +	if (handle == NULL) {
+    +		WARN("Failed loading %s: %s", path, dlerror());
+    +	}
+    +	free(path);

      #include <freeradius-devel/radiusd.h>
      #include <freeradius-devel/modules.h>
      #include <Python.h>
      #include <dlfcn.h>
+    +#include <link.h>

FreeRADIUS/freeradius-server

Dlopen the actual linked libpython #1592

spbnick commented Apr 25, 2016

arr2036 and 1 other commented on an outdated diff Apr 25, 2016

arr2036 commented on an outdated diff Apr 25, 2016

arr2036 and 1 other commented on an outdated diff Apr 25, 2016

arr2036 commented on an outdated diff Apr 25, 2016

spbnick commented Apr 25, 2016

arr2036 commented Apr 25, 2016

spbnick commented Apr 26, 2016

arr2036 commented Apr 26, 2016

spbnick commented Apr 26, 2016

arr2036 commented Apr 26, 2016

spbnick commented Apr 26, 2016 • edited

spbnick commented Apr 27, 2016

spbnick commented Apr 28, 2016

arr2036 commented on an outdated diff Apr 28, 2016

spbnick commented Apr 29, 2016

spbnick commented May 13, 2016

spbnick commented Feb 16, 2017

alandekok merged commit `1d9a88a` into FreeRADIUS:v3.0.x Feb 27, 2017
1 check passed

1 check passed

FreeRADIUS/freeradius-server

Join GitHub today

Dlopen the actual linked libpython #1592

Conversation

spbnick commented Apr 25, 2016

Show 6 comments Hide comments arr2036 and 1 other commented on an outdated diff Apr 25, 2016

Show 1 comment Hide comment arr2036 commented on an outdated diff Apr 25, 2016

Show 3 comments Hide comments arr2036 and 1 other commented on an outdated diff Apr 25, 2016

Show 1 comment Hide comment arr2036 commented on an outdated diff Apr 25, 2016

spbnick commented Apr 25, 2016

arr2036 commented Apr 25, 2016

spbnick commented Apr 26, 2016

arr2036 commented Apr 26, 2016

spbnick commented Apr 26, 2016

arr2036 commented Apr 26, 2016

spbnick commented Apr 26, 2016 • edited

spbnick commented Apr 27, 2016

spbnick commented Apr 28, 2016

Show 1 comment Hide comment arr2036 commented on an outdated diff Apr 28, 2016

spbnick commented Apr 29, 2016

spbnick commented May 13, 2016

spbnick commented Feb 16, 2017

Hide details View details alandekok merged commit 1d9a88a into FreeRADIUS:v3.0.x Feb 27, 2017 1 check passed

1 check passed

arr2036 and 1 other commented on an outdated diff Apr 25, 2016

arr2036 commented on an outdated diff Apr 25, 2016

arr2036 and 1 other commented on an outdated diff Apr 25, 2016

arr2036 commented on an outdated diff Apr 25, 2016

arr2036 commented on an outdated diff Apr 28, 2016

alandekok merged commit `1d9a88a` into FreeRADIUS:v3.0.x Feb 27, 2017
1 check passed