| Message ID | 0dae86e37516236d337e8433c63047504f1576f9.1415126141.git.baruch@tkos.co.il |
|---|---|
| State | Rejected |
| Headers | show |
Dear Baruch Siach, On Tue, 4 Nov 2014 20:35:41 +0200, Baruch Siach wrote: > Edit libtool to remove RPATH from generated binaries. This is similar to > upstream commit making the same change when buiding RPMs. > https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commitdiff;h=a4a624e4c5 > > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > --- > package/wireshark/wireshark.mk | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/package/wireshark/wireshark.mk b/package/wireshark/wireshark.mk > index ccf5a56f1f23..6c7c3920e7af 100644 > --- a/package/wireshark/wireshark.mk > +++ b/package/wireshark/wireshark.mk > @@ -15,6 +15,12 @@ WIRESHARK_AUTORECONF = YES > WIRESHARK_CONF_ENV = LIBGCRYPT_CONFIG=$(STAGING_DIR)/usr/bin/libgcrypt-config \ > ac_cv_path_PCAP_CONFIG=$(STAGING_DIR)/usr/bin/pcap-config > > +define WIRESHARK_REMOVE_RPATH > + $(SED) 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' $(@D)/libtool > + $(SED) 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' $(@D)/libtool > +endef > +WIRESHARK_POST_CONFIGURE_HOOKS += WIRESHARK_REMOVE_RPATH Isn't this problem potentially identical in many packages that use libtool? What leads the wireshark libtool to have this specifically? Thomas
Hi Thomas, On Wed, Nov 05, 2014 at 08:15:55AM +0100, Thomas Petazzoni wrote: > On Tue, 4 Nov 2014 20:35:41 +0200, Baruch Siach wrote: > > Edit libtool to remove RPATH from generated binaries. This is similar to > > upstream commit making the same change when buiding RPMs. > > https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commitdiff;h=a4a624e4c5 > > > > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > > --- > > package/wireshark/wireshark.mk | 6 ++++++ > > 1 file changed, 6 insertions(+) > > > > diff --git a/package/wireshark/wireshark.mk b/package/wireshark/wireshark.mk > > index ccf5a56f1f23..6c7c3920e7af 100644 > > --- a/package/wireshark/wireshark.mk > > +++ b/package/wireshark/wireshark.mk > > @@ -15,6 +15,12 @@ WIRESHARK_AUTORECONF = YES > > WIRESHARK_CONF_ENV = LIBGCRYPT_CONFIG=$(STAGING_DIR)/usr/bin/libgcrypt-config \ > > ac_cv_path_PCAP_CONFIG=$(STAGING_DIR)/usr/bin/pcap-config > > > > +define WIRESHARK_REMOVE_RPATH > > + $(SED) 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' $(@D)/libtool > > + $(SED) 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' $(@D)/libtool > > +endef > > +WIRESHARK_POST_CONFIGURE_HOOKS += WIRESHARK_REMOVE_RPATH > > Isn't this problem potentially identical in many packages that use > libtool? What leads the wireshark libtool to have this specifically? Probably. The Buildorot speex package does an almost identical edit to its own libtool. Do you think we should enable this for all libtool users? We'll only know if anything breaks at run time. baruch
Dear Baruch Siach, On Wed, 5 Nov 2014 09:22:14 +0200, Baruch Siach wrote: > > Isn't this problem potentially identical in many packages that use > > libtool? What leads the wireshark libtool to have this specifically? > > Probably. The Buildorot speex package does an almost identical edit to its own > libtool. Do you think we should enable this for all libtool users? We'll only > know if anything breaks at run time. Well, not having a rpath should not break things at runtime, unless a custom rpath is really necessary. But I'm pretty sure in those cases, the custom rpath will be wrong as it will point to some location on the build machine. What would be good is to understand why in certain situations libtool decides to set a rpath and not in other cases. Thomas
Hi Thomas, On Wed, Nov 05, 2014 at 08:31:24AM +0100, Thomas Petazzoni wrote: > On Wed, 5 Nov 2014 09:22:14 +0200, Baruch Siach wrote: > > > > Isn't this problem potentially identical in many packages that use > > > libtool? What leads the wireshark libtool to have this specifically? > > > > Probably. The Buildorot speex package does an almost identical edit to its own > > libtool. Do you think we should enable this for all libtool users? We'll only > > know if anything breaks at run time. > > Well, not having a rpath should not break things at runtime, unless a > custom rpath is really necessary. But I'm pretty sure in those cases, > the custom rpath will be wrong as it will point to some location on the > build machine. > > What would be good is to understand why in certain situations libtool > decides to set a rpath and not in other cases. It seems that libtool decides to set rpath based on .la files we have in staging. Some of them (e.g. glib provided libs, and libnl) set either 'dependency_libs' (glib) or 'libdir' (libnl) to paths under output/build or staging. wireshark also links its own libraries (e.g. wiretap, epan) using local source tree .la files. I'm not sure how to solve this in the general case. It probably requires fixing up all these .la files. baruch
On 11/17/2014 09:33 AM, Baruch Siach wrote: > It seems that libtool decides to set rpath based on .la files we have in > staging. Some of them (e.g. glib provided libs, and libnl) set either > 'dependency_libs' (glib) or 'libdir' (libnl) to paths under output/build or > staging. wireshark also links its own libraries (e.g. wiretap, epan) using > local source tree .la files. I'm not sure how to solve this in the general > case. It probably requires fixing up all these .la files. Hi. There's a good briefing at https://wiki.debian.org/RpathIssue Fixing up is nice, however doing it in a good manner will require many autoreconfs and being proactive about it. Stripping RPATHs is probably a faster way out (via chrpath). In the end this could be considered as a build information leak, not terribly useful but doesn't look good either. Regards.
On Mon, Nov 17, 2014 at 1:45 PM, Gustavo Zacarias <gustavo@zacarias.com.ar> wrote: > On 11/17/2014 09:33 AM, Baruch Siach wrote: > >> It seems that libtool decides to set rpath based on .la files we have in >> staging. Some of them (e.g. glib provided libs, and libnl) set either >> 'dependency_libs' (glib) or 'libdir' (libnl) to paths under output/build or >> staging. wireshark also links its own libraries (e.g. wiretap, epan) using >> local source tree .la files. I'm not sure how to solve this in the general >> case. It probably requires fixing up all these .la files. The weird thing is that not all autotools packages have a rpath... :-/ I've already try to have a look at libtool, but it requires quite a brain at 300% and some free time to dive into this hell. > > Hi. > There's a good briefing at https://wiki.debian.org/RpathIssue > Fixing up is nice, however doing it in a good manner will require many > autoreconfs and being proactive about it. > Stripping RPATHs is probably a faster way out (via chrpath). or patchelf (already in buildroot) ;-) > In the end this could be considered as a build information leak, not > terribly useful but doesn't look good either. > Regards. > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot Regards,
Hi Gustavo, On Mon, Nov 17, 2014 at 09:45:48AM -0300, Gustavo Zacarias wrote: > On 11/17/2014 09:33 AM, Baruch Siach wrote: > > It seems that libtool decides to set rpath based on .la files we have in > > staging. Some of them (e.g. glib provided libs, and libnl) set either > > 'dependency_libs' (glib) or 'libdir' (libnl) to paths under output/build > > or staging. wireshark also links its own libraries (e.g. wiretap, epan) > > using local source tree .la files. I'm not sure how to solve this in the > > general case. It probably requires fixing up all these .la files. > > There's a good briefing at https://wiki.debian.org/RpathIssue > Fixing up is nice, however doing it in a good manner will require many > autoreconfs and being proactive about it. > Stripping RPATHs is probably a faster way out (via chrpath). > In the end this could be considered as a build information leak, not > terribly useful but doesn't look good either. chrpath can indeed nullify the ELF RPATH tag, but it leaves the ELF string table alone. This means that the build paths information leak is still there, though somewhat less noticeable. Until we find a better solution I think this patch can go in for -next. baruch
Dear Baruch Siach, On Tue, 4 Nov 2014 20:35:41 +0200, Baruch Siach wrote: > Edit libtool to remove RPATH from generated binaries. This is similar to > upstream commit making the same change when buiding RPMs. > https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commitdiff;h=a4a624e4c5 > > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > --- > package/wireshark/wireshark.mk | 6 ++++++ > 1 file changed, 6 insertions(+) I'm still not sure what to do with this. Yann, Gustavo, Samuel, some suggestions ? Should we introduce a final check in target-finalize to warn about binaries having invalid rpaths, and start fixing them one by one ? Or a more brutal approach where we remove all rpaths in target-finalize ? I'm a bit uncomfortable with having a fix for just one package without having a globally defined policy on how to handle this rpath mess. Opinions welcome. Thanks, Thomas
Thomas, All, On 2015-04-04 19:59 +0200, Thomas Petazzoni spake thusly: > On Tue, 4 Nov 2014 20:35:41 +0200, Baruch Siach wrote: > > Edit libtool to remove RPATH from generated binaries. This is similar to > > upstream commit making the same change when buiding RPMs. > > https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commitdiff;h=a4a624e4c5 > > > > Signed-off-by: Baruch Siach <baruch@tkos.co.il> > > --- > > package/wireshark/wireshark.mk | 6 ++++++ > > 1 file changed, 6 insertions(+) > > I'm still not sure what to do with this. Yann, Gustavo, Samuel, some > suggestions ? > > Should we introduce a final check in target-finalize to warn about > binaries having invalid rpaths, and start fixing them one by one ? Or a > more brutal approach where we remove all rpaths in target-finalize ? I have a series doing exactly that; I'll be sending it shortly. > I'm a bit uncomfortable with having a fix for just one package without > having a globally defined policy on how to handle this rpath mess. > Opinions welcome. I feel the same. Regards, Yann E. MORIN.
Dear Yann E. MORIN, On Sun, 5 Apr 2015 11:05:00 +0200, Yann E. MORIN wrote: > > Should we introduce a final check in target-finalize to warn about > > binaries having invalid rpaths, and start fixing them one by one ? Or a > > more brutal approach where we remove all rpaths in target-finalize ? > > I have a series doing exactly that; I'll be sending it shortly. There was some concern in this thread that the approach to remove the rpath at the end of the build was indeed making the built-in rpath ineffective, but still present if you did a "strings" on the binary. Is this something you have a solution for? Best regards, Thomas
Thomas, All, On 2015-04-05 11:29 +0200, Thomas Petazzoni spake thusly: > On Sun, 5 Apr 2015 11:05:00 +0200, Yann E. MORIN wrote: > > > Should we introduce a final check in target-finalize to warn about > > > binaries having invalid rpaths, and start fixing them one by one ? Or a > > > more brutal approach where we remove all rpaths in target-finalize ? > > > > I have a series doing exactly that; I'll be sending it shortly. > > There was some concern in this thread that the approach to remove the > rpath at the end of the build was indeed making the built-in rpath > ineffective, but still present if you did a "strings" on the binary. Is > this something you have a solution for? Hmm... What I do is use patch-e;f to remove it altogether, yes. Now, if there is another reference to it in another section (e.g. .data or .rodata) I don't know. I can do a biggish test-build with lotta packages and do a string on there, but I'm afraid we will find it in those data sections, if only because some tools store the build dir and some other things in there. Regards, Yann E. MORIN.
diff --git a/package/wireshark/wireshark.mk b/package/wireshark/wireshark.mk index ccf5a56f1f23..6c7c3920e7af 100644 --- a/package/wireshark/wireshark.mk +++ b/package/wireshark/wireshark.mk @@ -15,6 +15,12 @@ WIRESHARK_AUTORECONF = YES WIRESHARK_CONF_ENV = LIBGCRYPT_CONFIG=$(STAGING_DIR)/usr/bin/libgcrypt-config \ ac_cv_path_PCAP_CONFIG=$(STAGING_DIR)/usr/bin/pcap-config +define WIRESHARK_REMOVE_RPATH + $(SED) 's|^hardcode_libdir_flag_spec=.*|hardcode_libdir_flag_spec=""|g' $(@D)/libtool + $(SED) 's|^runpath_var=LD_RUN_PATH|runpath_var=DIE_RPATH_DIE|g' $(@D)/libtool +endef +WIRESHARK_POST_CONFIGURE_HOOKS += WIRESHARK_REMOVE_RPATH + # wireshark adds -I$includedir to CFLAGS, causing host/target headers mixup. # Work around it by pointing includedir at staging WIRESHARK_CONF_OPTS = --disable-wireshark --without-krb5 --disable-usr-local \
Edit libtool to remove RPATH from generated binaries. This is similar to upstream commit making the same change when buiding RPMs. https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commitdiff;h=a4a624e4c5 Signed-off-by: Baruch Siach <baruch@tkos.co.il> --- package/wireshark/wireshark.mk | 6 ++++++ 1 file changed, 6 insertions(+)