diff mbox series

package/busybox: disable setfattr when attr package is enabled

Message ID 20180504203726.19657-1-yann.morin.1998@free.fr
State Rejected
Headers show
Series package/busybox: disable setfattr when attr package is enabled | expand

Commit Message

Yann E. MORIN May 4, 2018, 8:37 p.m. UTC
The install procedure of attr is kinda borked: it does not unlink any
existing destination file during installation, but directly wites into
it. So, when busybox is instaleld before attr and installs its applets
as hardlinks, attr will overwrite the setfattr file, thus borking the
busybox binary and all its applets.

So, when the attr package is enabled, we disable the setfattr applet.

Fixes: #10986.

Reported-by: mzweerspenko+bugzilla@gmail.com
Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: mzweerspenko+bugzilla@gmail.com
---
 package/busybox/busybox.mk | 9 +++++++++
 1 file changed, 9 insertions(+)

Comments

Peter Korsgaard May 5, 2018, 11:48 a.m. UTC | #1
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

 > The install procedure of attr is kinda borked: it does not unlink any
 > existing destination file during installation, but directly wites into
 > it. So, when busybox is instaleld before attr and installs its applets
 > as hardlinks, attr will overwrite the setfattr file, thus borking the
 > busybox binary and all its applets.

 > So, when the attr package is enabled, we disable the setfattr applet.

 > Fixes: #10986.

 > Reported-by: mzweerspenko+bugzilla@gmail.com
 > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
 > Cc: mzweerspenko+bugzilla@gmail.com

It would be nicer to instead fix the attr build system. I took a quick
look, and the problem seems to be that installation is done with their
own install-sh script which uses cp (and no --remove-destination).

I *THINK* a fix would just be to change the 'INSTALL =' line in
include/buildmacros to use normal install instead of this install-sh
script.

Could you give that a try?
Yann E. MORIN May 5, 2018, 3:58 p.m. UTC | #2
Peter, All,

On 2018-05-05 13:48 +0200, Peter Korsgaard spake thusly:
> >>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:
> 
>  > The install procedure of attr is kinda borked: it does not unlink any
>  > existing destination file during installation, but directly wites into
>  > it. So, when busybox is instaleld before attr and installs its applets
>  > as hardlinks, attr will overwrite the setfattr file, thus borking the
>  > busybox binary and all its applets.
> 
>  > So, when the attr package is enabled, we disable the setfattr applet.
> 
>  > Fixes: #10986.
> 
>  > Reported-by: mzweerspenko+bugzilla@gmail.com
>  > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>  > Cc: mzweerspenko+bugzilla@gmail.com
> 
> It would be nicer to instead fix the attr build system. I took a quick
> look, and the problem seems to be that installation is done with their
> own install-sh script which uses cp (and no --remove-destination).
> 
> I *THINK* a fix would just be to change the 'INSTALL =' line in
> include/buildmacros to use normal install instead of this install-sh
> script.
> 
> Could you give that a try?

I'm first going to bump to the latest version, 2.4.48 (from 2.4.47),
because 2.4.48 has had a complete overhaul of its buildsystem.

If that gets too comples, then I'll try to fix the current attr code.

Regards,
Yann E. MORIN.
Peter Korsgaard May 5, 2018, 10:26 p.m. UTC | #3
>>>>> "Yann" == Yann E MORIN <yann.morin.1998@free.fr> writes:

Hi,

 >> It would be nicer to instead fix the attr build system. I took a quick
 >> look, and the problem seems to be that installation is done with their
 >> own install-sh script which uses cp (and no --remove-destination).
 >> 
 >> I *THINK* a fix would just be to change the 'INSTALL =' line in
 >> include/buildmacros to use normal install instead of this install-sh
 >> script.
 >> 
 >> Could you give that a try?

 > I'm first going to bump to the latest version, 2.4.48 (from 2.4.47),
 > because 2.4.48 has had a complete overhaul of its buildsystem.

 > If that gets too comples, then I'll try to fix the current attr code.

Ok, thanks!
diff mbox series

Patch

diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index 60a55a2c78..5f4b18e47f 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -254,6 +254,14 @@  define BUSYBOX_LINUX_PAM
 endef
 endif
 
+# attr's install procedure is borked and overwrites any existing
+# destination file. So, disable conflicting applets in that case
+ifeq ($(BR2_PACKAGE_ATTR),y)
+define BUSYBOX_DISABLE_ATTR
+	$(call KCONFIG_DISABLE_OPT,CONFIG_SETFATTR,$(BUSYBOX_BUILD_CONFIG))
+endef
+endif
+
 # Telnet support
 define BUSYBOX_INSTALL_TELNET_SCRIPT
 	if grep -q CONFIG_FEATURE_TELNETD_STANDALONE=y $(@D)/.config; then \
@@ -294,6 +302,7 @@  define BUSYBOX_KCONFIG_FIXUP_CMDS
 	$(BUSYBOX_SET_SELINUX)
 	$(BUSYBOX_SET_INDIVIDUAL_BINARIES)
 	$(BUSYBOX_MUSL_TWEAKS)
+	$(BUSYBOX_DISABLE_ATTR)
 endef
 
 define BUSYBOX_CONFIGURE_CMDS