All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 0/4] Allow Busybox installation as individual binaries
@ 2017-03-26 21:43 Thomas Petazzoni
  2017-03-26 21:43 ` [Buildroot] [PATCH 1/4] system: do not overwrite /bin/sh Busybox symlink Thomas Petazzoni
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2017-03-26 21:43 UTC (permalink / raw)
  To: buildroot

Hello,

For SELinux, we need to be able to use the Busybox installation mode
of "one binary per applet". So, this patch series allows the Busybox
package to use this option.

However, this requires some preparation, due to two different aspects:

 1. In terms of permissions, it is no longer as easy as making
    /bin/busybox SUID root. We want only the actual applets that need
    to be SUID to be SUID, and therefore the list of files that should
    be SUID root is now generated depending on the Busybox
    configuration, thanks to the addition of the support for
    <pkg>_PERMISSIONS_FILE to the package infrastruture (implemented
    by Yann E. Morin)

 2. The /bin/sh symlink is normally created to point to /bin/busybox,
    which does not exist when using this "individual binary"
    mode. Therefore, a patch also tweaks the /bin/sh logic to not
    overwrite the /bin/sh implementation of Busybox.

I've runtime tested this on an ARM platform, and it works for me. I'm
not super interested by this feature personaly, but it's been waiting
in patchwork for ages, so it's time to do something with it. If too
much rework is requested, I'll just drop the patches and mark them as
Rejected.

Best regards,

Thomas

Clayton Shotwell (1):
  busybox: applets as individual binaries

Thomas Petazzoni (1):
  system: do not overwrite /bin/sh Busybox symlink

Yann E. MORIN (2):
  core: allow packages to declare a permission file
  docs/manual: document FOO_PERMISSIONS_FILE

 docs/manual/adding-packages-generic.txt |  8 +++++++-
 fs/common.mk                            |  3 +++
 package/busybox/Config.in               | 16 ++++++++++++++++
 package/busybox/busybox.mk              | 28 ++++++++++++++++++++++++++++
 package/pkg-generic.mk                  |  1 +
 package/skeleton/skeleton.mk            |  2 ++
 system/Config.in                        |  1 -
 7 files changed, 57 insertions(+), 2 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 1/4] system: do not overwrite /bin/sh Busybox symlink
  2017-03-26 21:43 [Buildroot] [PATCH 0/4] Allow Busybox installation as individual binaries Thomas Petazzoni
@ 2017-03-26 21:43 ` Thomas Petazzoni
  2017-03-27 16:43   ` Matthew Weber
                     ` (2 more replies)
  2017-03-26 21:43 ` [Buildroot] [PATCH 2/4] core: allow packages to declare a permission file Thomas Petazzoni
                   ` (2 subsequent siblings)
  3 siblings, 3 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2017-03-26 21:43 UTC (permalink / raw)
  To: buildroot

The BR2_SYSTEM_BIN_SH hidden option defines to what binary the /bin/sh
symlinks should point to. If busybox is chosen, then /bin/sh is created
to point to /bin/busybox.

This works fine with the default installation mode of Busybox, but it
fails with the upcoming "individual binaries" mode, in which each applet
is installed as its own binary, and /bin/busybox doesn't exist: we get
/bin/sh as a broken symlink to /bin/busybox.

Since Busybox already installs its own /bin/sh symlink, properly
pointing to /bin/ash or /bin/hush depending on the selected shell, it
doesn't make sense for the BR2_SYSTEM_BIN_SH logic to override
this. Just let Busybox install its own /bin/sh by making
BR2_SYSTEM_BIN_SH empty when Busybox shell is selected as /bin/sh.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
This is proposed as an alternative to
https://patchwork.ozlabs.org/patch/686673/. If people believe
https://patchwork.ozlabs.org/patch/686673/ is a better solution, I'm
fine as well.
---
 package/skeleton/skeleton.mk | 2 ++
 system/Config.in             | 1 -
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/package/skeleton/skeleton.mk b/package/skeleton/skeleton.mk
index 1000161..9940944 100644
--- a/package/skeleton/skeleton.mk
+++ b/package/skeleton/skeleton.mk
@@ -203,10 +203,12 @@ define SKELETON_BIN_SH
 	rm -f $(TARGET_DIR)/bin/sh
 endef
 else
+ifneq ($(SKELETON_TARGET_GENERIC_BIN_SH),)
 define SKELETON_BIN_SH
 	ln -sf $(SKELETON_TARGET_GENERIC_BIN_SH) $(TARGET_DIR)/bin/sh
 endef
 endif
+endif
 TARGET_FINALIZE_HOOKS += SKELETON_BIN_SH
 
 ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
diff --git a/system/Config.in b/system/Config.in
index 3ddf843..b47ae43 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -298,7 +298,6 @@ endchoice # /bin/sh
 
 config BR2_SYSTEM_BIN_SH
 	string
-	default "busybox" if BR2_SYSTEM_BIN_SH_BUSYBOX
 	default "bash"    if BR2_SYSTEM_BIN_SH_BASH
 	default "dash"    if BR2_SYSTEM_BIN_SH_DASH
 	default "mksh"    if BR2_SYSTEM_BIN_SH_MKSH
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 2/4] core: allow packages to declare a permission file
  2017-03-26 21:43 [Buildroot] [PATCH 0/4] Allow Busybox installation as individual binaries Thomas Petazzoni
  2017-03-26 21:43 ` [Buildroot] [PATCH 1/4] system: do not overwrite /bin/sh Busybox symlink Thomas Petazzoni
@ 2017-03-26 21:43 ` Thomas Petazzoni
  2017-03-27 16:44   ` Matthew Weber
  2017-03-28 22:12   ` Arnout Vandecappelle
  2017-03-26 21:43 ` [Buildroot] [PATCH 3/4] docs/manual: document FOO_PERMISSIONS_FILE Thomas Petazzoni
  2017-03-26 21:43 ` [Buildroot] [PATCH 4/4] busybox: applets as individual binaries Thomas Petazzoni
  3 siblings, 2 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2017-03-26 21:43 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Currently, packages can define a variable that holds all the permissions
to set on the files it installs. This can be used to set various
permissions, like ownership, mode, suid/sgid/sticky bits to individual
files.

However, this variable has to contain entries that are known the moment
we scan the .mk file; it is not possible to conditionally add permisions
for files which presence depend on post-parse conditions.

This is the case for example for Busybox, for which we don't know whether
a specific applet will be enabled or not until after the configure
command has run.

Introduce a new variable that packages can set to point to a file that
contains a permission table. That filewill only be used when a filesystem
image is asembled, so the file can be generated, either at configure or
build time, with no problem.

Since this variable can be empty (when no package provides such a
permissions file), we must ensure not to cat anything (or that would
stale, cat-ing stdin). But since this variable is not fully known by the
time we parse the fs infra (e.g. packages from a br2-external tree are
not yet parsed), we can not test it with make syntas (ifneq...endif).
Teting it with shell syntac is not trivial either, becaue the variable
would not be mpty (it would be only spaces). sO, we just iterate over
the the files and cat them one by one with a shell-level for-loop, which
is happy with nothing to iterate over.

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <peter@korsgaard.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 fs/common.mk           | 3 +++
 package/pkg-generic.mk | 1 +
 2 files changed, 4 insertions(+)

diff --git a/fs/common.mk b/fs/common.mk
index 396b1c2..0bbcca4 100644
--- a/fs/common.mk
+++ b/fs/common.mk
@@ -91,6 +91,9 @@ ifeq ($$(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
 endif
 endif
 	$$(call PRINTF,$$(PACKAGES_PERMISSIONS_TABLE)) >> $$(FULL_DEVICE_TABLE)
+	for f in $$(PACKAGES_PERMISSIONS_FILES); do \
+		cat $$$${f} >> $$(FULL_DEVICE_TABLE) || exit 1; \
+	done
 	echo "$$(HOST_DIR)/usr/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
 	$$(foreach s,$$(call qstrip,$$(BR2_ROOTFS_POST_FAKEROOT_SCRIPT)),\
 		echo "echo '$$(TERM_BOLD)>>>   Executing fakeroot script $$(s)$$(TERM_RESET)'" >> $$(FAKEROOT_SCRIPT); \
diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
index dd8a1e2..343d1ad 100644
--- a/package/pkg-generic.mk
+++ b/package/pkg-generic.mk
@@ -915,6 +915,7 @@ PACKAGES += $(1)
 ifneq ($$($(2)_PERMISSIONS),)
 PACKAGES_PERMISSIONS_TABLE += $$($(2)_PERMISSIONS)$$(sep)
 endif
+PACKAGES_PERMISSIONS_FILES += $$($(2)_PERMISSIONS_FILE)
 ifneq ($$($(2)_DEVICES),)
 PACKAGES_DEVICES_TABLE += $$($(2)_DEVICES)$$(sep)
 endif
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 3/4] docs/manual: document FOO_PERMISSIONS_FILE
  2017-03-26 21:43 [Buildroot] [PATCH 0/4] Allow Busybox installation as individual binaries Thomas Petazzoni
  2017-03-26 21:43 ` [Buildroot] [PATCH 1/4] system: do not overwrite /bin/sh Busybox symlink Thomas Petazzoni
  2017-03-26 21:43 ` [Buildroot] [PATCH 2/4] core: allow packages to declare a permission file Thomas Petazzoni
@ 2017-03-26 21:43 ` Thomas Petazzoni
  2017-03-27 16:44   ` Matthew Weber
  2017-03-26 21:43 ` [Buildroot] [PATCH 4/4] busybox: applets as individual binaries Thomas Petazzoni
  3 siblings, 1 reply; 15+ messages in thread
From: Thomas Petazzoni @ 2017-03-26 21:43 UTC (permalink / raw)
  To: buildroot

From: "Yann E. MORIN" <yann.morin.1998@free.fr>

Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Cc: Peter Korsgaard <peter@korsgaard.com>
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 docs/manual/adding-packages-generic.txt | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
index a08283c..23b07fa 100644
--- a/docs/manual/adding-packages-generic.txt
+++ b/docs/manual/adding-packages-generic.txt
@@ -386,7 +386,13 @@ information is (assuming the package name is +libfoo+) :
 * +LIBFOO_PERMISSIONS+ lists the changes of permissions to be done at
   the end of the build process. The syntax is once again the makedevs one.
   You can find some documentation for this syntax in the xref:makedev-syntax[].
-  This variable is optional.
+  This variable is optional; its value must be known when the .mk file
+  is parsed.
+
+* +LIBFOO_PERMISSIONS_FILE+, like +LIBFOO_PERMISSIONS+ but points to a
+  file that contains the list of permissions. Unlike +LIBFOO_PERMISSIONS+,
+  its content need not be known when the .mk file is parsed, so it can be
+  generated. This variable is optional, and you should seldom need it.
 
 * +LIBFOO_USERS+ lists the users to create for this package, if it installs
   a program you want to run as a specific user (e.g. as a daemon, or as a
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 4/4] busybox: applets as individual binaries
  2017-03-26 21:43 [Buildroot] [PATCH 0/4] Allow Busybox installation as individual binaries Thomas Petazzoni
                   ` (2 preceding siblings ...)
  2017-03-26 21:43 ` [Buildroot] [PATCH 3/4] docs/manual: document FOO_PERMISSIONS_FILE Thomas Petazzoni
@ 2017-03-26 21:43 ` Thomas Petazzoni
  2017-03-27 16:39   ` Matthew Weber
  3 siblings, 1 reply; 15+ messages in thread
From: Thomas Petazzoni @ 2017-03-26 21:43 UTC (permalink / raw)
  To: buildroot

From: Clayton Shotwell <clayton.shotwell@rockwellcollins.com>

The individual binaries option of busybox allows for the applets that
would usually be symlinks to be built as individual applications that
link against a shared library.

This feature is needed for SELinux to allow the applications to run
under the correct SELinux context.

The patch being added allows the individual applications to be installed
and will be upstreamed to the busybox developers.

The initial work for this change was done by Thomas Petazzoni
<thomas.petazzoni@free-electrons.com>.

The implementation of the BUSYBOX_PERMISSIONS_FILE generation has been
provided by Yann E. Morin.

Signed-off-by: Clayton Shotwell <clayton.shotwell@rockwellcollins.com>
Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
Reviewed-by: Samuel Martin <s.martin49@gmail.com>
Signed-off-by: Niranjan Reddy <niranjan.reddy@rockwellcollins.com>
Signed-off-by: Bryce Ferguson <bryce.ferguson@rockwellcollins.com>
[Thomas:
 - add help text in Config.in option.
 - remove change to BUSYBOX_PERMISSIONS, for now the permissions of the
   individual binaries are not correct. A follow-up patch will fix
   that. Therefore, the change to makedevs.c was removed.
 - rename BUSYBOX_CONFIGURE_INDIVIDUAL_BINARIES to
   BUSYBOX_SET_INDIVIDUAL_BINARIES to be consistent with other
   variables.
 - call BUSYBOX_INSTALL_INDIVIDUAL_BINARIES in
   BUSYBOX_INSTALL_TARGET_CMDS, not in BUSYBOX_INSTALL_INIT_SYSV.]
Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 package/busybox/Config.in  | 16 ++++++++++++++++
 package/busybox/busybox.mk | 28 ++++++++++++++++++++++++++++
 2 files changed, 44 insertions(+)

diff --git a/package/busybox/Config.in b/package/busybox/Config.in
index 504cd8a..9a2f411 100644
--- a/package/busybox/Config.in
+++ b/package/busybox/Config.in
@@ -54,6 +54,22 @@ config BR2_PACKAGE_BUSYBOX_SELINUX
 	  crond, then individual binaries have to be enabled for the
 	  SELinux type transitions to occur properly.
 
+config BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES
+	bool "Individual binaries"
+	depends on !BR2_STATIC_LIBS
+	depends on !BR2_bfin # libbusybox.so link issue
+	help
+	  By default (i.e with this option disabled), Busybox is
+	  installed as a single binary in /bin/busybox and all applets
+	  are a symbolic link to /bin/busybox.
+
+	  With this option enabled, each applet is a separate binary,
+	  which is needed for proper operation with SELinux.
+
+comment "Busybox individual binaries depends on dynamic libraries"
+	depends on BR2_STATIC_LIBS
+	depends on !BR2_bfin
+
 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 577f2f2..205b45a 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -60,9 +60,24 @@ BUSYBOX_KCONFIG_FRAGMENT_FILES = $(call qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG_FRAG
 BUSYBOX_KCONFIG_EDITORS = menuconfig xconfig gconfig
 BUSYBOX_KCONFIG_OPTS = $(BUSYBOX_MAKE_OPTS)
 
+ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
+BUSYBOX_PERMISSIONS_FILE = $(BUSYBOX_DIR)/busybox.permissions
+define BUSYBOX_GEN_PERMISSIONS
+	for app in `grep -r -e "APPLET.*BB_SUID_REQUIRE\|APPLET.*BB_SUID_MAYBE" $(@D)/include/applets.h \
+		| sed -e 's/,.*//' -e 's/.*(//'`; \
+	do \
+		temp=`grep -w $${app} $(@D)/busybox.links`; \
+		if [ -n "$${temp}" ]; then \
+			echo "$${temp} f 4755 0  0 - - - - -"; \
+		fi; \
+	done >$(BUSYBOX_PERMISSIONS_FILE)
+endef
+BUSYBOX_POST_INSTALL_TARGET_HOOKS += BUSYBOX_GEN_PERMISSIONS
+else
 define BUSYBOX_PERMISSIONS
 	/bin/busybox                     f 4755 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)
@@ -171,6 +186,17 @@ define BUSYBOX_SET_SELINUX
 endef
 endif
 
+ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
+define BUSYBOX_SET_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
+	rm -f $(TARGET_DIR)/bin/busybox
+endef
+endif
+
 define BUSYBOX_INSTALL_LOGGING_SCRIPT
 	if grep -q CONFIG_SYSLOGD=y $(@D)/.config; then \
 		$(INSTALL) -m 0755 -D package/busybox/S01logging \
@@ -228,6 +254,7 @@ define BUSYBOX_KCONFIG_FIXUP_CMDS
 	$(BUSYBOX_SET_INIT)
 	$(BUSYBOX_SET_WATCHDOG)
 	$(BUSYBOX_SET_SELINUX)
+	$(BUSYBOX_SET_INDIVIDUAL_BINARIES)
 	$(BUSYBOX_MUSL_TWEAKS)
 endef
 
@@ -244,6 +271,7 @@ define BUSYBOX_INSTALL_TARGET_CMDS
 	$(BUSYBOX_INSTALL_INITTAB)
 	$(BUSYBOX_INSTALL_UDHCPC_SCRIPT)
 	$(BUSYBOX_INSTALL_MDEV_CONF)
+	$(BUSYBOX_INSTALL_INDIVIDUAL_BINARIES)
 endef
 
 define BUSYBOX_INSTALL_INIT_SYSV
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 4/4] busybox: applets as individual binaries
  2017-03-26 21:43 ` [Buildroot] [PATCH 4/4] busybox: applets as individual binaries Thomas Petazzoni
@ 2017-03-27 16:39   ` Matthew Weber
  0 siblings, 0 replies; 15+ messages in thread
From: Matthew Weber @ 2017-03-27 16:39 UTC (permalink / raw)
  To: buildroot

Thomas,

On Sun, Mar 26, 2017 at 4:43 PM, Thomas Petazzoni <
thomas.petazzoni@free-electrons.com> wrote:

> From: Clayton Shotwell <clayton.shotwell@rockwellcollins.com>
>
> The individual binaries option of busybox allows for the applets that
> would usually be symlinks to be built as individual applications that
> link against a shared library.
>
> This feature is needed for SELinux to allow the applications to run
> under the correct SELinux context.
>
> The patch being added allows the individual applications to be installed
> and will be upstreamed to the busybox developers.
>
> The initial work for this change was done by Thomas Petazzoni
> <thomas.petazzoni@free-electrons.com>.
>
> The implementation of the BUSYBOX_PERMISSIONS_FILE generation has been
> provided by Yann E. Morin.
>
> Signed-off-by: Clayton Shotwell <clayton.shotwell@rockwellcollins.com>
> Signed-off-by: Matthew Weber <matthew.weber@rockwellcollins.com>
> Reviewed-by: Samuel Martin <s.martin49@gmail.com>
> Signed-off-by: Niranjan Reddy <niranjan.reddy@rockwellcollins.com>
> Signed-off-by: Bryce Ferguson <bryce.ferguson@rockwellcollins.com>
> [Thomas:
>  - add help text in Config.in option.
>  - remove change to BUSYBOX_PERMISSIONS, for now the permissions of the
>    individual binaries are not correct. A follow-up patch will fix
>    that. Therefore, the change to makedevs.c was removed.
>  - rename BUSYBOX_CONFIGURE_INDIVIDUAL_BINARIES to
>    BUSYBOX_SET_INDIVIDUAL_BINARIES to be consistent with other
>    variables.
>  - call BUSYBOX_INSTALL_INDIVIDUAL_BINARIES in
>    BUSYBOX_INSTALL_TARGET_CMDS, not in BUSYBOX_INSTALL_INIT_SYSV.]
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  package/busybox/Config.in  | 16 ++++++++++++++++
>  package/busybox/busybox.mk | 28 ++++++++++++++++++++++++++++
>  2 files changed, 44 insertions(+)
>
> diff --git a/package/busybox/Config.in b/package/busybox/Config.in
> index 504cd8a..9a2f411 100644
> --- a/package/busybox/Config.in
> +++ b/package/busybox/Config.in
> @@ -54,6 +54,22 @@ config BR2_PACKAGE_BUSYBOX_SELINUX
>           crond, then individual binaries have to be enabled for the
>           SELinux type transitions to occur properly.
>
> +config BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES
> +       bool "Individual binaries"
> +       depends on !BR2_STATIC_LIBS
> +       depends on !BR2_bfin # libbusybox.so link issue
> +       help
> +         By default (i.e with this option disabled), Busybox is
> +         installed as a single binary in /bin/busybox and all applets
> +         are a symbolic link to /bin/busybox.
> +
> +         With this option enabled, each applet is a separate binary,
> +         which is needed for proper operation with SELinux.
> +
> +comment "Busybox individual binaries depends on dynamic libraries"
> +       depends on BR2_STATIC_LIBS
> +       depends on !BR2_bfin
> +
>  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 577f2f2..205b45a 100644
> --- a/package/busybox/busybox.mk
> +++ b/package/busybox/busybox.mk
> @@ -60,9 +60,24 @@ BUSYBOX_KCONFIG_FRAGMENT_FILES = $(call
> qstrip,$(BR2_PACKAGE_BUSYBOX_CONFIG_FRAG
>  BUSYBOX_KCONFIG_EDITORS = menuconfig xconfig gconfig
>  BUSYBOX_KCONFIG_OPTS = $(BUSYBOX_MAKE_OPTS)
>
> +ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
> +BUSYBOX_PERMISSIONS_FILE = $(BUSYBOX_DIR)/busybox.permissions
> +define BUSYBOX_GEN_PERMISSIONS
> +       for app in `grep -r -e "APPLET.*BB_SUID_REQUIRE\|APPLET.*BB_SUID_MAYBE"
> $(@D)/include/applets.h \
> +               | sed -e 's/,.*//' -e 's/.*(//'`; \
> +       do \
> +               temp=`grep -w $${app} $(@D)/busybox.links`; \
> +               if [ -n "$${temp}" ]; then \
> +                       echo "$${temp} f 4755 0  0 - - - - -"; \
> +               fi; \
> +       done >$(BUSYBOX_PERMISSIONS_FILE)
> +endef
> +BUSYBOX_POST_INSTALL_TARGET_HOOKS += BUSYBOX_GEN_PERMISSIONS
> +else
>

I like the new approach vs the fixed table and the makedevs.c mod for a
optional file.

Reviewed-by: Matthew Weber <matthew.weber@rockwellcollins.com>


>  define BUSYBOX_PERMISSIONS
>         /bin/busybox                     f 4755 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)
> @@ -171,6 +186,17 @@ define BUSYBOX_SET_SELINUX
>  endef
>  endif
>
> +ifeq ($(BR2_PACKAGE_BUSYBOX_INDIVIDUAL_BINARIES),y)
> +define BUSYBOX_SET_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
> +       rm -f $(TARGET_DIR)/bin/busybox
> +endef
> +endif
> +
>  define BUSYBOX_INSTALL_LOGGING_SCRIPT
>         if grep -q CONFIG_SYSLOGD=y $(@D)/.config; then \
>                 $(INSTALL) -m 0755 -D package/busybox/S01logging \
> @@ -228,6 +254,7 @@ define BUSYBOX_KCONFIG_FIXUP_CMDS
>         $(BUSYBOX_SET_INIT)
>         $(BUSYBOX_SET_WATCHDOG)
>         $(BUSYBOX_SET_SELINUX)
> +       $(BUSYBOX_SET_INDIVIDUAL_BINARIES)
>         $(BUSYBOX_MUSL_TWEAKS)
>  endef
>
> @@ -244,6 +271,7 @@ define BUSYBOX_INSTALL_TARGET_CMDS
>         $(BUSYBOX_INSTALL_INITTAB)
>         $(BUSYBOX_INSTALL_UDHCPC_SCRIPT)
>         $(BUSYBOX_INSTALL_MDEV_CONF)
> +       $(BUSYBOX_INSTALL_INDIVIDUAL_BINARIES)
>  endef
>
>  define BUSYBOX_INSTALL_INIT_SYSV
> --
> 2.7.4
>
>


-- 
Matthew L Weber / Pr Software Engineer
Airborne Information Systems / Security Systems and Software / Secure
Platforms
MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA
www.rockwellcollins.com

Note: Any Export License Required Information and License Restricted Third
Party Intellectual Property (TPIP) content must be encrypted and sent to
matthew.weber at corp.rockwellcollins.com.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20170327/349bb423/attachment.html>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 1/4] system: do not overwrite /bin/sh Busybox symlink
  2017-03-26 21:43 ` [Buildroot] [PATCH 1/4] system: do not overwrite /bin/sh Busybox symlink Thomas Petazzoni
@ 2017-03-27 16:43   ` Matthew Weber
  2017-03-28 21:30   ` Arnout Vandecappelle
  2017-03-29 21:29   ` Thomas Petazzoni
  2 siblings, 0 replies; 15+ messages in thread
From: Matthew Weber @ 2017-03-27 16:43 UTC (permalink / raw)
  To: buildroot

Thomas,

On Sun, Mar 26, 2017 at 4:43 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> The BR2_SYSTEM_BIN_SH hidden option defines to what binary the /bin/sh
> symlinks should point to. If busybox is chosen, then /bin/sh is created
> to point to /bin/busybox.
>
> This works fine with the default installation mode of Busybox, but it
> fails with the upcoming "individual binaries" mode, in which each applet
> is installed as its own binary, and /bin/busybox doesn't exist: we get
> /bin/sh as a broken symlink to /bin/busybox.
>
> Since Busybox already installs its own /bin/sh symlink, properly
> pointing to /bin/ash or /bin/hush depending on the selected shell, it
> doesn't make sense for the BR2_SYSTEM_BIN_SH logic to override
> this. Just let Busybox install its own /bin/sh by making
> BR2_SYSTEM_BIN_SH empty when Busybox shell is selected as /bin/sh.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> This is proposed as an alternative to
> https://patchwork.ozlabs.org/patch/686673/. If people believe
> https://patchwork.ozlabs.org/patch/686673/ is a better solution, I'm
> fine as well.
> ---
>  package/skeleton/skeleton.mk | 2 ++
>  system/Config.in             | 1 -
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/package/skeleton/skeleton.mk b/package/skeleton/skeleton.mk
> index 1000161..9940944 100644
> --- a/package/skeleton/skeleton.mk
> +++ b/package/skeleton/skeleton.mk
> @@ -203,10 +203,12 @@ define SKELETON_BIN_SH
>         rm -f $(TARGET_DIR)/bin/sh
>  endef
>  else
> +ifneq ($(SKELETON_TARGET_GENERIC_BIN_SH),)
>  define SKELETON_BIN_SH
>         ln -sf $(SKELETON_TARGET_GENERIC_BIN_SH) $(TARGET_DIR)/bin/sh
>  endef
>  endif
> +endif
>  TARGET_FINALIZE_HOOKS += SKELETON_BIN_SH
>
>  ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
> diff --git a/system/Config.in b/system/Config.in
> index 3ddf843..b47ae43 100644
> --- a/system/Config.in
> +++ b/system/Config.in
> @@ -298,7 +298,6 @@ endchoice # /bin/sh
>
>  config BR2_SYSTEM_BIN_SH
>         string
> -       default "busybox" if BR2_SYSTEM_BIN_SH_BUSYBOX


Reviewed-by: Matthew Weber <matthew.weber@rockwellcollins.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 2/4] core: allow packages to declare a permission file
  2017-03-26 21:43 ` [Buildroot] [PATCH 2/4] core: allow packages to declare a permission file Thomas Petazzoni
@ 2017-03-27 16:44   ` Matthew Weber
  2017-03-28 22:12   ` Arnout Vandecappelle
  1 sibling, 0 replies; 15+ messages in thread
From: Matthew Weber @ 2017-03-27 16:44 UTC (permalink / raw)
  To: buildroot

All,

On Sun, Mar 26, 2017 at 4:43 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> Currently, packages can define a variable that holds all the permissions
> to set on the files it installs. This can be used to set various
> permissions, like ownership, mode, suid/sgid/sticky bits to individual
> files.
>
> However, this variable has to contain entries that are known the moment
> we scan the .mk file; it is not possible to conditionally add permisions
> for files which presence depend on post-parse conditions.
>
> This is the case for example for Busybox, for which we don't know whether
> a specific applet will be enabled or not until after the configure
> command has run.
>
> Introduce a new variable that packages can set to point to a file that
> contains a permission table. That filewill only be used when a filesystem
> image is asembled, so the file can be generated, either at configure or
> build time, with no problem.
>
> Since this variable can be empty (when no package provides such a
> permissions file), we must ensure not to cat anything (or that would
> stale, cat-ing stdin). But since this variable is not fully known by the
> time we parse the fs infra (e.g. packages from a br2-external tree are
> not yet parsed), we can not test it with make syntas (ifneq...endif).
> Teting it with shell syntac is not trivial either, becaue the variable
> would not be mpty (it would be only spaces). sO, we just iterate over
> the the files and cat them one by one with a shell-level for-loop, which
> is happy with nothing to iterate over.
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  fs/common.mk           | 3 +++
>  package/pkg-generic.mk | 1 +
>  2 files changed, 4 insertions(+)
>
> diff --git a/fs/common.mk b/fs/common.mk
> index 396b1c2..0bbcca4 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -91,6 +91,9 @@ ifeq ($$(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
>  endif
>  endif
>         $$(call PRINTF,$$(PACKAGES_PERMISSIONS_TABLE)) >> $$(FULL_DEVICE_TABLE)
> +       for f in $$(PACKAGES_PERMISSIONS_FILES); do \
> +               cat $$$${f} >> $$(FULL_DEVICE_TABLE) || exit 1; \
> +       done
>         echo "$$(HOST_DIR)/usr/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
>         $$(foreach s,$$(call qstrip,$$(BR2_ROOTFS_POST_FAKEROOT_SCRIPT)),\
>                 echo "echo '$$(TERM_BOLD)>>>   Executing fakeroot script $$(s)$$(TERM_RESET)'" >> $$(FAKEROOT_SCRIPT); \
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index dd8a1e2..343d1ad 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -915,6 +915,7 @@ PACKAGES += $(1)
>  ifneq ($$($(2)_PERMISSIONS),)
>  PACKAGES_PERMISSIONS_TABLE += $$($(2)_PERMISSIONS)$$(sep)
>  endif
> +PACKAGES_PERMISSIONS_FILES += $$($(2)_PERMISSIONS_FILE)


Reviewed-by: Matt Weber <matthew.weber@rockwellcollins.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 3/4] docs/manual: document FOO_PERMISSIONS_FILE
  2017-03-26 21:43 ` [Buildroot] [PATCH 3/4] docs/manual: document FOO_PERMISSIONS_FILE Thomas Petazzoni
@ 2017-03-27 16:44   ` Matthew Weber
  0 siblings, 0 replies; 15+ messages in thread
From: Matthew Weber @ 2017-03-27 16:44 UTC (permalink / raw)
  To: buildroot

Thomas,

On Sun, Mar 26, 2017 at 4:43 PM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
>
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  docs/manual/adding-packages-generic.txt | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/docs/manual/adding-packages-generic.txt b/docs/manual/adding-packages-generic.txt
> index a08283c..23b07fa 100644
> --- a/docs/manual/adding-packages-generic.txt
> +++ b/docs/manual/adding-packages-generic.txt
> @@ -386,7 +386,13 @@ information is (assuming the package name is +libfoo+) :
>  * +LIBFOO_PERMISSIONS+ lists the changes of permissions to be done at
>    the end of the build process. The syntax is once again the makedevs one.
>    You can find some documentation for this syntax in the xref:makedev-syntax[].
> -  This variable is optional.
> +  This variable is optional; its value must be known when the .mk file
> +  is parsed.
> +
> +* +LIBFOO_PERMISSIONS_FILE+, like +LIBFOO_PERMISSIONS+ but points to a
> +  file that contains the list of permissions. Unlike +LIBFOO_PERMISSIONS+,
> +  its content need not be known when the .mk file is parsed, so it can be
> +  generated. This variable is optional, and you should seldom need it.
>
>  * +LIBFOO_USERS+ lists the users to create for this package, if it installs
>    a program you want to run as a specific user (e.g. as a daemon, or as a

Reviewed-by: Matt Weber <matthew.weber@rockwellcollins.com>

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 1/4] system: do not overwrite /bin/sh Busybox symlink
  2017-03-26 21:43 ` [Buildroot] [PATCH 1/4] system: do not overwrite /bin/sh Busybox symlink Thomas Petazzoni
  2017-03-27 16:43   ` Matthew Weber
@ 2017-03-28 21:30   ` Arnout Vandecappelle
  2017-03-29 21:29   ` Thomas Petazzoni
  2 siblings, 0 replies; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-03-28 21:30 UTC (permalink / raw)
  To: buildroot



On 26-03-17 23:43, Thomas Petazzoni wrote:
> The BR2_SYSTEM_BIN_SH hidden option defines to what binary the /bin/sh
> symlinks should point to. If busybox is chosen, then /bin/sh is created
> to point to /bin/busybox.
> 
> This works fine with the default installation mode of Busybox, but it
> fails with the upcoming "individual binaries" mode, in which each applet
> is installed as its own binary, and /bin/busybox doesn't exist: we get
> /bin/sh as a broken symlink to /bin/busybox.
> 
> Since Busybox already installs its own /bin/sh symlink, properly
> pointing to /bin/ash or /bin/hush depending on the selected shell, it
> doesn't make sense for the BR2_SYSTEM_BIN_SH logic to override
> this. Just let Busybox install its own /bin/sh by making
> BR2_SYSTEM_BIN_SH empty when Busybox shell is selected as /bin/sh.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

 I was worried that the other shells might overwrite the symlink created by
busybox in their install commands, but apparently none of them do. I tried bash,
dash, mksh and zsh. So

Acked-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

> ---
> This is proposed as an alternative to
> https://patchwork.ozlabs.org/patch/686673/. If people believe
> https://patchwork.ozlabs.org/patch/686673/ is a better solution, I'm
> fine as well.

 No, that solution is wrong I think if /bin/sh is not busybox, because then the
symlink will not be created at all.

 Regards,
 Arnout

> ---
>  package/skeleton/skeleton.mk | 2 ++
>  system/Config.in             | 1 -
>  2 files changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/package/skeleton/skeleton.mk b/package/skeleton/skeleton.mk
> index 1000161..9940944 100644
> --- a/package/skeleton/skeleton.mk
> +++ b/package/skeleton/skeleton.mk
> @@ -203,10 +203,12 @@ define SKELETON_BIN_SH
>  	rm -f $(TARGET_DIR)/bin/sh
>  endef
>  else
> +ifneq ($(SKELETON_TARGET_GENERIC_BIN_SH),)
>  define SKELETON_BIN_SH
>  	ln -sf $(SKELETON_TARGET_GENERIC_BIN_SH) $(TARGET_DIR)/bin/sh
>  endef
>  endif
> +endif
>  TARGET_FINALIZE_HOOKS += SKELETON_BIN_SH
>  
>  ifeq ($(BR2_TARGET_GENERIC_GETTY),y)
> diff --git a/system/Config.in b/system/Config.in
> index 3ddf843..b47ae43 100644
> --- a/system/Config.in
> +++ b/system/Config.in
> @@ -298,7 +298,6 @@ endchoice # /bin/sh
>  
>  config BR2_SYSTEM_BIN_SH
>  	string
> -	default "busybox" if BR2_SYSTEM_BIN_SH_BUSYBOX
>  	default "bash"    if BR2_SYSTEM_BIN_SH_BASH
>  	default "dash"    if BR2_SYSTEM_BIN_SH_DASH
>  	default "mksh"    if BR2_SYSTEM_BIN_SH_MKSH
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 2/4] core: allow packages to declare a permission file
  2017-03-26 21:43 ` [Buildroot] [PATCH 2/4] core: allow packages to declare a permission file Thomas Petazzoni
  2017-03-27 16:44   ` Matthew Weber
@ 2017-03-28 22:12   ` Arnout Vandecappelle
  2017-03-29  6:43     ` Arnout Vandecappelle
  1 sibling, 1 reply; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-03-28 22:12 UTC (permalink / raw)
  To: buildroot



On 26-03-17 23:43, Thomas Petazzoni wrote:
> From: "Yann E. MORIN" <yann.morin.1998@free.fr>
> 
> Currently, packages can define a variable that holds all the permissions
> to set on the files it installs. This can be used to set various
> permissions, like ownership, mode, suid/sgid/sticky bits to individual
> files.
> 
> However, this variable has to contain entries that are known the moment
> we scan the .mk file; it is not possible to conditionally add permisions
                                                                     ^
> for files which presence depend on post-parse conditions.
> 
> This is the case for example for Busybox, for which we don't know whether
> a specific applet will be enabled or not until after the configure
> command has run.
> 
> Introduce a new variable that packages can set to point to a file that
> contains a permission table. That filewill only be used when a filesystem
                                        ^
> image is asembled, so the file can be generated, either at configure or
            ^
> build time, with no problem.
> 
> Since this variable can be empty (when no package provides such a
> permissions file), we must ensure not to cat anything (or that would
> stale, cat-ing stdin). But since this variable is not fully known by the
> time we parse the fs infra (e.g. packages from a br2-external tree are
> not yet parsed), we can not test it with make syntas (ifneq...endif).
                                                     ^
> Teting it with shell syntac is not trivial either, becaue the variable
    ^                       ^                             ^
> would not be mpty (it would be only spaces). sO, we just iterate over
               ^                                ^
> the the files and cat them one by one with a shell-level for-loop, which
> is happy with nothing to iterate over.
> 
> Signed-off-by: "Yann E. MORIN" <yann.morin.1998@free.fr>
> Cc: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Cc: Peter Korsgaard <peter@korsgaard.com>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  fs/common.mk           | 3 +++
>  package/pkg-generic.mk | 1 +
>  2 files changed, 4 insertions(+)
> 
> diff --git a/fs/common.mk b/fs/common.mk
> index 396b1c2..0bbcca4 100644
> --- a/fs/common.mk
> +++ b/fs/common.mk
> @@ -91,6 +91,9 @@ ifeq ($$(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
>  endif
>  endif
>  	$$(call PRINTF,$$(PACKAGES_PERMISSIONS_TABLE)) >> $$(FULL_DEVICE_TABLE)
> +	for f in $$(PACKAGES_PERMISSIONS_FILES); do \
> +		cat $$$${f} >> $$(FULL_DEVICE_TABLE) || exit 1; \
> +	done

 Actually I think it's simpler to do

	$$(if $$(strip $$(PACKAGES_PERMISSIONS_FILES)),\
		cat $$(PACKAGES_PERMISSIONS_FILES) >> $$(FULL_DEVICE_TABLE))

>  	echo "$$(HOST_DIR)/usr/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
>  	$$(foreach s,$$(call qstrip,$$(BR2_ROOTFS_POST_FAKEROOT_SCRIPT)),\
>  		echo "echo '$$(TERM_BOLD)>>>   Executing fakeroot script $$(s)$$(TERM_RESET)'" >> $$(FAKEROOT_SCRIPT); \
> diff --git a/package/pkg-generic.mk b/package/pkg-generic.mk
> index dd8a1e2..343d1ad 100644
> --- a/package/pkg-generic.mk
> +++ b/package/pkg-generic.mk
> @@ -915,6 +915,7 @@ PACKAGES += $(1)
>  ifneq ($$($(2)_PERMISSIONS),)
>  PACKAGES_PERMISSIONS_TABLE += $$($(2)_PERMISSIONS)$$(sep)
>  endif
> +PACKAGES_PERMISSIONS_FILES += $$($(2)_PERMISSIONS_FILE)

 I really don't like having two ways to do the same thing. So I looked at
alternatives, and found this:

define BUSYBOX_PERMISSIONS
	$(foreach app,$(shell \
		for app in `grep -r -e "APPLET.*BB_SUID_REQUIRE\|APPLET.*BB_SUID_MAYBE"
$(BUSYBOX_DIR)/include/applets.h \
			| sed -e 's/,.*//' -e 's/.*(//'`; \
		do \
			grep -w $${app} $(BUSYBOX_DIR)/busybox.links; \
		done #) ),$(app) f 4755 0  0 - - - - -$(sep))
endef

 For that to work, the condition around PACKAGES_PERMISSIONS_TABLES += must be
removed, but that's fine IMO (just adds a lot of empty lines to the device
table, but that doesn't hurt). The #) is a bit ugly but I'm sure a better
solution can be found.

 With that solution, patches 2 and 3 wouldn't be needed.

 Regards,
 Arnout


>  ifneq ($$($(2)_DEVICES),)
>  PACKAGES_DEVICES_TABLE += $$($(2)_DEVICES)$$(sep)
>  endif
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 2/4] core: allow packages to declare a permission file
  2017-03-28 22:12   ` Arnout Vandecappelle
@ 2017-03-29  6:43     ` Arnout Vandecappelle
  2017-07-03 16:03       ` Arnout Vandecappelle
  0 siblings, 1 reply; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-03-29  6:43 UTC (permalink / raw)
  To: buildroot



On 29-03-17 00:12, Arnout Vandecappelle wrote:
[snip]
> define BUSYBOX_PERMISSIONS
> 	$(foreach app,$(shell \
> 		for app in `grep -r -e "APPLET.*BB_SUID_REQUIRE\|APPLET.*BB_SUID_MAYBE"
> $(BUSYBOX_DIR)/include/applets.h \
> 			| sed -e 's/,.*//' -e 's/.*(//'`; \
> 		do \
> 			grep -w $${app} $(BUSYBOX_DIR)/busybox.links; \
> 		done #) ),$(app) f 4755 0  0 - - - - -$(sep))
> endef
> 
>  For that to work, the condition around PACKAGES_PERMISSIONS_TABLES += must be
> removed, but that's fine IMO (just adds a lot of empty lines to the device
> table, but that doesn't hurt). The #) is a bit ugly but I'm sure a better
> solution can be found.
> 
>  With that solution, patches 2 and 3 wouldn't be needed.

 And an even simpler approach would be to always run the installation commands
under fakeroot (with the -i -s options of course) That would simplify a lot of
things. But there are probably problems with that approach as well - e.g.
nothing should install/remove anything in target outside of the install-target step.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 1/4] system: do not overwrite /bin/sh Busybox symlink
  2017-03-26 21:43 ` [Buildroot] [PATCH 1/4] system: do not overwrite /bin/sh Busybox symlink Thomas Petazzoni
  2017-03-27 16:43   ` Matthew Weber
  2017-03-28 21:30   ` Arnout Vandecappelle
@ 2017-03-29 21:29   ` Thomas Petazzoni
  2 siblings, 0 replies; 15+ messages in thread
From: Thomas Petazzoni @ 2017-03-29 21:29 UTC (permalink / raw)
  To: buildroot

Hello,

On Sun, 26 Mar 2017 23:43:40 +0200, Thomas Petazzoni wrote:
> The BR2_SYSTEM_BIN_SH hidden option defines to what binary the /bin/sh
> symlinks should point to. If busybox is chosen, then /bin/sh is created
> to point to /bin/busybox.
> 
> This works fine with the default installation mode of Busybox, but it
> fails with the upcoming "individual binaries" mode, in which each applet
> is installed as its own binary, and /bin/busybox doesn't exist: we get
> /bin/sh as a broken symlink to /bin/busybox.
> 
> Since Busybox already installs its own /bin/sh symlink, properly
> pointing to /bin/ash or /bin/hush depending on the selected shell, it
> doesn't make sense for the BR2_SYSTEM_BIN_SH logic to override
> this. Just let Busybox install its own /bin/sh by making
> BR2_SYSTEM_BIN_SH empty when Busybox shell is selected as /bin/sh.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
> This is proposed as an alternative to
> https://patchwork.ozlabs.org/patch/686673/. If people believe
> https://patchwork.ozlabs.org/patch/686673/ is a better solution, I'm
> fine as well.
> ---
>  package/skeleton/skeleton.mk | 2 ++
>  system/Config.in             | 1 -
>  2 files changed, 2 insertions(+), 1 deletion(-)

Applied to master, thanks.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 2/4] core: allow packages to declare a permission file
  2017-03-29  6:43     ` Arnout Vandecappelle
@ 2017-07-03 16:03       ` Arnout Vandecappelle
  2017-07-03 16:11         ` Arnout Vandecappelle
  0 siblings, 1 reply; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-07-03 16:03 UTC (permalink / raw)
  To: buildroot

 Hi all,

On 29-03-17 08:43, Arnout Vandecappelle wrote:
> 
> 
> On 29-03-17 00:12, Arnout Vandecappelle wrote:
> [snip]
>> define BUSYBOX_PERMISSIONS
>> 	$(foreach app,$(shell \
>> 		for app in `grep -r -e "APPLET.*BB_SUID_REQUIRE\|APPLET.*BB_SUID_MAYBE"
>> $(BUSYBOX_DIR)/include/applets.h \
>> 			| sed -e 's/,.*//' -e 's/.*(//'`; \
>> 		do \
>> 			grep -w $${app} $(BUSYBOX_DIR)/busybox.links; \
>> 		done #) ),$(app) f 4755 0  0 - - - - -$(sep))
>> endef
>>
>>  For that to work, the condition around PACKAGES_PERMISSIONS_TABLES += must be
>> removed, but that's fine IMO (just adds a lot of empty lines to the device
>> table, but that doesn't hurt). The #) is a bit ugly but I'm sure a better
>> solution can be found.
>>
>>  With that solution, patches 2 and 3 wouldn't be needed.
> 
>  And an even simpler approach would be to always run the installation commands
> under fakeroot (with the -i -s options of course) That would simplify a lot of
> things. But there are probably problems with that approach as well - e.g.
> nothing should install/remove anything in target outside of the install-target step.

 We finally got around to discussing this at the Summer Camp. The conclusion is
that the simplest option is in fact to go back to your earlier solution, where
makedevs would be extended with a file type that ignores files that don't exist,
and the BUSYBOX_PERMISSIONS are set statically for all potential busybox
applets. Yann doesn't like it but he's a minority :-).

 Bryce, Adam, do you think you could reconstruct such a series?

 For now, I've marked these three patches as Rejected in patchwork.

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [Buildroot] [PATCH 2/4] core: allow packages to declare a permission file
  2017-07-03 16:03       ` Arnout Vandecappelle
@ 2017-07-03 16:11         ` Arnout Vandecappelle
  0 siblings, 0 replies; 15+ messages in thread
From: Arnout Vandecappelle @ 2017-07-03 16:11 UTC (permalink / raw)
  To: buildroot



On 03-07-17 18:03, Arnout Vandecappelle wrote:
>  Hi all,
> 
> On 29-03-17 08:43, Arnout Vandecappelle wrote:
>>
>>
>> On 29-03-17 00:12, Arnout Vandecappelle wrote:
>> [snip]
>>> define BUSYBOX_PERMISSIONS
>>> 	$(foreach app,$(shell \
>>> 		for app in `grep -r -e "APPLET.*BB_SUID_REQUIRE\|APPLET.*BB_SUID_MAYBE"
>>> $(BUSYBOX_DIR)/include/applets.h \
>>> 			| sed -e 's/,.*//' -e 's/.*(//'`; \
>>> 		do \
>>> 			grep -w $${app} $(BUSYBOX_DIR)/busybox.links; \
>>> 		done #) ),$(app) f 4755 0  0 - - - - -$(sep))
>>> endef
>>>
>>>  For that to work, the condition around PACKAGES_PERMISSIONS_TABLES += must be
>>> removed, but that's fine IMO (just adds a lot of empty lines to the device
>>> table, but that doesn't hurt). The #) is a bit ugly but I'm sure a better
>>> solution can be found.
>>>
>>>  With that solution, patches 2 and 3 wouldn't be needed.
>>
>>  And an even simpler approach would be to always run the installation commands
>> under fakeroot (with the -i -s options of course) That would simplify a lot of
>> things. But there are probably problems with that approach as well - e.g.
>> nothing should install/remove anything in target outside of the install-target step.
> 
>  We finally got around to discussing this at the Summer Camp. The conclusion is
> that the simplest option is in fact to go back to your earlier solution, where
> makedevs would be extended with a file type that ignores files that don't exist,
> and the BUSYBOX_PERMISSIONS are set statically for all potential busybox
> applets. Yann doesn't like it but he's a minority :-).
> 
>  Bryce, Adam, do you think you could reconstruct such a series?

 Thomas gently suggested that I should do this instead, so don't bother :-)

 Regards,
 Arnout

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-07-03 16:11 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-26 21:43 [Buildroot] [PATCH 0/4] Allow Busybox installation as individual binaries Thomas Petazzoni
2017-03-26 21:43 ` [Buildroot] [PATCH 1/4] system: do not overwrite /bin/sh Busybox symlink Thomas Petazzoni
2017-03-27 16:43   ` Matthew Weber
2017-03-28 21:30   ` Arnout Vandecappelle
2017-03-29 21:29   ` Thomas Petazzoni
2017-03-26 21:43 ` [Buildroot] [PATCH 2/4] core: allow packages to declare a permission file Thomas Petazzoni
2017-03-27 16:44   ` Matthew Weber
2017-03-28 22:12   ` Arnout Vandecappelle
2017-03-29  6:43     ` Arnout Vandecappelle
2017-07-03 16:03       ` Arnout Vandecappelle
2017-07-03 16:11         ` Arnout Vandecappelle
2017-03-26 21:43 ` [Buildroot] [PATCH 3/4] docs/manual: document FOO_PERMISSIONS_FILE Thomas Petazzoni
2017-03-27 16:44   ` Matthew Weber
2017-03-26 21:43 ` [Buildroot] [PATCH 4/4] busybox: applets as individual binaries Thomas Petazzoni
2017-03-27 16:39   ` Matthew Weber

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.