fs: fix and better handle capabilities
v2 cover fs: fix and better handle capabilities
Hello All!
As reported by Ricardo in #11216, and recently noticed thanks to the runtime tests he added, handling file capabilities is broken.
Ricardo did a very good job at pinpointing the issue, and that is caused by the recent-ish split of the filesystem infra with the use of the intermediate tarball.
It turns out that playing with fakeroot, tar, and capabilities is a lost game, as fakeroot behaves badly with the special handling tar does with the security.capability extended attribute.
This series fixes the issue with the first patch, in which we simply get rid of the intermediate tarball, and directly rsync from the global target/ directory to populate the per-filesystem target directory. This means that any common fincalisation under fakaeroot is no longer possible, so they have to be replicated for each filesystem instead.
The only remaining common part is now the generation of the various
tables: devices, permisions and users.
Then, the tar filesystem is fixed to store the xattrs.
Then there are tow clean-up patches, which pave the way to two fixes.
First, we allow permissions provided by users to override the internal permissions, as we do for all user-provided settings. Second, we fix the static device creation conditions.
Regards,
Yann E. MORIN.
The following changes since commit cd0ca09e43be8bd87fa35c96fa099a338b85de36
{linux, linux-headers}: bump 4.{4, 9, 14, 18}.x series (2018-11-11 22:11:04 +0100)
are available in the git repository at:
git://git.buildroot.org/~ymorin/git/buildroot.git
for you to fetch changes up to ae80646fbb9d0e7f40434b16ebe5173ee3a4654c
fs: make static device nodes creation more logical (2018-11-12 18:28:28 +0100)
----------------------------------------------------------------
Yann E. MORIN (6):
fs: get over the intermediate tarball
fs/tar: add support for xattrs (thus capabilties)
fs: rename internal variable
fs: split devices and permissions tables
fs: allow user provided permissions to override packages permissions
fs: make static device nodes creation more logical
fs/common.mk | 92 +++++++++++-------------- fs/tar/tar.mk | 2 +- support/testing/tests/core/test_post_scripts.py | 4 +- 3 files changed, 44 insertions(+), 54 deletions(-)
Signature · 6 lines
.-----------------.--------------------.------------------.--------------------. | Yann E. MORIN | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: | | +33 662 376 056 | Software Designer | \ / CAMPAIGN | ___ | | +33 223 225 172 `------------.-------: X AGAINST | \e/ There is no | | http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL | v conspiracy. | '------------------------------^-------^------------------^--------------------'
PATCH v2 1/6 fs: get over the intermediate tarball
Since 118534fe54b (fs: use a common tarball as base for the other filesystems), the filesystem creation is split in two steps, using an intermediate tarball to carry the generic, common finalisations to the per-filesystem finalisation and image creation.
However, this intermediate tarball causes an issue with capabilities, which are entirely missing in the generated filesystems.
Capabilties are stored in the extended attribute security.capabilti, which tar by default will not store/restore, unless explicitly told to, e.g. with --xattrs-include='*', which we don't pass.
Now, passing this option when creating and extracting the intermediate tarball, both done under fakeroot, will cause fakeroot to report an invalid filetype for files with capabilities. mksquashfs would report such unknown files as a warning, while mkfs.ext2 would fail (with a similar error message), e.g.:
File [...]/usr/sbin/getcap has unrecognised filetype 0, ignoring
This is due to a poor interaction between tar and fakeroot; running as root the exact same commands we run under fakeroot, works as expected. Unfortunately, short of fixing fakeroot (which would first require understanding the problem in there), we don't have much options.
The intermediate tarball was made to avoid redoing the same actions over and over again for each filesystem to build. However, most of the time, only one or two such filesystems would be enabled [0], and those actions are usually pretty lightweight. So, using an intermediate tarball does not provide a big optimisation.
What is interesting in the intermediate tarball, however, is that it allowed to postpone per-filesystem finalisations to be applied only for the corresponding filesystem, not for all of them.
So, we get rid of the intermediate tarball, and simply move all of the code to run under fakeroot to the per-filesystem fakeroot script. Instead of extracting the intermediate tarball, we just rsync the original target/ directory, and apply the filesystem finalisations on that copy.
Note: an alternate solution would have been to keep the intermediate tarball to keep most of the common finalisations, and move only the permissions to each filesystem, but that was getting a bit more complex and changed the ordering of permissions and post-fakeroot scripts. Once we bite the bullet of having some common finalisation done in each filesystem, so move all of them.
[0] Most probsably, users would enable the real filesystem to put on their device, plus the 'tar' filesystem, to be able to easily inspect the content on their development machine.
---
Changes v1 -> v2:
- move all the code to the per-fs steps, drop the intermediate tarball
--- fs/common.mk | 59 ++++++++----------------- support/testing/tests/core/test_post_scripts.py | 4 +- 2 files changed, 20 insertions(+), 43 deletions(-)
fs/common.mk +18 -41
@@ -40,48 +40,21 @@ define ROOTFS_REPRODUCIBLE endef endif -ROOTFS_COMMON_TAR = $(FS_DIR)/rootfs.common.tar - -# Command to create the common tarball from the base target directory. -define ROOTFS_COMMON_TAR_CMD - tar cf $(ROOTFS_COMMON_TAR) --numeric-owner \ - --exclude=$(notdir $(TARGET_DIR_WARNING_FILE)) \ - -C $(TARGET_DIR) . -endef - -# Command to extract the common tarball into the per-rootfs target directory -define ROOTFS_COMMON_UNTAR_CMD - mkdir -p $(TARGET_DIR) - tar xf $(ROOTFS_COMMON_TAR) -C $(TARGET_DIR) -endef - -.PHONY: rootfs-common -rootfs-common: $(ROOTFS_COMMON_TAR) - -# Emulate being in a filesystem, so that we can have our own TARGET_DIR. -ROOTFS_COMMON_TARGET_DIR = $(FS_DIR)/target - ROOTFS_COMMON_DEPENDENCIES = \ host-fakeroot host-makedevs \ $(BR2_TAR_HOST_DEPENDENCY) \ $(if $(PACKAGES_USERS)$(ROOTFS_USERS_TABLES),host-mkpasswd) -$(ROOTFS_COMMON_TAR): ROOTFS=COMMON -$(ROOTFS_COMMON_TAR): FAKEROOT_SCRIPT=$(FS_DIR)/fakeroot.fs -$(ROOTFS_COMMON_TAR): $(ROOTFS_COMMON_DEPENDENCIES) target-finalize - @$(call MESSAGE,"Generating common rootfs tarball") +.PHONY: rootfs-common +rootfs-common: $(ROOTFS_COMMON_DEPENDENCIES) target-finalize + @$(call MESSAGE,"Generating root filesystems common tables") rm -rf $(FS_DIR) mkdir -p $(FS_DIR) - rsync -auH $(BASE_TARGET_DIR)/ $(TARGET_DIR) - echo '#!/bin/sh' > $(FAKEROOT_SCRIPT) - echo "set -e" >> $(FAKEROOT_SCRIPT) - echo "chown -h -R 0:0 $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT) $(call PRINTF,$(PACKAGES_USERS)) >> $(USERS_TABLE) ifneq ($(ROOTFS_USERS_TABLES),) cat $(ROOTFS_USERS_TABLES) >> $(USERS_TABLE) endif - PATH=$(BR_PATH) $(TOPDIR)/support/scripts/mkusers $(USERS_TABLE) $(TARGET_DIR) >> $(FAKEROOT_SCRIPT) ifneq ($(ROOTFS_DEVICE_TABLES),) cat $(ROOTFS_DEVICE_TABLES) > $(FULL_DEVICE_TABLE) ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y)
@@ -89,16 +62,6 @@ ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) endif endif $(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(FULL_DEVICE_TABLE) - echo "$(HOST_DIR)/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); \ - echo $(EXTRA_ENV) $(s) $(TARGET_DIR) $(BR2_ROOTFS_POST_SCRIPT_ARGS) >> $(FAKEROOT_SCRIPT)$(sep)) - $(foreach hook,$(ROOTFS_PRE_CMD_HOOKS),\ - $(call PRINTF,$($(hook))) >> $(FAKEROOT_SCRIPT)$(sep)) - $(call PRINTF,$(ROOTFS_COMMON_TAR_CMD)) >> $(FAKEROOT_SCRIPT) - chmod a+x $(FAKEROOT_SCRIPT) - PATH=$(BR_PATH) $(HOST_DIR)/bin/fakeroot -- $(FAKEROOT_SCRIPT) - $(Q)rm -rf $(TARGET_DIR) rootfs-common-show-depends: @echo $(ROOTFS_COMMON_DEPENDENCIES)
@@ -147,9 +110,23 @@ $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) @$$(call MESSAGE,"Generating root filesystem image rootfs.$(1)") rm -rf $$(ROOTFS_$(2)_DIR) mkdir -p $$(ROOTFS_$(2)_DIR) + rsync -auH \ + --exclude=/$$(notdir $$(TARGET_DIR_WARNING_FILE)) \ + $$(BASE_TARGET_DIR)/ \ + $$(TARGET_DIR) + echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT) echo "set -e" >> $$(FAKEROOT_SCRIPT) - $$(call PRINTF,$$(ROOTFS_COMMON_UNTAR_CMD)) >> $$(FAKEROOT_SCRIPT) + + echo "chown -h -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) + PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT) + echo "$$(HOST_DIR)/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); \ + echo $$(EXTRA_ENV) $$(s) $$(TARGET_DIR) $$(BR2_ROOTFS_POST_SCRIPT_ARGS) >> $$(FAKEROOT_SCRIPT)$$(sep)) + $$(foreach hook,$$(ROOTFS_PRE_CMD_HOOKS),\ + $$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep)) + $$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),\ $$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep)) $$(call PRINTF,$$(ROOTFS_REPRODUCIBLE)) >> $$(FAKEROOT_SCRIPT)
support/testing/tests/core/test_post_scripts.py +2 -2
@@ -41,8 +41,8 @@ class TestPostScripts(infra.basetest.BRTest): os.path.join(self.builddir, "target"), os.path.join(self.builddir, "target")) self.check_post_log_file("post-fakeroot.log", - os.path.join(self.builddir, "build/buildroot-fs/target"), - os.path.join(self.builddir, "build/buildroot-fs/target")) + os.path.join(self.builddir, "build/buildroot-fs/tar/target"), + os.path.join(self.builddir, "build/buildroot-fs/tar/target")) self.check_post_log_file("post-image.log", os.path.join(self.builddir, "images"), os.path.join(self.builddir, "target"))
Signature · 1 line
2.14.1
PATCH v2 1/6 fs: get over the intermediate tarball
Quoted 35 lines · Yann E. MORIN
Since
118534fe54b(fs: use a common tarball as base for the other filesystems), the filesystem creation is split in two steps, using an intermediate tarball to carry the generic, common finalisations to the per-filesystem finalisation and image creation.However, this intermediate tarball causes an issue with capabilities, which are entirely missing in the generated filesystems.
Capabilties are stored in the extended attribute security.capabilti, which tar by default will not store/restore, unless explicitly told to, e.g. with --xattrs-include='*', which we don't pass.
Now, passing this option when creating and extracting the intermediate tarball, both done under fakeroot, will cause fakeroot to report an invalid filetype for files with capabilities. mksquashfs would report such unknown files as a warning, while mkfs.ext2 would fail (with a similar error message), e.g.:
File [...]/usr/sbin/getcap has unrecognised filetype 0, ignoring
This is due to a poor interaction between tar and fakeroot; running as root the exact same commands we run under fakeroot, works as expected. Unfortunately, short of fixing fakeroot (which would first require understanding the problem in there), we don't have much options.
The intermediate tarball was made to avoid redoing the same actions over and over again for each filesystem to build. However, most of the time, only one or two such filesystems would be enabled [0], and those actions are usually pretty lightweight. So, using an intermediate tarball does not provide a big optimisation.
What is interesting in the intermediate tarball, however, is that it allowed to postpone per-filesystem finalisations to be applied only for the corresponding filesystem, not for all of them.
Quoted 7 lines
So, we get rid of the intermediate tarball, and simply move all of the code to run under fakeroot to the per-filesystem fakeroot script. Instead of extracting the intermediate tarball, we just rsync the original target/ directory, and apply the filesystem finalisations on that copy.
Fixes #11216
A full URL is more convenient to find it back. So I changed that, as well as
some spelling fixes, and applied to master, thanks.
Regards,
Arnout
Quoted 19 lines
Note: an alternate solution would have been to keep the intermediate tarball to keep most of the common finalisations, and move only the permissions to each filesystem, but that was getting a bit more complex and changed the ordering of permissions and post-fakeroot scripts. Once we bite the bullet of having some common finalisation done in each filesystem, so move all of them.
[0] Most probsably, users would enable the real filesystem to put on their device, plus the 'tar' filesystem, to be able to easily inspect the content on their development machine.
Reported-by Ricardo Martincoski <[email protected]>Signed-off-by "Yann E. MORIN" <[email protected]>Cc Ricardo Martincoski <[email protected]>Cc Thomas Petazzoni <[email protected]>Cc Arnout Vandecappelle <[email protected]>Cc Peter Korsgaard <[email protected]>fs/common.mk +0
-0
PATCH v2 2/6 fs/tar: add support for xattrs (thus capabilties)
In reply to Yann E. MORINBy default, tar will not include any extended attribute (xattr) when creating archives, and thus will not store capabilties either (as they are stored in the xattr 'security.capability').
Using option --xattrs is enough to create a tarball with all the xattrs attached to a file. However, extracting all xattrs from a tarball requires that --xattrs-include='*' be used. This is not symetric (but on purpose, as per the documentation), and so is confusing to some.
So, we use --xattrs-include='*' to create the archive, so as to be explicit that we want all xattrs to be stored.
--- fs/tar/tar.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/tar/tar.mk +1 -1
@@ -10,7 +10,7 @@ ROOTFS_TAR_DEPENDENCIES = $(BR2_TAR_HOST_DEPENDENCY) define ROOTFS_TAR_CMD (cd $(TARGET_DIR); find -print0 | LC_ALL=C sort -z | \ - tar $(TAR_OPTS) -cf $@ --null --no-recursion -T - --numeric-owner) + tar $(TAR_OPTS) -cf $@ --null --xattrs-include='*' --no-recursion -T - --numeric-owner) endef $(eval $(rootfs))
Signature · 1 line
2.14.1
PATCH v2 2/6 fs/tar: add support for xattrs (thus capabilties)
Quoted 3 lines · Yann E. MORIN
By default, tar will not include any extended attribute (xattr) when creating archives, and thus will not store capabilties either (as they are stored in the xattr 'security.capability').
So... currently capabilities are not included at all in the tarball, since
--xattrs is not passed at the moment?
But after this patch, they are still not included? Or does --xattrs-include
imply --xattrs?
Quoted 22 lines
Using option --xattrs is enough to create a tarball with all the xattrs attached to a file. However, extracting all xattrs from a tarball requires that --xattrs-include='*' be used. This is not symetric (but on purpose, as per the documentation), and so is confusing to some.
So, we use --xattrs-include='*' to create the archive, so as to be explicit that we want all xattrs to be stored.
Signed-off-by "Yann E. MORIN" <[email protected]>Cc Ricardo Martincoski <[email protected]>Cc Peter Korsgaard <[email protected]>Cc Arnout Vandecappelle <[email protected]>--- fs/tar/tar.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)fs/tar/tar.mk +0
-0
fs/tar/tar.mk ROOTFS_TAR_DEPENDENCIES = $(BR2_TAR_HOST_DEPENDENCY) · line 13
Quoted 5 lines
fs/tar/tar.mk +1
-1@@ -10,7 +10,7 @@ ROOTFS_TAR_DEPENDENCIES = $(BR2_TAR_HOST_DEPENDENCY) define ROOTFS_TAR_CMD (cd $(TARGET_DIR); find -print0 | LC_ALL=C sort -z | \ - tar $(TAR_OPTS) -cf $@ --null --no-recursion -T - --numeric-owner) + tar $(TAR_OPTS) -cf $@ --null --xattrs-include='*' --no-recursion -T - --numeric-owner)
I think --xattrs-include should come before TAR_OPTS, so it can still be
overridden. Ah, darn, never mind, it can't be overridden, several --xattrs-include options are cumulative and their order doesn't matter.
Regards,
Arnout
Quoted 4 lines
fs/tar/tar.mk +0
-0endef $(eval $(rootfs))
PATCH v2 2/6 fs/tar: add support for xattrs (thus capabilties)
Quoted 10 lines · Arnout Vandecappelle
Quoted 3 lines · Yann E. MORIN
By default, tar will not include any extended attribute (xattr) when creating archives, and thus will not store capabilties either (as they are stored in the xattr 'security.capability').
So... currently capabilities are not included at all in the tarball, since
--xattrs is not passed at the moment?But after this patch, they are still not included? Or does --xattrs-include
imply --xattrs?
?Tested and confirmed: --xattrs-include implies --xattrs. At least in my tar 1.29. So
?Regards,
?Arnout
Quoted 38 lines
Quoted 22 lines
Using option --xattrs is enough to create a tarball with all the xattrs attached to a file. However, extracting all xattrs from a tarball requires that --xattrs-include='*' be used. This is not symetric (but on purpose, as per the documentation), and so is confusing to some.
So, we use --xattrs-include='*' to create the archive, so as to be explicit that we want all xattrs to be stored.
Signed-off-by "Yann E. MORIN" <[email protected]>Cc Ricardo Martincoski <[email protected]>Cc Peter Korsgaard <[email protected]>Cc Arnout Vandecappelle <[email protected]>--- fs/tar/tar.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)fs/tar/tar.mk +0
-0fs/tar/tar.mk ROOTFS_TAR_DEPENDENCIES = $(BR2_TAR_HOST_DEPENDENCY) · line 13
Quoted 5 lines
fs/tar/tar.mk +1
-1@@ -10,7 +10,7 @@ ROOTFS_TAR_DEPENDENCIES = $(BR2_TAR_HOST_DEPENDENCY) define ROOTFS_TAR_CMD (cd $(TARGET_DIR); find -print0 | LC_ALL=C sort -z | \ - tar $(TAR_OPTS) -cf $@ --null --no-recursion -T - --numeric-owner) + tar $(TAR_OPTS) -cf $@ --null --xattrs-include='*' --no-recursion -T - --numeric-owner)I think --xattrs-include should come before
TAR_OPTS, so it can still be
overridden. Ah, darn, never mind, it can't be overridden, several --xattrs-include options are cumulative and their order doesn't matter.Regards,
ArnoutQuoted 4 lines
fs/tar/tar.mk +0
-0endef $(eval $(rootfs))
PATCH v2 2/6 fs/tar: add support for xattrs (thus capabilties)
In reply to Yann E. MORINQuoted 11 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Yann" == Yann E MORIN <[email protected]> writes:
By default, tar will not include any extended attribute (xattr) when creating archives, and thus will not store capabilties either (as they are stored in the xattr 'security.capability').
Using option --xattrs is enough to create a tarball with all the xattrs attached to a file. However, extracting all xattrs from a tarball requires that --xattrs-include='*' be used. This is not symetric (but on purpose, as per the documentation), and so is confusing to some.
Quoted 7 lines
So, we use --xattrs-include='*' to create the archive, so as to be explicit that we want all xattrs to be stored.
Signed-off-by "Yann E. MORIN" <[email protected]>Cc Ricardo Martincoski <[email protected]>Cc Peter Korsgaard <[email protected]>Cc Arnout Vandecappelle <[email protected]>
Committed, thanks.
Signature · 1 line
Bye, Peter Korsgaard
PATCH v2 3/6 fs: rename internal variable
In reply to Yann E. MORINIn preparation of more renames, rename the variable that points to the final users table.
--- fs/common.mk | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
fs/common.mk +6 -4
@@ -31,9 +31,11 @@ FS_DIR = $(BUILD_DIR)/buildroot-fs FULL_DEVICE_TABLE = $(FS_DIR)/device_table.txt ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \ $(BR2_ROOTFS_STATIC_DEVICE_TABLE)) -USERS_TABLE = $(FS_DIR)/users_table.txt + ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES)) +ROOTFS_FINAL_USERS_TABLE = $(FS_DIR)/full_users_table.txt + ifeq ($(BR2_REPRODUCIBLE),y) define ROOTFS_REPRODUCIBLE find $(TARGET_DIR) -print0 | xargs -0 -r touch -hd @$(SOURCE_DATE_EPOCH)
@@ -51,9 +53,9 @@ rootfs-common: $(ROOTFS_COMMON_DEPENDENCIES) target-finalize rm -rf $(FS_DIR) mkdir -p $(FS_DIR) - $(call PRINTF,$(PACKAGES_USERS)) >> $(USERS_TABLE) + $(call PRINTF,$(PACKAGES_USERS)) >> $(ROOTFS_FINAL_USERS_TABLE) ifneq ($(ROOTFS_USERS_TABLES),) - cat $(ROOTFS_USERS_TABLES) >> $(USERS_TABLE) + cat $(ROOTFS_USERS_TABLES) >> $(ROOTFS_FINAL_USERS_TABLE) endif ifneq ($(ROOTFS_DEVICE_TABLES),) cat $(ROOTFS_DEVICE_TABLES) > $(FULL_DEVICE_TABLE)
@@ -119,7 +121,7 @@ $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) echo "set -e" >> $$(FAKEROOT_SCRIPT) echo "chown -h -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) - PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT) + PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(ROOTFS_FINAL_USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT) echo "$$(HOST_DIR)/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); \
Signature · 1 line
2.14.1
PATCH v2 3/6 fs: rename internal variable
Quoted 13 lines · Yann E. MORIN
In preparation of more renames, rename the variable that points to the final users table.
Signed-off-by "Yann E. MORIN" <[email protected]>--- fs/common.mk | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)fs/common.mk +0
-0
fs/common.mk FS_DIR = $(BUILD_DIR)/buildroot-fs · line 37
Quoted 8 lines
fs/common.mk +2
-1@@ -31,9 +31,11 @@ FS_DIR = $(BUILD_DIR)/buildroot-fs FULL_DEVICE_TABLE = $(FS_DIR)/device_table.txt ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \ $(BR2_ROOTFS_STATIC_DEVICE_TABLE)) -USERS_TABLE = $(FS_DIR)/users_table.txt + ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES)) +ROOTFS_FINAL_USERS_TABLE = $(FS_DIR)/full_users_table.txt
Nitpicking: make sure the file name is the same as the variable name. Either
name is OK for me, with a light preference for full rather than final.
With that:
Regards,
Arnout
Quoted 26 lines
fs/common.mk +4
-3+ ifeq ($(BR2_REPRODUCIBLE),y) define ROOTFS_REPRODUCIBLE find $(TARGET_DIR) -print0 | xargs -0 -r touch -hd @$(SOURCE_DATE_EPOCH)@@ -51,9 +53,9 @@ rootfs-common: $(ROOTFS_COMMON_DEPENDENCIES) target-finalize rm -rf $(FS_DIR) mkdir -p $(FS_DIR) - $(call PRINTF,$(PACKAGES_USERS)) >> $(USERS_TABLE) + $(call PRINTF,$(PACKAGES_USERS)) >> $(ROOTFS_FINAL_USERS_TABLE) ifneq ($(ROOTFS_USERS_TABLES),) - cat $(ROOTFS_USERS_TABLES) >> $(USERS_TABLE) + cat $(ROOTFS_USERS_TABLES) >> $(ROOTFS_FINAL_USERS_TABLE) endif ifneq ($(ROOTFS_DEVICE_TABLES),) cat $(ROOTFS_DEVICE_TABLES) > $(FULL_DEVICE_TABLE)@@ -119,7 +121,7 @@ $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) echo "set -e" >> $$(FAKEROOT_SCRIPT) echo "chown -h -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) - PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT) + PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(ROOTFS_FINAL_USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT) echo "$$(HOST_DIR)/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); \
Thomas Petazzoni
AppliedPATCH v2 3/6 fs: rename internal variable
In reply to Yann E. MORINHello,
Quoted 7 lines · Yann E. MORIN
In preparation of more renames, rename the variable that points to the final users table.
Signed-off-by "Yann E. MORIN" <[email protected]>--- fs/common.mk | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
Applied to master after changing the name to ROOTFS_FULL_USERS_TABLE as suggested by Arnout.
Thanks!
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
PATCH v2 4/6 fs: split devices and permissions tables
In reply to Yann E. MORINCurrently, we conflate device tables and permissions tables, on the premise they are both created and applied with the same tool, makedevs.
Split the two, in to their own final aggregated tables and their own call to makedevs.
--- fs/common.mk | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)
fs/common.mk +19 -8
@@ -28,13 +28,16 @@ # macro will automatically generate a compressed filesystem image. FS_DIR = $(BUILD_DIR)/buildroot-fs -FULL_DEVICE_TABLE = $(FS_DIR)/device_table.txt -ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \ - $(BR2_ROOTFS_STATIC_DEVICE_TABLE)) +# BR2_ROOTFS_STATIC_DEVICE_TABLE is really for /dev nodes +# BR2_ROOTFS_DEVICE_TABLE is only for permissions (legacy name...) ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES)) +ROOTFS_STATIC_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE)) +ROOTFS_PERMISSIONS_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) ROOTFS_FINAL_USERS_TABLE = $(FS_DIR)/full_users_table.txt +ROOTFS_FINAL_STATIC_DEVICES_TABLE = $(FS_DIR)/full_static_devices_table.txt +ROOTFS_FINAL_PERMISSIONS_TABLE = $(FS_DIR)/full_permissions_table.txt ifeq ($(BR2_REPRODUCIBLE),y) define ROOTFS_REPRODUCIBLE
@@ -57,13 +60,20 @@ rootfs-common: $(ROOTFS_COMMON_DEPENDENCIES) target-finalize ifneq ($(ROOTFS_USERS_TABLES),) cat $(ROOTFS_USERS_TABLES) >> $(ROOTFS_FINAL_USERS_TABLE) endif -ifneq ($(ROOTFS_DEVICE_TABLES),) - cat $(ROOTFS_DEVICE_TABLES) > $(FULL_DEVICE_TABLE) + + $(Q)touch $(ROOTFS_FINAL_STATIC_DEVICES_TABLE) +ifneq ($(ROOTFS_STATIC_DEVICE_TABLES),) + cat $(ROOTFS_STATIC_DEVICE_TABLES) >> $(ROOTFS_FINAL_STATIC_DEVICES_TABLE) ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) - $(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(FULL_DEVICE_TABLE) + $(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(ROOTFS_FINAL_STATIC_DEVICES_TABLE) endif endif - $(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(FULL_DEVICE_TABLE) + +ifneq ($(ROOTFS_PERMISSIONS_TABLES),) + cat $(ROOTFS_PERMISSIONS_TABLES) >>$(ROOTFS_FINAL_PERMISSIONS_TABLE) +endif + $(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(ROOTFS_FINAL_PERMISSIONS_TABLE) + rootfs-common-show-depends: @echo $(ROOTFS_COMMON_DEPENDENCIES)
@@ -122,7 +132,8 @@ $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) echo "chown -h -R 0:0 $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) PATH=$$(BR_PATH) $$(TOPDIR)/support/scripts/mkusers $$(ROOTFS_FINAL_USERS_TABLE) $$(TARGET_DIR) >> $$(FAKEROOT_SCRIPT) - echo "$$(HOST_DIR)/bin/makedevs -d $$(FULL_DEVICE_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) + echo "$$(HOST_DIR)/bin/makedevs -d $$(ROOTFS_FINAL_STATIC_DEVICES_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) + echo "$$(HOST_DIR)/bin/makedevs -d $$(ROOTFS_FINAL_PERMISSIONS_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); \ echo $$(EXTRA_ENV) $$(s) $$(TARGET_DIR) $$(BR2_ROOTFS_POST_SCRIPT_ARGS) >> $$(FAKEROOT_SCRIPT)$$(sep))
Signature · 1 line
2.14.1
PATCH v2 4/6 fs: split devices and permissions tables
Quoted 5 lines · Yann E. MORIN
Currently, we conflate device tables and permissions tables, on the premise they are both created and applied with the same tool, makedevs.
Split the two, in to their own final aggregated tables and their own call to makedevs.
Sorry, I don't think there is a good reason to split them up. I don't think
this enables any other improvements either - the following two patches don't depend on this, right? I also don't feel that this change makes the code easier to understand (not more difficult either, but if it ain't broken, don't fix it).
So nack from me.
Regards,
Arnout
Quoted 7 lines
Signed-off-by "Yann E. MORIN" <[email protected]>Cc Thomas Petazzoni <[email protected]>Cc Arnout Vandecappelle <[email protected]>--- fs/common.mk | 27 +++++++++++++++++++-------- 1 file changed, 19 insertions(+), 8 deletions(-)
[snip]
PATCH v2 5/6 fs: allow user provided permissions to override packages permissions
In reply to Yann E. MORIN--- fs/common.mk | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/common.mk +1 -1
@@ -69,10 +69,10 @@ ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) endif endif + $(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(ROOTFS_FINAL_PERMISSIONS_TABLE) ifneq ($(ROOTFS_PERMISSIONS_TABLES),) cat $(ROOTFS_PERMISSIONS_TABLES) >>$(ROOTFS_FINAL_PERMISSIONS_TABLE) endif - $(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(ROOTFS_FINAL_PERMISSIONS_TABLE) rootfs-common-show-depends:
Signature · 1 line
2.14.1
PATCH v2 6/6 fs: make static device nodes creation more logical
In reply to Yann E. MORINCurrently, the creation of the static device nodes is under a weird (but correct) condition: it depends on whether a statice device table is defined or not. Since that option already depend on static /dev management, that is coorrect, even if not totally obvious at first sight.
Yet, the creation of the per-package static devices is conditional to using a static /dev management, which is already implied by the enclosing condition, as explained above.
However, there is still a gotcha: if doing static /dev management, but using no static device table [0], then the per-package static devices are not created.
Fix that by inverting the logic, with the outter condition being on using a static /dev management, conditionng per-package devices to that, and with the inner condition on static device tables only conditioning the use of said device tables.
--- fs/common.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
fs/common.mk +2 -2
@@ -62,10 +62,10 @@ ifneq ($(ROOTFS_USERS_TABLES),) endif $(Q)touch $(ROOTFS_FINAL_STATIC_DEVICES_TABLE) -ifneq ($(ROOTFS_STATIC_DEVICE_TABLES),) - cat $(ROOTFS_STATIC_DEVICE_TABLES) >> $(ROOTFS_FINAL_STATIC_DEVICES_TABLE) ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) $(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(ROOTFS_FINAL_STATIC_DEVICES_TABLE) +ifneq ($(ROOTFS_STATIC_DEVICE_TABLES),) + cat $(ROOTFS_STATIC_DEVICE_TABLES) >> $(ROOTFS_FINAL_STATIC_DEVICES_TABLE) endif endif
Signature · 1 line
2.14.1
PATCH v2 6/6 fs: make static device nodes creation more logical
Hello,
fs/common.mk ifneq ($(ROOTFS_USERS_TABLES),) · line 70
Quoted 9 lines · Yann E. MORIN
fs/common.mk +2
-2$(Q)touch $(ROOTFS_FINAL_STATIC_DEVICES_TABLE) -ifneq ($(ROOTFS_STATIC_DEVICE_TABLES),) - cat $(ROOTFS_STATIC_DEVICE_TABLES) >> $(ROOTFS_FINAL_STATIC_DEVICES_TABLE) ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) $(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(ROOTFS_FINAL_STATIC_DEVICES_TABLE) +ifneq ($(ROOTFS_STATIC_DEVICE_TABLES),) + cat $(ROOTFS_STATIC_DEVICE_TABLES) >> $(ROOTFS_FINAL_STATIC_DEVICES_TABLE) endif endif
Ah, I thought this change was not correct. But in fact, it was correct in your series (sorry!). It was correct because you did split the permission table from the static device table, which was done in the patch that Arnout didn't like.
In fact, there is a way to do the same without the split of permission/device table. Let me propose something.
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
v2 cover fs: fix and better handle capabilities
In reply to Yann E. MORINQuoted 13 lines · Yann E. MORIN
Hello All!
As reported by Ricardo in #11216, and recently noticed thanks to the runtime tests he added, handling file capabilities is broken.
Ricardo did a very good job at pinpointing the issue, and that is caused by the recent-ish split of the filesystem infra with the use of the intermediate tarball.
It turns out that playing with fakeroot, tar, and capabilities is a lost game, as fakeroot behaves badly with the special handling tar does with the security.capability extended attribute.
Quoted 8 lines
This series fixes the issue with the first patch, in which we simply get rid of the intermediate tarball, and directly rsync from the global target/ directory to populate the per-filesystem target directory. This means that any common fincalisation under fakaeroot is no longer possible, so they have to be replicated for each filesystem instead.
The only remaining common part is now the generation of the various
tables: devices, permisions and users.
Just to be clear: this is the only patch for master, right? The rest is for next?
Quoted 2 lines
Then, the tar filesystem is fixed to store the xattrs.
Like this one, tar has never contained xattrs, right?
Regards,
Arnout
Quoted 40 lines
Then there are tow clean-up patches, which pave the way to two fixes.
First, we allow permissions provided by users to override the internal permissions, as we do for all user-provided settings. Second, we fix the static device creation conditions.
Regards,
Yann E. MORIN.The following changes since commit
cd0ca09e43be8bd87fa35c96fa099a338b85de36{linux, linux-headers}: bump 4.{4, 9, 14, 18}.x series (2018-11-11 22:11:04 +0100)
are available in the git repository at:
git://git.buildroot.org/~ymorin/git/buildroot.gitfor you to fetch changes up to
ae80646fbb9d0e7f40434b16ebe5173ee3a4654cfs: make static device nodes creation more logical (2018-11-12 18:28:28 +0100)
----------------------------------------------------------------
Yann E. MORIN (6):
fs: get over the intermediate tarball
fs/tar: add support for xattrs (thus capabilties)
fs: rename internal variable
fs: split devices and permissions tables
fs: allow user provided permissions to override packages permissions
fs: make static device nodes creation more logicalfs/common.mk | 92 +++++++++++-------------- fs/tar/tar.mk | 2 +- support/testing/tests/core/test_post_scripts.py | 4 +- 3 files changed, 44 insertions(+), 54 deletions(-)