diff mbox

[v4,16/20] busybox: add option to install individual binaries

Message ID 1386872840-22051-17-git-send-email-clshotwe@rockwellcollins.com
State Superseded
Headers show

Commit Message

Clayton Shotwell Dec. 12, 2013, 6:27 p.m. UTC 
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Acked-by: Clayton Shotwell <clshotwe@rockwellcollins.com>
---
 package/busybox/Config.in  |    3 +++
 package/busybox/busybox.mk |   24 ++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 0 deletions(-)

Comments

Arnout Vandecappelle Dec. 12, 2013, 11:02 p.m. UTC | #1 
On 12/12/13 19:27, Clayton Shotwell wrote:
> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Acked-by: Clayton Shotwell <clshotwe@rockwellcollins.com>
> ---
>   package/busybox/Config.in  |    3 +++
>   package/busybox/busybox.mk |   24 ++++++++++++++++++++++++
>   2 files changed, 27 insertions(+), 0 deletions(-)
>
> diff --git a/package/busybox/Config.in b/package/busybox/Config.in
> index 2d55aa6..7f00fb8 100644
> --- a/package/busybox/Config.in
> +++ b/package/busybox/Config.in
> @@ -55,6 +55,9 @@ config BR2_PACKAGE_BUSYBOX_SHOW_OTHERS
>   	  Show packages in menuconfig that are potentially also provided
>   	  by busybox.
>
> +config BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES
> +	bool "Individual binaries"

  This really lacks some help text, even I had no idea what it meant at 
first. I guess the text from busybox can just be reused - though for 
SELinux there's obviously a different reason.

         If your CPU architecture doesn't allow for sharing text/rodata
         sections of running binaries, but allows for runtime dynamic
         libraries, this option will allow you to reduce memory footprint
         when you have many different applets running at once.

         If your CPU architecture allows for sharing text/rodata,
         having single binary is more optimal.

         Each applet will be a tiny program, dynamically linked
         against libbusybox.so.N.N.N.

         You need to have a working dynamic linker.

  That last sentence makes me think that we're missing a depends on 
!BR2_PREFER_STATIC here.

> +
>   config BR2_PACKAGE_BUSYBOX_WATCHDOG
>   	bool "Install the watchdog daemon startup script"
>   	help
> diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
> index f6f542e..13de7ed 100644
> --- a/package/busybox/busybox.mk
> +++ b/package/busybox/busybox.mk
> @@ -50,10 +50,16 @@ ifndef BUSYBOX_CONFIG_FILE
>   	BUSYBOX_CONFIG_FILE = $(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG))
>   endif
>
> +ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
> +define BUSYBOX_PERMISSIONS
> +/usr/share/udhcpc/default.script f 755  0 0 - - - - -

  Isn't setuid root needed anymore for /bin/login? Same for passwd, su, 
probably others...


> +endef
> +else
>   define BUSYBOX_PERMISSIONS
>   /bin/busybox			 f 4755	0 0 - - - - -
>   /usr/share/udhcpc/default.script f 755  0 0 - - - - -
>   endef
> +endif
>
>   # If mdev will be used for device creation enable it and copy S10mdev to /etc/init.d
>   ifeq ($(BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_MDEV),y)
> @@ -152,6 +158,22 @@ define BUSYBOX_SET_INIT
>   endef
>   endif
>
> +ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
> +define BUSYBOX_CONFIGURE_INDIVIDUAL_BINARIES
> +	$(call KCONFIG_ENABLE_OPT,CONFIG_BUILD_LIBBUSYBOX,$(BUSYBOX_BUILD_CONFIG))
> +	$(call KCONFIG_ENABLE_OPT,CONFIG_FEATURE_INDIVIDUAL,$(BUSYBOX_BUILD_CONFIG))
> +endef
> +
> +define BUSYBOX_INSTALL_INDIVIDUAL_BINARIES
> +	for i in `find -L $(TARGET_DIR) -xtype l -samefile $(TARGET_DIR)/bin/busybox` ; do \
> +		$(INSTALL) -D -m 0755 $(@D)/0_lib/`basename $$i` $$i ; \
> +	done

  Icky, perhaps it's better to make an upstreamable patch that fixes the 
Makefile so it properly installs the binaries if INDIVIDUAL_BINARIES is 
selected?


  Regards,
  Arnout

