fs/common: ROOTFS_USERS_TABLES has priority

PATCH New Matt Weber 3 participants 3 messages Sep 27, 2018 – Oct 2, 2018 VFS

Share & tools

Matt Weber

author

2018-09-27 14:36 UTC #0 lore raw

PATCH 1/1 fs/common: ROOTFS_USERS_TABLES has priority

This patch moves the PACKAGES_USERS to be appended to the USERS_TABLES before ROOTFS_USERS_TABLES. This allows the ROOTFS_USERS_TABLES to update the existing users created by packages with different settings like home directory, pid, gid, etc.

Signed-off-by Matthew Weber <[email protected]>

---
 fs/common.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

fs/common.mk +2 -1

index abf3541..453da60 100644
@@ -75,10 +75,11 @@ $(ROOTFS_COMMON_TAR): $(ROOTFS_COMMON_DEPENDENCIES) target-finalize
 	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
-	$(call PRINTF,$(PACKAGES_USERS)) >> $(USERS_TABLE)
 	PATH=$(BR_PATH) $(TOPDIR)/support/scripts/mkusers $(USERS_TABLE) $(TARGET_DIR) >> $(FAKEROOT_SCRIPT)
 ifneq ($(ROOTFS_DEVICE_TABLES),)
 	cat $(ROOTFS_DEVICE_TABLES) > $(FULL_DEVICE_TABLE)

Signature · 1 line

1.9.1

Yann E. MORIN

2018-10-02 19:26 UTC #1 lore raw

PATCH 1/1 fs/common: ROOTFS_USERS_TABLES has priority

Matt, All,

On 2018-09-27 09:36 -0500, Matt Weber spake thusly:

Quoted 4 lines

This patch moves the PACKAGES_USERS to be appended to the USERS_TABLES before ROOTFS_USERS_TABLES. This allows the ROOTFS_USERS_TABLES to update the existing users created by packages with different settings like home directory, pid, gid, etc.

So, I do understand the rationale behind this, and I was almost ready to ack the patch, when I thought of a potential trap this could open under the unsuspectign user's feet.

So, let's consider a hypotetical package that:

- defines a user, say 'foo', with home set to e.g. /run/foo

- installs a daemon to run under user 'foo'

- installs a config file that contains (e.g.) chroot_dir=/run/foo

Then your custom users table overrides the home dir to (e.g.) /home/foo.

You better be prepared to also override the config file with your own, and so on (e.g. maybe the startup script too...)

I don't think it is too bad, though, so I'm OK with the change.

I would however suggest a rephrasing of the commit log, something along the lines of:

fs/common: allow custom user table to override package-defined users

Currently, when a custom user table and a package define the same user, the settings from the package takes precedence over the ones from the custom user table.

Hwever, it makes sense to allow the settings from the custom user table take precedence. For example, it woould allow redirecting the user's home directory to an alternate location (e.g. away from tmp and into a partition that is persistent).

The support/scripts/mkusers script will only retain settings from the latest definition it finds.

Thus, by passing the custom user table after the package defined users, it is possible to override the packageprovided user definitions.

With that commit log updated:

Acked-by "Yann E. MORIN" <[email protected]>

Regards,
Yann E. MORIN.

Quoted 29 lines

Signed-off-by Matthew Weber <[email protected]>

---
 fs/common.mk | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

fs/common.mk +2 -1

index abf3541..453da60 100644
@@ -75,10 +75,11 @@ $(ROOTFS_COMMON_TAR): $(ROOTFS_COMMON_DEPENDENCIES) target-finalize
 	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
-	$(call PRINTF,$(PACKAGES_USERS)) >> $(USERS_TABLE)
 	PATH=$(BR_PATH) $(TOPDIR)/support/scripts/mkusers $(USERS_TABLE) $(TARGET_DIR) >> $(FAKEROOT_SCRIPT)
 ifneq ($(ROOTFS_DEVICE_TABLES),)
 	cat $(ROOTFS_DEVICE_TABLES) > $(FULL_DEVICE_TABLE)

Signature · 6 lines

1.9.1

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

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.  |
'------------------------------^-------^------------------^--------------------'

Matthew Weber

2018-10-02 23:03 UTC #2 lore raw

PATCH 1/1 fs/common: ROOTFS_USERS_TABLES has priority

Yann, All,

Quoted 42 lines · Yann E. MORIN

Matt, All,
On 2018-09-27 09:36 -0500, Matt Weber spake thusly:
Quoted 4 lines
This patch moves the PACKAGES_USERS to be appended to the USERS_TABLES before ROOTFS_USERS_TABLES. This allows the ROOTFS_USERS_TABLES to update the existing users created by packages with different settings like home directory, pid, gid, etc.
So, I do understand the rationale behind this, and I was almost ready to ack the patch, when I thought of a potential trap this could open under the unsuspectign user's feet.
So, let's consider a hypotetical package that:
- defines a user, say 'foo', with home set to e.g. /run/foo
- installs a daemon to run under user 'foo'
- installs a config file that contains (e.g.) chroot_dir=/run/foo
Then your custom users table overrides the home dir to (e.g.) /home/foo.
You better be prepared to also override the config file with your own, and so on (e.g. maybe the startup script too...)
I don't think it is too bad, though, so I'm OK with the change.
I would however suggest a rephrasing of the commit log, something along the lines of:
fs/common: allow custom user table to override package-defined users
Currently, when a custom user table and a package define the same user, the settings from the package takes precedence over the ones from the custom user table.
Hwever, it makes sense to allow the settings from the custom user table take precedence. For example, it woould allow redirecting the user's home directory to an alternate location (e.g. away from tmp and into a partition that is persistent).

Quoted 8 lines

The support/scripts/mkusers script will only retain settings from the latest definition it finds.
Thus, by passing the custom user table after the package defined users, it is possible to override the packageprovided user definitions.
With that commit log updated:

v2 - https://patchwork.ozlabs.org/patch/978067/

Matt