[Buildroot] [PATCH 0/4] Improve verification of custom rootfs skeletons and overlays

[Buildroot] [PATCH 0/4] Improve verification of custom rootfs skeletons and overlays

[Carlos Santos]

This series makes some improvements in the verification of custom rootfs
skeletons and overlays, regarding mergerd /usr:

Patch 1 adds a script to check if a given path complies to the merged /usr
requirements and makes skeleton-custom.mk use it instead of a bunch of
variables filled by $(shell ...) macros.

Patch 2 ensures that /bin, /lib and /sbin are created for custom skeletons,
either as directories or symlinks, according to BR2_ROOTFS_MERGED_USR.

Patch 3 uses the script added in patch 1 to check rootfs overlays, in
target-finalize.

Patch 3 removes the restriction of using merged /usr only with the default
skeleton or when systemd is selected.

Carlos Santos (4):
  skeleton-custom: use a script to check merged usr structure
  skeleton-custom: install /bin, /lib, and /sbin
  Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled
  system: allow selecting merged /usr along with custom rootfs skeleton

 Makefile                                   | 20 +++++++++--
 docs/manual/customize-rootfs.txt           | 10 ++++++
 package/skeleton-custom/skeleton-custom.mk | 25 ++------------
 support/scripts/check-merged-usr.sh        | 39 ++++++++++++++++++++++
 system/Config.in                           |  8 ++---
 5 files changed, 71 insertions(+), 31 deletions(-)
 create mode 100755 support/scripts/check-merged-usr.sh

-- 
2.17.0

[Buildroot] [PATCH 1/4] skeleton-custom: use a script to check merged usr structure

[Carlos Santos]

Introduce support/scripts/check-merged-usr.sh, a script that check if a
given path complies to the merged /usr requirements:

    /
    /bin -> usr/bin
    /lib -> usr/lib
    /sbin -> usr/sbin
    /usr/bin/
    /usr/lib/
    /usr/sbin/

Use this script in skeleton-custom.mk instead of a bunch of variables
filled by $(shell ...) macros. The same script will be used to check
rootfs overlays, in a forthcoming change.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
 package/skeleton-custom/skeleton-custom.mk | 23 +------------
 support/scripts/check-merged-usr.sh        | 39 ++++++++++++++++++++++
 2 files changed, 40 insertions(+), 22 deletions(-)
 create mode 100755 support/scripts/check-merged-usr.sh

diff --git a/package/skeleton-custom/skeleton-custom.mk b/package/skeleton-custom/skeleton-custom.mk
index 8c57531782..b1cddd9146 100644
--- a/package/skeleton-custom/skeleton-custom.mk
+++ b/package/skeleton-custom/skeleton-custom.mk
@@ -23,32 +23,11 @@ $(error No path specified for the custom skeleton)
 endif
 endif
 
-# Extract the inode numbers for all of those directories. In case any is
-# a symlink, we want to get the inode of the pointed-to directory, so we
-# append '/.' to be sure we get the target directory. Since the symlinks
-# can be anyway (/bin -> /usr/bin or /usr/bin -> /bin), we do that for
-# all of them.
-#
-SKELETON_CUSTOM_LIB_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/lib/. 2>/dev/null)
-SKELETON_CUSTOM_BIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/bin/. 2>/dev/null)
-SKELETON_CUSTOM_SBIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/sbin/. 2>/dev/null)
-SKELETON_CUSTOM_USR_LIB_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/lib/. 2>/dev/null)
-SKELETON_CUSTOM_USR_BIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/bin/. 2>/dev/null)
-SKELETON_CUSTOM_USR_SBIN_INODE = $(shell stat -c '%i' $(SKELETON_CUSTOM_PATH)/usr/sbin/. 2>/dev/null)
-
 # For a merged /usr, ensure that /lib, /bin and /sbin and their /usr
 # counterparts are appropriately setup as symlinks ones to the others.
 ifeq ($(BR2_ROOTFS_MERGED_USR),y)
 
-ifneq ($(SKELETON_CUSTOM_LIB_INODE),$(SKELETON_CUSTOM_USR_LIB_INODE))
-SKELETON_CUSTOM_NOT_MERGED_USR_DIRS += /lib
-endif
-ifneq ($(SKELETON_CUSTOM_BIN_INODE),$(SKELETON_CUSTOM_USR_BIN_INODE))
-SKELETON_CUSTOM_NOT_MERGED_USR_DIRS += /bin
-endif
-ifneq ($(SKELETON_CUSTOM_SBIN_INODE),$(SKELETON_CUSTOM_USR_SBIN_INODE))
-SKELETON_CUSTOM_NOT_MERGED_USR_DIRS += /sbin
-endif
+SKELETON_CUSTOM_NOT_MERGED_USR_DIRS = $(shell support/scripts/check-merged-usr.sh $(SKELETON_CUSTOM_PATH))
 
 endif # merged /usr
 
diff --git a/support/scripts/check-merged-usr.sh b/support/scripts/check-merged-usr.sh
new file mode 100755
index 0000000000..74c43c89fd
--- /dev/null
+++ b/support/scripts/check-merged-usr.sh
@@ -0,0 +1,39 @@
+#!/bin/sh
+#
+# Check if a given custom skeleton or overlay complies to the merged /usr
+# requirements:
+# /
+# /bin -> usr/bin
+# /lib -> usr/lib
+# /sbin -> usr/sbin
+# /usr/bin/
+# /usr/lib/
+# /usr/sbin/
+#
+# Output: the list non-compliant paths (empty if compliant).
+#
+
+# Extract the inode numbers for all of those directories. In case any is
+# a symlink, we want to get the inode of the pointed-to directory, so we
+# append '/.' to be sure we get the target directory. Since the symlinks
+# can be anyway (/bin -> /usr/bin or /usr/bin -> /bin), we do that for
+# all of them.
+#
+lib_inode=$(stat -c '%i' "${1}/lib/." 2>/dev/null)
+bin_inode=$(stat -c '%i' "${1}/bin/." 2>/dev/null)
+sbin_inode=$(stat -c '%i' "${1}/sbin/." 2>/dev/null)
+usr_lib_inode=$(stat -c '%i' "${1}/usr/lib/." 2>/dev/null)
+usr_bin_inode=$(stat -c '%i' "${1}/usr/bin/." 2>/dev/null)
+usr_sbin_inode=$(stat -c '%i' "${1}/usr/sbin/." 2>/dev/null)
+
+not_merged_dirs=""
+test -z "$lib_inode" || \
+	test "$lib_inode" = "$usr_lib_inode" || \
+		not_merged_dirs="/lib"
+test -z "$bin_inode" || \
+	test "$bin_inode" = "$usr_bin_inode" || \
+		not_merged_dirs="$not_merged_dirs /bin"
+test -z "$sbin_inode" || \
+	test "$sbin_inode" = "$usr_sbin_inode" || \
+		not_merged_dirs="$not_merged_dirs /sbin"
+echo "${not_merged_dirs# }"
-- 
2.17.0

[Buildroot] [PATCH 2/4] skeleton-custom: install /bin, /lib, and /sbin

[Carlos Santos]

skeleton-custom does not install the required /bin, /lib and /sbin
directories (or symlinks), which may result in an imcomplete tree, The
user could add the required directories/symlinks to the skeleton but
they maye be invalid, depending on the state of BR2_ROOTFS_MERGED_USR.

Steps to reproduce:

- Enable BR2_ROOTFS_MERGED_USR and BR2_INIT_SYSTEMD
- Set BR2_ROOTFS_SKELETON_CUSTOM_PATH to "system/skeleton"
- Run "make skeleton"
- target/{bin.lib,sbin} will not exist

Add calls to SYSTEM_USR_SYMLINKS_OR_DIRS to INSTALL_TARGET_CMDS and
INSTALL_STAGING_CMDS, so the required directories or symlinks are
created.

Add a paragraph to the documentation clarifying that custom skeletons
should not contain /bin, /lib or /sbin.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
 docs/manual/customize-rootfs.txt           | 5 +++++
 package/skeleton-custom/skeleton-custom.mk | 2 ++
 2 files changed, 7 insertions(+)

diff --git a/docs/manual/customize-rootfs.txt b/docs/manual/customize-rootfs.txt
index 44fc460670..8982e03498 100644
--- a/docs/manual/customize-rootfs.txt
+++ b/docs/manual/customize-rootfs.txt
@@ -100,6 +100,11 @@ To enable this feature, enable config option
   +System configuration+ menu. If you specify a relative path, it will
   be relative to the root of the Buildroot tree.
 +
+Custom skeletons should not contain the '/bin', '/lib' or '/sbin' paths,
+  since they are created during the build, either as directories or as
+  symbolic links to their counterparts under 'usr', depending on the
+  system configuration.
++
 This method is not recommended because it duplicates the entire
   skeleton, which prevents taking advantage of the fixes or improvements
   brought to the default skeleton in later Buildroot releases.
diff --git a/package/skeleton-custom/skeleton-custom.mk b/package/skeleton-custom/skeleton-custom.mk
index b1cddd9146..01cd62794d 100644
--- a/package/skeleton-custom/skeleton-custom.mk
+++ b/package/skeleton-custom/skeleton-custom.mk
@@ -43,6 +43,7 @@ endif
 # things we customise in the custom skeleton.
 define SKELETON_CUSTOM_INSTALL_TARGET_CMDS
 	$(call SYSTEM_RSYNC,$(SKELETON_CUSTOM_PATH),$(TARGET_DIR))
+	$(call SYSTEM_USR_SYMLINKS_OR_DIRS,$(TARGET_DIR))
 	$(call SYSTEM_LIB_SYMLINK,$(TARGET_DIR))
 	$(INSTALL) -m 0644 support/misc/target-dir-warning.txt \
 		$(TARGET_DIR_WARNING_FILE)
@@ -54,6 +55,7 @@ endef
 # skeleton to staging.
 define SKELETON_CUSTOM_INSTALL_STAGING_CMDS
 	$(call SYSTEM_RSYNC,$(SKELETON_CUSTOM_PATH),$(STAGING_DIR))
+	$(call SYSTEM_USR_SYMLINKS_OR_DIRS,$(STAGING_DIR))
 	$(call SYSTEM_LIB_SYMLINK,$(STAGING_DIR))
 endef
 
-- 
2.17.0

[Buildroot] [PATCH 2/4] skeleton-custom: install /bin, /lib, and /sbin

[Thomas Petazzoni]

Hello Carlos,

On Sun,  6 May 2018 18:31:14 -0300, Carlos Santos wrote:
> skeleton-custom does not install the required /bin, /lib and /sbin
> directories (or symlinks), which may result in an imcomplete tree, The
> user could add the required directories/symlinks to the skeleton but
> they maye be invalid, depending on the state of BR2_ROOTFS_MERGED_USR.

maye -> may

> 
> Steps to reproduce:
> 
> - Enable BR2_ROOTFS_MERGED_USR and BR2_INIT_SYSTEMD
> - Set BR2_ROOTFS_SKELETON_CUSTOM_PATH to "system/skeleton"
> - Run "make skeleton"
> - target/{bin.lib,sbin} will not exist
> 
> Add calls to SYSTEM_USR_SYMLINKS_OR_DIRS to INSTALL_TARGET_CMDS and
> INSTALL_STAGING_CMDS, so the required directories or symlinks are
> created.
> 
> Add a paragraph to the documentation clarifying that custom skeletons
> should not contain /bin, /lib or /sbin.

Maybe I don't understand the full scope of the problem, but to me, it
looks weird that the full contents of the skeleton (default or custom)
are not copied to the target root filesystem. It goes against the basic
expectations of what the skeleton is.

Of course, I am fine with checking that the skeleton complies with the
BR2_ROOTFS_MERGED_USR option, but preventing the skeleton from
having /bin, /lib and /sbin looks weird. So much that it needs to be
mentioned in the documentation.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 2/4] skeleton-custom: install /bin, /lib, and /sbin

[Carlos Santos]

> From: "Thomas Petazzoni" <thomas.petazzoni@bootlin.com>
> To: "Carlos Santos" <casantos@datacom.ind.br>
> Cc: "buildroot" <buildroot@buildroot.org>, "yann morin 1998" <yann.morin.1998@gmail.com>, "Thomas De Schampheleire"
> <thomas.de_schampheleire@nokia.com>
> Sent: Monday, May 7, 2018 4:08:11 AM
> Subject: Re: [Buildroot] [PATCH 2/4] skeleton-custom: install /bin, /lib, and /sbin

> Hello Carlos,
> 
> On Sun,  6 May 2018 18:31:14 -0300, Carlos Santos wrote:
>> skeleton-custom does not install the required /bin, /lib and /sbin
>> directories (or symlinks), which may result in an imcomplete tree, The
>> user could add the required directories/symlinks to the skeleton but
>> they maye be invalid, depending on the state of BR2_ROOTFS_MERGED_USR.
> 
> maye -> may

:-)

> 
>> 
>> Steps to reproduce:
>> 
>> - Enable BR2_ROOTFS_MERGED_USR and BR2_INIT_SYSTEMD
>> - Set BR2_ROOTFS_SKELETON_CUSTOM_PATH to "system/skeleton"
>> - Run "make skeleton"
>> - target/{bin.lib,sbin} will not exist
>> 
>> Add calls to SYSTEM_USR_SYMLINKS_OR_DIRS to INSTALL_TARGET_CMDS and
>> INSTALL_STAGING_CMDS, so the required directories or symlinks are
>> created.
>> 
>> Add a paragraph to the documentation clarifying that custom skeletons
>> should not contain /bin, /lib or /sbin.
> 
> Maybe I don't understand the full scope of the problem, but to me, it
> looks weird that the full contents of the skeleton (default or custom)
> are not copied to the target root filesystem. It goes against the basic
> expectations of what the skeleton is.
> 
> Of course, I am fine with checking that the skeleton complies with the
> BR2_ROOTFS_MERGED_USR option, but preventing the skeleton from
> having /bin, /lib and /sbin looks weird. So much that it needs to be
> mentioned in the documentation.

Notice that I used ?should? in the documentation, not ?must?. It is
still OK to have /bin, /lib and /sbin in custom skeletons and rootfs
overlays if BR2_ROOTFS_MERGED_USR is not set.

-- 
Carlos Santos (Casantos) - DATACOM, P&D
?The greatest triumph that modern PR can offer is the transcendent 
success of having your words and actions judged by your reputation, 
rather than the other way about.? ? Christopher Hitchens

[Buildroot] [PATCH 2/4] skeleton-custom: install /bin, /lib, and /sbin

[Thomas Petazzoni]

Hello Carlos,

On Mon, 7 May 2018 08:16:48 -0300 (BRT), Carlos Santos wrote:

> > Of course, I am fine with checking that the skeleton complies with the
> > BR2_ROOTFS_MERGED_USR option, but preventing the skeleton from
> > having /bin, /lib and /sbin looks weird. So much that it needs to be
> > mentioned in the documentation.  
> 
> Notice that I used ?should? in the documentation, not ?must?. It is
> still OK to have /bin, /lib and /sbin in custom skeletons and rootfs
> overlays if BR2_ROOTFS_MERGED_USR is not set.

Ah, OK, but then the explanation should be worded differently. What
about something like:

"""

When +BR2_ROOTFS_MERGED_USR+ is enabled, then the custom skeleton must
not contain the +/bin+, +/lib+ or +/sbin+ directories, as Buildroot
will create them as symbolic links to the relevant folders in +/usr+.
In such a situation, should the skeleton have any programs or
libraries, they should be placed in +/usr/bin+, +/usr/sbin+ and
+/usr/lib+.

"""

Or something along those lines.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 3/4] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

[Carlos Santos]

Since commit 0db34529f48 we use rsync with the --keep-dirlinks option to
prevent overlays from accidentally overwriding /{usr,bin,sbin,lib} links
when BR2_ROOTFS_MERGED_USR option is enabled. Unfortunately this also
prevents replacing a symlink by a directory on purpose (e.g. /var/log,
to persist system logs).

Steps to reproduce:

- enable BR2_ROOTFS_MERGED_USR and BR2_PACKAGE_SKELETON_INIT_SYSV
- mkdir some_path/rootfs-overlay/var/log
- enable BR2_ROOTFS_OVERLAY="some_path/rootfs-overlay"
- run 'make'
- 'target/var/log' is still a symlink to '../tmp', not a directory

Fix the problem by adding a step in target-finalize that checks each
overlay, using the same criteria used in skeleton-custom.mk.

Add a paragraph to the documentation clarifying that rootfs overlays
should not contain /bin, /lib or /sbin.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
 Makefile                         | 20 +++++++++++++++++---
 docs/manual/customize-rootfs.txt |  5 +++++
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index c024c65f78..dc51269143 100644
--- a/Makefile
+++ b/Makefile
@@ -746,11 +746,25 @@ endif
 	@$(call MESSAGE,"Sanitizing RPATH in target tree")
 	$(TOPDIR)/support/scripts/fix-rpath target
 
+# For a merged /usr, ensure that /lib, /bin and /sbin and their /usr
+# counterparts are appropriately setup as symlinks ones to the others.
+ifeq ($(BR2_ROOTFS_MERGED_USR),y)
+
+	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
+		$(call MESSAGE,"Sanity check in overlay $(d)"); \
+		not_merged_dirs="$$(support/scripts/check-merged-usr.sh $(d))"; \
+		test -n "$$not_merged_dirs" && { \
+			echo "ERROR: The overlay in $(d) is not" \
+				"using a merged /usr for the following directories:" \
+				$$not_merged_dirs; \
+			exit 1; \
+		} || true$(sep))
+
+endif # merged /usr
+
 	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
 		$(call MESSAGE,"Copying overlay $(d)"); \
-		rsync -a --ignore-times --keep-dirlinks $(RSYNC_VCS_EXCLUSIONS) \
-			--chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
-			$(d)/ $(TARGET_DIR)$(sep))
+		$(call SYSTEM_RSYNC,$(d),$(TARGET_DIR))$(sep))
 
 	@$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
 		$(call MESSAGE,"Executing post-build script $(s)"); \
diff --git a/docs/manual/customize-rootfs.txt b/docs/manual/customize-rootfs.txt
index 8982e03498..5dbbe86044 100644
--- a/docs/manual/customize-rootfs.txt
+++ b/docs/manual/customize-rootfs.txt
@@ -22,6 +22,11 @@ A filesystem overlay is a tree of files that is copied directly
   etc., files called +.empty+ and files ending in +~+ are excluded from
   the copy.
 +
+Overlays should not contain the '/bin', '/lib' or '/sbin' paths, since
+  they are created during the build, either as directories or as
+  symbolic links to their counterparts under 'usr', depending on the
+  system configuration.
++
 As shown in xref:customize-dir-structure[], the recommended path for
   this overlay is +board/<company>/<boardname>/rootfs-overlay+.
 
-- 
2.17.0

[Buildroot] [PATCH 3/4] Makefile: check rootfs overlays with BR2_ROOTFS_MERGED_USR enabled

[Thomas Petazzoni]

Hello,

On Sun,  6 May 2018 18:31:15 -0300, Carlos Santos wrote:

> Fix the problem by adding a step in target-finalize that checks each
> overlay, using the same criteria used in skeleton-custom.mk.
> 
> Add a paragraph to the documentation clarifying that rootfs overlays
> should not contain /bin, /lib or /sbin.

Same comment here: I'm sure plenty of people want to add some stuff to
their filesystem (scripts, etc.) that might need to be in /bin or /sbin
(/lib is less likely).

So I'm not a big fan of this additional constraint, just like I said
for the skeleton case.

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

[Buildroot] [PATCH 4/4] system: allow selecting merged /usr along with custom rootfs skeleton

[Carlos Santos]

If the user is brave enough to use a custom rootfs skeleton then we must
not prevent using merged /usr too. Actually it is already possible to do
this, although indirectly, by selecting BR2_INIT_SYSTEMD.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
 system/Config.in | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/system/Config.in b/system/Config.in
index d14a864ca5..911cd424ba 100644
--- a/system/Config.in
+++ b/system/Config.in
@@ -31,10 +31,6 @@ config BR2_ROOTFS_SKELETON_CUSTOM_PATH
 	help
 	  Path to custom target skeleton.
 
-# dummy config so merged /usr workarounds can also be activated for
-# custom rootfs skeleton
-config BR2_ROOTFS_MERGED_USR
-
 endif
 
 if BR2_ROOTFS_SKELETON_DEFAULT
@@ -209,8 +205,6 @@ config BR2_ROOTFS_DEVICE_TABLE_SUPPORTS_EXTENDED_ATTRIBUTES
 	help
 	  Support extended attributes handling in device tables
 
-if BR2_ROOTFS_SKELETON_DEFAULT
-
 config BR2_ROOTFS_MERGED_USR
 	bool "Use symlinks to /usr for /bin, /sbin and /lib"
 	help
@@ -223,6 +217,8 @@ config BR2_ROOTFS_MERGED_USR
 	  symlinks to their counterparts in /usr. In this case, /usr can
 	  not be a separate filesystem.
 
+if BR2_ROOTFS_SKELETON_DEFAULT
+
 config BR2_TARGET_ENABLE_ROOT_LOGIN
 	bool "Enable root login with password"
 	default y
-- 
2.17.0