Improve verification of custom rootfs skeletons and overlays

PATCH Superseded Carlos Santos 2 participants 9 messages May 6, 2018 – May 7, 2018 Kbuild

Share & tools

Carlos Santos

author

2018-05-06 21:31 UTC #0 lore raw

cover Improve verification of custom rootfs skeletons and overlays

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

Signature · 1 line

2.17.0

Carlos Santos

author

2018-05-06 21:31 UTC #1 lore raw

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

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 <[email protected]>

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

package/skeleton-custom/skeleton-custom.mk +1 ~~-22~~

index 8c57531782..b1cddd9146 100644
@@ -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

support/scripts/check-merged-usr.sh new file +39 -0

new file mode 100755
index 0000000000..74c43c89fd
@@ -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# }"

Signature · 1 line

2.17.0

Carlos Santos

author

2018-05-06 21:31 UTC #2 lore raw

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

In reply to 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 <[email protected]>

---
 docs/manual/customize-rootfs.txt           | 5 +++++
 package/skeleton-custom/skeleton-custom.mk | 2 ++
 2 files changed, 7 insertions(+)

docs/manual/customize-rootfs.txt +5 -0

index 44fc460670..8982e03498 100644
@@ -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.

package/skeleton-custom/skeleton-custom.mk +2 -0

index b1cddd9146..01cd62794d 100644
@@ -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

Signature · 1 line

2.17.0

Thomas Petazzoni

2018-05-07 07:08 UTC #3 lore raw

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

Hello Carlos,

Quoted 4 lines · 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.

maye -> may

Quoted 8 lines

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

Quoted 6 lines

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

Signature · 3 lines

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

Carlos Santos

author

2018-05-07 11:16 UTC #4 lore raw

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

Quoted 9 lines

From: "Thomas Petazzoni" <[email protected]>
To: "Carlos Santos" <[email protected]>
Cc: "buildroot" <[email protected]>, "yann morin 1998" <[email protected]>, "Thomas De Schampheleire" <[email protected]>
Sent: Monday, May 7, 2018 4:08:11 AM
Subject: Re: [Buildroot] [PATCH 2/4] skeleton-custom: install /bin, /lib, and /sbin
Hello Carlos,

Quoted 7 lines

Quoted 4 lines · 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.
maye -> may

:-)

Quoted 21 lines

Quoted 9 lines
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
Quoted 6 lines
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.

Quoted 4 lines

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.

Signature · 4 lines

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

Thomas Petazzoni

2018-05-07 11:28 UTC #5 lore raw

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

Hello Carlos,

Quoted 8 lines · Carlos Santos

Quoted 4 lines
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

Signature · 3 lines

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

Carlos Santos

author

2018-05-06 21:31 UTC #6 lore raw

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

In reply to 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 <[email protected]>

---
 Makefile                         | 20 +++++++++++++++++---
 docs/manual/customize-rootfs.txt |  5 +++++
 2 files changed, 22 insertions(+), 3 deletions(-)

Makefile +17 -3

index c024c65f78..dc51269143 100644
@@ -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)"); \

docs/manual/customize-rootfs.txt +5 -0

index 8982e03498..5dbbe86044 100644
@@ -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+.

Signature · 1 line

2.17.0

Thomas Petazzoni

2018-05-07 07:13 UTC #7 lore raw

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

Hello,

Quoted 5 lines · Carlos Santos

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

Signature · 3 lines

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

Carlos Santos

author

2018-05-06 21:31 UTC #8 lore raw

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

In reply to 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 <[email protected]>

---
 system/Config.in | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

system/Config.in +2 -6

index d14a864ca5..911cd424ba 100644
@@ -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

Signature · 1 line

2.17.0