diff mbox series

[1/1] package/libselinux: fix build with python 3.8

Message ID 20191020203119.99538-1-james.hilliard1@gmail.com
State Changes Requested
Headers show
Series [1/1] package/libselinux: fix build with python 3.8 | expand

Commit Message

James Hilliard Oct. 20, 2019, 8:31 p.m. UTC 
Fixes:
 - http://autobuild.buildroot.net/results/839e81c5b968c47842b4d0c25b902397af8ea5a5

Details:
 - https://bugs.python.org/issue36721
 - https://github.com/python/cpython/commit/7efc526e5cfb929a79c192ac2dcf7eb78d3a4401

Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
---
 package/libselinux/libselinux.mk | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

Comments

Thomas Petazzoni Oct. 21, 2019, 8:06 a.m. UTC | #1 
On Sun, 20 Oct 2019 14:31:19 -0600
James Hilliard <james.hilliard1@gmail.com> wrote:

> diff --git a/package/libselinux/libselinux.mk b/package/libselinux/libselinux.mk
> index c707c8a25e..75f01a84b7 100644
> --- a/package/libselinux/libselinux.mk
> +++ b/package/libselinux/libselinux.mk
> @@ -33,7 +33,7 @@ endif
>  ifeq ($(BR2_PACKAGE_PYTHON)$(BR2_PACKAGE_PYTHON3),y)
>  ifeq ($(BR2_PACKAGE_PYTHON3),y)
>  LIBSELINUX_DEPENDENCIES += python3 host-swig
> -LIBSELINUX_PYINC = -I$(STAGING_DIR)/usr/include/python$(PYTHON3_VERSION_MAJOR)m
> +LIBSELINUX_PYINC = -I$(STAGING_DIR)/usr/include/python$(PYTHON3_VERSION_MAJOR)

Considering the number of packages that hardcode this path, perhaps we
should have a variable provided by some common code, so that if it
changes again in the future, we don't have to chase down multiple
packages ?

Thomas
James Hilliard Oct. 21, 2019, 3:18 p.m. UTC | #2 
On Mon, Oct 21, 2019 at 10:06 AM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Sun, 20 Oct 2019 14:31:19 -0600
> James Hilliard <james.hilliard1@gmail.com> wrote:
>
> > diff --git a/package/libselinux/libselinux.mk b/package/libselinux/libselinux.mk
> > index c707c8a25e..75f01a84b7 100644
> > --- a/package/libselinux/libselinux.mk
> > +++ b/package/libselinux/libselinux.mk
> > @@ -33,7 +33,7 @@ endif
> >  ifeq ($(BR2_PACKAGE_PYTHON)$(BR2_PACKAGE_PYTHON3),y)
> >  ifeq ($(BR2_PACKAGE_PYTHON3),y)
> >  LIBSELINUX_DEPENDENCIES += python3 host-swig
> > -LIBSELINUX_PYINC = -I$(STAGING_DIR)/usr/include/python$(PYTHON3_VERSION_MAJOR)m
> > +LIBSELINUX_PYINC = -I$(STAGING_DIR)/usr/include/python$(PYTHON3_VERSION_MAJOR)
>
> Considering the number of packages that hardcode this path, perhaps we
> should have a variable provided by some common code, so that if it
> changes again in the future, we don't have to chase down multiple
> packages ?
Maybe, although there aren't all that many from what I can tell and some seem
to use slightly different variables. It doesn't seem all that likely
that this will change
in the near future again at least since this change was only finalized
after many
python releases.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Thomas Petazzoni Oct. 21, 2019, 7:06 p.m. UTC | #3 
On Sun, 20 Oct 2019 14:31:19 -0600
James Hilliard <james.hilliard1@gmail.com> wrote:

> +ifeq ($(BR2_PACKAGE_PYTHON3),y)
> +LIBSELINUX_MAKE_OPTS += \
> +	PYLIBS="`$(STAGING_DIR)/usr/bin/python3-config --libs --embed`"
> +endif

Same question: libselinux is not embedding Python, it is providing a
Python extension written in C, so according to what's in
https://bugs.python.org/issue36721, we should not need --embed.