> +	cp $(@D)/0_lib/libbusybox.so.* $(TARGET_DIR)/lib
> +	rm -f $(TARGET_DIR)/lib/libbusybox.so.*_unstripped*
> +	rm -f $(TARGET_DIR)/bin/busybox
> +endef
> +endif
> +
>   define BUSYBOX_INSTALL_LOGGING_SCRIPT
>   	if grep -q CONFIG_SYSLOGD=y $(@D)/.config; then \
>   		[ -f $(TARGET_DIR)/etc/init.d/S01logging ] || \
> @@ -187,6 +209,7 @@ define BUSYBOX_CONFIGURE_CMDS
>   	$(BUSYBOX_INTERNAL_SHADOW_PASSWORDS)
>   	$(BUSYBOX_SET_INIT)
>   	$(BUSYBOX_SET_WATCHDOG)
> +	$(BUSYBOX_CONFIGURE_INDIVIDUAL_BINARIES)
>   	@yes "" | $(MAKE) ARCH=$(KERNEL_ARCH) CROSS_COMPILE="$(TARGET_CROSS)" \
>   		-C $(@D) oldconfig
>   endef
> @@ -205,6 +228,7 @@ define BUSYBOX_INSTALL_TARGET_CMDS
>   	$(BUSYBOX_INSTALL_MDEV_CONF)
>   	$(BUSYBOX_INSTALL_LOGGING_SCRIPT)
>   	$(BUSYBOX_INSTALL_WATCHDOG_SCRIPT)
> +	$(BUSYBOX_INSTALL_INDIVIDUAL_BINARIES)
>   endef
>
>   $(eval $(generic-package))
>
Thomas Petazzoni Dec. 15, 2013, 4:10 p.m. UTC | #2 
Dear Arnout Vandecappelle,

On Fri, 13 Dec 2013 00:02:01 +0100, Arnout Vandecappelle wrote:

> >
> > +config BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES
> > +	bool "Individual binaries"
> 
>   This really lacks some help text, even I had no idea what it meant
> at first. I guess the text from busybox can just be reused - though
> for SELinux there's obviously a different reason.

In fact, the patches from me in this series were not really meant to be
sent to the list. They are proof-of-concept patches that I did to help
Clayton, but some more work was needed before they could be submit for
real.

>          If your CPU architecture doesn't allow for sharing
> text/rodata sections of running binaries, but allows for runtime
> dynamic libraries, this option will allow you to reduce memory
> footprint when you have many different applets running at once.
> 
>          If your CPU architecture allows for sharing text/rodata,
>          having single binary is more optimal.
> 
>          Each applet will be a tiny program, dynamically linked
>          against libbusybox.so.N.N.N.
> 
>          You need to have a working dynamic linker.

In the context of SELinux, it's because SELinux can only apply
different security policies to the various applets if they use
different binaries.

>   That last sentence makes me think that we're missing a depends on 
> !BR2_PREFER_STATIC here.

Indeed.

Also, I am not sure we want to show this option in the top-level
package menuconfig. So, we can either:

 * Have a sub-menu for Busybox customization options
 * Make this behavior automatic whenever SELinux support is enabled,
   and only in this case

> > +ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
> > +define BUSYBOX_PERMISSIONS
> > +/usr/share/udhcpc/default.script f 755  0 0 - - - - -
> 
>   Isn't setuid root needed anymore for /bin/login? Same for passwd,
> su, probably others...

Most likely yes. I have been able to log into the system with this
patch, though.

> > +define BUSYBOX_INSTALL_INDIVIDUAL_BINARIES
> > +	for i in `find -L $(TARGET_DIR) -xtype l -samefile
> > $(TARGET_DIR)/bin/busybox` ; do \
> > +		$(INSTALL) -D -m 0755 $(@D)/0_lib/`basename $$i`
> > $$i ; \
> > +	done
> 
>   Icky, perhaps it's better to make an upstreamable patch that fixes
> the Makefile so it properly installs the binaries if
> INDIVIDUAL_BINARIES is selected?

Yeah, could be interesting indeed.

Thanks for your comments,

Thomas
Clayton Shotwell Dec. 16, 2013, 3:44 p.m. UTC | #3 
Arnout,

Arnout Vandecappelle <arnout@mind.be> wrote on 12/12/2013 05:02:01 PM:

