diff mbox

[v6,03/13] core: re-enter make if $(CURDIR) or $(O) are not absolute canonical path

Message ID 1454342021-22960-4-git-send-email-s.martin49@gmail.com
State Superseded
Headers show

Commit Message

Samuel Martin Feb. 1, 2016, 3:53 p.m. UTC
When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
they can be resolved differently, depending on each package build-system
(whether it uses the given paths or get the absolute canonical ones).

Thus, to make easier tracking down host machine paths leaking into the
host, target or staging trees, the CURDIR and O variables are set to
their absolute canonical paths.

Note that this change takes care of the makefile wrapper installed in
$(O) to avoid unneeded make recursion.

Signed-off-by: Samuel Martin <s.martin49@gmail.com>

---
changes v5 -> v6:
- new patch
---
 Makefile | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

Comments

Arnout Vandecappelle Feb. 1, 2016, 6:58 p.m. UTC | #1
On 01-02-16 16:53, Samuel Martin wrote:
> When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
> they can be resolved differently, depending on each package build-system
> (whether it uses the given paths or get the absolute canonical ones).
> 
> Thus, to make easier tracking down host machine paths leaking into the
> host, target or staging trees, the CURDIR and O variables are set to
> their absolute canonical paths.
> 
> Note that this change takes care of the makefile wrapper installed in
> $(O) to avoid unneeded make recursion.
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>
> 
> ---
> changes v5 -> v6:
> - new patch
> ---
>  Makefile | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 13c16af..10193ba 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -26,16 +26,30 @@
>  
>  # Trick for always running with a fixed umask
>  UMASK = 0022
> -ifneq ($(shell umask),$(UMASK))
> +
> +# Check if we need to re-enter make for one or several of the following reasons:
> +# 1- Wrong (too restrictive) umask:
> +#    This prevents Buildroot and packages from creating files and directories.
> +# 2- CWD (i.e. $(CURDIR)) not being the absolute canonical path:
> +#    This makes harder tracking and fixing host machine path leaks.
> +# 3- Output location (i.e. $(O)) not being the absolute canonical path:
> +#    This makes harder tracking and fixing host machine path leaks.
> +#
> +# Note:
> +# - remove the trailing '/.' from $(O) as it can be added by the makefile
> +#   wrapper installed in the $(O).

 When is the trailing /. added? Can't we avoid that it is added by mkmakefile?

 Otherwise looks good to me.

 Regards,
 Arnout

> +ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))
>  .PHONY: _all $(MAKECMDGOALS)
>  
>  $(MAKECMDGOALS): _all
>  	@:
>  
>  _all:
> -	@umask $(UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
> +	umask $(UMASK) && \
> +		$(MAKE) -C $(realpath $(CURDIR)) --no-print-directory \
> +			$(MAKECMDGOALS) O=$(realpath $(O))
>  
> -else # umask
> +else # umask / $(CURDIR) / $(O)
>  
>  # This is our default rule, so must come first
>  all:
> @@ -1001,4 +1015,4 @@ include docs/manual/manual.mk
>  
>  .PHONY: $(noconfig_targets)
>  
> -endif #umask
> +endif #umask / $(CURDIR) / $(O)
>
Thomas Petazzoni Feb. 3, 2016, 8:15 p.m. UTC | #2
Hello,

On Mon,  1 Feb 2016 16:53:31 +0100, Samuel Martin wrote:
> When $(CURDIR) or $(O) contain symlinks (or mount-bind) in their path,
> they can be resolved differently, depending on each package build-system
> (whether it uses the given paths or get the absolute canonical ones).
> 
> Thus, to make easier tracking down host machine paths leaking into the
> host, target or staging trees, the CURDIR and O variables are set to
> their absolute canonical paths.
> 
> Note that this change takes care of the makefile wrapper installed in
> $(O) to avoid unneeded make recursion.
> 
> Signed-off-by: Samuel Martin <s.martin49@gmail.com>

I have to say I kind of hate how complicated our Makefile "entry" logic
has become over time. But well, certainly not your fault.

> +# Check if we need to re-enter make for one or several of the following reasons:
> +# 1- Wrong (too restrictive) umask:
> +#    This prevents Buildroot and packages from creating files and directories.
> +# 2- CWD (i.e. $(CURDIR)) not being the absolute canonical path:
> +#    This makes harder tracking and fixing host machine path leaks.
> +# 3- Output location (i.e. $(O)) not being the absolute canonical path:
> +#    This makes harder tracking and fixing host machine path leaks.
> +#
> +# Note:
> +# - remove the trailing '/.' from $(O) as it can be added by the makefile
> +#   wrapper installed in the $(O).
> +ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))

Can we simplify this a bit with an intermediate "NEED_RECURSE" or
something like that ?

# umask is too restrictive
ifneq ($(shell umask),$(UMASK))
NEED_RECURSE = YES
endif

...

ifeq ($(NEED_RECURSE),YES)

Maybe "RECURSE" is not the appropriate word. Maybe "NEED_SUBMAKE" is
better. Or other folks might have better suggestions.

Thanks!

Thomas
diff mbox

Patch

diff --git a/Makefile b/Makefile
index 13c16af..10193ba 100644
--- a/Makefile
+++ b/Makefile
@@ -26,16 +26,30 @@ 
 
 # Trick for always running with a fixed umask
 UMASK = 0022
-ifneq ($(shell umask),$(UMASK))
+
+# Check if we need to re-enter make for one or several of the following reasons:
+# 1- Wrong (too restrictive) umask:
+#    This prevents Buildroot and packages from creating files and directories.
+# 2- CWD (i.e. $(CURDIR)) not being the absolute canonical path:
+#    This makes harder tracking and fixing host machine path leaks.
+# 3- Output location (i.e. $(O)) not being the absolute canonical path:
+#    This makes harder tracking and fixing host machine path leaks.
+#
+# Note:
+# - remove the trailing '/.' from $(O) as it can be added by the makefile
+#   wrapper installed in the $(O).
+ifneq ($(shell umask):$(CURDIR):$(patsubst %/.,%,$(O)),$(UMASK):$(realpath $(CURDIR)):$(realpath $(O)))
 .PHONY: _all $(MAKECMDGOALS)
 
 $(MAKECMDGOALS): _all
 	@:
 
 _all:
-	@umask $(UMASK) && $(MAKE) --no-print-directory $(MAKECMDGOALS)
+	umask $(UMASK) && \
+		$(MAKE) -C $(realpath $(CURDIR)) --no-print-directory \
+			$(MAKECMDGOALS) O=$(realpath $(O))
 
-else # umask
+else # umask / $(CURDIR) / $(O)
 
 # This is our default rule, so must come first
 all:
@@ -1001,4 +1015,4 @@  include docs/manual/manual.mk
 
 .PHONY: $(noconfig_targets)
 
-endif #umask
+endif #umask / $(CURDIR) / $(O)