Thomas
James Hilliard Oct. 21, 2019, 7:25 p.m. UTC | #4 
On Mon, Oct 21, 2019 at 9:06 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Sun, 20 Oct 2019 14:31:19 -0600
> James Hilliard <james.hilliard1@gmail.com> wrote:
>
> > +ifeq ($(BR2_PACKAGE_PYTHON3),y)
> > +LIBSELINUX_MAKE_OPTS += \
> > +     PYLIBS="`$(STAGING_DIR)/usr/bin/python3-config --libs --embed`"
> > +endif
>
> Same question: libselinux is not embedding Python, it is providing a
> Python extension written in C, so according to what's in
> https://bugs.python.org/issue36721, we should not need --embed.
I think it might depend on if python is built with static vs shared libraries.
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Thomas Petazzoni Oct. 21, 2019, 9:18 p.m. UTC | #5 
On Mon, 21 Oct 2019 21:25:52 +0200
James Hilliard <james.hilliard1@gmail.com> wrote:

> On Mon, Oct 21, 2019 at 9:06 PM Thomas Petazzoni
> <thomas.petazzoni@bootlin.com> wrote:
> >
> > On Sun, 20 Oct 2019 14:31:19 -0600
> > James Hilliard <james.hilliard1@gmail.com> wrote:
> >  
> > > +ifeq ($(BR2_PACKAGE_PYTHON3),y)
> > > +LIBSELINUX_MAKE_OPTS += \
> > > +     PYLIBS="`$(STAGING_DIR)/usr/bin/python3-config --libs --embed`"
> > > +endif  
> >
> > Same question: libselinux is not embedding Python, it is providing a
> > Python extension written in C, so according to what's in
> > https://bugs.python.org/issue36721, we should not need --embed.  
> I think it might depend on if python is built with static vs shared libraries.

That is not really what the bug report says. It says the reason to not
link Python extensions with libpython was to allow loading Python
modules in a statically-compiled Python, if I understood correctly.

Thomas
James Hilliard Oct. 21, 2019, 9:42 p.m. UTC | #6 
On Mon, Oct 21, 2019 at 11:18 PM Thomas Petazzoni
<thomas.petazzoni@bootlin.com> wrote:
>
> On Mon, 21 Oct 2019 21:25:52 +0200
> James Hilliard <james.hilliard1@gmail.com> wrote:
>
> > On Mon, Oct 21, 2019 at 9:06 PM Thomas Petazzoni
> > <thomas.petazzoni@bootlin.com> wrote:
> > >
> > > On Sun, 20 Oct 2019 14:31:19 -0600
> > > James Hilliard <james.hilliard1@gmail.com> wrote:
> > >
> > > > +ifeq ($(BR2_PACKAGE_PYTHON3),y)
> > > > +LIBSELINUX_MAKE_OPTS += \
> > > > +     PYLIBS="`$(STAGING_DIR)/usr/bin/python3-config --libs --embed`"
> > > > +endif
> > >
> > > Same question: libselinux is not embedding Python, it is providing a
> > > Python extension written in C, so according to what's in
> > > https://bugs.python.org/issue36721, we should not need --embed.
> > I think it might depend on if python is built with static vs shared libraries.
>
> That is not really what the bug report says. It says the reason to not
> link Python extensions with libpython was to allow loading Python
> modules in a statically-compiled Python, if I understood correctly.
I think it's probably due to a quirk with the kmod build system where it's doing
something weird here that's different from the typical setuptool/distutils build
process:
https://git.kernel.org/pub/scm/utils/kernel/kmod/kmod.git/tree/Makefile.am?id=58133a96c894c043e48c74ddf0bfe8db90bac62f#n178
>
> Thomas
> --
> Thomas Petazzoni, CTO, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Thomas Petazzoni Oct. 22, 2019, 7:40 a.m. UTC | #7 
Hello,

On Mon, 21 Oct 2019 23:18:48 +0200
Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

> > > Same question: libselinux is not embedding Python, it is providing a
> > > Python extension written in C, so according to what's in
> > > https://bugs.python.org/issue36721, we should not need --embed.    
> > I think it might depend on if python is built with static vs shared libraries.  
> 
> That is not really what the bug report says. It says the reason to not
> link Python extensions with libpython was to allow loading Python
> modules in a statically-compiled Python, if I understood correctly.

So, if you try to build python-alsaaudio, which has some native code,
you will see that it builds fine. The resulting .so Python extension is
not linked against libpython, even though it calls functions like
PyList_New(). This is fine, as the module will be dlopened() in the
Python interpreter, which will provide PyList_New().

libselinux also uses PyList_New(), but complains that it is not
available. So I looked at the libselinux link of the Python extension.

When I use the link command from the Makefile, i.e:

arm-linux-gcc -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -Os -I../include -D_GNU_SOURCE  -DNO_ANDROID_BACKEND  -lpcre -lpthread -L. -shared -o python-3.8audit2why.so python-3.8audit2why.lo -lselinux -l:libsepol.a  -Wl,-soname,audit2why.so,--version-script=audit2why.map,-z,defs

Then the build fails with lots of undefined symbols, such as
PyList_New().

If I change the command line to just:

arm-linux-gcc -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -Os -I../include -D_GNU_SOURCE  -DNO_ANDROID_BACKEND  -lpcre -lpthread -L. -shared -o python-3.8audit2why.so python-3.8audit2why.lo -lselinux -l:libsepol.a  -Wl,-soname,audit2why.so

i.e, I simply drop the version script. Then doh, it builds fine!

It does have undefined references to PyList_New, but it is not linked
against libpython:

$ arm-linux-gnu-readelf -W -a python-3.8audit2why.so | grep PyList_New
0003c29c  00002316 R_ARM_JUMP_SLOT        00000000   PyList_New
    35: 00000000     0 NOTYPE  GLOBAL DEFAULT  UND PyList_New
  1145: 00000000     0 NOTYPE  GLOBAL DEFAULT  UND PyList_New

$ arm-linux-gnu-readelf -W -a python-3.8audit2why.so | grep NEEDED
 0x00000001 (NEEDED)                     Shared library: [libpcre.so.1]
 0x00000001 (NEEDED)                     Shared library: [libselinux.so.1]
 0x00000001 (NEEDED)                     Shared library: [libc.so.0]

So as you can see linking against libpython is *not* the solution, it
is just a workaround for another real problem. I have not found (yet)
what is the problem with the versioning script, but I guess we should
research in this direction.

Best regards,

Thomas
Thomas Petazzoni Oct. 22, 2019, 7:45 a.m. UTC | #8 
Hello,

On Tue, 22 Oct 2019 09:40:25 +0200
Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

> So as you can see linking against libpython is *not* the solution, it
> is just a workaround for another real problem. I have not found (yet)
> what is the problem with the versioning script, but I guess we should
> research in this direction.

So, it is not the versioning script. In fact, when testing I removed
both the --version-script option and the -z defs option. But it is
actually the -z defs option that causes the problem.

And indeed:

       -z defs

           Report unresolved symbol references from regular object
           files.  This is done even if the linker is creating a
           non-symbolic shared library.  The switch
           --[no-]allow-shlib-undefined controls the behaviour for
           reporting unresolved references found in shared libraries
           being linked in.

Using -z defs in this particular case doesn't make any sense: we do
acknowledge that we can have remaining unresolved symbols. Then will be
provided by the Python interpreter.

So I believe the right fix is:

+++ src/Makefile	2019-10-22 09:44:26.700158005 +0200
@@ -165,7 +165,7 @@
 	$(CC) $(filter-out -Werror, $(CFLAGS)) $(PYINC) -fPIC -DSHARED -c -o $@ $<
 
 $(AUDIT2WHYSO): $(AUDIT2WHYLOBJ) $(LIBSEPOLA)
-	$(CC) $(CFLAGS) $(LDFLAGS) -L. -shared -o $@ $^ -lselinux $(LDLIBS_LIBSEPOLA) $(PYLIBS) -Wl,-soname,audit2why.so,--version-script=audit2why.map,-z,defs
+	$(CC) $(CFLAGS) $(LDFLAGS) -L. -shared -o $@ $^ -lselinux $(LDLIBS_LIBSEPOLA) $(PYLIBS) -Wl,-soname,audit2why.so,--version-script=audit2why.map
 
 %.o:  %.c policy.h
 	$(CC) $(CFLAGS) $(TLSFLAGS) -c -o $@ $<

Best regards,

Thomas
Thomas Petazzoni Oct. 22, 2019, 9:10 a.m. UTC | #9 
Hello,

+Matt Weber, SELinux stuff inside.

On Tue, 22 Oct 2019 09:45:00 +0200
Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

> So I believe the right fix is:

With this fix, the module builds properly. With
http://patchwork.ozlabs.org/patch/1181164/, it gets installed to the
target. However, importing the module fails:

Python 3.8.0 (default, Oct 21 2019, 21:15:12) 
[GCC 4.9.4] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import selinux
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "usr/lib/python3.8/site-packages/selinux/__init__.py", line 13, in <module>
ImportError: cannot import name '_selinux' from partially initialized module 'selinux' (most likely due to a circular import) (/usr/lib/python3.8/site-packages/selinux/__init__.pyc)

One interesting thing is that the name of the _selinux module is incorrect:

_selinux.cpython-38-x86_64-linux-gnu.so

