dog and sheep have executable stack

[rubenk]

rpmlint results_sheepdog/0.9.3/0.1.rc0.fc24/sheepdog-0.9.3-0.1.rc0.fc24.x86_64.rpm
sheepdog.x86_64: W: executable-stack /usr/sbin/dog
sheepdog.x86_64: W: executable-stack /usr/sbin/sheep

I think this is because the objects in lib/isa-l are compiled from asm and lack the GNU-stack elf note.

The easiest fix is to link against the shared isa library instead of the static one.

[vtolstov]

@zhanghongzhou can you helps with this?

[zhanghongzhou]

@rubenk Is this a new or legacy issue?

[rubenk]

@zhanghongzhou afaict the isa lib has always had executable stack.

[rubenk]

ceph had a patch for this but looking at this commit, this is fixed in ISA 2.14: https://github.com/ceph/ceph/commit/97c66e3f3f17eea0a40d20619d171c85aec3a9ed

[mitake]

a little bit off topic: ceph seems to bundle isa-l in the repository, isn't it problematic for packaging? cc @marcin-github

[rubenk]

As does sheepdog. At least on Fedora that is indeed problematic: https://fedoraproject.org/wiki/Packaging:Guidelines#Duplication_of_system_libraries

[marcin-github]

Dispute about bundling gtest in ceph: https://bugs.gentoo.org/show_bug.cgi?id=463326 (litttle off topic to off topic;) ) I removed executable stack from isa-l using patch http://repoz.mejor.pl/svn/gentoo/portage/dev-libs/isa-l/files/isa-l-makefile.patch In make.inc:

 $(so_lib_name): LDFLAGS+=-Wl,-soname,$(soname)
 $(so_lib_name): $(shared_objs) $(aobjs)
        @echo "  ---> Creating Shared Lib $@"
-       @$(CC) $(CFLAGS) --shared  $(LDFLAGS) -o $@ $^
+       $(CC) $(CFLAGS) --shared  $(LDFLAGS) -Wl,-z,noexecstack -o $@ $^
        @(cd $(@D); ln -f -s $(so_lib_inst) $(soname))

Code of isa-l bundled in ceph has such lines:

; inform linker that this doesn't require executable stack
section .note.GNU-stack noalloc noexec nowrite progbits

I don't know if it's added by ceph team or by upstream (which version of isa-l in in ceph? ) . In Gentoo ceph also has bundled isa-l library, as you can see above isn't well welcome but sometimes unbundling takes too much time for package maintainer and it leaves it as is. I'll try to suggest to unbundle isa-l, it could be used by sheepdog (when sheepdog will be added to Gentoo).

[rubenk]

@marcin-github yeah, I see those bundling issues all the time. From a vendor perspective it makes sense to provide a good out-of-the-box experience. From a distro perspective, not so much.

Your patch only works for shared libs, but upstream sheepdog currently uses the static lib, so it has no effect. Do you have another patch to make sheepdog use the shared (.so) libisa?

The ceph maintainers added the GNU-stack note a while ago, but that's now upstream in ISA 2.14. I think sheepdog needs to:

• update the shipped ISA version to 2.14
• use the shared library version of the isa lib.
• add a build option to optionally use a system version of libisa.

Does that make sense?

[marcin-github]

I'm unbundling isa-l from sheepdog using http://repoz.mejor.pl/svn/gentoo/portage/net-fs/sheepdog/files/unbundling-isa-l.patch

diff --git a/dog/Makefile.am b/dog/Makefile.am
index 9615468..86c3a45 100644
--- a/dog/Makefile.am
+++ b/dog/Makefile.am
@@ -37,7 +37,7 @@ if BUILD_NFS
 dog_SOURCES            += nfs.c
 endif

-dog_LDADD              = ../lib/libsd.a -lpthread
+dog_LDADD              = ../lib/libsd.a -lpthread -lisal
 dog_DEPENDENCIES       = ../lib/libsd.a

 noinst_HEADERS         = treeview.h dog.h farm/farm.h