>   That last sentence makes me think that we're missing a depends on 
> !BR2_PREFER_STATIC here.

Agreed. I'll add that in.

> > +
> >   config BR2_PACKAGE_BUSYBOX_WATCHDOG
> >      bool "Install the watchdog daemon startup script"
> >      help
> > diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
> > index f6f542e..13de7ed 100644
> > --- a/package/busybox/busybox.mk
> > +++ b/package/busybox/busybox.mk
> > @@ -50,10 +50,16 @@ ifndef BUSYBOX_CONFIG_FILE
> >      BUSYBOX_CONFIG_FILE = $(call 
qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG))
> >   endif
> >
> > +ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
> > +define BUSYBOX_PERMISSIONS
> > +/usr/share/udhcpc/default.script f 755  0 0 - - - - -
> 
>   Isn't setuid root needed anymore for /bin/login? Same for passwd, su, 
> probably others...

Good point. I'll look into adding support for that.

> > +endef
> > +else
> >   define BUSYBOX_PERMISSIONS
> >   /bin/busybox          f 4755   0 0 - - - - -
> >   /usr/share/udhcpc/default.script f 755  0 0 - - - - -
> >   endef
> > +endif
> >
> >   # If mdev will be used for device creation enable it and copy 
> S10mdev to /etc/init.d
> >   ifeq ($(BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_MDEV),y)
> > @@ -152,6 +158,22 @@ define BUSYBOX_SET_INIT
> >   endef
> >   endif
> >
> > +ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
> > +define BUSYBOX_CONFIGURE_INDIVIDUAL_BINARIES
> > +   $(call KCONFIG_ENABLE_OPT,CONFIG_BUILD_LIBBUSYBOX,$
> (BUSYBOX_BUILD_CONFIG))
> > +   $(call KCONFIG_ENABLE_OPT,CONFIG_FEATURE_INDIVIDUAL,$
> (BUSYBOX_BUILD_CONFIG))
> > +endef
> > +
> > +define BUSYBOX_INSTALL_INDIVIDUAL_BINARIES
> > +   for i in `find -L $(TARGET_DIR) -xtype l -samefile $
> (TARGET_DIR)/bin/busybox` ; do \
> > +      $(INSTALL) -D -m 0755 $(@D)/0_lib/`basename $$i` $$i ; \
> > +   done
> 
>   Icky, perhaps it's better to make an upstreamable patch that fixes the 

> Makefile so it properly installs the binaries if INDIVIDUAL_BINARIES is 

I'll see what I can do.

Thanks,
Clayton

Clayton Shotwell
Software Engineer
clshotwe@rockwellcollins.com
www.rockwellcollins.com
Clayton Shotwell Dec. 17, 2013, 5:47 p.m. UTC | #4 
Thomas,

Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote on 12/15/2013 
10:10:59 AM:

> > >
> > > +config BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES
> > > +   bool "Individual binaries"
> > 
> >   This really lacks some help text, even I had no idea what it meant
> > at first. I guess the text from busybox can just be reused - though
> > for SELinux there's obviously a different reason.
> 
> In fact, the patches from me in this series were not really meant to be
> sent to the list. They are proof-of-concept patches that I did to help
> Clayton, but some more work was needed before they could be submit for
> real.

I apologize for just sending these out without any context or much of a 
second review.
I will get them cleaned up a little bit and resubmit them.
 
> >          If your CPU architecture doesn't allow for sharing
> > text/rodata sections of running binaries, but allows for runtime
> > dynamic libraries, this option will allow you to reduce memory
> > footprint when you have many different applets running at once.
> > 
> >          If your CPU architecture allows for sharing text/rodata,
> >          having single binary is more optimal.
> > 
> >          Each applet will be a tiny program, dynamically linked
> >          against libbusybox.so.N.N.N.
> > 
> >          You need to have a working dynamic linker.
> 
> In the context of SELinux, it's because SELinux can only apply
> different security policies to the various applets if they use
> different binaries.
> 
> >   That last sentence makes me think that we're missing a depends on 
> > !BR2_PREFER_STATIC here.
> 
> Indeed.
> 
> Also, I am not sure we want to show this option in the top-level
> package menuconfig. So, we can either:
> 
>  * Have a sub-menu for Busybox customization options
>  * Make this behavior automatic whenever SELinux support is enabled,
>    and only in this case
> 
> > > +ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
> > > +define BUSYBOX_PERMISSIONS
> > > +/usr/share/udhcpc/default.script f 755  0 0 - - - - -
> > 
> >   Isn't setuid root needed anymore for /bin/login? Same for passwd,
> > su, probably others...
> 
> Most likely yes. I have been able to log into the system with this
> patch, though.