See the "x86_64-linux-gnu" ? A properly working module has the following name:

alsaaudio.cpython-38-arm-linux-gnueabi.so

You might think _selinux.cpython-38-x86_64-linux-gnu.so is built for
x86-64, but it's not, it's properly cross-compiled for ARM:

$ file _selinux.cpython-38-x86_64-linux-gnu.so 
_selinux.cpython-38-x86_64-linux-gnu.so: ELF 32-bit LSB shared object, ARM, EABI5 version 1 (SYSV), dynamically linked, stripped

More investigation is needed.

Thomas
Thomas Petazzoni Oct. 25, 2019, 9:28 a.m. UTC | #10 
On Tue, 22 Oct 2019 11:10:17 +0200
Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:

> Hello,
> 
> +Matt Weber, SELinux stuff inside.
> 
> On Tue, 22 Oct 2019 09:45:00 +0200
> Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote:
> 
> > So I believe the right fix is:  
> 
> With this fix, the module builds properly. With
> http://patchwork.ozlabs.org/patch/1181164/, it gets installed to the
> target. However, importing the module fails:
> 
> Python 3.8.0 (default, Oct 21 2019, 21:15:12) 
> [GCC 4.9.4] on linux
> Type "help", "copyright", "credits" or "license" for more information.
> >>> import selinux  
> Traceback (most recent call last):
>   File "<stdin>", line 1, in <module>
>   File "usr/lib/python3.8/site-packages/selinux/__init__.py", line 13, in <module>
> ImportError: cannot import name '_selinux' from partially initialized module 'selinux' (most likely due to a circular import) (/usr/lib/python3.8/site-packages/selinux/__init__.pyc)

This problem, as well as the initial -z defs issue, has been resolved upstream by:

  https://github.com/SELinuxProject/selinux/commit/2efa06857575e4118e91ca250b6b92da68b130d5

I'm now trying to backport that to 2.9.

Best regards,

Thomas
diff mbox series

Patch

diff --git a/package/libselinux/libselinux.mk b/package/libselinux/libselinux.mk
index c707c8a25e..75f01a84b7 100644
--- a/package/libselinux/libselinux.mk
+++ b/package/libselinux/libselinux.mk
@@ -33,7 +33,7 @@  endif
 ifeq ($(BR2_PACKAGE_PYTHON)$(BR2_PACKAGE_PYTHON3),y)
 ifeq ($(BR2_PACKAGE_PYTHON3),y)
 LIBSELINUX_DEPENDENCIES += python3 host-swig
-LIBSELINUX_PYINC = -I$(STAGING_DIR)/usr/include/python$(PYTHON3_VERSION_MAJOR)m
+LIBSELINUX_PYINC = -I$(STAGING_DIR)/usr/include/python$(PYTHON3_VERSION_MAJOR)
 LIBSELINUX_PYLIBVER = python$(PYTHON3_VERSION_MAJOR)
 else ifeq ($(BR2_PACKAGE_PYTHON),y)
 LIBSELINUX_DEPENDENCIES += python host-swig
@@ -47,6 +47,11 @@  LIBSELINUX_MAKE_OPTS += \
 	PYSITEDIR=$(TARGET_DIR)/usr/lib/$(LIBSELINUX_PYLIBVER)/site-packages \
 	SWIG_LIB="$(HOST_DIR)/share/swig/$(SWIG_VERSION)/"
 
+ifeq ($(BR2_PACKAGE_PYTHON3),y)
+LIBSELINUX_MAKE_OPTS += \
+	PYLIBS="`$(STAGING_DIR)/usr/bin/python3-config --libs --embed`"
+endif
+
 LIBSELINUX_MAKE_INSTALL_TARGETS += install-pywrap
 
 # dependencies are broken and result in file truncation errors at link
@@ -84,7 +89,7 @@  HOST_LIBSELINUX_DEPENDENCIES = \
 
 ifeq ($(BR2_PACKAGE_PYTHON3),y)
 HOST_LIBSELINUX_DEPENDENCIES += host-python3
-HOST_LIBSELINUX_PYINC = -I$(HOST_DIR)/include/python$(PYTHON3_VERSION_MAJOR)m/
+HOST_LIBSELINUX_PYINC = -I$(HOST_DIR)/include/python$(PYTHON3_VERSION_MAJOR)/
 HOST_LIBSELINUX_PYLIBVER = python$(PYTHON3_VERSION_MAJOR)
 else
 HOST_LIBSELINUX_DEPENDENCIES += host-python