fs: fix and better handle capabilities
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.
To fix that, we postpone handling of capabilities later, down into each filesystem commands, right after extracting the intermediate tarball.
Discussion about this at the developers days lead to the suggestion that, maybe, we should in fact not use an intermediate tarball, and instead have each filesystem duplicate the currently common actions.
This is a bigger endeavour, and one that needs more thinking into. In the meantime, this patchset is a pragmatic approach to solve the problem.
Regards,
Yann E. MORIN.
The following changes since commit cbf62fc5692cc04a2f721260d5e7f8a2558b4bb1
mysql: properly order "depends on" vs. bool (2018-10-26 21:28:31 +0200)
are available in the git repository at:
git://git.buildroot.org/~ymorin/git/buildroot.git
for you to fetch changes up to 99d38f609529976ec574c3a05b6665cf3dd0669d
fs: fix condition to create static devices (2018-10-27 09:43:55 +0200)
----------------------------------------------------------------
Yann E. MORIN (3):
fs: apply permissions late
fs: be oblivious of pre-existing xattrs
fs: fix condition to create static devices
fs/common.mk | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 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 1/3 fs: apply permissions late
The combination of fakeroot, tar, and capabilities is broken, because fakeroot currently badly handles capabilities, which are currently simply ignored.
As described in #11216, asking tar to explicitly store and restore capabilities ends up with a failling build, when tar actually tries to restore the capabilities. Adding support for capabilities to fakeroot (by adding host-libcap as dependency) does not fix the problem.
Capabilities are stored in the extended attribute security.capabilty. It turns out that tar does have special handling when extracting and restoring that extended attribute, and that fails miserably when running under fakeroot...
We fix that by offloading the permissions handling down to individual filesystems.
This needs a split of the makedevs call, with the current and first one now only responsible for creating the pseudo devices, while the new, second call does only set the permissions.
This changes the order of steps, and post-fakeroot scripts are now called before the permissions are set. This could mean breaking existing setups, but more probably, this woudl sovle some, where files created in post-fakeroot scripts can now see their permissions appropriately set.
This also slightly breaks the idea behind the intermediate image, which was supposed to gather all actions common to all filesystems, so that they are not repeated. Still, most actions are still created only once, and moving just this is purely a practical and pragmatic workaround.
--- fs/common.mk | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)
fs/common.mk +11 -6
@@ -29,8 +29,8 @@ 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)) +ROOTFS_PERMISSION_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) +ROOTFS_STATIC_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE)) USERS_TABLE = $(FS_DIR)/users_table.txt ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES))
@@ -81,14 +81,13 @@ 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) +ifneq ($(ROOTFS_STATIC_DEVICE_TABLES),) + cat $(ROOTFS_STATIC_DEVICE_TABLES) > $(FULL_DEVICE_TABLE) ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) $(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(FULL_DEVICE_TABLE) endif -endif - $(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(FULL_DEVICE_TABLE) echo "$(HOST_DIR)/bin/makedevs -d $(FULL_DEVICE_TABLE) $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT) +endif $(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))
@@ -108,6 +107,7 @@ define inner-rootfs ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1) ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target +ROOTFS_$(2)_PERMISSION_TABLE = $$(ROOTFS_$(2)_DIR)/permissions.txt ROOTFS_$(2)_DEPENDENCIES += rootfs-common
@@ -149,6 +149,11 @@ $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT) echo "set -e" >> $$(FAKEROOT_SCRIPT) $$(call PRINTF,$$(ROOTFS_COMMON_UNTAR_CMD)) >> $$(FAKEROOT_SCRIPT) +ifneq ($$(ROOTFS_PERMISSION_TABLES),) + cat $$(ROOTFS_PERMISSION_TABLES) > $$(ROOTFS_$(2)_PERMISSION_TABLE) +endif + $$(call PRINTF,$$(PACKAGES_PERMISSIONS_TABLE)) >> $$(ROOTFS_$(2)_PERMISSION_TABLE) + echo "$$(HOST_DIR)/bin/makedevs -d $$(ROOTFS_$(2)_PERMISSION_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) $$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),\ $$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep)) $$(call PRINTF,$$(ROOTFS_REPRODUCIBLE)) >> $$(FAKEROOT_SCRIPT)
Signature · 1 line
2.14.1
PATCH 1/3 fs: apply permissions late
Yann,
Quoted 7 lines · Yann E. MORIN
The combination of fakeroot, tar, and capabilities is broken, because fakeroot currently badly handles capabilities, which are currently simply ignored.
As described in #11216, asking tar to explicitly store and restore capabilities ends up with a failling build, when tar actually tries to
failling -> failing
Quoted 4 lines
restore the capabilities. Adding support for capabilities to fakeroot (by adding host-libcap as dependency) does not fix the problem.
Capabilities are stored in the extended attribute security.capabilty.
Capabilities are stored in the extended attribute security capability.
Quoted 11 lines
It turns out that tar does have special handling when extracting and restoring that extended attribute, and that fails miserably when running under fakeroot...
We fix that by offloading the permissions handling down to individual filesystems.
This needs a split of the makedevs call, with the current and first one now only responsible for creating the pseudo devices, while the new, second call does only set the permissions.
Quoted 5 lines
Fixes #11216This changes the order of steps, and post-fakeroot scripts are now called before the permissions are set. This could mean breaking existing setups, but more probably, this woudl sovle some, where files created in
setups, but more probably, this would solve some, where files created in
Quoted 11 lines
post-fakeroot scripts can now see their permissions appropriately set.
This also slightly breaks the idea behind the intermediate image, which was supposed to gather all actions common to all filesystems, so that they are not repeated. Still, most actions are still created only once, and moving just this is purely a practical and pragmatic workaround.
Signed-off-by "Yann E. MORIN" <[email protected]>Cc Ricardo Martincoski <[email protected]>Cc Thomas Petazzoni <[email protected]>Cc Matthew Weber <[email protected]>
Quoted 46 lines
--- fs/common.mk | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)fs/common.mk +6
-6@@ -29,8 +29,8 @@ 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)) +ROOTFS_PERMISSION_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) +ROOTFS_STATIC_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE)) USERS_TABLE = $(FS_DIR)/users_table.txt ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES))@@ -81,14 +81,13 @@ 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) +ifneq ($(ROOTFS_STATIC_DEVICE_TABLES),) + cat $(ROOTFS_STATIC_DEVICE_TABLES) > $(FULL_DEVICE_TABLE) ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) $(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(FULL_DEVICE_TABLE) endif -endif - $(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(FULL_DEVICE_TABLE) echo "$(HOST_DIR)/bin/makedevs -d $(FULL_DEVICE_TABLE) $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT) +endif $(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))@@ -108,6 +107,7 @@ define inner-rootfs ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1) ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target +ROOTFS_$(2)_PERMISSION_TABLE = $$(ROOTFS_$(2)_DIR)/permissions.txt ROOTFS_$(2)_DEPENDENCIES += rootfs-common
fs/common.mk $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) · line 155
Quoted 7 lines
fs/common.mk +4
-0@@ -149,6 +149,11 @@ $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT) echo "set -e" >> $$(FAKEROOT_SCRIPT) $$(call PRINTF,$$(ROOTFS_COMMON_UNTAR_CMD)) >> $$(FAKEROOT_SCRIPT) +ifneq ($$(ROOTFS_PERMISSION_TABLES),) + cat $$(ROOTFS_PERMISSION_TABLES) > $$(ROOTFS_$(2)_PERMISSION_TABLE) +endif + $$(call PRINTF,$$(PACKAGES_PERMISSIONS_TABLE)) >> $$(ROOTFS_$(2)_PERMISSION_TABLE)
If a package duplicates an entry and is below a user provided rootfs permissions table similar item, I assume makedev uses the last entry as the one to set? If so, should the two lines above be flipped so the "user provided" can always fixup/override the package default?
Matt
PATCH 1/3 fs: apply permissions late
Matt, All,
On 2018-10-27 08:14 -0500, Matthew Weber spake thusly:
Quoted 8 lines · Yann E. MORIN
Quoted 7 lines
The combination of fakeroot, tar, and capabilities is broken, because fakeroot currently badly handles capabilities, which are currently simply ignored.
As described in #11216, asking tar to explicitly store and restore capabilities ends up with a failling build, when tar actually tries to
Quoted 7 lines
failling -> failing
Quoted 4 lines
restore the capabilities. Adding support for capabilities to fakeroot (by adding host-libcap as dependency) does not fix the problem.
Capabilities are stored in the extended attribute security.capabilty.
Capabilities are stored in the extended attribute security capability.
Thanks for the fixes! :-)
Yet, the extended attribute is really named "security.capabilty" (i.e. with a dot in-between the two words): https://linux.die.net/man/7/capabilities
Since kernel 2.6.24, the kernel supports associating capability sets [...] stored in an extended attribute (see setxattr(2)) named security.capability.o
Regards,
Yann E. MORIN.
Quoted 92 lines
Quoted 11 lines
It turns out that tar does have special handling when extracting and restoring that extended attribute, and that fails miserably when running under fakeroot...
We fix that by offloading the permissions handling down to individual filesystems.
This needs a split of the makedevs call, with the current and first one now only responsible for creating the pseudo devices, while the new, second call does only set the permissions.
Quoted 5 lines
Fixes #11216This changes the order of steps, and post-fakeroot scripts are now called before the permissions are set. This could mean breaking existing setups, but more probably, this woudl sovle some, where files created in
setups, but more probably, this would solve some, where files created in
Quoted 11 lines
post-fakeroot scripts can now see their permissions appropriately set.
This also slightly breaks the idea behind the intermediate image, which was supposed to gather all actions common to all filesystems, so that they are not repeated. Still, most actions are still created only once, and moving just this is purely a practical and pragmatic workaround.
Signed-off-by "Yann E. MORIN" <[email protected]>Cc Ricardo Martincoski <[email protected]>Cc Thomas Petazzoni <[email protected]>Cc Matthew Weber <[email protected]>Reviewed-by Matthew Weber <[email protected]>Quoted 46 lines
--- fs/common.mk | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)fs/common.mk +6
-6@@ -29,8 +29,8 @@ 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)) +ROOTFS_PERMISSION_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) +ROOTFS_STATIC_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE)) USERS_TABLE = $(FS_DIR)/users_table.txt ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES))@@ -81,14 +81,13 @@ 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) +ifneq ($(ROOTFS_STATIC_DEVICE_TABLES),) + cat $(ROOTFS_STATIC_DEVICE_TABLES) > $(FULL_DEVICE_TABLE) ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) $(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(FULL_DEVICE_TABLE) endif -endif - $(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(FULL_DEVICE_TABLE) echo "$(HOST_DIR)/bin/makedevs -d $(FULL_DEVICE_TABLE) $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT) +endif $(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))@@ -108,6 +107,7 @@ define inner-rootfs ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1) ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target +ROOTFS_$(2)_PERMISSION_TABLE = $$(ROOTFS_$(2)_DIR)/permissions.txt ROOTFS_$(2)_DEPENDENCIES += rootfs-commonfs/common.mk $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) · line 155
Quoted 7 lines
fs/common.mk +4
-0@@ -149,6 +149,11 @@ $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT) echo "set -e" >> $$(FAKEROOT_SCRIPT) $$(call PRINTF,$$(ROOTFS_COMMON_UNTAR_CMD)) >> $$(FAKEROOT_SCRIPT) +ifneq ($$(ROOTFS_PERMISSION_TABLES),) + cat $$(ROOTFS_PERMISSION_TABLES) > $$(ROOTFS_$(2)_PERMISSION_TABLE) +endif + $$(call PRINTF,$$(PACKAGES_PERMISSIONS_TABLE)) >> $$(ROOTFS_$(2)_PERMISSION_TABLE)If a package duplicates an entry and is below a user provided rootfs permissions table similar item, I assume makedev uses the last entry as the one to set? If so, should the two lines above be flipped so the "user provided" can always fixup/override the package default?
Matt
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 1/3 fs: apply permissions late
Yann,
Quoted 20 lines · Yann E. MORIN
Matt, All,
On 2018-10-27 08:14 -0500, Matthew Weber spake thusly:
Quoted 8 lines · Yann E. MORIN
Quoted 7 lines
The combination of fakeroot, tar, and capabilities is broken, because fakeroot currently badly handles capabilities, which are currently simply ignored.
As described in #11216, asking tar to explicitly store and restore capabilities ends up with a failling build, when tar actually tries to
Quoted 7 lines
failling -> failing
Quoted 4 lines
restore the capabilities. Adding support for capabilities to fakeroot (by adding host-libcap as dependency) does not fix the problem.
Capabilities are stored in the extended attribute security.capabilty.
Capabilities are stored in the extended attribute security capability.
Quoted 8 lines
Thanks for the fixes! :-)
Yet, the extended attribute is really named "security.capabilty" (i.e. with a dot in-between the two words): https://linux.die.net/man/7/capabilities
Since kernel 2.6.24, the kernel supports associating capability sets [...] stored in an extended attribute (see setxattr(2)) named security.capability.o
:-) Fair, security.capabilty -> security.capability
Quoted 104 lines
Regards,
Yann E. MORIN.Quoted 91 lines
Quoted 11 lines
It turns out that tar does have special handling when extracting and restoring that extended attribute, and that fails miserably when running under fakeroot...
We fix that by offloading the permissions handling down to individual filesystems.
This needs a split of the makedevs call, with the current and first one now only responsible for creating the pseudo devices, while the new, second call does only set the permissions.
Quoted 5 lines
Fixes #11216This changes the order of steps, and post-fakeroot scripts are now called before the permissions are set. This could mean breaking existing setups, but more probably, this woudl sovle some, where files created in
setups, but more probably, this would solve some, where files created in
Quoted 11 lines
post-fakeroot scripts can now see their permissions appropriately set.
This also slightly breaks the idea behind the intermediate image, which was supposed to gather all actions common to all filesystems, so that they are not repeated. Still, most actions are still created only once, and moving just this is purely a practical and pragmatic workaround.
Signed-off-by "Yann E. MORIN" <[email protected]>Cc Ricardo Martincoski <[email protected]>Cc Thomas Petazzoni <[email protected]>Cc Matthew Weber <[email protected]>Reviewed-by Matthew Weber <[email protected]>Quoted 46 lines
--- fs/common.mk | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)fs/common.mk +6
-6@@ -29,8 +29,8 @@ 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)) +ROOTFS_PERMISSION_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) +ROOTFS_STATIC_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE)) USERS_TABLE = $(FS_DIR)/users_table.txt ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES))@@ -81,14 +81,13 @@ 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) +ifneq ($(ROOTFS_STATIC_DEVICE_TABLES),) + cat $(ROOTFS_STATIC_DEVICE_TABLES) > $(FULL_DEVICE_TABLE) ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) $(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(FULL_DEVICE_TABLE) endif -endif - $(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(FULL_DEVICE_TABLE) echo "$(HOST_DIR)/bin/makedevs -d $(FULL_DEVICE_TABLE) $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT) +endif $(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))@@ -108,6 +107,7 @@ define inner-rootfs ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1) ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target +ROOTFS_$(2)_PERMISSION_TABLE = $$(ROOTFS_$(2)_DIR)/permissions.txt ROOTFS_$(2)_DEPENDENCIES += rootfs-commonfs/common.mk $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) · line 155
Quoted 7 lines
fs/common.mk +4
-0@@ -149,6 +149,11 @@ $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT) echo "set -e" >> $$(FAKEROOT_SCRIPT) $$(call PRINTF,$$(ROOTFS_COMMON_UNTAR_CMD)) >> $$(FAKEROOT_SCRIPT) +ifneq ($$(ROOTFS_PERMISSION_TABLES),) + cat $$(ROOTFS_PERMISSION_TABLES) > $$(ROOTFS_$(2)_PERMISSION_TABLE) +endif + $$(call PRINTF,$$(PACKAGES_PERMISSIONS_TABLE)) >> $$(ROOTFS_$(2)_PERMISSION_TABLE)If a package duplicates an entry and is below a user provided rootfs permissions table similar item, I assume makedev uses the last entry as the one to set? If so, should the two lines above be flipped so the "user provided" can always fixup/override the package default?
Quoted 1 line
Matt
--
.-----------------.--------------------.------------------.--------------------.| 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. |'------------------------------^-------^------------------^--------------------'
Signature · 8 lines
Matthew L Weber / Pr Software Engineer Airborne Information Systems / RC Linux 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.
PATCH 1/3 fs: apply permissions late
Matt, All,
On 2018-10-30 20:18 -0500, Matthew Weber spake thusly:
Quoted 5 lines · Yann E. MORIN
Quoted 3 lines
Since kernel 2.6.24, the kernel supports associating capability sets [...] stored in an extended attribute (see setxattr(2)) named security.capability.o
:-) Fair, security.capabilty -> security.capability
Muahaha! Thanks :-)
Regards,
Yann E. MORIN.
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 1/3 fs: apply permissions late
Yann,
Quoted 10 lines · Yann E. MORIN
Matt, All,
On 2018-10-30 20:18 -0500, Matthew Weber spake thusly:
Quoted 5 lines · Yann E. MORIN
Quoted 3 lines
Since kernel 2.6.24, the kernel supports associating capability sets [...] stored in an extended attribute (see setxattr(2)) named security.capability.o
:-) Fair, security.capabilty -> security.capability
Ran some scenaios as a regression on existing target builds before and after the change.
PATCH 1/3 fs: apply permissions late
In reply to Yann E. MORINHello,
Quoted 3 lines · Yann E. MORIN
This needs a split of the makedevs call, with the current and first one now only responsible for creating the pseudo devices, while the new, second call does only set the permissions.
What happens if there are special permissions/extended attributes set on pseudo devices in the static device table ?
fs/common.mk line 31
Quoted 8 lines
fs/common.mk +0
-0@@ -29,8 +29,8 @@ FS_DIR = $(BUILD_DIR)/buildroot-fs FULL_DEVICE_TABLE = $(FS_DIR)/device_table.txt
I no longer like the name FULL_DEVICE_TABLE, nor device_table.txt, because it's no longer the "full device table".
Quoted 26 lines
fs/common.mk +5
-6-ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \ - $(BR2_ROOTFS_STATIC_DEVICE_TABLE)) +ROOTFS_PERMISSION_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) +ROOTFS_STATIC_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE)) USERS_TABLE = $(FS_DIR)/users_table.txt ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES))@@ -81,14 +81,13 @@ 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) +ifneq ($(ROOTFS_STATIC_DEVICE_TABLES),) + cat $(ROOTFS_STATIC_DEVICE_TABLES) > $(FULL_DEVICE_TABLE) ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) $(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(FULL_DEVICE_TABLE) endif -endif - $(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(FULL_DEVICE_TABLE) echo "$(HOST_DIR)/bin/makedevs -d $(FULL_DEVICE_TABLE) $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT) +endif $(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))
fs/common.mk define inner-rootfs · line 110
Quoted 4 lines
fs/common.mk +1
-0@@ -108,6 +107,7 @@ define inner-rootfs ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1) ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target +ROOTFS_$(2)_PERMISSION_TABLE = $$(ROOTFS_$(2)_DIR)/permissions.txt
There is no reason for this variable to be filesystem-specific, nor for this permissions.txt file to be generated for each filesystem format.
So I'd prefer to see something like this in fs/common.mk:
ROOTFS_FULL_STATIC_DEVICE_TABLE = $(FS_DIR)/full_static_device_table.txt ROOTFS_FULL_PERMISSION_TABLE = $(FS_DIR)/full_permission_table.txt
both are generated in fs/common.mk, but only ROOTFS_FULL_STATIC_DEVICE_TABLE is used in the makedevs call in fs/common.mk.
fs/common.mk $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) · line 155
Quoted 10 lines
fs/common.mk +4
-0ROOTFS_$(2)_DEPENDENCIES += rootfs-common@@ -149,6 +149,11 @@ $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT) echo "set -e" >> $$(FAKEROOT_SCRIPT) $$(call PRINTF,$$(ROOTFS_COMMON_UNTAR_CMD)) >> $$(FAKEROOT_SCRIPT) +ifneq ($$(ROOTFS_PERMISSION_TABLES),) + cat $$(ROOTFS_PERMISSION_TABLES) > $$(ROOTFS_$(2)_PERMISSION_TABLE) +endif + $$(call PRINTF,$$(PACKAGES_PERMISSIONS_TABLE)) >> $$(ROOTFS_$(2)_PERMISSION_TABLE)
i.e, those lines would migrate back into fs/common.mk.
fs/common.mk $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) · line 156
Quoted 1 line
fs/common.mk +1
-0+ echo "$$(HOST_DIR)/bin/makedevs -d $$(ROOTFS_$(2)_PERMISSION_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)
And this would use $(ROOTFS_FULL_PERMISSION_TABLE)
This would make the whole thing more consistent IMO. Of course unless we decide that pseudo devices in the static device table can have extended attributes, in which case, the makedevs for static devices anyway needs to be moved to the per-filesystem code.
Best regards,
Thomas
Signature · 3 lines
Thomas Petazzoni, CTO, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
PATCH 1/3 fs: apply permissions late
Thomas, All,
On 2018-11-03 14:38 +0100, Thomas Petazzoni spake thusly:
Quoted 8 lines · Yann E. MORIN
Quoted 4 lines
This needs a split of the makedevs call, with the current and first one now only responsible for creating the pseudo devices, while the new, second call does only set the permissions.
What happens if there are special permissions/extended attributes set on pseudo devices in the static device table ?
This is of no consequence, because that is not possible to do with our makedevs tool: it only ever supports setting capabilties; even though we claim xattr, it's really capabilties:
https://git.buildroot.org/buildroot/tree/package/makedevs/makedevs.c#n355
Capabilities are 100% useless on device nodes, because they are only used for read/write/ioctl. capabilties are used to give a process more permissions than it would otherwise have, i.e. capabilities are to be set on executables.
So it does not make sense to set capabilties on device nodes in /dev...
Quoted 11 lines
fs/common.mk line 31
Quoted 8 lines
fs/common.mk +0
-0@@ -29,8 +29,8 @@ FS_DIR = $(BUILD_DIR)/buildroot-fs FULL_DEVICE_TABLE = $(FS_DIR)/device_table.txtI no longer like the name
FULL_DEVICE_TABLE, nor device_table.txt, because it's no longer the "full device table".
Well, now at least, it only contains devices, and not permissions. And yes, it is the full device table now, because it does contain all the _devices_ that are to be created: those from the static table(S), and those from packages, and nothing more.
One could argue that the previous denomination was in fact wrong, and that it is now correct. ;-)
However, see below...
Quoted 34 lines
Quoted 26 lines
fs/common.mk +5
-6-ROOTFS_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE) \ - $(BR2_ROOTFS_STATIC_DEVICE_TABLE)) +ROOTFS_PERMISSION_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) +ROOTFS_STATIC_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE)) USERS_TABLE = $(FS_DIR)/users_table.txt ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES))@@ -81,14 +81,13 @@ 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) +ifneq ($(ROOTFS_STATIC_DEVICE_TABLES),) + cat $(ROOTFS_STATIC_DEVICE_TABLES) > $(FULL_DEVICE_TABLE) ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) $(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(FULL_DEVICE_TABLE) endif -endif - $(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(FULL_DEVICE_TABLE) echo "$(HOST_DIR)/bin/makedevs -d $(FULL_DEVICE_TABLE) $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT) +endif $(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))fs/common.mk define inner-rootfs · line 110
Quoted 4 lines
fs/common.mk +1
-0@@ -108,6 +107,7 @@ define inner-rootfs ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1) ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target +ROOTFS_$(2)_PERMISSION_TABLE = $$(ROOTFS_$(2)_DIR)/permissions.txtThere is no reason for this variable to be filesystem-specific, nor for this permissions.txt file to be generated for each filesystem format.
Quoted 8 lines
So I'd prefer to see something like this in
fs/common.mk:
ROOTFS_FULL_STATIC_DEVICE_TABLE= $(FS_DIR)/full_static_device_table.txtROOTFS_FULL_PERMISSION_TABLE= $(FS_DIR)/full_permission_table.txtboth are generated in
fs/common.mk, but onlyROOTFS_FULL_STATIC_DEVICE_TABLEis used in the makedevs call infs/common.mk.
ACK.
Quoted 13 lines
fs/common.mk $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) · line 155
Quoted 10 lines
fs/common.mk +4
-0ROOTFS_$(2)_DEPENDENCIES += rootfs-common@@ -149,6 +149,11 @@ $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT) echo "set -e" >> $$(FAKEROOT_SCRIPT) $$(call PRINTF,$$(ROOTFS_COMMON_UNTAR_CMD)) >> $$(FAKEROOT_SCRIPT) +ifneq ($$(ROOTFS_PERMISSION_TABLES),) + cat $$(ROOTFS_PERMISSION_TABLES) > $$(ROOTFS_$(2)_PERMISSION_TABLE) +endif + $$(call PRINTF,$$(PACKAGES_PERMISSIONS_TABLE)) >> $$(ROOTFS_$(2)_PERMISSION_TABLE)i.e, those lines would migrate back into
fs/common.mk.
Quoted 3 lines
fs/common.mk $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) · line 156
Quoted 1 line
fs/common.mk +1
-0+ echo "$$(HOST_DIR)/bin/makedevs -d $$(ROOTFS_$(2)_PERMISSION_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT)And this would use $(ROOTFS_FULL_PERMISSION_TABLE)
ACK
Quoted 4 lines
This would make the whole thing more consistent IMO. Of course unless we decide that pseudo devices in the static device table can have extended attributes, in which case, the makedevs for static devices anyway needs to be moved to the per-filesystem code.
As I explained above, I believe it does not make sense.
However, as we previously discussed, we maybe should drop the inter,mediate rootfs step, and revert to duplicating everything in each filesystem steps, each starting off by copying the origian target/ to their own copy and hack that.
I'm not sure which is the simplest to do to be ready for this release cycle, I'll be investigating this more by the end of the WE...
Regards,
Yann E. MORIN.
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 1/3 fs: apply permissions late
In reply to Yann E. MORINQuoted 3 lines · Yann E. MORIN
The combination of fakeroot, tar, and capabilities is broken, because fakeroot currently badly handles capabilities, which are currently simply ignored.
"simply ignored" can't be the case, otherwise it still wouldn't work after this
patch.
I *think* the real problem is that fakeroot forgets to wrap the setxattrat and
lsetxattrat syscalls, and these are the ones that are used by tar.
Quoted 4 lines
As described in #11216, asking tar to explicitly store and restore capabilities ends up with a failling build, when tar actually tries to restore the capabilities. Adding support for capabilities to fakeroot (by adding host-libcap as dependency) does not fix the problem.
Just to be clear: with this patch, the 'tar' filesystem does still work because
the creation of the tarball works OK, it's just the extraction that goes wrong, right?
Quoted 11 lines
Capabilities are stored in the extended attribute security.capabilty. It turns out that tar does have special handling when extracting and restoring that extended attribute, and that fails miserably when running under fakeroot...
We fix that by offloading the permissions handling down to individual filesystems.
This needs a split of the makedevs call, with the current and first one now only responsible for creating the pseudo devices, while the new, second call does only set the permissions.
Why? Is that just to reduce the impact on post-fakeroot scripts?
I'd rather move the entire makedevs call to the per-filesystem phase.
Quoted 7 lines
Fixes #11216This changes the order of steps, and post-fakeroot scripts are now called before the permissions are set. This could mean breaking existing setups, but more probably, this woudl sovle some, where files created in post-fakeroot scripts can now see their permissions appropriately set.
Since extracting xattrs doesn't work correctly, this is not true. And normally
in a post-fakeroot script, you would do any permission setting in the script itself and not use the permissions table. So I think this statement is utterly wrong. That said, I doubt that post-build scripts would be affected by the permissions tables anyway.
Quoted 4 lines
This also slightly breaks the idea behind the intermediate image, which was supposed to gather all actions common to all filesystems, so that they are not repeated. Still, most actions are still created only once, and moving just this is purely a practical and pragmatic workaround.
Still, I'd prefer to fix fakeroot :-)
Quoted 43 lines
Signed-off-by "Yann E. MORIN" <[email protected]>Cc Ricardo Martincoski <[email protected]>Cc Thomas Petazzoni <[email protected]>Cc Matthew Weber <[email protected]>--- fs/common.mk | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-)fs/common.mk +5
-6@@ -29,8 +29,8 @@ 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)) +ROOTFS_PERMISSION_TABLES = $(call qstrip,$(BR2_ROOTFS_DEVICE_TABLE)) +ROOTFS_STATIC_DEVICE_TABLES = $(call qstrip,$(BR2_ROOTFS_STATIC_DEVICE_TABLE)) USERS_TABLE = $(FS_DIR)/users_table.txt ROOTFS_USERS_TABLES = $(call qstrip,$(BR2_ROOTFS_USERS_TABLES))@@ -81,14 +81,13 @@ 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) +ifneq ($(ROOTFS_STATIC_DEVICE_TABLES),) + cat $(ROOTFS_STATIC_DEVICE_TABLES) > $(FULL_DEVICE_TABLE) ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) $(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(FULL_DEVICE_TABLE) endif -endif - $(call PRINTF,$(PACKAGES_PERMISSIONS_TABLE)) >> $(FULL_DEVICE_TABLE) echo "$(HOST_DIR)/bin/makedevs -d $(FULL_DEVICE_TABLE) $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT) +endif $(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))
fs/common.mk define inner-rootfs · line 110
Quoted 4 lines
fs/common.mk +1
-0@@ -108,6 +107,7 @@ define inner-rootfs ROOTFS_$(2)_DIR = $$(FS_DIR)/$(1) ROOTFS_$(2)_TARGET_DIR = $$(ROOTFS_$(2)_DIR)/target +ROOTFS_$(2)_PERMISSION_TABLE = $$(ROOTFS_$(2)_DIR)/permissions.txt
BTW I agree with Thomas that it's better to create this file only once and only
do the makedevs call in the per-filesystem stage.
Regards,
Arnout
Quoted 16 lines
fs/common.mk +5
-0ROOTFS_$(2)_DEPENDENCIES += rootfs-common@@ -149,6 +149,11 @@ $$(BINARIES_DIR)/rootfs.$(1): $$(ROOTFS_$(2)_DEPENDENCIES) echo '#!/bin/sh' > $$(FAKEROOT_SCRIPT) echo "set -e" >> $$(FAKEROOT_SCRIPT) $$(call PRINTF,$$(ROOTFS_COMMON_UNTAR_CMD)) >> $$(FAKEROOT_SCRIPT) +ifneq ($$(ROOTFS_PERMISSION_TABLES),) + cat $$(ROOTFS_PERMISSION_TABLES) > $$(ROOTFS_$(2)_PERMISSION_TABLE) +endif + $$(call PRINTF,$$(PACKAGES_PERMISSIONS_TABLE)) >> $$(ROOTFS_$(2)_PERMISSION_TABLE) + echo "$$(HOST_DIR)/bin/makedevs -d $$(ROOTFS_$(2)_PERMISSION_TABLE) $$(TARGET_DIR)" >> $$(FAKEROOT_SCRIPT) $$(foreach hook,$$(ROOTFS_$(2)_PRE_GEN_HOOKS),\ $$(call PRINTF,$$($$(hook))) >> $$(FAKEROOT_SCRIPT)$$(sep)) $$(call PRINTF,$$(ROOTFS_REPRODUCIBLE)) >> $$(FAKEROOT_SCRIPT)
PATCH 1/3 fs: apply permissions late
Quoted 2 lines · Arnout Vandecappelle
I *think* the real problem is that fakeroot forgets to wrap the setxattrat and
lsetxattrat syscalls, and these are the ones that are used by tar.
?So this was a stupid mistake - there is no setxattrat syscall, it's a function that is defined by tar itself.
?Peter's analysis looks a lot more accurate.
?Regards,
?Arnout
PATCH 1/3 fs: apply permissions late
Arnout, All,
On 2018-11-09 21:13 +0100, Arnout Vandecappelle spake thusly:
Quoted 6 lines · Arnout Vandecappelle
Quoted 2 lines
I *think* the real problem is that fakeroot forgets to wrap the setxattrat and
lsetxattrat syscalls, and these are the ones that are used by tar.?So this was a stupid mistake - there is no setxattrat syscall, it's a function that is defined by tar itself.
ACK, thanks for the feedback.
Quoted 1 line
?Peter's analysis looks a lot more accurate.
Except he was not using fakeroot, but sudo, which does work (as explained in BZ-11216).
Regards,
Yann E. MORIN.
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 1/3 fs: apply permissions late
In reply to Arnout VandecappelleArnout, All,
On 2018-11-08 00:43 +0100, Arnout Vandecappelle spake thusly:
Quoted 7 lines · Yann E. MORIN
Quoted 3 lines
The combination of fakeroot, tar, and capabilities is broken, because fakeroot currently badly handles capabilities, which are currently simply ignored.
"simply ignored" can't be the case, otherwise it still wouldn't work after this
patch.
No, because fakeroot really badly handles capabilties, as demonstrated in another mail in that thread. And yes, capabilties *are* currently simply ignored, and that's on purpose (not that changing would fix it anyway, and I'm not even totally sure this is related, and neither am I really sure to understand what it even really means), added in c994c3db1f7d (fakeroot: disable capabilities):
https://git.buildroot.org/buildroot/tree/package/fakeroot/fakeroot.mk#n12
# Force capabilities detection off
# For now these are process capabilities (faked) rather than file # so they're of no real use
HOST_FAKEROOT_CONF_ENV = \
ac_cv_header_sys_capability_h=no \
ac_cv_func_capset=no
[--SNIP--]
Quoted 8 lines
Quoted 4 lines
As described in #11216, asking tar to explicitly store and restore capabilities ends up with a failling build, when tar actually tries to restore the capabilities. Adding support for capabilities to fakeroot (by adding host-libcap as dependency) does not fix the problem.
Just to be clear: with this patch, the 'tar' filesystem does still work because
the creation of the tarball works OK, it's just the extraction that goes wrong, right?
Oh, you poinpoint an issue: the tar filesystem must be also modified to add --xattrs-include='*' otherwise it would not contain any xattrs, and even less so, any capabilty.
So, with this patch, the 'tar' filesystem is not fixed, but this is not a regression either, as it did not have capabilties, and it still does not.
Now, should we fix the tar filesystem, a tarball of the rootfs is supposed to be extracted by root, the real root, not a fake root, and as Peter demonstrated, there is no problem with capabilities in a tarball that is extracted by root.
Quoted 5 lines
Quoted 5 lines
Capabilities are stored in the extended attribute security.capabilty. It turns out that tar does have special handling when extracting and restoring that extended attribute, and that fails miserably when running under fakeroot...
Quoted 8 lines
Quoted 6 lines
We fix that by offloading the permissions handling down to individual filesystems.
This needs a split of the makedevs call, with the current and first one now only responsible for creating the pseudo devices, while the new, second call does only set the permissions.
Why? Is that just to reduce the impact on post-fakeroot scripts?
The idea of the common, intermediate tarball was to include all the common actions, to avoid repeating them for each filesystem.
Since the creating of device nodes in /dev is not impacted by the capabilties, I left that in the common part, and only moved the offending part (setting permissions, which includes capabilties) to the per-filesystem actions.
It had about nothing to do with the post-fakeroot scripts that I had thought about.
Quoted 1 line
I'd rather move the entire makedevs call to the per-filesystem phase.
I'm still not convinced, as this is not needed (capabilties don't make sense on device nodes), except if we were to entirely drop the intemediate tarball altogether.
Quoted 8 lines
Quoted 6 lines
Fixes #11216This changes the order of steps, and post-fakeroot scripts are now called before the permissions are set. This could mean breaking existing setups, but more probably, this woudl sovle some, where files created in post-fakeroot scripts can now see their permissions appropriately set.
Since extracting xattrs doesn't work correctly, this is not true.i
Sorry, re-reading this, I see where you choked on my explanations. The underlying reasoning I had, was that files created in a post-fakeroot scripts could have appropriate capabilties by providing a permissions table, which is _now_ applied after those files are created.
So, before this patch, capabilities created in a post-fakeroot scripts were not working, and there was no way to set capabilties on those files. With this patch, they are not working either, but there is a workaround by providing a permissions table.
Quoted 3 lines
And normally
in a post-fakeroot script, you would do any permission setting in the script itself and not use the permissions table.
Except that was not working so far, and nobody complained, so nobody was doing that anyway.
Quoted 2 lines
So I think this statement is utterly
wrong.
s/utterly// makes it easier to read, thanks. And yes, it was wrong, except it did not properly convey my thinking. I hope the above explanations make it a bit less wrong.
Quoted 9 lines
That said, I doubt that post-build scripts would be affected by the permissions tables anyway.
Quoted 4 lines
This also slightly breaks the idea behind the intermediate image, which was supposed to gather all actions common to all filesystems, so that they are not repeated. Still, most actions are still created only once, and moving just this is purely a practical and pragmatic workaround.
Still, I'd prefer to fix fakeroot :-)
Then, please be my guest! ;-) I already fixed something in fakeroot, and I dread working on that code base any more...
Regards,
Yann E. MORIN.
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 1/3 fs: apply permissions late
Quoted 3 lines · Yann E. MORIN
Arnout, All,
On 2018-11-08 00:43 +0100, Arnout Vandecappelle spake thusly:
[snip]
Quoted 4 lines
Quoted 1 line
Still, I'd prefer to fix fakeroot :-)
Then, please be my guest! ;-) I already fixed something in fakeroot, and I dread working on that code base any more...
Time to switch to pseudo? The .11-rc period is ideal for that, isn't it? :-)
Regards,
Arnout
PATCH 1/3 fs: apply permissions late
Arnout, All,
On 2018-11-11 17:02 +0100, Arnout Vandecappelle spake thusly:
Quoted 11 lines · Yann E. MORIN
Quoted 3 lines
Arnout, All,
On 2018-11-08 00:43 +0100, Arnout Vandecappelle spake thusly:
[snip]
Quoted 4 lines
Quoted 1 line
Still, I'd prefer to fix fakeroot :-)
Then, please be my guest! ;-) I already fixed something in fakeroot, and I dread working on that code base any more...
Time to switch to pseudo? The .11-rc period is ideal for that, isn't it? :-)
I already made that joke on IRC the other day! ;-)
2018-11-09 22:01 < y_morin> Maybe we will eventually have to switch
to an alternative (pseudo?) NOOOO!!!!!! :-]Regards,
Yann E. MORIN.
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 1/3 fs: apply permissions late
In reply to Arnout VandecappelleQuoted 5 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Arnout" == Arnout Vandecappelle <[email protected]> writes:
Quoted 2 lines · Yann E. MORIN
Arnout, All,
Quoted 8 lines
Quoted 1 line
On 2018-11-08 00:43 +0100, Arnout Vandecappelle spake thusly:
[snip]
Quoted 4 lines
Quoted 1 line
Still, I'd prefer to fix fakeroot :-)
Then, please be my guest! ;-) I already fixed something in fakeroot, and I dread working on that code base any more...
Time to switch to pseudo? The .11-rc period is ideal for that, isn't it? :-)
I see the smiley, but does pseudo correctly handle xattrs/capabilities?
Signature · 1 line
Bye, Peter Korsgaard
PATCH 1/3 fs: apply permissions late
In reply to Yann E. MORINQuoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Yann" == Yann E MORIN <[email protected]> writes:
Hi,
Quoted 7 lines
Quoted 3 lines
Just to be clear: with this patch, the 'tar' filesystem does still work because the creation of the tarball works OK, it's just the extraction that goes wrong, right?
Oh, you poinpoint an issue: the tar filesystem must be also modified to add --xattrs-include='*' otherwise it would not contain any xattrs, and even less so, any capabilty.
I believe my tests showed that --xattrs-include isn't needed when creating the tarball (but _IS_ needed when extracting), as long as --xattrs is passed.
Signature · 1 line
Bye, Peter Korsgaard
PATCH 1/3 fs: apply permissions late
On 2018-11-11 21:02 +0100, Peter Korsgaard spake thusly:
Quoted 8 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Yann" == Yann E MORIN <[email protected]> writes:
Oh, you poinpoint an issue: the tar filesystem must be also modified to add --xattrs-include='*' otherwise it would not contain any xattrs, and even less so, any capabilty.
I believe my tests showed that --xattrs-include isn't needed when creating the tarball (but _IS_ needed when extracting), as long as --xattrs is passed.
Right, but the tar filesystem does have to be fixed, anyway.
I don't like it that the extraction is not symetric to the creation, and since this has puzzled us (and a lot of other people), I'd prefer if we were explicit which xattrs we ionclude.
Regards,
Yann E. MORIN.
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 1/3 fs: apply permissions late
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:
The combination of fakeroot, tar, and capabilities is broken, because fakeroot currently badly handles capabilities, which are currently simply ignored.
As described in #11216, asking tar to explicitly store and restore capabilities ends up with a failling build, when tar actually tries to restore the capabilities. Adding support for capabilities to fakeroot (by adding host-libcap as dependency) does not fix the problem.
Quoted 4 lines
Capabilities are stored in the extended attribute security.capabilty. It turns out that tar does have special handling when extracting and restoring that extended attribute, and that fails miserably when running under fakeroot...
Hmm, playing a bit around with tar here (1.29, Debian) it looks like it doesn't actually extract the security.capability xattrs when --xattrs is used unless --xattrs-include='*.*' is used:
getcap /usr/bin/i3status/usr/bin/i3status = cap_net_admin+ep
sudo tar -cvvvf foo.tar /usr/bin/i3status
tar: Removing leading `/' from member names
-rwxr-xr-x root/root 84888 2017-01-21 15:42 /usr/bin/i3status
hexdump -C foo.tar | grep -A2 ecu
No security.capability in the .tar
sudo tar --xattrs -cvvvf foo.tar /usr/bin/i3status
tar: Removing leading `/' from member names
-rwxr-xr-x root/root 84888 2017-01-21 15:42 /usr/bin/i3status
hexdump -C foo.tar | grep -A2 ecu
00000240 43 48 49 4c 59 2e 78 61 74 74 72 2e 73 65 63 75 |CHILY.xattr.secu| 00000250 72 69 74 79 2e 63 61 70 61 62 69 6c 69 74 79 3d |rity.capability=| 00000260 01 00 00 02 00 10 00 00 00 00 00 00 00 00 00 00 |................|
With --xattrs they are included but not listed in the verbose output:
sudo tar --xattrs --xattrs-include='*.*' -cvvvf foo.tar /usr/bin/i3status
tar: Removing leading `/' from member names
-rwxr-xr-x* root/root 84888 2017-01-21 15:42 /usr/bin/i3status
x: 20 security.capability
hexdump -C foo.tar | grep -A2 ecu
00000240 43 48 49 4c 59 2e 78 61 74 74 72 2e 73 65 63 75 |CHILY.xattr.secu| 00000250 72 69 74 79 2e 63 61 70 61 62 69 6c 69 74 79 3d |rity.capability=| 00000260 01 00 00 02 00 10 00 00 00 00 00 00 00 00 00 00 |................|
Same .tar content but now also listed in the verbose output.
For extraction:
tar -xvvvf ../foo.tar && getcap usr/bin/i3status
-rwxr-xr-x root/root 84888 2017-01-21 15:42 usr/bin/i3status
No capability.
tar --xattrs -xvvvf ../foo.tar && getcap usr/bin/i3status
-rwxr-xr-x root/root 84888 2017-01-21 15:42 usr/bin/i3status
Still no capability.
tar --xattrs --xattrs-include='*.*' -xvvvf ../foo.tar && getcap usr/bin/i3status
-rwxr-xr-x* root/root 84888 2017-01-21 15:42 usr/bin/i3status
x: 20 security.capabilityusr/bin/i3status = cap_net_admin+ep
Now it works.
I don't see where we are passing --xattrs-include. Are we sure this is a fakeroot issue after all?
Signature · 1 line
Bye, Peter Korsgaard
PATCH 1/3 fs: apply permissions late
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Peter" == Peter Korsgaard <[email protected]> writes:
Hi,
Quoted 9 lines
tar --xattrs --xattrs-include='*.*' -xvvvf ../foo.tar && getcap
usr/bin/i3status
-rwxr-xr-x* root/root 84888 2017-01-21 15:42usr/bin/i3status
x: 20 security.capabilityusr/bin/i3status= cap_net_admin+epNow it works.
I don't see where we are passing --xattrs-include. Are we sure this is a fakeroot issue after all?
Digging further, this is apparently expected (if confusing) behaviour:
https://bugzilla.redhat.com/show_bug.cgi?id=771927#c21
Signature · 1 line
Bye, Peter Korsgaard
PATCH 1/3 fs: apply permissions late
In reply to Peter KorsgaardPeter, All,
On 2018-11-08 23:58 +0100, Peter Korsgaard spake thusly:
Quoted 16 lines
Quoted 11 lines
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Yann" == Yann E MORIN <[email protected]> writes:
The combination of fakeroot, tar, and capabilities is broken, because fakeroot currently badly handles capabilities, which are currently simply ignored.
As described in #11216, asking tar to explicitly store and restore capabilities ends up with a failling build, when tar actually tries to restore the capabilities. Adding support for capabilities to fakeroot (by adding host-libcap as dependency) does not fix the problem.
Quoted 4 lines
Capabilities are stored in the extended attribute security.capabilty. It turns out that tar does have special handling when extracting and restoring that extended attribute, and that fails miserably when running under fakeroot...
Quoted 8 lines
Hmm, playing a bit around with tar here (1.29, Debian) it looks like it doesn't actually extract the security.capability xattrs when --xattrs is used unless --xattrs-include='*.*' is used:
getcap
/usr/bin/i3status/usr/bin/i3status= cap_net_admin+epsudo tar -cvvvf foo.tar
/usr/bin/i3status
Sorry, but as discussed on IRC, your analysis is incorrect, because you are really root, by mean of sudo. The issue manifests itself only under fakeroot.
[...]
Quoted 1 line
I don't see where we are passing --xattrs-include.
We are not, and this is all the grief reported in BZ-11216.
Quoted 2 lines
Are we sure this is a
fakeroot issue after all?
So, here is a report of a bit more experimentations. 'master' is 6a5e9a7ac6. The configuration is from support/testing/tests/core/test_file_capabilities.py The tar that is used is the one from my system: tar (GNU tar) 1.29
master
no capability in rootfs
master
tar with --xattrs-include='*'
no capability in rootfs
master
untar with --xattrs-include='*'
no capability in rootfs
master
tar with --xattrs-include='*'
untar with --xattrs-include='*'
File [...]/usr/sbin/getcap has unrecognised filetype 0, ignoring /usr/sbin/getcap is missing from rootfs
master
fakeroot with libcap
no capability in rootfs
master
fakeroot with libcap
tar with --xattrs-include='*'
untar with --xattrs-include='*'
File [...]/usr/sbin/getcap has unrecognised filetype 0, ignoring /usr/sbin/getcap is missing from rootfs
To be noted: the rootfs in that config is a squashfs. mksquashfs simply ignores the unknown filetype, so getcap is missing in the filesystem. However, should one switch to using ext4 for the test, then mkfs.ext4 would choke on that unknown filetype, and error out, which we would catch (same behaviour whether fakeroot is linked with libcap or not):
Copying files into the device: __populate_fs: ignoring entry "getcap"
getcap: File not found by ext2_lookup while looking up "getcap"
mkfs.ext4: File not found by ext2_lookup while populating file system
*** Maybe you need to increase the filesystem size (BR2_TARGET_ROOTFS_EXT2_SIZE) fs/ext2/ext2.mk:46: recipe for target '/home/ymorin/dev/buildroot/O/images/rootfs.ext2' failed make[1]: *** [/home/ymorin/dev/buildroot/O/images/rootfs.ext2] Error 1 Makefile:23: recipe for target '_all' failed
make: *** [_all] Error 2
Note that only the first three lines are messages from mkfs.ext4. It also fails for ext2 and ext3, btw, and possibly some other filesystems.
Regards,
Yann E. MORIN.
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 2/3 fs: be oblivious of pre-existing xattrs
In reply to Yann E. MORINNow that we handle permissions in each filesystems, and no longer in the intermediate tarball, we no longer need tar to store extended attributes.
What's more, we really want it to store no attributes at all, to be sure we don't have rogue extended attributes leaking in...
--- fs/common.mk | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
fs/common.mk +4 -1
@@ -45,6 +45,7 @@ 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 \ + --no-xattrs \ --exclude=$(notdir $(TARGET_DIR_WARNING_FILE)) \ -C $(TARGET_DIR) . endef
@@ -52,7 +53,9 @@ 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) + tar xf $(ROOTFS_COMMON_TAR) \ + --no-xattrs \ + -C $(TARGET_DIR) endef .PHONY: rootfs-common
Signature · 1 line
2.14.1
PATCH 2/3 fs: be oblivious of pre-existing xattrs
Yann,
Quoted 11 lines · Yann E. MORIN
Now that we handle permissions in each filesystems, and no longer in the intermediate tarball, we no longer need tar to store extended attributes.
What's more, we really want it to store no attributes at all, to be sure we don't have rogue extended attributes leaking in...
Signed-off-by "Yann E. MORIN" <[email protected]>Cc Ricardo Martincoski <[email protected]>Cc Matthew Weber <[email protected]>Cc Thomas Petazzoni <[email protected]>
Ran some scenaios as a regression on existing target builds before and after the change. I did not use a SElinux enabled system. I'll keep that in mind as a scenario that probably needs checked.
Quoted 30 lines
--- fs/common.mk | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)fs/common.mk +4
-2@@ -45,6 +45,7 @@ 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 \ + --no-xattrs \ --exclude=$(notdir $(TARGET_DIR_WARNING_FILE)) \ -C $(TARGET_DIR) . endef@@ -52,7 +53,9 @@ 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) + tar xf $(ROOTFS_COMMON_TAR) \ + --no-xattrs \ + -C $(TARGET_DIR) endef .PHONY: rootfs-common --2.14.1
PATCH 3/3 fs: fix condition to create static devices
In reply to Yann E. MORINCurrently, we parse the static devices tables even when we're not using static device nodes; this is a left over from when we mixed devices tables and permissions tables together. Creating package-defined device nodes is guarded by the condition that there is at elast one devices table.
This means that, when using static device nodes, but no device table, we would miss on the package-defined device nodes.
We fix that by inverting the condition: static device tables are now parsed only when using static devices, but the package-defined device nodes are no longer conditioned to there being at least one devices table.
Note: when there is no device table, the package-defined device ndoes are appended to the file. That file can't exist from a previous build, because the whole directory is rmoved at the beginning of the rule.
--- fs/common.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
fs/common.mk +2 -2
@@ -84,11 +84,11 @@ ifneq ($(ROOTFS_USERS_TABLES),) cat $(ROOTFS_USERS_TABLES) >> $(USERS_TABLE) endif PATH=$(BR_PATH) $(TOPDIR)/support/scripts/mkusers $(USERS_TABLE) $(TARGET_DIR) >> $(FAKEROOT_SCRIPT) +ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) ifneq ($(ROOTFS_STATIC_DEVICE_TABLES),) cat $(ROOTFS_STATIC_DEVICE_TABLES) > $(FULL_DEVICE_TABLE) -ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) +endif $(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(FULL_DEVICE_TABLE) -endif echo "$(HOST_DIR)/bin/makedevs -d $(FULL_DEVICE_TABLE) $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT) endif $(foreach s,$(call qstrip,$(BR2_ROOTFS_POST_FAKEROOT_SCRIPT)),\
Signature · 1 line
2.14.1
PATCH 3/3 fs: fix condition to create static devices
Yann,
Quoted 5 lines · Yann E. MORIN
Currently, we parse the static devices tables even when we're not using static device nodes; this is a left over from when we mixed devices tables and permissions tables together. Creating package-defined device nodes is guarded by the condition that there is at elast one devices
elast -> least
Quoted 11 lines
table.
This means that, when using static device nodes, but no device table, we would miss on the package-defined device nodes.
We fix that by inverting the condition: static device tables are now parsed only when using static devices, but the package-defined device nodes are no longer conditioned to there being at least one devices table.
Note: when there is no device table, the package-defined device ndoes
ndoes -> nodes
Quoted 2 lines
are appended to the file. That file can't exist from a previous build, because the whole directory is rmoved at the beginning of the rule.
rmoved -> removed
Quoted 3 lines
Signed-off-by "Yann E. MORIN" <[email protected]>Cc Thomas Petazzoni <[email protected]>
Quoted 23 lines
--- fs/common.mk | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)fs/common.mk +2
-3@@ -84,11 +84,11 @@ ifneq ($(ROOTFS_USERS_TABLES),) cat $(ROOTFS_USERS_TABLES) >> $(USERS_TABLE) endif PATH=$(BR_PATH) $(TOPDIR)/support/scripts/mkusers $(USERS_TABLE) $(TARGET_DIR) >> $(FAKEROOT_SCRIPT) +ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) ifneq ($(ROOTFS_STATIC_DEVICE_TABLES),) cat $(ROOTFS_STATIC_DEVICE_TABLES) > $(FULL_DEVICE_TABLE) -ifeq ($(BR2_ROOTFS_DEVICE_CREATION_STATIC),y) +endif $(call PRINTF,$(PACKAGES_DEVICES_TABLE)) >> $(FULL_DEVICE_TABLE) -endif echo "$(HOST_DIR)/bin/makedevs -d $(FULL_DEVICE_TABLE) $(TARGET_DIR)" >> $(FAKEROOT_SCRIPT) endif $(foreach s,$(call qstrip,$(BR2_ROOTFS_POST_FAKEROOT_SCRIPT)),\ --