I will come up with a base list of applications that need suid and add
it to the patch.

Thanks,
Clayton

Clayton Shotwell
Software Engineer
clshotwe@rockwellcollins.com
www.rockwellcollins.com
diff mbox

Patch

diff --git a/package/busybox/Config.in b/package/busybox/Config.in
index 2d55aa6..7f00fb8 100644
--- a/package/busybox/Config.in
+++ b/package/busybox/Config.in
@@ -55,6 +55,9 @@  config BR2_PACKAGE_BUSYBOX_SHOW_OTHERS
 	  Show packages in menuconfig that are potentially also provided
 	  by busybox.
 
+config BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES
+	bool "Individual binaries"
+
 config BR2_PACKAGE_BUSYBOX_WATCHDOG
 	bool "Install the watchdog daemon startup script"
 	help
diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index f6f542e..13de7ed 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -50,10 +50,16 @@  ifndef BUSYBOX_CONFIG_FILE
 	BUSYBOX_CONFIG_FILE = $(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG))
 endif
 
+ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
+define BUSYBOX_PERMISSIONS
+/usr/share/udhcpc/default.script f 755  0 0 - - - - -
+endef
+else
 define BUSYBOX_PERMISSIONS
 /bin/busybox			 f 4755	0 0 - - - - -
 /usr/share/udhcpc/default.script f 755  0 0 - - - - -
 endef
+endif
 
 # If mdev will be used for device creation enable it and copy S10mdev to /etc/init.d
 ifeq ($(BR2_ROOTFS_DEVICE_CREATION_DYNAMIC_MDEV),y)
@@ -152,6 +158,22 @@  define BUSYBOX_SET_INIT
 endef
 endif
 
+ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
+define BUSYBOX_CONFIGURE_INDIVIDUAL_BINARIES
+	$(call KCONFIG_ENABLE_OPT,CONFIG_BUILD_LIBBUSYBOX,$(BUSYBOX_BUILD_CONFIG))
+	$(call KCONFIG_ENABLE_OPT,CONFIG_FEATURE_INDIVIDUAL,$(BUSYBOX_BUILD_CONFIG))
+endef
+
+define BUSYBOX_INSTALL_INDIVIDUAL_BINARIES
+	for i in `find -L $(TARGET_DIR) -xtype l -samefile $(TARGET_DIR)/bin/busybox` ; do \
+		$(INSTALL) -D -m 0755 $(@D)/0_lib/`basename $$i` $$i ; \
+	done
+	cp $(@D)/0_lib/libbusybox.so.* $(TARGET_DIR)/lib
+	rm -f $(TARGET_DIR)/lib/libbusybox.so.*_unstripped*
+	rm -f $(TARGET_DIR)/bin/busybox
+endef
+endif
+
 define BUSYBOX_INSTALL_LOGGING_SCRIPT
 	if grep -q CONFIG_SYSLOGD=y $(@D)/.config; then \
 		[ -f $(TARGET_DIR)/etc/init.d/S01logging ] || \
@@ -187,6 +209,7 @@  define BUSYBOX_CONFIGURE_CMDS
 	$(BUSYBOX_INTERNAL_SHADOW_PASSWORDS)
 	$(BUSYBOX_SET_INIT)
 	$(BUSYBOX_SET_WATCHDOG)
+	$(BUSYBOX_CONFIGURE_INDIVIDUAL_BINARIES)
 	@yes "" | $(MAKE) ARCH=$(KERNEL_ARCH) CROSS_COMPILE="$(TARGET_CROSS)" \
 		-C $(@D) oldconfig
 endef
@@ -205,6 +228,7 @@  define BUSYBOX_INSTALL_TARGET_CMDS
 	$(BUSYBOX_INSTALL_MDEV_CONF)
 	$(BUSYBOX_INSTALL_LOGGING_SCRIPT)
 	$(BUSYBOX_INSTALL_WATCHDOG_SCRIPT)
+	$(BUSYBOX_INSTALL_INDIVIDUAL_BINARIES)
 endef
 
 $(eval $(generic-package))