From: Arnout Vandecappelle <arnout@mind.be>
To: buildroot@busybox.net
Subject: [Buildroot] [RFC/PATCH] Makefile: guarantee reproducible permissions
Date: Mon, 03 Nov 2014 23:23:53 +0100 [thread overview]
Message-ID: <54580079.2070603@mind.be> (raw)
In-Reply-To: <1415018556-28336-2-git-send-email-guido@vanguardiasur.com.ar>
On 03/11/14 13:42, Guido Mart?nez wrote:
> Currently, the permission mode on many target rootfs files depend on the
> user's umask at the time of building, and at the time of cloning the
> repo. This is caused by two things:
>
> 1) Some packages and BR itself create files and directories on the
> target with cp/mkdir/etc which depend on the umask at the time of
> building.
>
> 2) We use rsync -a to copy the skeleton and overlay, leaving permissions
> on the target exactly as they were on the host. These permissions are
> not tracked by Git and depend on the user's umask at the time of cloning
> (assuming no mode changes).
>
> To fix (1), change the Makefile's $(SHELL) to always call a wrapper
> script first that sets the umask to a sane fixed value (022) and then
> calls the real shell.
>
> To fix (2), use the --chmod option on rsync calls so we don't depend on
> the current mode of those files.
>
> Signed-off-by: Guido Mart?nez <guido@vanguardiasur.com.ar>
I build-tested with and without the patch, compared the permissions and they're
identical.
I was also a bit concerned about the effect on build time of the additional
fork, but the difference turns out to be smaller than the noise margin.
However, you did miss a few rsync/cp instances that could be problematic.
- The external toolchain uses a combination of rsync and cp to copy the sysroot
to the target. Normally not an issue, unless when you download and extract the
external toolchain yourself and use BR2_TOOLCHAIN_EXTERNAL_PATH.
- The OVERRIDE_SRCDIR infrastructure in general may show the same problem if
stuff is then cp'ed into the target. However, that one we probably shouldn't
bother with too much. Still, adding a chmod to the rsync in the package
infrastructure is a possibility.
I think these two should be fixed before accepting the patch.
Also, there are probably a couple of packages that use cp to copy stuff from
the buildroot directory to the target. A quick search only turns up
matchbox-keyboard, but there may be more. So perhaps you could do an
allpackageyes build and see if anything turns up in the target directory that is
not world-readable. But even without that, the patch is acceptable after just
fixing the external toolchain and OVERRIDE_SRCDIR support.
Regards,
Arnout
> ---
> Makefile | 7 ++++---
> support/scripts/shell-wrapper.sh | 5 +++++
> 2 files changed, 9 insertions(+), 3 deletions(-)
> create mode 100755 support/scripts/shell-wrapper.sh
>
> diff --git a/Makefile b/Makefile
> index 907a0fc..0f7db1e 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -203,7 +203,8 @@ else
> endif
>
> # we want bash as shell
> -SHELL := $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
> +SHELL := $(TOPDIR)/support/scripts/shell-wrapper.sh \
> + $(shell if [ -x "$$BASH" ]; then echo $$BASH; \
> else if [ -x /bin/bash ]; then echo /bin/bash; \
> else echo sh; fi; fi)
>
> @@ -476,7 +477,7 @@ RSYNC_VCS_EXCLUSIONS = \
> $(BUILD_DIR)/.root:
> mkdir -p $(TARGET_DIR)
> rsync -a --ignore-times $(RSYNC_VCS_EXCLUSIONS) \
> - --chmod=Du+w --exclude .empty --exclude '*~' \
> + --chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
> $(TARGET_SKELETON)/ $(TARGET_DIR)/
> $(INSTALL) -m 0644 support/misc/target-dir-warning.txt $(TARGET_DIR_WARNING_FILE)
> @ln -snf lib $(TARGET_DIR)/$(LIB_SYMLINK)
> @@ -612,7 +613,7 @@ endif
> @$(foreach d, $(call qstrip,$(BR2_ROOTFS_OVERLAY)), \
> $(call MESSAGE,"Copying overlay $(d)"); \
> rsync -a --ignore-times $(RSYNC_VCS_EXCLUSIONS) \
> - --chmod=Du+w --exclude .empty --exclude '*~' \
> + --chmod=u=rwX,go=rX --exclude .empty --exclude '*~' \
> $(d)/ $(TARGET_DIR)$(sep))
>
> @$(foreach s, $(call qstrip,$(BR2_ROOTFS_POST_BUILD_SCRIPT)), \
> diff --git a/support/scripts/shell-wrapper.sh b/support/scripts/shell-wrapper.sh
> new file mode 100755
> index 0000000..513b927
> --- /dev/null
> +++ b/support/scripts/shell-wrapper.sh
> @@ -0,0 +1,5 @@
> +#!/bin/sh
> +
> +umask 022
> +
> +exec "$@"
>
--
Arnout Vandecappelle arnout at mind be
Senior Embedded Software Architect +32-16-286500
Essensium/Mind http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint: 7CB5 E4CC 6C2E EFD4 6E3D A754 F963 ECAB 2450 2F1F
next prev parent reply other threads:[~2014-11-03 22:23 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-11-03 12:42 [Buildroot] [RFC/PATCH] target permissions Guido Martínez
2014-11-03 12:42 ` [Buildroot] [RFC/PATCH] Makefile: guarantee reproducible permissions Guido Martínez
2014-11-03 22:23 ` Arnout Vandecappelle [this message]
2014-11-03 22:31 ` Ezequiel Garcia
2014-11-05 14:46 ` Guido Martínez
2014-11-05 15:51 ` Angelo Compagnucci
2014-11-07 13:57 ` Guido Martínez
2014-11-07 3:50 ` [Buildroot] [PATCH v2 0/5] target permissions Guido Martínez
2014-11-07 3:50 ` [Buildroot] [PATCH v2 1/5] Makefile: don't depend on the umask Guido Martínez
2014-11-07 3:50 ` [Buildroot] [PATCH v2 2/5] Makefile: don't depend on current skeleton/overlay permissions Guido Martínez
2014-11-07 3:50 ` [Buildroot] [PATCH v2 3/5] pkg-generic.mk: don't depend on external package permissions Guido Martínez
2014-11-07 3:50 ` [Buildroot] [PATCH v2 4/5] package: matchbox-keyboard: use install instead of cp Guido Martínez
2014-11-07 3:50 ` [Buildroot] [PATCH v2 5/5] package: linux-fusion: " Guido Martínez
2014-11-07 12:44 ` Danomi Manchego
2014-11-07 13:16 ` Guido Martínez
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=54580079.2070603@mind.be \
--to=arnout@mind.be \
--cc=buildroot@busybox.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox