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

[Buildroot] [PATCH v3 0/5] 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 4 allows rootfs overlays to override symbolic links on purpose (e.g.
/var/log, to persist system logs).

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

Carlos Santos (5):
  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
  Makefile: allow rootfs overlays to override symbolic links
  system: allow selecting merged /usr along with custom rootfs skeleton

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

-- 
2.14.3

[Buildroot] [PATCH v3 1/5] 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>
---
Changes v1->v2:

- Rebase series to HEAD of master branch
---
 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.14.3

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

[Arnout Vandecappelle]

On 07/05/2018 16:44, Carlos Santos wrote:

> +SKELETON_CUSTOM_NOT_MERGED_USR_DIRS = $(shell support/scripts/check-merged-usr.sh $(SKELETON_CUSTOM_PATH))

 This line was too long.


> +# Output: the list non-compliant paths (empty if compliant).
                     ^of

 Regards,
 Arnout

[Buildroot] [PATCH v3 2/5] 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 may 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
don't need to contain /bin, /lib or /sbin and must not contain them when
BR2_ROOTFS_MERGED_USR is enabled.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
Changes v1->v2:

- Rebase series to HEAD of master branch
- Rework commit message and documentation, as suggested by Thomas
  Petazzoni
---
 docs/manual/customize-rootfs.txt           | 9 +++++++++
 package/skeleton-custom/skeleton-custom.mk | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/docs/manual/customize-rootfs.txt b/docs/manual/customize-rootfs.txt
index 44fc460670..9d3a62ddaf 100644
--- a/docs/manual/customize-rootfs.txt
+++ b/docs/manual/customize-rootfs.txt
@@ -100,6 +100,15 @@ 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 don't need to contain the '/bin', '/lib' or '/sbin'
+  directories, since they are created automatically during the build.
+  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'.
++
 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.14.3

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

[Carlos Santos]

Add a step to target-finalize that checks each rootfs overlay, following
the criteria established for custom skeletons and using the same script
uesd by skeleton-custom.mk.

Add a paragraph to the documentation clarifying that rootfs overlays
don't need to contain /bin, /lib or /sbin and must not contain them when
BR2_ROOTFS_MERGED_USR is enabled.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
Changes v1->v2:

- Rebase series to HEAD of master branch
- Rework commit message and documentation, as suggested by Thomas
  Petazzoni

Changes v2-v3:

- Move the removal of --keep-dirlinks in the call to rsync to a separate
  patch.
---
 Makefile                         | 16 ++++++++++++++++
 docs/manual/customize-rootfs.txt |  8 ++++++++
 2 files changed, 24 insertions(+)

diff --git a/Makefile b/Makefile
index c024c65f78..64c97ce6e2 100644
--- a/Makefile
+++ b/Makefile
@@ -746,6 +746,22 @@ 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) \
diff --git a/docs/manual/customize-rootfs.txt b/docs/manual/customize-rootfs.txt
index 9d3a62ddaf..bb6d8da6bf 100644
--- a/docs/manual/customize-rootfs.txt
+++ b/docs/manual/customize-rootfs.txt
@@ -22,6 +22,14 @@ 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.
 +
+Filesystem overlays don't need to contain the '/bin', '/lib' or '/sbin'
+  directories, since they are created automatically during the build.
+  When +BR2_ROOTFS_MERGED_USR+ is enabled, then the overlay 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 overlay have any programs or libraries,
+  they should be placed in '/usr/bin', '/usr/sbin' and '/usr/lib'.
++
 As shown in xref:customize-dir-structure[], the recommended path for
   this overlay is +board/<company>/<boardname>/rootfs-overlay+.
 
-- 
2.14.3

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

[Arnout Vandecappelle]

On 07/05/2018 16:44, Carlos Santos wrote:

> +# 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))

 This is really not OK, it should be done already before building, using the
approach similar to how it's done in skeleton-custom. It should then probably go
at the end of system/system.mk as well.

 I've applied anyway since it works :-)

 Oh, one more thing, both for this one and the skeleton check: we should also do
the check in case MERGED_USR is not selected. Indeed, we have 5 packages that
rely in the value of MERGED_USR, and that do the wrong thing if MERGED_USR is
not selected but the symlinks exist.

> +
> +endif # merged /usr
> +
>  	@$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
>  		$(call MESSAGE,"Copying overlay $(d)"); \
>  		rsync -a --ignore-times --keep-dirlinks $(RSYNC_VCS_EXCLUSIONS) \
> diff --git a/docs/manual/customize-rootfs.txt b/docs/manual/customize-rootfs.txt
> index 9d3a62ddaf..bb6d8da6bf 100644
> --- a/docs/manual/customize-rootfs.txt
> +++ b/docs/manual/customize-rootfs.txt
> @@ -22,6 +22,14 @@ 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.
>  +
> +Filesystem overlays don't need to contain the '/bin', '/lib' or '/sbin'
> +  directories, since they are created automatically during the build.

 This sentence doesn't make much sense for the overlay (I mean, why would you
put /bin in an overlay, unless there is some file in it? and in that case, the
following sentence applies and is more important.), so I've removed it.

 Regards,
 Arnout
> +  When +BR2_ROOTFS_MERGED_USR+ is enabled, then the overlay 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 overlay have any programs or libraries,
> +  they should be placed in '/usr/bin', '/usr/sbin' and '/usr/lib'.
> ++
>  As shown in xref:customize-dir-structure[], the recommended path for
>    this overlay is +board/<company>/<boardname>/rootfs-overlay+.
>  
> 

[Buildroot] [PATCH v3 4/5] Makefile: allow rootfs overlays to override symbolic links

[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

The --keep-dirlinks option can be dropped, since we run sanity checks
on overlays. Now the rsync invocation is identical to the SYSTEM_RSYNC
logic we have in system/system.mk, so use that variable.

Signed-off-by: Carlos Santos <casantos@datacom.ind.br>
---
Changes v1->v2:

- Rebase series to HEAD of master branch
- Rework commit message and documentation, as suggested by Thomas
  Petazzoni

Changes v2-v3:

- Move the removal of --keep-dirlinks in the call to rsync to a separate
  patch.
---
 Makefile | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 64c97ce6e2..dc51269143 100644
--- a/Makefile
+++ b/Makefile
@@ -764,9 +764,7 @@ 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)"); \
-- 
2.14.3

[Buildroot] [PATCH v3 5/5] 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>
---
Changes v1->v2:

- Rebase series to HEAD of master branch
---
 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.14.3

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

[Arnout Vandecappelle]

On 07/05/2018 16:44, Carlos Santos wrote:
> 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 4 allows rootfs overlays to override symbolic links on purpose (e.g.
> /var/log, to persist system logs).
> 
> Patch 5 removes the restriction of using merged /usr only with the default
> skeleton or when systemd is selected.
> 
> Carlos Santos (5):
>   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
>   Makefile: allow rootfs overlays to override symbolic links
>   system: allow selecting merged /usr along with custom rootfs skeleton


 Series applied to master, thanks.

 I still have a few individual comments on some of the patches.

 Regards,
 Arnout

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