diff --git a/include/fec.h b/include/fec.h
index 690a775..d05a343 100644
--- a/include/fec.h
+++ b/include/fec.h
@@ -65,7 +65,7 @@
 #include "util.h"
 #include "logger.h"
 #include "sheepdog_proto.h"
-#include "../lib/isa-l/include/erasure_code.h"
+#include "erasure_code.h"

 struct fec {
        unsigned long magic;
diff --git a/lib/Makefile.am b/lib/Makefile.am
index dec81c7..edde053 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -26,30 +26,12 @@ libsheepdog_a_CPPFLAGS  = $(AM_CPPFLAGS) -DNO_SHEEPDOG_LOGGER

 libsd_a_DEPENDENCIES =

-noinst_LIBRARIES       = libisa.a libsd.a
-
-libisa_a_SOURCES       = $(shell find isa-l/ -type f -regex ".*\.\(c\|h\|asm\)") \
-                         isa-l/erasure_code/Makefile \
-                         isa-l/Makefile \
-                         isa-l/Makefile.nmake \
-                         isa-l/make.inc
+noinst_LIBRARIES       = libsd.a

 libsd_a_SOURCES                = event.c logger.c net.c util.c rbtree.c strbuf.c \
                          sha1.c option.c work.c sockfd_cache.c fec.c \
                          sd_inode.c common.c

-libsd_a_LIBADD         = isa-l/bin/ec_base.o \
-                         isa-l/bin/ec_highlevel_func.o \
-                         isa-l/bin/ec_multibinary.o \
-                         isa-l/bin/gf_2vect_dot_prod_sse.o \
-                         isa-l/bin/gf_3vect_dot_prod_sse.o \
-                         isa-l/bin/gf_4vect_dot_prod_sse.o \
-                         isa-l/bin/gf_5vect_dot_prod_sse.o \
-                         isa-l/bin/gf_6vect_dot_prod_sse.o \
-                         isa-l/bin/gf_vect_dot_prod_sse.o \
-                         isa-l/bin/gf_vect_mul_avx.o \
-                         isa-l/bin/gf_vect_mul_sse.o
-
 if BUILD_SHA1_HW
 libsd_a_SOURCES                += sha1_ssse3.S
 endif
@@ -73,9 +55,6 @@ check-syntax:
 check-style:
        @$(CHECK_STYLE) $(libsd_a_SOURCES)

-libisa.a:
-       cd isa-l/ && $(MAKE) && cd ..
-
 clean-local:
        rm -f *.o gmon.out *.da *.bb *.bbg
        cd isa-l/ && $(MAKE) clean && cd ..
diff --git a/sheep/Makefile.am b/sheep/Makefile.am
index 70ee0fc..cae9ac8 100644
--- a/sheep/Makefile.am
+++ b/sheep/Makefile.am
@@ -56,7 +56,7 @@ AM_CPPFLAGS           += -DENABLE_TRACE
 sheep_SOURCES          += trace/trace.c trace/mcount.S trace/graph.c trace/checker.c
 endif

-sheep_LDADD            = ../lib/libsd.a -lpthread -lm \
+sheep_LDADD            = ../lib/libsd.a -lpthread -lm -lisal \
                          $(libcpg_LIBS) $(libcfg_LIBS) $(libacrd_LIBS) $(LIBS) \
                          $(libqb_LIBS) $(libcorosync_common_LIBS)

"option to optionally use a system version of libisa" sounds for me as good settlement.

[rubenk]

Thanks for the patch @marcin-github. Am I right in that this only works with an unbundled version and not with the included libisa code?

[marcin-github]

Yes, for my purposes I've got isa-l package which gives me shared libraries and header files so I didn't need to have chooice to switch beetwen bundled or system wide libraries (to be sure I'm removing isa-l code from sources of sheepdog before compilation) .