[Buildroot] [PATCH] package/busybox: disable setfattr when attr package is enabled

[Buildroot] [PATCH] package/busybox: disable setfattr when attr package is enabled

[Yann E. MORIN]

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

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
-- 
2.14.1

[Buildroot] [PATCH] package/busybox: disable setfattr when attr package is enabled

[Peter Korsgaard]

>>>>> "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 at gmail.com
 > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
 > Cc: mzweerspenko+bugzilla at 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?

-- 
Bye, Peter Korsgaard

[Buildroot] [PATCH] package/busybox: disable setfattr when attr package is enabled

[Yann E. MORIN]

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 at gmail.com
>  > Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
>  > Cc: mzweerspenko+bugzilla at 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.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] package/busybox: disable setfattr when attr package is enabled

[Peter Korsgaard]

>>>>> "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!

-- 
Bye, Peter Korsgaard