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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.